Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-90548: Make musl test skips smarter (fixes Alpine errors) #131313

Merged
merged 9 commits into from
Mar 19, 2025
31 changes: 30 additions & 1 deletion Doc/library/test.rst
Original file line number Diff line number Diff line change
@@ -246,7 +246,27 @@ The :mod:`test.support` module defines the following constants:

.. data:: is_android

``True`` if the system is Android.
``True`` if ``sys.platform`` is ``android``.


.. data:: is_emscripten

``True`` if ``sys.platform`` is ``emscripten``.


.. data:: is_wasi

``True`` if ``sys.platform`` is ``wasi``.


.. data:: is_apple_mobile

``True`` if ``sys.platform`` is ``ios``, ``tvos``, or ``watchos``.


.. data:: is_apple

``True`` if ``sys.platform`` is ``darwin`` or ``is_apple_mobile`` is ``True``.


.. data:: unix_shell
@@ -831,6 +851,15 @@ The :mod:`test.support` module defines the following functions:
Decorator for tests that fill the address space.


.. function:: linked_with_musl()

Return ``False`` if there is no evidence the interperter was compiled with
``musl``, otherwise return a version triple, either ``(0, 0, 0)`` if the
version is unknown, or the actual version if it is known. Intended for use
in ``skip`` decorators. ``emscripten`` and ``wasi`` are assumed to be
compiled with ``musl``; othewise ``platform.libc_ver`` is checked.


.. function:: check_syntax_error(testcase, statement, errtext='', *, lineno=None, offset=None)

Test for syntax errors in *statement* by attempting to compile *statement*.
40 changes: 25 additions & 15 deletions Lib/platform.py
Original file line number Diff line number Diff line change
@@ -189,22 +189,28 @@ def libc_ver(executable=None, lib='', version='', chunksize=16384):
# sys.executable is not set.
return lib, version

libc_search = re.compile(b'(__libc_init)'
b'|'
b'(GLIBC_([0-9.]+))'
b'|'
br'(libc(_\w+)?\.so(?:\.(\d[0-9.]*))?)', re.ASCII)
libc_search = re.compile(
br'(__libc_init)'
br'|'
br'(GLIBC_([0-9.]+))'
br'|'
br'(libc(_\w+)?\.so(?:\.(\d[0-9.]*))?)'
br'|'
br'(musl-([0-9.]+))'
br'',
re.ASCII)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, we can switch to the VERBOSE mode, which would be slightly more clear.

Suggested change
libc_search = re.compile(
br'(__libc_init)'
br'|'
br'(GLIBC_([0-9.]+))'
br'|'
br'(libc(_\w+)?\.so(?:\.(\d[0-9.]*))?)'
br'|'
br'(musl-([0-9.]+))'
br'',
re.ASCII)
libc_search = re.compile(br'''
(__libc_init)
| (GLIBC_([0-9.]+))
| (libc(_\w+)?\.so(?:\.(\d[0-9.]*))?)
| (musl-([0-9.]+))
''',
re.ASCII | re.VERBOSE)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that.


V = _comparable_version
# We use os.path.realpath()
# here to work around problems with Cygwin not being
# able to open symlinks for reading
executable = os.path.realpath(executable)
ver = None
with open(executable, 'rb') as f:
binary = f.read(chunksize)
pos = 0
while pos < len(binary):
if b'libc' in binary or b'GLIBC' in binary:
if b'libc' in binary or b'GLIBC' or 'musl' in binary:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if b'libc' in binary or b'GLIBC' or 'musl' in binary:
if b'libc' in binary or b'GLIBC' or b'musl' in binary:

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that's a nasty one. The actual fix is b'GLIBC in binary or b'musl' in binary`. That's why it passed without the b...it just ran slower ;) (A lot slower, somewhat to my surprise.)

m = libc_search.search(binary, pos)
else:
m = None
@@ -216,26 +222,30 @@ def libc_ver(executable=None, lib='', version='', chunksize=16384):
continue
if not m:
break
libcinit, glibc, glibcversion, so, threads, soversion = [
libcinit, glibc, glibcversion, so, threads, soversion, musl, muslversion = [
s.decode('latin1') if s is not None else s
for s in m.groups()]
if libcinit and not lib:
lib = 'libc'
elif glibc:
if lib != 'glibc':
lib = 'glibc'
version = glibcversion
elif V(glibcversion) > V(version):
version = glibcversion
ver = glibcversion
elif V(glibcversion) > V(ver):
ver = glibcversion
elif so:
if lib != 'glibc':
lib = 'libc'
if soversion and (not version or V(soversion) > V(version)):
version = soversion
if threads and version[-len(threads):] != threads:
version = version + threads
if soversion and (not ver or V(soversion) > V(ver)):
ver = soversion
if threads and ver[-len(threads):] != threads:
ver = ver + threads
elif musl:
lib = 'musl'
if not ver or V(muslversion) > V(ver):
ver = muslversion
pos = m.end()
return lib, version
return lib, version if ver is None else ver

def _norm_version(version, build=''):

35 changes: 23 additions & 12 deletions Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
@@ -3017,20 +3017,31 @@ def is_libssl_fips_mode():
return get_fips_mode() != 0


_linked_to_musl = None
def linked_to_musl():
"""
Test if the Python executable is linked to the musl C library.
Report if the Python executable is linked to the musl C library.
Return False if we don't think it is, or a version triple otherwise.
"""
# This is can be a relatively expensive check, so we use a cache.
global _linked_to_musl
if _linked_to_musl is not None:
return _linked_to_musl

# emscripten (at least as far as we're concerned) and wasi use musl,
# but platform doesn't know how to get the version, so set it to zero.
if is_emscripten or is_wasi:
return (_linked_to_musl := (0, 0, 0))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that it is necessary to use the := operator here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is never necessary to use the := operator, so do you mean you prefer the more verbose style (which I'm fine with changing to), or am I missing something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the more verbose style, which is also more sequential and clear.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


# On all other non-linux platforms assume no musl.
if sys.platform != 'linux':
return False
return (_linked_to_musl := False)

import subprocess
exe = getattr(sys, '_base_executable', sys.executable)
cmd = ['ldd', exe]
try:
stdout = subprocess.check_output(cmd,
text=True,
stderr=subprocess.STDOUT)
except (OSError, subprocess.CalledProcessError):
return False
return ('musl' in stdout)
# On linux, we'll depend on the platform module to do the check, so new
# musl platforms should add support in that module if possible.
import platform
lib, version = platform.libc_ver()
if lib != 'musl':
return (_linked_to_musl := False)
return (_linked_to_musl := tuple(map(int, version.split('.'))))
20 changes: 4 additions & 16 deletions Lib/test/test__locale.py
Original file line number Diff line number Diff line change
@@ -137,10 +137,7 @@ def numeric_tester(self, calc_type, calc_value, data_type, used_locale):
return True

@unittest.skipUnless(nl_langinfo, "nl_langinfo is not available")
@unittest.skipIf(
support.is_emscripten or support.is_wasi,
"musl libc issue on Emscripten, bpo-46390"
)
@unittest.skipIf(support.linked_to_musl(), "musl libc issue, bpo-46390")
def test_lc_numeric_nl_langinfo(self):
# Test nl_langinfo against known values
tested = False
@@ -158,10 +155,7 @@ def test_lc_numeric_nl_langinfo(self):
if not tested:
self.skipTest('no suitable locales')

@unittest.skipIf(
support.is_emscripten or support.is_wasi,
"musl libc issue on Emscripten, bpo-46390"
)
@unittest.skipIf(support.linked_to_musl(), "musl libc issue, bpo-46390")
def test_lc_numeric_localeconv(self):
# Test localeconv against known values
tested = False
@@ -210,10 +204,7 @@ def test_lc_numeric_basic(self):

@unittest.skipUnless(nl_langinfo, "nl_langinfo is not available")
@unittest.skipUnless(hasattr(locale, 'ALT_DIGITS'), "requires locale.ALT_DIGITS")
@unittest.skipIf(
support.is_emscripten or support.is_wasi,
"musl libc issue on Emscripten, bpo-46390"
)
@unittest.skipIf(support.linked_to_musl(), "musl libc issue, bpo-46390")
def test_alt_digits_nl_langinfo(self):
# Test nl_langinfo(ALT_DIGITS)
tested = False
@@ -245,10 +236,7 @@ def test_alt_digits_nl_langinfo(self):

@unittest.skipUnless(nl_langinfo, "nl_langinfo is not available")
@unittest.skipUnless(hasattr(locale, 'ERA'), "requires locale.ERA")
@unittest.skipIf(
support.is_emscripten or support.is_wasi,
"musl libc issue on Emscripten, bpo-46390"
)
@unittest.skipIf(support.linked_to_musl(), "musl libc issue, bpo-46390")
def test_era_nl_langinfo(self):
# Test nl_langinfo(ERA)
tested = False
12 changes: 3 additions & 9 deletions Lib/test/test_locale.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from decimal import Decimal
from test.support import verbose, is_android, is_emscripten, is_wasi, os_helper
from test.support import verbose, is_android, linked_to_musl, os_helper
from test.support.warnings_helper import check_warnings
from test.support.import_helper import import_fresh_module
from unittest import mock
@@ -351,21 +351,15 @@ def setUp(self):

@unittest.skipIf(sys.platform.startswith('aix'),
'bpo-29972: broken test on AIX')
@unittest.skipIf(
is_emscripten or is_wasi,
"musl libc issue on Emscripten/WASI, bpo-46390"
)
@unittest.skipIf(linked_to_musl(), "musl libc issue, bpo-46390")
@unittest.skipIf(sys.platform.startswith("netbsd"),
"gh-124108: NetBSD doesn't support UTF-8 for LC_COLLATE")
def test_strcoll_with_diacritic(self):
self.assertLess(locale.strcoll('à', 'b'), 0)

@unittest.skipIf(sys.platform.startswith('aix'),
'bpo-29972: broken test on AIX')
@unittest.skipIf(
is_emscripten or is_wasi,
"musl libc issue on Emscripten/WASI, bpo-46390"
)
@unittest.skipIf(linked_to_musl(), "musl libc issue, bpo-46390")
@unittest.skipIf(sys.platform.startswith("netbsd"),
"gh-124108: NetBSD doesn't support UTF-8 for LC_COLLATE")
def test_strxfrm_with_diacritic(self):
3 changes: 3 additions & 0 deletions Lib/test/test_math.py
Original file line number Diff line number Diff line change
@@ -2772,6 +2772,9 @@ def test_fma_infinities(self):
or (sys.platform == "android" and platform.machine() == "x86_64")
or support.linked_to_musl(), # gh-131032
f"this platform doesn't implement IEE 754-2008 properly")
# XXX musl is fixed but the fix is not yet released; when the fixed version
# is known change this to:
# or support.linked_to_musl() < (1, <m>, <p>)
def test_fma_zero_result(self):
nonnegative_finites = [0.0, 1e-300, 2.3, 1e300]

15 changes: 9 additions & 6 deletions Lib/test/test_os.py
Original file line number Diff line number Diff line change
@@ -2555,15 +2555,18 @@ def test_fchown(self):
@unittest.skipUnless(hasattr(os, 'fpathconf'), 'test needs os.fpathconf()')
def test_fpathconf(self):
self.assertIn("PC_NAME_MAX", os.pathconf_names)
if not (support.is_emscripten or support.is_wasi):
# musl libc pathconf ignores the file descriptor and always returns
# a constant, so the assertion that it should notice a bad file
# descriptor and return EBADF fails.
self.check(os.pathconf, "PC_NAME_MAX")
self.check(os.fpathconf, "PC_NAME_MAX")
self.check_bool(os.pathconf, "PC_NAME_MAX")
self.check_bool(os.fpathconf, "PC_NAME_MAX")

@unittest.skipUnless(hasattr(os, 'fpathconf'), 'test needs os.fpathconf()')
@unittest.skipIf(
support.linked_to_musl(),
'musl pathconf ignores the file descriptor and returns a constant',
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
)
)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the style I used because it lines up better with how python's statements work (first line only is outdented, lack of outdent signals return to outer logic block). This is accepted by PEP 8, are there additional style constrains on the cpython code base these days?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen lots of use of hanging indents in the lib. There is no additional constraints but usually we try to be consistent with the surrounding code when possible.

def test_fpathconf_with_bad_fd(self):
self.check(os.pathconf, "PC_NAME_MAX")
self.check(os.fpathconf, "PC_NAME_MAX")

@unittest.skipUnless(hasattr(os, 'ftruncate'), 'test needs os.ftruncate()')
def test_ftruncate(self):
self.check(os.truncate, 0)
32 changes: 24 additions & 8 deletions Lib/test/test_platform.py
Original file line number Diff line number Diff line change
@@ -552,6 +552,7 @@ def test_libc_ver(self):
(b'GLIBC_2.9', ('glibc', '2.9')),
(b'libc.so.1.2.5', ('libc', '1.2.5')),
(b'libc_pthread.so.1.2.5', ('libc', '1.2.5_pthread')),
(b'/aports/main/musl/src/musl-1.2.5', ('musl', '1.2.5')),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add tests on version with 2 numbers and 4 numbers? Like /aports/main/musl/src/musl-1.2 and /aports/main/musl/src/musl-1.2.5.1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could, but I don't believe musl will ever have such release numbers, so is it worth doing?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parsing a binary file using a regular expression is a strange thing for me. So I prefer to have tests to make sure that the implementation is reliable. The question is more if the regex will match versions with 2 members (x.y) or 4 members (x.y.z.a), or if it only matchs version with 3 members (x.y.z).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, though the existing tests don't do that, nor do they test the no-match cases. I could add such tests, but at that point I'd want to rewrite this test method to split it up into multiple tests, and I don't think that's in scope for this PR :) I'll add the couple of extra checks, though.

(b'', ('', '')),
):
with open(filename, 'wb') as fp:
@@ -563,14 +564,29 @@ def test_libc_ver(self):
expected)

# binary containing multiple versions: get the most recent,
# make sure that 1.9 is seen as older than 1.23.4
chunksize = 16384
with open(filename, 'wb') as f:
# test match at chunk boundary
f.write(b'x'*(chunksize - 10))
f.write(b'GLIBC_1.23.4\0GLIBC_1.9\0GLIBC_1.21\0')
self.assertEqual(platform.libc_ver(filename, chunksize=chunksize),
('glibc', '1.23.4'))
# make sure that eg 1.9 is seen as older than 1.23.4, and that
# the arguments don't count even if they are set.
chunksize = 200
for data, expected in (
(b'GLIBC_1.23.4\0GLIBC_1.9\0GLIBC_1.21\0', ('glibc', '1.23.4')),
(b'libc.so.2.4\0libc.so.9\0libc.so.23.1\0', ('libc', '23.1')),
(b'musl-1.4.1\0musl-2.1.1\0musl-2.0.1\0', ('musl', '2.1.1')),
(b'no match here, so defaults are used', ('test', '100.1.0')),
):
with open(filename, 'wb') as f:
# test match at chunk boundary
f.write(b'x'*(chunksize - 10))
f.write(data)
self.assertEqual(
expected,
platform.libc_ver(
filename,
lib='test',
version='100.1.0',
chunksize=chunksize,
),
)


def test_android_ver(self):
res = platform.android_ver()
12 changes: 3 additions & 9 deletions Lib/test/test_re.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from test.support import (gc_collect, bigmemtest, _2G,
cpython_only, captured_stdout,
check_disallow_instantiation, is_emscripten, is_wasi,
check_disallow_instantiation, linked_to_musl,
warnings_helper, SHORT_TIMEOUT, CPUStopwatch, requires_resource)
import locale
import re
@@ -2172,10 +2172,7 @@ def test_bug_20998(self):
# with ignore case.
self.assertEqual(re.fullmatch('[a-c]+', 'ABC', re.I).span(), (0, 3))

@unittest.skipIf(
is_emscripten or is_wasi,
"musl libc issue on Emscripten/WASI, bpo-46390"
)
@unittest.skipIf(linked_to_musl(), "musl libc issue, bpo-46390")
def test_locale_caching(self):
# Issue #22410
oldlocale = locale.setlocale(locale.LC_CTYPE)
@@ -2212,10 +2209,7 @@ def check_en_US_utf8(self):
self.assertIsNone(re.match(b'(?Li)\xc5', b'\xe5'))
self.assertIsNone(re.match(b'(?Li)\xe5', b'\xc5'))

@unittest.skipIf(
is_emscripten or is_wasi,
"musl libc issue on Emscripten/WASI, bpo-46390"
)
@unittest.skipIf(linked_to_musl(), "musl libc issue, bpo-46390")
def test_locale_compiled(self):
oldlocale = locale.setlocale(locale.LC_CTYPE)
self.addCleanup(locale.setlocale, locale.LC_CTYPE, oldlocale)
5 changes: 1 addition & 4 deletions Lib/test/test_strptime.py
Original file line number Diff line number Diff line change
@@ -544,10 +544,7 @@ def test_date_locale(self):
self.roundtrip('%x', slice(0, 3), time.localtime(now - 366*24*3600))

# NB: Dates before 1969 do not roundtrip on many locales, including C.
@unittest.skipIf(
support.is_emscripten or support.is_wasi,
"musl libc issue on Emscripten, bpo-46390"
)
@unittest.skipIf(support.linked_to_musl(), "musl libc issue, bpo-46390")
@run_with_locales('LC_TIME', 'en_US', 'fr_FR', 'de_DE', 'ja_JP',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add the linked_to_musl() check in run_with_locales() and run_with_locale(). This may be non-trivial, depending on arguments, a dummy run can be supported even if no suitable locale was found. For now, the proposed change is good as is.

'eu_ES', 'ar_AE', 'my_MM', 'shn_MM')
def test_date_locale2(self):
13 changes: 12 additions & 1 deletion Lib/test/test_support.py
Original file line number Diff line number Diff line change
@@ -746,7 +746,18 @@ def test_get_signal_name(self):

def test_linked_to_musl(self):
linked = support.linked_to_musl()
self.assertIsInstance(linked, bool)
self.assertIsNotNone(linked)
if support.is_wasi:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about is_emscripten?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used is_wasi as an example, but it doesn't hurt to add emscripten as well.

self.assertTrue(linked)
# The value is cached, so make sure it returns the same value again.
self.assertEqual(linked, support.linked_to_musl())
# The unlike libc, the musl version is a triple.
if linked:
self.assertIsInstance(linked, tuple)
self.assertEqual(3, len(linked))
for v in linked:
self.assertIsInstance(v, int)


# XXX -follows a list of untested API
# make_legacy_pyc