Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python3 bin #596

Merged
merged 2 commits into from
Sep 8, 2019
Merged

Python3 bin #596

merged 2 commits into from
Sep 8, 2019

Conversation

expobrain
Copy link
Contributor

Explicitly use the python3 binary instead of lain python to target Python 3.x even on systems where Python 2.x is not installed

build.rs Outdated
@@ -425,22 +426,22 @@ fn find_interpreter_and_get_config() -> Result<(InterpreterConfig, HashMap<Strin
};

// check default python
let interpreter_path = "python";
// let interpreter_path = "python3";
Copy link
Member

Choose a reason for hiding this comment

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

Could you please remove this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@kngwyu
Copy link
Member

kngwyu commented Sep 7, 2019

It's a really nice refactoring 👍 , thank you!
And maybe this resolves #584?

@expobrain
Copy link
Contributor Author

@kngwyu yes, it solves exactly that issue

@kngwyu
Copy link
Member

kngwyu commented Sep 7, 2019

Ah, sorry, now I noticed the Windows CI fails because of this fix.
Wait for a minute...

@expobrain
Copy link
Contributor Author

Interesting, I'm wondering if make sense to check in order for both python3 and python binaries and use the first found

build.rs Outdated

lazy_static! {
static ref PYTHON_INTERPRETER: &'static str = {
vec!["python3", "python"]
Copy link
Member

@kngwyu kngwyu Sep 8, 2019

Choose a reason for hiding this comment

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

Suggested change
vec!["python3", "python"]
["python3", "python"]
.iter()
.find_map(|bin| Command::new(bin).spawn().ok().map(|_| bin))
.expect("Python 3.x interpreter not found")

@kngwyu
Copy link
Member

kngwyu commented Sep 8, 2019

Thanks!
I left a comment about coding, but other than that LGTM.

@kngwyu kngwyu merged commit a700e3d into PyO3:master Sep 8, 2019
@kngwyu
Copy link
Member

kngwyu commented Sep 8, 2019

Thank you!

@vadixidav
Copy link

Thank you for the fix 👍. Just saw that the issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants