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

Consider requires-python when searching for interpreters #495

Merged
merged 1 commit into from
Apr 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## Unreleased

* Interpreter search now uses python 3.6 to 3.12
* Consider requires-python when searching for interpreters

## 0.10.3 - 2021-04-13

* The `upload` command is now implemented, it is mostly similar to `twine upload`. [#484](https://github.com/PyO3/maturin/pull/484)
Expand Down
45 changes: 42 additions & 3 deletions src/build_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::PythonInterpreter;
use crate::Target;
use anyhow::{bail, format_err, Context, Result};
use cargo_metadata::{Metadata, MetadataCommand, Node};
use regex::Regex;
use serde::{Deserialize, Serialize};
use std::collections::{HashMap, HashSet};
use std::env;
Expand Down Expand Up @@ -176,9 +177,9 @@ impl BuildOptions {
// Only build a source distribution
Some(ref interpreter) if interpreter.is_empty() => vec![],
// User given list of interpreters
Some(interpreter) => find_interpreter(&bridge, &interpreter, &target)?,
Some(interpreter) => find_interpreter(&bridge, &interpreter, &target, None)?,
Copy link
Member

@messense messense Apr 9, 2021

Choose a reason for hiding this comment

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

Should we also filter the user given list of interpreters if requires-python specified?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, in that case I'd trust the users list more than the PEP 440 heuristic

// Auto-detect interpreters
None => find_interpreter(&bridge, &[], &target)?,
None => find_interpreter(&bridge, &[], &target, get_min_python_minor(&metadata21))?,
};

let rustc_extra_args = split_extra_args(&self.rustc_extra_args)?;
Expand Down Expand Up @@ -225,6 +226,29 @@ impl BuildOptions {
}
}

/// Uses very simple PEP 440 subset parsing to determine the
/// minimum supported python minor version for interpreter search
fn get_min_python_minor(metadata21: &Metadata21) -> Option<usize> {
if let Some(requires_python) = &metadata21.requires_python {
let regex = Regex::new(r#"(?:\^|>=)3\.(\d+)(?:\.\d)?"#).unwrap();
if let Some(captures) = regex.captures(&requires_python) {
let min_python_minor = captures[1]
.parse::<usize>()
.expect("Regex must only match usize");
Some(min_python_minor)
} else {
println!(
"⚠ Couldn't parse the value of requires-python, \
not taking it into account when searching for python interpreter.\
Note: Only the forms `^3.x` and `>=3.x.y` are currently supported."
);
None
}
} else {
None
}
}

/// pyo3 supports building abi3 wheels if the unstable-api feature is not selected
fn has_abi3(cargo_metadata: &Metadata) -> Result<Option<(u8, u8)>> {
let resolve = cargo_metadata
Expand Down Expand Up @@ -384,14 +408,15 @@ pub fn find_interpreter(
bridge: &BridgeModel,
interpreter: &[PathBuf],
target: &Target,
min_python_minor: Option<usize>,
) -> Result<Vec<PythonInterpreter>> {
match bridge {
BridgeModel::Bindings(_) => {
let interpreter = if !interpreter.is_empty() {
PythonInterpreter::check_executables(&interpreter, &target, &bridge)
.context("The given list of python interpreters is invalid")?
} else {
PythonInterpreter::find_all(&target, &bridge)
PythonInterpreter::find_all(&target, &bridge, min_python_minor)
.context("Finding python interpreters failed")?
};

Expand Down Expand Up @@ -660,4 +685,18 @@ mod test {

assert_eq!(extract_cargo_metadata_args(&args).unwrap(), expected);
}

#[test]
fn test_get_min_python_minor() {
// Nothing specified
let cargo_toml = CargoToml::from_path("test-crates/pyo3-pure/Cargo.toml").unwrap();
let metadata21 =
Metadata21::from_cargo_toml(&cargo_toml, &"test-crates/pyo3-pure").unwrap();
assert_eq!(get_min_python_minor(&metadata21), None);
// ^3.7
let cargo_toml = CargoToml::from_path("test-crates/pyo3-mixed/Cargo.toml").unwrap();
let metadata21 =
Metadata21::from_cargo_toml(&cargo_toml, &"test-crates/pyo3-mixed").unwrap();
assert_eq!(get_min_python_minor(&metadata21), Some(7));
}
}
2 changes: 1 addition & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ fn run() -> Result<()> {
Opt::ListPython => {
let target = Target::from_target_triple(None)?;
// We don't know the targeted bindings yet, so we use the most lenient
let found = PythonInterpreter::find_all(&target, &BridgeModel::Cffi)?;
let found = PythonInterpreter::find_all(&target, &BridgeModel::Cffi, None)?;
println!("🐍 {} python interpreter found:", found.len());
for interpreter in found {
println!(" - {}", interpreter);
Expand Down
29 changes: 15 additions & 14 deletions src/python_interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ fn windows_interpreter_no_build(
minor: usize,
target_width: usize,
pointer_width: usize,
min_python_minor: usize,
) -> bool {
// Python 2 support has been dropped
if major == 2 {
return true;
}

// Ignore python 3.0 - 3.5
if major == 3 && minor < MINIMUM_PYTHON_MINOR {
if major == 3 && minor < min_python_minor {
return true;
}

Expand Down Expand Up @@ -83,7 +84,7 @@ fn windows_interpreter_no_build(
/// As well as the version numbers, etc. of the interpreters we also have to find the
/// pointer width to make sure that the pointer width (32-bit or 64-bit) matches across
/// platforms.
fn find_all_windows(target: &Target) -> Result<Vec<String>> {
fn find_all_windows(target: &Target, min_python_minor: usize) -> Result<Vec<String>> {
let code = "import sys; print(sys.executable or '')";
let mut interpreter = vec![];
let mut versions_found = HashSet::new();
Expand Down Expand Up @@ -123,6 +124,7 @@ fn find_all_windows(target: &Target) -> Result<Vec<String>> {
minor,
target.pointer_width(),
pointer_width,
min_python_minor,
) {
continue;
}
Expand Down Expand Up @@ -201,6 +203,7 @@ fn find_all_windows(target: &Target) -> Result<Vec<String>> {
minor,
target.pointer_width(),
pointer_width,
min_python_minor,
) {
continue;
}
Expand All @@ -219,15 +222,6 @@ fn find_all_windows(target: &Target) -> Result<Vec<String>> {
Ok(interpreter)
}

/// Since there is no known way to list the installed python versions on unix
/// (or just generally to list all binaries in $PATH, which could then be
/// filtered down), we use this workaround.
fn find_all_unix() -> Vec<String> {
(MINIMUM_PYTHON_MINOR..MAXIMUM_PYTHON_MINOR)
.map(|minor| format!("python3.{}", minor))
.collect()
}

#[derive(Debug, Clone, Eq, PartialEq)]
pub enum InterpreterKind {
CPython,
Expand Down Expand Up @@ -482,11 +476,18 @@ impl PythonInterpreter {

/// Tries to find all installed python versions using the heuristic for the
/// given platform
pub fn find_all(target: &Target, bridge: &BridgeModel) -> Result<Vec<PythonInterpreter>> {
pub fn find_all(
target: &Target,
bridge: &BridgeModel,
min_python_minor: Option<usize>,
) -> Result<Vec<PythonInterpreter>> {
let min_python_minor = min_python_minor.unwrap_or(MINIMUM_PYTHON_MINOR);
let executables = if target.is_windows() {
find_all_windows(&target)?
find_all_windows(&target, min_python_minor)?
} else {
find_all_unix()
(min_python_minor..MAXIMUM_PYTHON_MINOR)
.map(|minor| format!("python3.{}", minor))
.collect()
};
let mut available_versions = Vec::new();
for executable in executables {
Expand Down
1 change: 1 addition & 0 deletions test-crates/pyo3-mixed/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ classifier = [
"Programming Language :: Rust"
]
requires-dist = ["boltons"]
requires-python = "^3.7"

[dependencies]
pyo3 = { version = "0.13.2", features = ["extension-module"] }
Expand Down