Skip to content

[wheel] Remove pythonX.Y symlink indirection on macOS - #24919

Merged
tyler-yankee merged 1 commit into
RobotLocomotion:masterfrom
tyler-yankee:wheel-mac-X.Y-link
Aug 26, 2026
Merged

[wheel] Remove pythonX.Y symlink indirection on macOS#24919
tyler-yankee merged 1 commit into
RobotLocomotion:masterfrom
tyler-yankee:wheel-mac-X.Y-link

Conversation

@tyler-yankee

@tyler-yankee tyler-yankee commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

The python spelling is needed for install-wheel.sh and test-wheel.sh to be reusable across the Linux and macOS builders, but it being a symlink pointing to pythonX.Y (the directory where the venv lives) is unnecessary. provision-build.sh already creates a unique test root each time a wheel is tested (via
macos/provision-test-python.sh).

The broader motivation for the change is to remove confusion around the uniqueness of the test root and Python virtual environment ahead of a future commit to support testing a single wheel with multiple Python versions.

Towards #24903.


This change is Reviewable

The `python` spelling is needed for `install-wheel.sh` and
`test-wheel.sh` to be reusable across the Linux and macOS builders, but
it being a symlink pointing to `pythonX.Y` (the directory where the
venv lives) is unnecessary. `provision-build.sh` already creates a
unique test root each time a wheel is tested (via
`macos/provision-test-python.sh`).

The broader motivation for the change is to remove confusion around
the uniqueness of the test root and Python virtual environment ahead of
a future commit to support testing a single wheel with multiple Python
versions.

Co-authored-by: Matthew Woehlke <matthew.woehlke@kitware.com>
@tyler-yankee

Copy link
Copy Markdown
Contributor Author

@drake-jenkins-bot mac-arm-tahoe-clang-wheel-experimental-release please.

@mwoehlke-kitware mwoehlke-kitware left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:lgtm: (modulo suggestion for improved clarity in commit message)

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


-- commits line 9 at r1:

Suggestion:

  venv lives) is unnecessary. `provision-build.sh` already creates (via
  `macos/provision-test-python.sh`) a unique test root each time a wheel
  is tested.

@tyler-yankee tyler-yankee left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

+a:@SeanCurtis-TRI for platform review per schedule (tomorrow), please.

@tyler-yankee reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on SeanCurtis-TRI and tyler-yankee).


-- commits line 9 at r1:
Happy to take this suggestion upon merge.

@SeanCurtis-TRI SeanCurtis-TRI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:LGTM: I'll let you merge so you can patch the merge commit on the way.

@SeanCurtis-TRI reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: 1 unresolved discussion (waiting on tyler-yankee).

@tyler-yankee tyler-yankee left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tyler-yankee resolved 1 discussion.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignees SeanCurtis-TRI(platform),mwoehlke-kitware (waiting on tyler-yankee).

@tyler-yankee
tyler-yankee merged commit fa5b069 into RobotLocomotion:master Aug 26, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: medium release notes: none This pull request should not be mentioned in the release notes

Development

Successfully merging this pull request may close these issues.

3 participants