TST: fix a small abi3t tag test issue, and add a new test for multiarch filename - #882
Conversation
The assertion parsed as:
assert (abi == 'abi3.abi3t') if IS_ABI3T else 'abi3'
which will never fail for GIL-enabled builds.
Python 3.15 and later append the multiarch tuple to the stable ABI filename suffix. The multiarch tuple is not part of the ABI tag and must be ignored: when cross compiling it is the one of the host platform and differs from the one of the build interpreter. This test fails without the regular expression fix in commit 865aa61.
| # Python 3.15 and later append the multiarch tuple to the stable ABI | ||
| # filename suffix. Verify that it is ignored rather than causing the | ||
| # module to be rejected: when cross compiling it differs from the one | ||
| # of the build interpreter. |
There was a problem hiding this comment.
I had this kind of issue on my mind when thinking about the support for build-detail.json in #829. We have this check right now
meson-python/mesonpy/__init__.py
Lines 436 to 437 in 7c78da3
There was a problem hiding this comment.
The regular expression used to extract the ABI from the filename matches both f'extension.{STABLE_ABI_KIND}-aarch64-linux-gnu.so' and f'extension.{STABLE_ABI_KIND}.so' and any other valid extension module filename, and this is verified at runtime. One test more is better than one test less, thus wer should include this one, but I would like to add some proper cross-compilation tests.
There was a problem hiding this comment.
I would like to add some proper cross-compilation tests.
💯 me too. However, that's for the build-details.json PR or a follow-up to that I think. This test is cheap (just regex/strings, no building), and would have caught an issue.
There was a problem hiding this comment.
which makes it impossible to cross-compile between a Python platform that uses abi3t and one that does not.
I think we can fix that up by extending support to more combinations once gh-829 is in? In practice there are lots of combinations of build/host-platform that are difficult. The most common one uses the same Python version and just changes the host platform. That's the test case I'd add first. Then extend from there.
There was a problem hiding this comment.
This test is cheap (just regex/strings, no building), and would have caught an issue.
Out of curiosity, which issue would this test have caught that other tests did have not?
There was a problem hiding this comment.
There was a problem hiding this comment.
My review comment on gh-876: #876 (comment)
Which I think was a recurrence of the thing fixed in gh-322.
There was a problem hiding this comment.
Should we try to get gh-829 in for this release?
Yes, that'd be great.
There was a problem hiding this comment.
Should we try to get gh-829 in for this release?
Yes, that'd be great.
I can try to find time to finish the PR. However, I would be hesitant to merge it without an integration test. Ideally the test would compiler an extension module, thus it requires a C cross compiler and a build-details.json for the target foreign architecture. What's the easiest way to get these?
There was a problem hiding this comment.
We can also get the release out, merge it right after, and do another release. To take it off of the critical path (we shouldn't drop it for months though).
However, I would be hesitant to merge it without an integration test. Ideally the test would compiler an extension module, thus it requires a C cross compiler and a build-details.json for the target foreign architecture. What's the easiest way to get these?
Conda/pixi is the easiest way to get a cross compilation toolchain. I already have an integration test for scipy; would need to scope that down to running on a test package (or set of packages) and then adding it in a new CI job.
Let's move the CI coverage part to gh-829 though? I think we can discuss the release plan here; this PR is ready to go, and we can probably push to get 1-2 more PRs in, but then ship 0.21.0
| name = artifact.parsed_filename | ||
| assert name.group('pyver') == INTERPRETER | ||
| assert name.group('abi') == 'abi3.abi3t' if FREE_THREADED_BUILD else 'abi3' | ||
| assert name.group('abi') == ('abi3.abi3t' if FREE_THREADED_BUILD else 'abi3') |
There was a problem hiding this comment.
Ups! Thanks for catching this.
2 participants
This is a follow-up to gh-876. The new multiarch test addresses a gap in test coverage that is important - we've stumbled over this case a few times over the past years.