GeezerVison is showing github.com — Both test_flush_reparse_deferral_disabled tests fail with older libexpat (2.4.4) · Issue #117187 · python/cpython
Both test_flush_reparse_deferral_disabled tests fail with older libexpat (2.4.4) #117187DescriptionBug report
Bug description:
Running the test suite for 3.9.19 (and other currently released versions) breaks with the old libexpat (of course, patched for the security issues):
[ 684s] 0:04:10 load avg: 0.64 Re-running test_sax in verbose mode (matching: test_flush_re
parse_deferral_disabled)
[ 684s] test_flush_reparse_deferral_disabled (test.test_sax.ExpatReaderTest) ... FAIL
[ 684s]
[ 684s] ======================================================================
[ 684s] FAIL: test_flush_reparse_deferral_disabled (test.test_sax.ExpatReaderTest)
[ 684s] ----------------------------------------------------------------------
[ 684s] Traceback (most recent call last):
[ 684s] File "/home/abuild/rpmbuild/BUILD/Python-3.9.19/Lib/test/test_sax.py", line 1251,
in test_flush_reparse_deferral_disabled
[ 684s] self.assertEqual(result.getvalue(), start) # i.e. no elements started
[ 684s] AssertionError: b'<?xml version="1.0" encoding="iso-8859-1"?>\n<doc>' != b'<?xml ve
rsion="1.0" encoding="iso-8859-1"?>\n'
[ 684s]
[ 684s] ----------------------------------------------------------------------
[ 684s] Ran 1 test in 0.001s
[ 684s]
[ 684s] FAILED (failures=1)
[ 684s] test test_sax failed
[ 684s] 0:04:10 load avg: 0.64 Re-running test_xml_etree in verbose mode (matching: test_fl
ush_reparse_deferral_disabled)
[ 684s] test_flush_reparse_deferral_disabled (test.test_xml_etree.XMLPullParserTest) ... FA
IL
[ 684s]
[ 684s] ======================================================================
[ 684s] FAIL: test_flush_reparse_deferral_disabled (test.test_xml_etree.XMLPullParserTest)
[ 684s] ----------------------------------------------------------------------
[ 684s] Traceback (most recent call last):
[ 684s] File "/home/abuild/rpmbuild/BUILD/Python-3.9.19/Lib/test/test_xml_etree.py", line
1659, in test_flush_reparse_deferral_disabled
[ 684s] self.assert_event_tags(parser, []) # i.e. no elements started
[ 684s] File "/home/abuild/rpmbuild/BUILD/Python-3.9.19/Lib/test/test_xml_etree.py", line
1395, in assert_event_tags
[ 684s] self.assertEqual([(action, elem.tag) for action, elem in events],
[ 684s] AssertionError: Lists differ: [('start', 'doc')] != []
[ 684s]
[ 684s] First list contains 1 additional elements.
[ 684s] First extra element 0:
[ 684s] ('start', 'doc')
[ 684s]
[ 684s] - [('start', 'doc')]
[ 684s] + []
[ 684s]
[ 684s] ----------------------------------------------------------------------
[ 684s] Ran 1 test in 0.001s
Complete build log with all package versions and steps taken recorded.
CPython versions tested on:
3.8, 3.9, 3.10
Operating systems tested on:
Linux
Linked PRs
Activitymcepl commented on Mar 24, 2024 ContributorAuthor More actions I am rather confused by the coding style of #115623 … isn’t there a preference for @unittest.skipIf instead of randomly included self.SkipTest commands, which are even applied rather inconsistently (as this issue shows)? Shouldn’t all tests including SetReparseDeferralEnabled and GetReparseDeferralEnabled be just covered by @unittest.skipIf(pyexpat.version_info < (2, 6, 0)) and be done with it, if those methods (as mentioned in the documentation) were introduced just in libexpat 2.6.0?
Wouldn’t something like this be more appropriate?
---
Lib/test/test_sax.py | 10 +++++-----
Lib/test/test_xml_etree.py | 17 ++++++++---------
2 files changed, 13 insertions(+), 14 deletions(-)
--- a/Lib/test/test_sax.py
+++ b/Lib/test/test_sax.py
@@ -1211,10 +1211,9 @@ class ExpatReaderTest(XmlTestBase):
self.assertEqual(result.getvalue(), start + b"<doc>text</doc>")
+ @unittest.skipIf(pyexpat.version_info < (2, 6, 0),
+ "Reparse deferral not defined for libexpat < 2.6.0")
def test_flush_reparse_deferral_enabled(self):
- if pyexpat.version_info < (2, 6, 0):
- self.skipTest(f'Expat {pyexpat.version_info} does not support reparse deferral')
-
result = BytesIO()
xmlgen = XMLGenerator(result)
parser = create_parser()
@@ -1236,6 +1235,8 @@ class ExpatReaderTest(XmlTestBase):
self.assertEqual(result.getvalue(), start + b"<doc></doc>")
+ @unittest.skipIf(pyexpat.version_info < (2, 6, 0),
+ "Reparse deferral not defined for libexpat < 2.6.0")
def test_flush_reparse_deferral_disabled(self):
result = BytesIO()
xmlgen = XMLGenerator(result)
@@ -1245,8 +1246,7 @@ class ExpatReaderTest(XmlTestBase):
for chunk in ("<doc", ">"):
parser.feed(chunk)
- if pyexpat.version_info >= (2, 6, 0):
- parser._parser.SetReparseDeferralEnabled(False)
+ parser._parser.SetReparseDeferralEnabled(False)
self.assertEqual(result.getvalue(), start) # i.e. no elements started
self.assertFalse(parser._parser.GetReparseDeferralEnabled())
--- a/Lib/test/test_xml_etree.py
+++ b/Lib/test/test_xml_etree.py
@@ -1619,11 +1619,9 @@ class XMLPullParserTest(unittest.TestCas
with self.assertRaises(ValueError):
ET.XMLPullParser(events=('start', 'end', 'bogus'))
+ @unittest.skipIf(pyexpat.version_info < (2, 6, 0),
+ "Reparse deferral not defined for libexpat < 2.6.0")
def test_flush_reparse_deferral_enabled(self):
- if pyexpat.version_info < (2, 6, 0):
- self.skipTest(f'Expat {pyexpat.version_info} does not '
- 'support reparse deferral')
-
parser = ET.XMLPullParser(events=('start', 'end'))
for chunk in ("<doc", ">"):
@@ -1644,17 +1642,18 @@ class XMLPullParserTest(unittest.TestCas
self.assert_event_tags(parser, [('end', 'doc')])
+ @unittest.skipIf(pyexpat.version_info < (2, 6, 0),
+ "Reparse deferral not defined for libexpat < 2.6.0")
def test_flush_reparse_deferral_disabled(self):
parser = ET.XMLPullParser(events=('start', 'end'))
for chunk in ("<doc", ">"):
parser.feed(chunk)
- if pyexpat.version_info >= (2, 6, 0):
- if not ET is pyET:
- self.skipTest(f'XMLParser.(Get|Set)ReparseDeferralEnabled '
- 'methods not available in C')
- parser._parser._parser.SetReparseDeferralEnabled(False)
+ if not ET is pyET:
+ self.skipTest(f'XMLParser.(Get|Set)ReparseDeferralEnabled '
+ 'methods not available in C')
+ parser._parser._parser.SetReparseDeferralEnabled(False)
self.assert_event_tags(parser, []) # i.e. no elements started
if ET is pyET:Eclips4 commented on Mar 24, 2024 hartwork commented on Mar 24, 2024
breaks with the old libexpat (of course, patched for the security issues):
@mcepl that's expected and distros backporting libexpat security fixed need to adjust the tests (and C pre-processor checks!) of CPython at the same time if doing so. This is a downstream issue, not an upstream one. I'm not alone in this view, e.g. see #115623 (review) .
I am rather confused by the coding style of #115623 … isn’t there a preference for @unittest.skipIf instead of randomly included self.SkipTest commands, which are even applied rather inconsistently (as this issue shows)? Shouldn’t all tests including SetReparseDeferralEnabled and GetReparseDeferralEnabled be just covered by @unittest.skipIf(pyexpat.version_info < (2, 6, 0)) and be done with it, if those methods (as mentioned in the documentation) were introduced just in libexpat 2.6.0?
Wouldn’t something like this be more appropriate?
@mcepl hunk 1 and 3 are purely cosmetic, and hence of limited interest to me:
Hunk 1:
---
Lib/test/test_sax.py | 10 +++++-----
Lib/test/test_xml_etree.py | 17 ++++++++---------
2 files changed, 13 insertions(+), 14 deletions(-)
--- a/Lib/test/test_sax.py
+++ b/Lib/test/test_sax.py
@@ -1211,10 +1211,9 @@ class ExpatReaderTest(XmlTestBase):
self.assertEqual(result.getvalue(), start + b"<doc>text</doc>")
+ @unittest.skipIf(pyexpat.version_info < (2, 6, 0),
+ "Reparse deferral not defined for libexpat < 2.6.0")
def test_flush_reparse_deferral_enabled(self):
- if pyexpat.version_info < (2, 6, 0):
- self.skipTest(f'Expat {pyexpat.version_info} does not support reparse deferral')
-
result = BytesIO()
xmlgen = XMLGenerator(result)
parser = create_parser()
Hunk 3:
--- a/Lib/test/test_xml_etree.py
+++ b/Lib/test/test_xml_etree.py
@@ -1619,11 +1619,9 @@ class XMLPullParserTest(unittest.TestCas
with self.assertRaises(ValueError):
ET.XMLPullParser(events=('start', 'end', 'bogus'))
+ @unittest.skipIf(pyexpat.version_info < (2, 6, 0),
+ "Reparse deferral not defined for libexpat < 2.6.0")
def test_flush_reparse_deferral_enabled(self):
- if pyexpat.version_info < (2, 6, 0):
- self.skipTest(f'Expat {pyexpat.version_info} does not '
- 'support reparse deferral')
-
parser = ET.XMLPullParser(events=('start', 'end'))
for chunk in ("<doc", ">"):
Hunk 2 and 4 are a very different story:
Hunk 2:
@@ -1236,6 +1235,8 @@ class ExpatReaderTest(XmlTestBase):
self.assertEqual(result.getvalue(), start + b"<doc></doc>")
+ @unittest.skipIf(pyexpat.version_info < (2, 6, 0),
+ "Reparse deferral not defined for libexpat < 2.6.0")
def test_flush_reparse_deferral_disabled(self):
result = BytesIO()
xmlgen = XMLGenerator(result)
@@ -1245,8 +1246,7 @@ class ExpatReaderTest(XmlTestBase):
for chunk in ("<doc", ">"):
parser.feed(chunk)
- if pyexpat.version_info >= (2, 6, 0):
- parser._parser.SetReparseDeferralEnabled(False)
+ parser._parser.SetReparseDeferralEnabled(False)
self.assertEqual(result.getvalue(), start) # i.e. no elements started
self.assertFalse(parser._parser.GetReparseDeferralEnabled())
Hunk 4:
@@ -1644,17 +1642,18 @@ class XMLPullParserTest(unittest.TestCas
self.assert_event_tags(parser, [('end', 'doc')])
+ @unittest.skipIf(pyexpat.version_info < (2, 6, 0),
+ "Reparse deferral not defined for libexpat < 2.6.0")
def test_flush_reparse_deferral_disabled(self):
parser = ET.XMLPullParser(events=('start', 'end'))
for chunk in ("<doc", ">"):
parser.feed(chunk)
- if pyexpat.version_info >= (2, 6, 0):
- if not ET is pyET:
- self.skipTest(f'XMLParser.(Get|Set)ReparseDeferralEnabled '
- 'methods not available in C')
- parser._parser._parser.SetReparseDeferralEnabled(False)
+ if not ET is pyET:
+ self.skipTest(f'XMLParser.(Get|Set)ReparseDeferralEnabled '
+ 'methods not available in C')
+ parser._parser._parser.SetReparseDeferralEnabled(False)
self.assert_event_tags(parser, []) # i.e. no elements started
if ET is pyET:
@mcepl both hunk 2 and 4 seem to change semantics for the worse, as tests that were previously running with <2.6.0 fine are now skipped without need to. This inconsistency was done on full purpose.
I'm not against simplifying the two of the four skip markers that keep semantics intact, but I don't consider it of much value, personally.
My vote for closing this issue as "works as expected, no upstream issue to fix".
CC @serhiy-storchaka @gpshead Reacted by Serhiy Storchaka hartwork commented on Mar 24, 2024
Shouldn’t all tests including SetReparseDeferralEnabled and GetReparseDeferralEnabled be just covered by @unittest.skipIf(pyexpat.version_info < (2, 6, 0)) and be done with it, if those methods (as mentioned in the documentation) were introduced just in libexpat 2.6.0?
@mcepl PS: Please note that these functions are exposed by CPython even with libexpat <2.6.0. They just don't do that much then. mcepl commented on Mar 24, 2024 ContributorAuthor More actions @mcepl PS: Please note that these functions are exposed by CPython even with libexpat <2.6.0. They just don't do that much then.
So, you admit that the documentation is wrong. Also, why then these tests fail with older libexpat? hartwork commented on Mar 24, 2024 Hi @mcepl,
@mcepl PS: Please note that these functions are exposed by CPython even with libexpat <2.6.0. They just don't do that much then.
So, you admit that the documentation is wrong.
I don't, and I don't see how it would be wrong. What does the documentation state that is incorrect and which documentation exactly?
Also, why then these tests fail with older libexpat?
Because of the security patches that distros backported into older releases of libexpat that got you into unsupported territory between old and new releases. The failing version is not vanilla 2.4.4 or it would not fail. It's 2.4.4 with backported patches. Reacted by Serhiy Storchaka mcepl commented on Mar 24, 2024 ContributorAuthor More actions @mcepl that's expected and distros backporting libexpat security fixed need to adjust the tests (and C pre-processor checks!) of CPython at the same time if doing so. This is a downstream issue, not an upstream one. I'm not alone in this view, e.g. see #115623 (review) .
I am still confused, but it probably doesn’t matter. Either you expect everybody to use the latest and greatest libraries (which is kind of strange for the universal interpreter), or you would like to support even older libraries. For the first I don’t understand why you have those `if pyexpat.version_info` spread over the code, for the latter I don’t understand why you are not interested when I report you that the support for the older libraries is broken. Anyway, I will just use my patch in *SUSE and let it be.
@mcepl this seems to change semantics for the worse, as a test that was previously running with <2.6.0 fine is now skipped without need to.
No it wasn’t running, as this ticket is all about tests not running with older libexpat.
My vote for closing this issue as "works as expected, no upstream issue to fix".
Sure, I will just patch the Python for ourselves as long as you don’t care. hartwork commented on Mar 24, 2024 @mcepl I have sent you an e-mail offering a voice call in the mean time, about 30 minutes ago. I will refrain from replying here more for now, before we had a real chance to talk. Thanks for your consideration. mcepl commented on Mar 24, 2024 ContributorAuthor More actions
@mcepl I have sent you an e-mail offering a voice call in the mean time, about 30 minutes ago. I will refrain from replying here more for now, before we had a real chance to talk. Thanks for your consideration.
I am in the Central European Time zone, and it is Sunday night here. As I see you are in Berlin, we can talk tomorrow (mcepl on Libera.chat (preferable), or @mcepl:one.ems.host on Matrix). serhiy-storchaka commented on Mar 24, 2024 I completely agree with @hartwork. Python tests work pretty well with both new and old versions of libexpat. They do not work with a distro-specific build of libexpat with distro-specific patches applied. This is not supported case. You perhaps need to apply a distro-specific patch for Python tests to make them consistent with the rest of of the distro. Or what is the standard way to solve such issues in your distro. Reacted by Sebastian Pipping hartwork commented on Mar 24, 2024 @mcepl Matrix and IRC do not work for me, but we'll find something else. Let's take call org to e-mail to keep the issue on topic. See you on Monday, have a good start to the week. gpshead commented on Mar 24, 2024 Hi, glad you two are chatting.
Looking at the build log you link to from the opening message, the important bit is:
- This was a
./configure --with-system-expat build. (not our default build mode)
- The system expat is version 2.4.4.
There were a pile of distro patches on top of that expat and Python applied, but none of those matter. (No backport of the related expat 2.6.0 security improvement feature was involved).
This is easy to reproduce on current non-bleeding-edge Debian and Ubuntu today (sporting expat 2.5.0 or earlier), make sure libexpat1-dev is installed, and presumbly a bunch of other distros (like opensuse or SuSE as was being used):
./configure --with-system-expat && ./python -m test test_sax test_xml_etree
Unfortunately, we do not test --with-system-expat in any way as part of the CPython project. So this failing right now isn't too surprising. That flag exists primarily for use by external distributors who unvendor libraries.
A good way to help prevent this kind of building against legacy libraries failure from happening in the future is to contribute & maintain a relevant builtbot. https://devguide.python.org/testing/new-buildbot-worker/ Reacted by Sebastian Pipping gpshead commented on Mar 24, 2024
Python tests work pretty well with both new and old versions of libexpat.
This currently appears to unfortunately be false because we don't test this anywhere. Reacted by Sebastian Pipping gpshead commented on Mar 26, 2024 Thanks! main merged, 3.12 and 3.11 set to automerge, and PRs assigned to a RM with releaseblocker status for the security only branches that received the expat security fix leading to this issue.
No decision has been made on whether to try and handle the "expat with local backported behavior changes" case that causes test suite failures as found with Ubuntu's expat_2.5.0-2ubuntu0.1 package. Ideally a distro needing that would contribute that? hartwork commented on Mar 26, 2024
Thanks! main merged, 3.12 and 3.11 set to automerge, and PRs assigned to a RM with releaseblocker status for the security only branches that received the expat security fix leading to this issue.
@gpshead thank you!
No decision has been made on whether to try and handle the "expat with local backported behavior changes" case that causes test suite failures as found with Ubuntu's expat_2.5.0-2ubuntu0.1 package. Ideally a distro needing that would contribute that?
I would argue that support for downstream backports would be out of place upstream. Supporting multiple versions of vanilla Expat is complex enough. So instead of asking for these patches for upstream, let's not patch upstream and keep the whole backporting cake downstream.
MetadataMetadataStatusShow more project fields
DevelopmentNo branches or pull requestsIssue actions
|