bootstrap: respect build.python on macOS
#148636
Open
+13
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `python()` method was hardcoded to return `/usr/bin/python3` on macOS, ignoring the `build.python` config option. This change respects the config while maintaining the system Python as the default.
rustbot
added
S-waiting-on-author
rustbot
added
the
S-waiting-on-review
rustbot
removed
the
S-waiting-on-author
Collaborator
|
r? @clubby789 rustbot has assigned @clubby789. Use |
Contributor
Author
|
I've tested the default is set locally (x86_64-apple-darwin), but it'd be great if a CI job on an *-apple-darwin worker could also confirm. |
jieyouxu
reviewed
Nov 7, 2025
Comment on lines
+1528
to
+1532
| // LLDB tests require the Python version the LLDB plugin was built for. | ||
| // On macOS, the system Python/LLDB are compatible. Many users install | ||
| // Homebrew Python but not Homebrew LLVM, so default to system Python. | ||
| // Can be overridden via `build.python` for custom LLVM installations. | ||
| self.config.python.as_deref().unwrap_or_else(|| Path::new("/usr/bin/python3")) |
Member
There was a problem hiding this comment.
Suggestion: can you also update the doc on bootstrap.example.toml for build.python?
Lines 322 to 326 in c90bcb9
Contributor
Author
There was a problem hiding this comment.
Wording is adjusted, please take a look. rustbot also suggested I modify src/bootstrap/src/utils/change_tracker.rs. I'll add a note there too.
Member
There was a problem hiding this comment.
Yes, an info-level advisory is indeed wise.
Member
|
@bors try jobs=aarch64-apple |
This comment has been minimized.
This comment has been minimized.
rust-bors bot
added a commit
that referenced
this pull request
Nov 7, 2025
bootstrap: respect `build.python` on macOS try-job: aarch64-apple
hkBst
reviewed
Nov 7, 2025
Comment on lines
1526
to
1539
Member
There was a problem hiding this comment.
Both branches are now much more similar than before your change, which makes me wonder where the difference went. Or maybe the default is good for most non-Windows systems generally?
Member
There was a problem hiding this comment.
This PR actually makes them more consistent, in that now for *-apple-darwin vs non *-apple-darwin targets we both first try to use an explicitly configured build.python, then we try our luck of discovering an environmental Python. Or in the case of *-apple-darwin, the matter of "environmental python" is currently still hijacked due to lldb reasons cf. #148361 #123621 #134682.
Member
There was a problem hiding this comment.
Which I agree is good, but makes me think perhaps we can go even further then? "/usr/bin/python3/ is a good default on many (most?) linux systems.
Member
There was a problem hiding this comment.
But I don't have build.python set and bootstrap still works for me on Linux, so some other place also sets a default python?
Member
There was a problem hiding this comment.
I don't think that would work for all cases, and I would prefer to keep the scope of this PR focused. I think we'd be open for a follow-up PR, though I cannot say I know for sure /usr/bin/python3 is universally valid. For instance, picking up a python3 from PATH is also a reasonable choice.
Contributor
Author
There was a problem hiding this comment.
But I don't have build.python set and bootstrap still works for me on Linux, so some other place also sets a default python?
It's ultimately set here:
https://github.com/rust-lang/rust/blob/1.91.0/src/bootstrap/src/core/sanity.rs#L166
Note that BOOTSTRAP_PYTHON is the interpreter invoking bootstrap.py, which is itself called by x.py. x.py can actually choose a different Python than what it's invoked with, e.g. switching from Python 2 if it finds a Python 3 install:
https://github.com/rust-lang/rust/blob/1.91.0/x.py#L18
build.python should be available to override all of that guesswork, except it (and all of the indirection) were ignored by a hardcoded value on macOS. This PR fixes that particular issue.
Member
There was a problem hiding this comment.
thanks @xSetech! So that makes me think finding the right python is already happening in sanity.rs, except it would fail is the PATH does not have "/usr/bin/" at higher priority than some other path that contains a python install. What does your path look like?
Collaborator
|
This PR modifies If appropriate, please update |
jieyouxu
approved these changes
Nov 7, 2025
Member
There was a problem hiding this comment.
Thanks, @xSetech! I'll r+ once the try job comes back.
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test for 37f35bb failed: CI. Failed jobs:
|
Member
|
Curious, I'll take a look tmrw at how they are wired together 🤔 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
python()method was hardcoded to return/usr/bin/python3on macOS, ignoring thebuild.pythonconfig option. This change respects the config while maintaining the system Python as the default.