Always use system `python3` on MacOS #95441

AlecGoncharow · 2022-03-29T18:30:13Z

This PR includes 2 changes:

Always use the system Python found at /usr/bin/python3 on MacOS
Removes the hard requirement on having python in your system path if you didn't specify alternatives. The proposed change will instead attempt to find and use in order: python -> python3 -> python2. This change isn't strictly necessary but without any change to this check, the original issue inspiring this change will still exist.

Fixes #95204
r? @jyn514

jyn514 · 2022-03-29T18:33:18Z

I no longer have r+ privileges.

r? @Mark-Simulacrum (but I'm happy to own this review and let you just be a rubber stamp if that's what you prefer)

jyn514

@Walther Can you confirm that this fixes cargo +beta build --bins && cargo +beta run check for you?

jyn514 · 2022-03-29T18:35:44Z

src/bootstrap/sanity.rs

+        .or_else(|| cmd_finder.maybe_have("python"))
+        .or_else(|| cmd_finder.maybe_have("python3"))
+        .or_else(|| cmd_finder.maybe_have("python2"));


Nice, this also matches what src/tools/x does :)

rust/src/tools/x/src/main.rs

Lines 28 to 43 in 05d2221

for dir in env::split_paths(&val) {

if dir.join(PYTHON).exists() {

return PYTHON;

}

python2 |= dir.join(PYTHON2).exists();

python3 |= dir.join(PYTHON3).exists();

}

if python3 {

PYTHON3

} else if python2 {

PYTHON2

} else {

PYTHON

}

I chose a slightly different order, I went with letting whatever the system deems the "One True python" would be safer to go with first. Seems that one goes python3 -> python2 -> python, happy to mimic that order.

No, your logic matches what it does: #95443

jyn514 · 2022-03-29T18:42:56Z

src/bootstrap/lib.rs

+            // (namely not Homebrew-installed python)
+            Path::new("/usr/bin/python3")
+        } else {
+            self.config.python.as_ref().unwrap()


ah, one comment - this unwrap will fail if no python at all is installed. So it would be nice to give a more helpful error message, something like

Suggested change

self.config.python.as_ref().unwrap()

self.config.python.as_ref().expect("python is required for running LLDB or rustdoc tests")

(I think right now we'll require it unconditionally for anything that uses compiletest, but we can fix that later.)

Walther · 2022-03-29T19:16:33Z

@Walther Can you confirm that this fixes cargo +beta build --bins && cargo +beta run check for you?

Can confirm, on commit 03c5f0d2182cc36548b782599412408349b13cbb running cargo +beta build --bins && cargo +beta run check completes successfully 👍 Thanks for the fix ❤️

This was on macOS Monterey 12.3 - I don't have a mac with an older OS installed so I cannot double-check that it works on the older ones. I think python3 should exist on those too, but might be worth double-checking if someone has a device that hasn't been updated to latest yet.

jyn514 · 2022-03-29T19:26:43Z

If the older OS's don't have python3, they would have failed even before this change. No one has complained so far, so I think it's fine not to worry about it.

jyn514 · 2022-04-13T00:05:05Z

@rustbot label +S-waiting-on-review

@AlecGoncharow I'm not actually part of the rust org - can you run r? @Mark-Simulacrum to assign him as the reviewer?

Mark-Simulacrum · 2022-04-13T01:13:45Z

@bors r+ rollup

bors · 2022-04-13T01:13:46Z

📌 Commit 03c5f0d has been approved by Mark-Simulacrum

jyn514 · 2022-04-13T01:18:12Z

src/bootstrap/test.rs

-        } else {
-            cmd.arg("--lldb-python").arg(builder.python());
-        }
+        cmd.arg("--lldb-python").arg(builder.python());


doesn't need to be part of this PR, but it would be nice eventually to just have --python and remove the --lldb-python / --docck-python args from compiletest.

…rk-Simulacrum Always use system `python3` on MacOS This PR includes 2 changes: 1. Always use the system Python found at `/usr/bin/python3` on MacOS 2. Removes the hard requirement on having `python` in your system path if you didn't specify alternatives. The proposed change will instead attempt to find and use in order: `python` -> `python3` -> `python2`. This change isn't strictly necessary but without any change to this check, the original issue inspiring this change will still exist. Fixes rust-lang#95204 r? `@jyn514`

…rk-Simulacrum Always use system `python3` on MacOS This PR includes 2 changes: 1. Always use the system Python found at `/usr/bin/python3` on MacOS 2. Removes the hard requirement on having `python` in your system path if you didn't specify alternatives. The proposed change will instead attempt to find and use in order: `python` -> `python3` -> `python2`. This change isn't strictly necessary but without any change to this check, the original issue inspiring this change will still exist. Fixes rust-lang#95204 r? ``@jyn514``

Rollup of 4 pull requests Successful merges: - rust-lang#95441 (Always use system `python3` on MacOS) - rust-lang#95954 (Fix broken link in coverage tools docs) - rust-lang#95984 (Fix spelling in docs for `can_not_overflow`) - rust-lang#95989 (diagnostics: regression test for spurrious "help: store this in the heap") Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

bootstrap: loosen and expand python check bounds

b3ff483

jyn514 approved these changes Mar 29, 2022

View reviewed changes

jyn514 reviewed Mar 29, 2022

View reviewed changes

AlecGoncharow force-pushed the issue-95204-fix branch from 1442b21 to f58acb3 Compare March 29, 2022 18:57

bootstrap: force system python3 on MacOS

03c5f0d

AlecGoncharow force-pushed the issue-95204-fix branch from f58acb3 to 03c5f0d Compare March 29, 2022 19:00

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 13, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2022

Mark-Simulacrum self-assigned this Apr 13, 2022

jyn514 reviewed Apr 13, 2022

View reviewed changes

Dylan-DPC mentioned this pull request Apr 13, 2022

Rollup of 4 pull requests #95997

Closed

This was referenced Apr 13, 2022

Rollup of 4 pull requests #95998

Closed

Rollup of 4 pull requests #95999

Merged

bors merged commit f6dfbfe into rust-lang:master Apr 13, 2022

rustbot added this to the 1.62.0 milestone Apr 13, 2022

jyn514 mentioned this pull request Apr 13, 2022

compiletest: only take a --python argument and remove the --lldb-python / --docck-python args #96011

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Always use system `python3` on MacOS #95441

Always use system `python3` on MacOS #95441

AlecGoncharow commented Mar 29, 2022

jyn514 commented Mar 29, 2022

jyn514 left a comment

jyn514 Mar 29, 2022

AlecGoncharow Mar 29, 2022 •

edited

Loading

jyn514 Mar 29, 2022

jyn514 Mar 29, 2022 •

edited

Loading

Walther commented Mar 29, 2022

jyn514 commented Mar 29, 2022

jyn514 commented Apr 13, 2022

Mark-Simulacrum commented Apr 13, 2022

bors commented Apr 13, 2022

jyn514 Apr 13, 2022 •

edited

Loading

	for dir in env::split_paths(&val) {
	if dir.join(PYTHON).exists() {
	return PYTHON;
	}

	python2 \|= dir.join(PYTHON2).exists();
	python3 \|= dir.join(PYTHON3).exists();
	}

	if python3 {
	PYTHON3
	} else if python2 {
	PYTHON2
	} else {
	PYTHON
	}

	self.config.python.as_ref().unwrap()
	self.config.python.as_ref().expect("python is required for running LLDB or rustdoc tests")

Always use system python3 on MacOS #95441

Always use system python3 on MacOS #95441

Conversation

AlecGoncharow commented Mar 29, 2022

jyn514 commented Mar 29, 2022

jyn514 left a comment

Choose a reason for hiding this comment

jyn514 Mar 29, 2022

Choose a reason for hiding this comment

AlecGoncharow Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

jyn514 Mar 29, 2022

Choose a reason for hiding this comment

jyn514 Mar 29, 2022 • edited Loading

Choose a reason for hiding this comment

Walther commented Mar 29, 2022

jyn514 commented Mar 29, 2022

jyn514 commented Apr 13, 2022

Mark-Simulacrum commented Apr 13, 2022

bors commented Apr 13, 2022

jyn514 Apr 13, 2022 • edited Loading

Choose a reason for hiding this comment

Always use system `python3` on MacOS #95441

Always use system `python3` on MacOS #95441

AlecGoncharow Mar 29, 2022 •

edited

Loading

jyn514 Mar 29, 2022 •

edited

Loading

jyn514 Apr 13, 2022 •

edited

Loading