support upstream libxml2 master up to gnome/libxml2@05c147c3 #3161
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is proving to be useful when narrowing down which tests have memory leaks.
An upcoming libxml2 change will force us to make a behavioral change of some kind, so I'm choosing to bring CData in line with the other character data classes (Comment and Text). The alternative would have been to continue accepting nil; but then the constructed node would contain a blank string (currently in this case Node#content returns nil). I think it's preferable to force callers to be explicit here. See upstream commit GNOME/libxml2@9991fae4
Upstream GNOME/libxml2@7e462425 updated `xmlAddChild` to do nothing if the subject node's `->next` or `->prev` are non-NULL. For fragments that are parsed in-node-context, the nodes are parsed as siblings of each other, meaning that unless we explicitly set `next` and `prev` to NULL, the node will not actually be inserted into the tree by `xml_document.c:dealloc_node_i2` and so `xmlFreeDoc` will not be able to free those nodes, resulting in a leak.
A text node coalescing bug was fixed in GNOME/libxml2@4ccd3eb8, and although Nokogiri handles this fine from a memory perspective, the tests need to be updated to reflect the new behavior.
flavorjones
changed the title
flavorjones
added a commit
that referenced
this pull request
Apr 7, 2024
**What problem is this PR intended to solve?** Get upstream libxml2 integration tests passing again. GNOME/libxml2@5bb84b47 changed behavior around text node coalescing again. See related #3161. Notably, with this change, blank text nodes are no longer merged by `#add_{next,previous}_sibling`. See the tests originally added in 3c8c21e back in 2009 for some context.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What problem is this PR intended to solve?
Closes #3156 and gets CI green against libxml2 master.
nilfor contentSee commit logs for deeper explanations on these changes.
Have you included adequate test coverage?
Yes.
Does this change affect the behavior of either the C or the Java implementations?
Both C and Java implementations no longer accept
nilfor CDATA content.