[wheel] Test ABI3 wheels across Python versions

[tyler-yankee]

By definition, an ABI3 wheel can be installed and run under newer versions of Python than what it was built with. Amend 0c490ac by adapting the wheel builder to handle testing a wheel on multiple versions of Python within a given target (i.e., version with which it was built).

To make the builder more efficient under this workflow, split up test image provisioning into two steps, for base prerequisites and Python environment setup, so that the former can be reused across tests of different Python versions.

Closes #24854.

---

This change is 

[tyler-yankee]

+a:@mwoehlke-kitware for feature review, please.

@tyler-yankee reviewed 4 files and all commit messages, and made 2 comments.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware and tyler-yankee).

---

a discussion (no related file):
working

Needs wheel CI. I've run it locally on Linux and macOS so I'm confident enough that it works for now, and I'll wait for review churn before triggering.

[tyler-yankee]

@tyler-yankee reviewed 1 file and all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

[mwoehlke-kitware]

I'm not going to do a detailed review (yet). Instead, I am just going to ask why the Linux and macOS approaches are different. IIUC, the Linux tests are meant to run on the set product of {hosts} and {pythons}. It seems that following the macOS approach of having a build_python and test_pythons, as separate members from build_platform and test_platforms (rather than moving the PythonTarget into the Platform) would be a cleaner approach.

@mwoehlke-kitware made 2 comments.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

---

tools/wheel/wheel_builder/linux.py line 39 at r2 (raw file):

# but the user may down-select from this set. The platform alias is used for
# Docker tag names, and, when combined with the Python version, must be unique.
# fmt: off

Aside from being not ideal, isn't this redundant with # noqa?

[mwoehlke-kitware]

@mwoehlke-kitware made 2 comments.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

---

tools/wheel/wheel_builder/macos.py line 52 at r2 (raw file):

        python_binder=PythonBinder.PYBIND11,
        build_python=PythonTarget(3, 13),
        test_pythons=(PythonTarget(3, 13),),

Consider omitting this and adding if self.test_pythons is None: self.test_pythons = (self.build_python,) to the ctor. (Same for Linux.)

---

tools/wheel/wheel_builder/macos.py line 97 at r2 (raw file):

def _test_wheel(wheel, target, env):
    """
    Runs the test script on `wheel` for each `PythonTarget` belonging to

(...or perhaps "testing". Not sure if the quote are useful or should be omitted.)

...although, I'd actually be very tempted to take a PythonTarget argument and kick looping over them up to the caller.

Suggestion:

for each 'test' `PythonTarget`

[tyler-yankee]

IIUC, the Linux tests are meant to run on the set product of {hosts} and {pythons}.

Aside from the verbosity of build_platforms, build_pythons, test_platforms, and test_pythons, my more functional concern with implicitly assuming this Cartesian product is the python_manager. The platform affects the availability of a given version of Python ¹, on the system package manager vs. some tool like uv. How does that get tracked in the matrix if Python and platform are decoupled?

¹ On Ubuntu, anyway. When we test on AL2023 the versions we need to test with are always available via dnf.

@tyler-yankee made 3 comments.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

---

tools/wheel/wheel_builder/linux.py line 39 at r2 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Aside from being not ideal, isn't this redundant with # noqa?

I hate adding both, but it was the only way I could placate both the formatter and the linter.

---

tools/wheel/wheel_builder/macos.py line 97 at r2 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

(...or perhaps "testing". Not sure if the quote are useful or should be omitted.)

...although, I'd actually be very tempted to take a PythonTarget argument and kick looping over them up to the caller.

happy to! I had considered this and didn't have much of a preference initially.

[mwoehlke-kitware]

Hmm, if anything, that means we are under-testing. Given we now always get build-Python with uv, maybe we should do the same for test-Python? If we're worried about them being different, wouldn't we want to test both distro- and uv-provided Python for all available versions? (Ugh, I note now that "PythonManager" is sort of... terrible. "PythonProvider[={uv,distro}]" would be more accurate.)

BTW, I am also surprised we don't pin the patch version of Python for testing. If uv doesn't need that, should we be pinning it for builds? If "yes", shouldn't it also be pinned for testing?

@mwoehlke-kitware made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

[mwoehlke-kitware]

On an unrelated note, is there a reason we need upstream uv vs. apt-get install astral-uv?

@mwoehlke-kitware made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

[tyler-yankee]

If we're worried about them being different, wouldn't we want to test both distro- and uv-provided Python for all available versions?

Under nanobind, that's effectively what this PR does. It doesn't quite do that, since it doesn't double-test distro- and uv-provided Python for the versions that are distro-provided, but the infrastructure can handle it if those additional lines of the matrix are added. Is your suggestion that the builder should handle that behind the scenes?

BTW, I am also surprised we don't pin the patch version of Python for testing.

uv can handle either way (not pinning the patch installs the latest patch), distro obviously installs whatever latest patch they ship at the time. I'd be open to adding the patch versions in the former case. I opted to not add it in the initial revision because in contrast to the build, where we'd like it to be reproducible, conceivably in a real-world environment someone will be installing a drake wheel with a different patch version of Python than what we built with, and that's a case our tests can easily simulate.

@tyler-yankee made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

[mwoehlke-kitware]

that's effectively what this PR does

Er... no? I don't think we ever test a particular Python under uv that we test using distro packages, or vice versa.

Maybe another approach is to disassociate the provider from the platform. That is, have one global constant that lists what Pythons can be distro-provided, and then go ahead and refactor testing to be a Cartesian product, but for each (host, Python) pair, use the distro's Python if available, else use uv.

...but I am honestly wondering if it makes sense to do the same for testing as we did for build, and just always use uv... (Thoughts from @jwnimmer-tri?)

[tyler-yankee]

It depends on the context of downstream usage and how "realistic" the testing environment should be to match that. Since drake's Python toolchain outside of the wheel/PyPI setting has dependencies on system Python packages, it makes sense to me to test that we can be installed into that environment, but I don't want to speculate.

global constant ... Cartesian product

Depending on the shape the testing takes (above), I'm open to considering it, but I'd have to sketch it out locally and compare to the current revision.

On an unrelated note, is there a reason we need upstream uv vs. apt-get install astral-uv?

I don't remember if there was a particular motivation either way at the time, no. Of course, upstream recommends you install from them directly, but e.g. in drake-ci we also get Docker from apt.

@tyler-yankee made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

[jwnimmer-tri]

We definitely need the test matrix to always cover the Ubuntu-provided default Python, e.g., on Noble we should test both wheels (pybind11/cp312 and nanobind/abi3) against /usr/bin/python3.12 and on Resolute we should test both wheels (pybind11/cp314 and nanobind/abi3) against /usr/bin/python3.14. Whether we also test against a uv-downloaded python for those same versions isn't really important. I don't think it will catch any additional bugs if we did it, but the only trouble it would cost is additional time. If the code is a lot simpler with the full uv matrix that's fine, as long as it's not too slow.

We do need to be careful about how long the full matrix takes, and how much it downloads. For example, the Docker image for x86-64 + Noble + uv python 3.13 and x86-64 + Noble + uv python 3.14 should either be the same image, or at least share a base layer where the apt update && apt install steps happen just one. Interactions with the OS package manager are actually pretty slow (and sometimes flaky) so we need to minimize that work now that there are going to be so many test cases. (I haven't looked at whether the current PR accomplishes this or not.) Even with the current matrix on master, I find it slower that I prefer.

[tyler-yankee]

For example, the Docker image for x86-64 + Noble + uv python 3.13 and x86-64 + Noble + uv python 3.14 should either be the same image, or at least share a base layer where the apt update && apt install steps happen just one.

I don't think the current revision handles reusing of images between steps; this is a good point. I'll look at it.

@tyler-yankee made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

[mwoehlke-kitware]

Hmm. Well, I think the right way to do that requires splitting test/provision.sh into two pieces. And we probably want to recycle the install of uv also.

Actually, if that sort of initial setup is really a concern, have we considered rolling our own base images? It might make sense to have something automated that generates and publishes images that have already run apt update periodically, allowing CI to just download those directly rather than having to run apt update everywhere.

[tyler-yankee]

The new diff is certainly larger, but I believe it accomplishes both:

• @mwoehlke-kitware's request to have the builder manage the Cartesian product and choose managers based on a static list of what package managers provide what Python versions;
• @jwnimmer-tri's request to tweak the test Dockerfile+tagging semantics to reuse a base image across test cases.

Happy to hear additional review from here.

@tyler-yankee partially reviewed 6 files and all commit messages, made 4 comments, and resolved 2 discussions.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

---

tools/wheel/wheel_builder/linux.py line 39 at r2 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

I hate adding both, but it was the only way I could placate both the formatter and the linter.

With the new matrix this is a moot point.

---

tools/wheel/wheel_builder/macos.py line 52 at r2 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Consider omitting this and adding if self.test_pythons is None: self.test_pythons = (self.build_python,) to the ctor. (Same for Linux.)

I actually prefer seeing the build and test versions spelled out in the matrix. Keeping it without a default also avoids confusion in the future when pybind11 goes away and the default would actually be "all" instead of build_python.

---

tools/wheel/wheel_builder/macos.py line 97 at r2 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

happy to! I had considered this and didn't have much of a preference initially.

Done.

[mwoehlke-kitware]

(Incomplete review...)

@mwoehlke-kitware partially reviewed 3 files and made 14 comments.
Reviewable status: 12 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

---

tools/wheel/wheel_builder/linux.py line 39 at r2 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

With the new matrix this is a moot point.

...which was a hoped-for outcome. 🙂

---

tools/wheel/wheel_builder/macos.py line 52 at r2 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

I actually prefer seeing the build and test versions spelled out in the matrix. Keeping it without a default also avoids confusion in the future when pybind11 goes away and the default would actually be "all" instead of build_python.

Okay, I guess I can live with it given it's eventually going to be irrelevant. (Note, 'all' would always be spelled out.)

---

-- commits line 10 at r3:
BTW, this doesn't necessitate said split, said split is simply more efficient.

---

tools/wheel/test/Dockerfile line 22 at r3 (raw file):

ARG PYTHON=3
ARG PYTHON_MANAGER=pip

BTW, does this (PYTHON_MANAGER) need to be duplicated?

---

tools/wheel/test/provision-base.sh line 3 at r3 (raw file):

#!/bin/bash

# Internal script to provision a bare Docker image for testing a Drake wheel.

BTW, might want to say something like "...to prepare a bare Docker image to be provisioned for...", since this is now only starting the process.

---

tools/wheel/wheel_builder/linux.py line 36 at r3 (raw file):

# Supported platforms on which every wheel is tested.
_TEST_PLATFORMS = (

Oh! I like that we were able to factor this out! 🙂

---

tools/wheel/wheel_builder/linux.py line 65 at r3 (raw file):

            build_python=PythonTarget(3, 12, 13),
            test_platforms=_TEST_PLATFORMS,
            test_pythons=(

BTW, consider factoring this out as _TEST_PYTHONS?

---

tools/wheel/wheel_builder/linux_types.py line 36 at r3 (raw file):

    """A (platform, python) combination with which to test.

    python_manager will be selected automatically based on _DISTRO_PYTHONS.

This seems inaccurate, and I sort of wish it wasn't. Though, TBH, I wonder if we even need this class (and the test_matrix method). How much are we gaining vs. having a 'determine python manager' function and just using a bi-level loop (or itertools.product) for running tests?

---

tools/wheel/wheel_builder/macos.py line 99 at r3 (raw file):

    Runs the test script on `wheel`.
    """
    # Setup the environment.

https://grammarist.com/spelling/set-up-vs-setup/

Suggestion:

Set up

---

tools/wheel/wheel_builder/macos.py line 106 at r3 (raw file):

        ["bash", setup_script, python_target.version], env=env
    )
    test_python_venv = os.path.join(test_root, "python")

Can we keep the blank line here?

Suggestion:

    )
    
    test_python_venv = os.path.join(test_root, "python")

---

tools/wheel/wheel_builder/macos.py line 213 at r3 (raw file):

                _test_wheel(wheel, python_target, env=environment)
                if not options.keep_build:
                    shutil.rmtree(os.path.realpath(test_root))

This is very non-optimal. I'm also not sure it's necessary, though we might need to nuke {test_root}/python. In order to be able to keep test results in case of issues, however, we should ensure that the tests for each unique Python version are isolated. (Which they might already be, mostly?) Then the only nuke of all of test_root should be the one that pre-exists.

I also don't know that the makedirs is needed in any case, as we didn't seem to do that previously?

---

tools/wheel/wheel_builder/macos_types.py line 28 at r3 (raw file):

    def __post_init__(self):
        assert len(self.build_python.version_tuple) == 2, (

Given this now happens four times, I'm somewhat tempted to make it a method on PythonTarget. WDYT?

Something like self.build_python.validate(components=2)...

---

tools/wheel/wheel_builder/macos_types.py line 31 at r3 (raw file):

            self.build_python.version_tuple
        )
        assert isinstance(self.test_pythons, tuple)

TBH, I don't know that we "should" care that this is exactly a tuple. We really only care that it's something iterable.

[tyler-yankee]

Most of my addressing the review comments is squashed into the main commit, with the other wip on top mentioned in the other thread. I haven't tested the latest rev on macOS.

@tyler-yankee reviewed 7 files and all commit messages, made 9 comments, and resolved 7 discussions.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on mwoehlke-kitware).

---

-- commits line 10 at r3:

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

BTW, this doesn't necessitate said split, said split is simply more efficient.

Fair

---

tools/wheel/test/Dockerfile line 22 at r3 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

BTW, does this (PYTHON_MANAGER) need to be duplicated?

removed (when I was adapting this file and/or testing --tag-stages I probably added it by rote)

---

tools/wheel/wheel_builder/linux.py line 36 at r3 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Oh! I like that we were able to factor this out! 🙂

😄

---

tools/wheel/wheel_builder/linux.py line 65 at r3 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

BTW, consider factoring this out as _TEST_PYTHONS?

Could, but similar logic as the default suggestion of test_pythons from build_pythons; I think I'd prefer to do this when pybind11 is gone.

That said it's not a strong preference either way.

---

tools/wheel/wheel_builder/linux_types.py line 36 at r3 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

This seems inaccurate, and I sort of wish it wasn't. Though, TBH, I wonder if we even need this class (and the test_matrix method). How much are we gaining vs. having a 'determine python manager' function and just using a bi-level loop (or itertools.product) for running tests?

I can go either way on this one; I've left the change in a second wip commit, LMK what you think and we can revert or adopt it as needed.

Particularly I'm seeing that we end up needing to pass around the collection of platform, python, and sometimes python_manager in a few places, where we might as well collect those things into an object like TestCase.

---

tools/wheel/wheel_builder/macos.py line 213 at r3 (raw file):
Something must be implicitly making the directory on subsequent iterations of the loop, because it fell over somewhere without makedirs. In any case, I opted for this approach to ensure our tests get a clean working slate between Python versions, but I guess if they're not writing files or etc. then it's fine?

though we might need to nuke {test_root}/python

_test_wheel already removes the python symlink, and the venv it points to is named python{version}, so I think we are safe as-is there.

---

tools/wheel/wheel_builder/macos_types.py line 28 at r3 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Given this now happens four times, I'm somewhat tempted to make it a method on PythonTarget. WDYT?

Something like self.build_python.validate(components=2)...

yep!

---

tools/wheel/wheel_builder/macos_types.py line 31 at r3 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

TBH, I don't know that we "should" care that this is exactly a tuple. We really only care that it's something iterable.

Agh. That's technically true, but IMO it's beneficial for consistency and simplicity to simply assert that we will always work with tuples. One could imagine an incidental change where only macOS starts using lists or something which would be weird.

[mwoehlke-kitware]

@mwoehlke-kitware reviewed 2 files and all commit messages, made 6 comments, and resolved 1 discussion.
Reviewable status: 6 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on tyler-yankee).

---

tools/wheel/test/Dockerfile line 22 at r3 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

removed (when I was adapting this file and/or testing --tag-stages I probably added it by rote)

It was there originally, I was guessing you just overlooked de-duplicating it. 🙂 🤷

---

tools/wheel/wheel_builder/linux.py line 65 at r3 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Could, but similar logic as the default suggestion of test_pythons from build_pythons; I think I'd prefer to do this when pybind11 is gone.

That said it's not a strong preference either way.

Okay, I guess I can live with that.

---

tools/wheel/wheel_builder/linux_types.py line 36 at r3 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

I can go either way on this one; I've left the change in a second wip commit, LMK what you think and we can revert or adopt it as needed.

Particularly I'm seeing that we end up needing to pass around the collection of platform, python, and sometimes python_manager in a few places, where we might as well collect those things into an object like TestCase.

Okay, that seems valid. But maybe just construct a TestCase from a Platform and PythonTarget and have it figure out the manager as part of its construction?

Okay, I am even reconsidering test_matrix, though maybe test_cases() is a better name? And ideally it would look something like return map(TestCase, itertools.product(...)).

---

tools/wheel/wheel_builder/macos.py line 213 at r3 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Something must be implicitly making the directory on subsequent iterations of the loop, because it fell over somewhere without makedirs. In any case, I opted for this approach to ensure our tests get a clean working slate between Python versions, but I guess if they're not writing files or etc. then it's fine?

though we might need to nuke {test_root}/python

_test_wheel already removes the python symlink, and the venv it points to is named python{version}, so I think we are safe as-is there.

Oh. I think we are not. This worked when we only tested on the build Python, which is unique. I think we need to change the 'real' venv to be something like <root>/<unique_build_id>/<test_python>. Otherwise multiple builds that test the same Python version will collide. But we do want to keep it so that we don't need to nuke the specific test venv after each test, so that we can keep the ability to retain the artifacts.

(Note: right now we use <root>/<test_python> which was unique prior to these changes...)

IIUC, the tests write everything to <root>/python, which should be (and used to be) a symlink to a unique test directory. And, yes, I missed that _test_wheel already nukes that. So we're good in that respect, we just need to tweak things a little so that the test directory is still unique now that the same Python can be tested against multiple builds.

Also, yikes, this isn't newly broken with this MR...

---

tools/wheel/wheel_builder/common.py line 70 at r4 (raw file):

    def validate(self, n_components: int) -> None:
        """XXX"""

Forgot to finish writing doc?

---

tools/wheel/wheel_builder/macos.py line 222 at r4 (raw file):

            if options.test:
                os.unlink(test_root)

This one needs to stay. After we are all done, we want to clean up, unless the user requested otherwise. (Note that this is guarded by if not options.keep_build.)

Suggestion:

            if options.test:
                shutil.rmtree(os.path.realpath(test_root))
                os.unlink(test_root)

[tyler-yankee]

@tyler-yankee reviewed 5 files and all commit messages, and made 4 comments.
Reviewable status: 6 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on mwoehlke-kitware).

---

tools/wheel/wheel_builder/common.py line 70 at r4 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Forgot to finish writing doc?

ha

---

tools/wheel/wheel_builder/linux_types.py line 36 at r3 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Okay, that seems valid. But maybe just construct a TestCase from a Platform and PythonTarget and have it figure out the manager as part of its construction?

Okay, I am even reconsidering test_matrix, though maybe test_cases() is a better name? And ideally it would look something like return map(TestCase, itertools.product(...)).

Yes, I think I'm happiest with the latest rev. TestCase and test_matrix still exist (on the latter, I guess test_cases also works, but I like the association of "matrix" with the notion that we're returning a product). But we've simplified down to deciding the manager via __post_init__ and using a starmap to generate the cases.

I've still left it in a separate commit for now pending your stamp / further discussion.

---

tools/wheel/wheel_builder/macos.py line 213 at r3 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Oh. I think we are not. This worked when we only tested on the build Python, which is unique. I think we need to change the 'real' venv to be something like <root>/<unique_build_id>/<test_python>. Otherwise multiple builds that test the same Python version will collide. But we do want to keep it so that we don't need to nuke the specific test venv after each test, so that we can keep the ability to retain the artifacts.

(Note: right now we use <root>/<test_python> which was unique prior to these changes...)

IIUC, the tests write everything to <root>/python, which should be (and used to be) a symlink to a unique test directory. And, yes, I missed that _test_wheel already nukes that. So we're good in that respect, we just need to tweak things a little so that the test directory is still unique now that the same Python can be tested against multiple builds.

Also, yikes, this isn't newly broken with this MR...

Yes, good find, this must have been missed on the macOS builder in the initial nanobind implementation.

Latest rev uses ~/.drake-wheel-build/test-<binder>-<py_tag><junk>/ as the test root for a given target, so we should be resistant to multiple Python versions for build (two binders) and tests (within the nanobind target).

---

tools/wheel/wheel_builder/macos.py line 222 at r4 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

This one needs to stay. After we are all done, we want to clean up, unless the user requested otherwise. (Note that this is guarded by if not options.keep_build.)

right, I missed restoring this. thanks!

[mwoehlke-kitware]

tools/wheel/wheel_builder/linux_types.py line 45 at r5 (raw file):

    def __post_init__(self):
        provided = _DISTRO_PYTHONS[self.platform.alias]

BTW, maybe consider something like distro_pythons instead of provided?

[tyler-yankee]

r6 is not yet tested on Linux, but hopefully the changes there are harmless.

@tyler-yankee reviewed 4 files and all commit messages, made 4 comments, and resolved 2 discussions.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

---

tools/wheel/wheel_builder/linux_types.py line 36 at r3 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Yes, I think I'm happiest with the latest rev. TestCase and test_matrix still exist (on the latter, I guess test_cases also works, but I like the association of "matrix" with the notion that we're returning a product). But we've simplified down to deciding the manager via __post_init__ and using a starmap to generate the cases.

I've still left it in a separate commit for now pending your stamp / further discussion.

I've squashed this

---

tools/wheel/wheel_builder/linux_types.py line 45 at r5 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

BTW, maybe consider something like distro_pythons instead of provided?

I went with platform_pythons since we're indexing distro by platform

---

tools/wheel/wheel_builder/macos.py line 213 at r3 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Yes, good find, this must have been missed on the macOS builder in the initial nanobind implementation.

Latest rev uses ~/.drake-wheel-build/test-<binder>-<py_tag><junk>/ as the test root for a given target, so we should be resistant to multiple Python versions for build (two binders) and tests (within the nanobind target).

(for reference, see also #24919; there was some offline discussion here, but I think we've now resolved that the test roots are unique, even with this change, since it's local to _test_wheel and relies on mktemp -d)

[mwoehlke-kitware]

@mwoehlke-kitware reviewed 4 files and all commit messages, made 4 comments, and resolved 2 discussions.
Reviewable status: 6 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

---

tools/wheel/wheel_builder/macos.py line 213 at r3 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

(for reference, see also #24919; there was some offline discussion here, but I think we've now resolved that the test roots are unique, even with this change, since it's local to _test_wheel and relies on mktemp -d)

Correct (my initial assessment was wrong, because I was missing that everything is tucked under a mktemp -d "real" location). Per aforementioned PR, the pythonX.Y was actually superfluous.

The way we create the underlying directories means it is not obvious which binder a particular directory is associated with, but since that is a transient problem, IMHO it isn't worth solving.

---

tools/wheel/wheel_builder/linux.py line 242 at r6 (raw file):

    Generates a Docker tag name for a test-role TestCase and stage/prefix.
    """
    platform = test_case.alias

BTW, do we really need this property, vs. just writing .platform.alias? The latter seems more clear.

---

tools/wheel/wheel_builder/linux_types.py line 74 at r6 (raw file):

    def test_cases(self) -> tuple[TestCase, ...]:
        """Returns the Cartesian product of `test_platforms` and
        `test_pythons` as TestCases."""

minor

Suggestion:

as a tuple of `TestCase`[ instance]s

---

tools/wheel/wheel_builder/common.py line 71 at r6 (raw file):

    def validate(self, n_components: int) -> None:
        """Asserts that the number of version components belonging to `self`
        matches `n_components`."""

minor (or, "equals"):

Suggestion:

is equal to

[mwoehlke-kitware]

@mwoehlke-kitware resolved 2 discussions.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

[tyler-yankee]

@tyler-yankee made 1 comment.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge" (waiting on mwoehlke-kitware).

---

a discussion (no related file):
working

per offline, going to pull the test provisioning splitting into a separate PR to make this change more clear. +(status: do not review) +(status: do not merge)