Update regression tests to handle Apple's local patching of libxml2#94
Open
nwc10 wants to merge 5 commits intoshlomif:masterfrom
Open
Update regression tests to handle Apple's local patching of libxml2#94nwc10 wants to merge 5 commits intoshlomif:masterfrom
nwc10 wants to merge 5 commits intoshlomif:masterfrom
Conversation
Line numbers are always enabled since libxml2 2.15.0.
Apple have patched their libxml2 to call the system malloc etc directly, bypassing libxml2's wrappers that track memory usage. The reporting function xmlMemUsed remains, but it always returns 0. This regression test is validating that memory is allocated at the expected time, and is correctly released at the expected time. It was added back in 2014 and is untouched since then, As it's not possible to run it without memory stats, simply skip it.
t/13dtd.t tests this case, but assumes the upstream behaviour of that C code
returning NULL and hence XML::LibXML::Dtd->new("",""); returning undef
Handle the Apple variant behaviour, rather than having the test script die.
I'm fine with this Perl-space behaviour for the corner case being
"implementation defined", rather than trying to work around the OS patching,
given that the original bug report from 2004 was closed as
NOT A BUG but a misunderstanding of the package.
https://rt.cpan.org/Public/Bug/Display.html?id=2021
before the test was written.
This file contains hidden or 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
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.
This MR builds on the commits in #87
Probably that MR should be merged, then this branch rebased on the merge commit
t/13dtd.tandt/50devel.tboth fail on macOS because Apple have made local patches to their version of libxml2, which I believe to be v2.9.13. Apple have published their changes at https://github.com/apple-oss-distributions/libxml2Apple's libxml2 throws an error in xmlSAX2ResolveEntity if URI is NULL.
t/13dtd.ttests this case, but assumes the upstream behaviour of that C code returningNULLand henceXML::LibXML::Dtd->new("","");returningundefHandle the Apple variant behaviour, rather than having the test script die. I'm fine with this Perl-space behaviour for the corner case being "implementation defined", rather than trying to work around the OS patching, given that the original bug report from 2004 was closed as
https://rt.cpan.org/Public/Bug/Display.html?id=2021
well before
t/13dtd.twas written.The failure of
t/50devel.tis messier. The tests that fail are the three instances ofcmp_ok(mem_used(), '>', mem_before);This turns out to be because
mem_used()is now always reporting0- 0 bytes allocated. Upstream libxml2 vectors all allocation through functions inxmlmemory.c, and uses a static variable to record total allocation. Hence inxmlMallocLocand similar functions about 20 lines in we haveand then there is this function to read it
exposed to Perl space as
mem_usedIn their patched
xmlmemory.hthey have this:and in
xmlmemory.cchanges such as this:see apple-oss-distributions/libxml2@5d4e3a3#diff-51ff3727803a305d66aca10db9fa81b1bdb2d5e3d201e99661df23df80f14af8 and apple-oss-distributions/libxml2@5d4e3a3#diff-66a9bd352f0152960995cc8bfe359e35d7d03c0023705b26f84e919590d7a518
suggesting that there are both compile time and run time tests to determine whether to shortcut the wrapper.
The upshot is that
mem_usedalways returns 0Hence I propose that we just skip the test if
xmlMemUsedreturns 0, as it's clearly inaccurate that there is no memory allocated, and it seems to be the easiest way to code a skip.