Skip to content

Commit

Permalink
Don't resolve python interpreter when building sdist only
Browse files Browse the repository at this point in the history
  • Loading branch information
@messense
messense committed Nov 6, 2024
1 parent dbe456b commit 78f84e7
Show file tree
Hide file tree
Showing 6 changed files with 240 additions and 61 deletions.
152 changes: 112 additions & 40 deletions src/build_options.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,12 +487,59 @@ impl BuildOptions {

/// Tries to fill the missing metadata for a BuildContext by querying cargo and python
#[instrument(skip_all)]
pub fn into_build_context(
self,
release: bool,
strip: bool,
editable: bool,
) -> Result<BuildContext> {
pub fn into_build_context(self) -> BuildContextBuilder {
BuildContextBuilder::new(self)
}
}

#[derive(Debug)]
pub struct BuildContextBuilder {
build_options: BuildOptions,
release: bool,
strip: bool,
editable: bool,
sdist_only: bool,
}

impl BuildContextBuilder {
fn new(build_options: BuildOptions) -> Self {
Self {
build_options,
release: false,
strip: false,
editable: false,
sdist_only: false,
}
}

pub fn release(mut self, release: bool) -> Self {
self.release = release;
self
}

pub fn strip(mut self, strip: bool) -> Self {
self.strip = strip;
self
}

pub fn editable(mut self, editable: bool) -> Self {
self.editable = editable;
self
}

pub fn sdist_only(mut self, sdist_only: bool) -> Self {
self.sdist_only = sdist_only;
self
}

pub fn build(self) -> Result<BuildContext> {
let Self {
build_options,
release,
strip,
editable,
sdist_only,
} = self;
let ProjectResolver {
project_layout,
cargo_toml_path,
Expand All @@ -504,12 +551,15 @@ impl BuildOptions {
mut cargo_options,
cargo_metadata,
mut pyproject_toml_maturin_options,
} = ProjectResolver::resolve(self.manifest_path.clone(), self.cargo.clone())?;
} = ProjectResolver::resolve(
build_options.manifest_path.clone(),
build_options.cargo.clone(),
)?;
let pyproject = pyproject_toml.as_ref();

let bridge = find_bridge(
&cargo_metadata,
self.bindings.as_deref().or_else(|| {
build_options.bindings.as_deref().or_else(|| {
pyproject.and_then(|x| {
if x.bindings().is_some() {
pyproject_toml_maturin_options.push("bindings");
Expand All @@ -527,7 +577,7 @@ impl BuildOptions {
);
}

let mut target_triple = self.target.clone();
let mut target_triple = build_options.target.clone();

let mut universal2 = target_triple.as_deref() == Some("universal2-apple-darwin");
// Also try to determine universal2 from ARCHFLAGS environment variable
Expand Down Expand Up @@ -560,7 +610,7 @@ impl BuildOptions {

let mut target = Target::from_target_triple(target_triple)?;
if !target.user_specified && !universal2 {
if let Some(interpreter) = self.interpreter.first() {
if let Some(interpreter) = build_options.interpreter.first() {
if let Some(detected_target) =
crate::target::detect_arch_from_python(interpreter, &target)
{
Expand All @@ -569,38 +619,23 @@ impl BuildOptions {
}
}

let wheel_dir = match self.out {
let wheel_dir = match build_options.out {
Some(ref dir) => dir.clone(),
None => PathBuf::from(&cargo_metadata.target_directory).join("wheels"),
};

let generate_import_lib = is_generating_import_lib(&cargo_metadata)?;
let interpreter = if self.find_interpreter {
// Auto-detect interpreters
self.find_interpreters(
let interpreter = if sdist_only {
// We don't need a python interpreter to build sdist only
Vec::new()
} else {
resolve_interpreters(
&build_options,
&bridge,
&[],
&target,
metadata23.requires_python.as_ref(),
generate_import_lib,
)?
} else {
// User given list of interpreters
let interpreter = if self.interpreter.is_empty() && !target.cross_compiling() {
if cfg!(test) {
match env::var_os("MATURIN_TEST_PYTHON") {
Some(python) => vec![python.into()],
None => vec![target.get_python()],
}
} else {
vec![target.get_python()]
}
} else {
// XXX: False positive clippy warning
#[allow(clippy::redundant_clone)]
self.interpreter.clone()
};
self.find_interpreters(&bridge, &interpreter, &target, None, generate_import_lib)?
};

if cargo_options.args.is_empty() {
Expand All @@ -613,19 +648,19 @@ impl BuildOptions {
}

let strip = pyproject.map(|x| x.strip()).unwrap_or_default() || strip;
let skip_auditwheel =
pyproject.map(|x| x.skip_auditwheel()).unwrap_or_default() || self.skip_auditwheel;
let auditwheel = self
let skip_auditwheel = pyproject.map(|x| x.skip_auditwheel()).unwrap_or_default()
|| build_options.skip_auditwheel;
let auditwheel = build_options
.auditwheel
.or_else(|| pyproject.and_then(|x| x.auditwheel()))
.unwrap_or(if skip_auditwheel {
AuditWheelMode::Skip
} else {
AuditWheelMode::Repair
});
let platform_tags = if self.platform_tag.is_empty() {
let platform_tags = if build_options.platform_tag.is_empty() {
#[cfg(feature = "zig")]
let use_zig = self.zig;
let use_zig = build_options.zig;
#[cfg(not(feature = "zig"))]
let use_zig = false;
let compatibility = pyproject
Expand Down Expand Up @@ -658,7 +693,7 @@ impl BuildOptions {
Vec::new()
}
} else {
self.platform_tag
build_options.platform_tag
};

for platform_tag in &platform_tags {
Expand All @@ -683,7 +718,7 @@ impl BuildOptions {
);
}

let target_dir = self
let target_dir = build_options
.cargo
.target_dir
.clone()
Expand Down Expand Up @@ -713,7 +748,7 @@ impl BuildOptions {
strip,
auditwheel,
#[cfg(feature = "zig")]
zig: self.zig,
zig: build_options.zig,
platform_tag: platform_tags,
interpreter,
cargo_metadata,
Expand All @@ -724,6 +759,43 @@ impl BuildOptions {
}
}

fn resolve_interpreters(
build_options: &BuildOptions,
bridge: &BridgeModel,
target: &Target,
requires_python: Option<&VersionSpecifiers>,
generate_import_lib: bool,
) -> Result<Vec<PythonInterpreter>, anyhow::Error> {
let interpreter = if build_options.find_interpreter {
// Auto-detect interpreters
build_options.find_interpreters(
bridge,
&[],
target,
requires_python,
generate_import_lib,
)?
} else {
// User given list of interpreters
let interpreter = if build_options.interpreter.is_empty() && !target.cross_compiling() {
if cfg!(test) {
match env::var_os("MATURIN_TEST_PYTHON") {
Some(python) => vec![python.into()],
None => vec![target.get_python()],
}
} else {
vec![target.get_python()]
}
} else {
// XXX: False positive clippy warning
#[allow(clippy::redundant_clone)]
build_options.interpreter.clone()
};
build_options.find_interpreters(bridge, &interpreter, target, None, generate_import_lib)?
};
Ok(interpreter)
}

/// Checks for bridge/platform type edge cases
fn validate_bridge_type(
bridge: &BridgeModel,
Expand Down
7 changes: 6 additions & 1 deletion src/develop.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,12 @@ pub fn develop(develop_options: DevelopOptions, venv_dir: &Path) -> Result<()> {
},
};

let build_context = build_options.into_build_context(release, strip, true)?;
let build_context = build_options
.into_build_context()
.release(release)
.strip(strip)
.editable(true)
.build()?;

let interpreter =
PythonInterpreter::check_executable(&python, &target, build_context.bridge())?.ok_or_else(
Expand Down
45 changes: 39 additions & 6 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,13 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
strip,
} => {
assert_eq!(build_options.interpreter.len(), 1);
let context = build_options.into_build_context(true, strip, false)?;
let context = build_options
.into_build_context()
.release(true)
.strip(strip)
.editable(false)
.sdist_only(true)
.build()?;

// Since afaik all other PEP 517 backends also return linux tagged wheels, we do so too
let tags = match context.bridge() {
Expand All @@ -298,7 +304,12 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
strip,
editable,
} => {
let build_context = build_options.into_build_context(true, strip, editable)?;
let build_context = build_options
.into_build_context()
.release(true)
.strip(strip)
.editable(editable)
.build()?;
let wheels = build_context.build_wheels()?;
assert_eq!(wheels.len(), 1);
println!("{}", wheels[0].0.to_str().unwrap());
Expand All @@ -318,7 +329,13 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
},
..Default::default()
};
let build_context = build_options.into_build_context(false, false, false)?;
let build_context = build_options
.into_build_context()
.release(false)
.strip(false)
.editable(false)
.sdist_only(true)
.build()?;
let (path, _) = build_context
.build_source_distribution()?
.context("Failed to build source distribution, pyproject.toml not found")?;
Expand Down Expand Up @@ -361,7 +378,12 @@ fn run() -> Result<()> {
strip,
sdist,
} => {
let build_context = build.into_build_context(release, strip, false)?;
let build_context = build
.into_build_context()
.release(release)
.strip(strip)
.editable(false)
.build()?;
if sdist {
build_context
.build_source_distribution()?
Expand All @@ -378,7 +400,12 @@ fn run() -> Result<()> {
no_strip,
no_sdist,
} => {
let build_context = build.into_build_context(!debug, !no_strip, false)?;
let build_context = build
.into_build_context()
.release(!debug)
.strip(!no_strip)
.editable(false)
.build()?;

if !build_context.release {
eprintln!("⚠️ Warning: You're publishing debug wheels");
Expand Down Expand Up @@ -427,7 +454,13 @@ fn run() -> Result<()> {
},
..Default::default()
};
let build_context = build_options.into_build_context(false, false, false)?;
let build_context = build_options
.into_build_context()
.release(false)
.strip(false)
.editable(false)
.sdist_only(true)
.build()?;
build_context
.build_source_distribution()?
.context("Failed to build source distribution, pyproject.toml not found")?;
Expand Down
26 changes: 22 additions & 4 deletions tests/common/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,12 @@ pub fn abi3_without_version() -> Result<()> {
];

let options = BuildOptions::try_parse_from(cli)?;
let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
let result = options
.into_build_context()
.release(false)
.strip(cfg!(feature = "faster-tests"))
.editable(false)
.build();
if let Err(err) = result {
assert_eq!(err.to_string(),
"You have selected the `abi3` feature but not a minimum version (e.g. the `abi3-py36` feature). \
Expand Down Expand Up @@ -48,7 +53,11 @@ pub fn pyo3_no_extension_module() -> Result<()> {

let options = BuildOptions::try_parse_from(cli)?;
let result = options
.into_build_context(false, cfg!(feature = "faster-tests"), false)?
.into_build_context()
.release(false)
.strip(cfg!(feature = "faster-tests"))
.editable(false)
.build()?
.build_wheels();
if let Err(err) = result {
if !(err
Expand Down Expand Up @@ -81,7 +90,12 @@ pub fn locked_doesnt_build_without_cargo_lock() -> Result<()> {
"test-crates/targets/locked_doesnt_build_without_cargo_lock",
];
let options = BuildOptions::try_parse_from(cli)?;
let result = options.into_build_context(false, cfg!(feature = "faster-tests"), false);
let result = options
.into_build_context()
.release(false)
.strip(cfg!(feature = "faster-tests"))
.editable(false)
.build();
if let Err(err) = result {
let err_string = err
.source()
Expand Down Expand Up @@ -115,7 +129,11 @@ pub fn invalid_manylinux_does_not_panic() -> Result<()> {
];
let options: BuildOptions = BuildOptions::try_parse_from(cli)?;
let result = options
.into_build_context(false, cfg!(feature = "faster-tests"), false)?
.into_build_context()
.release(false)
.strip(cfg!(feature = "faster-tests"))
.editable(false)
.build()?
.build_wheels();
if let Err(err) = result {
assert_eq!(err.to_string(), "Error ensuring manylinux_2_99 compliance");
Expand Down
Loading

0 comments on commit 78f84e7

Please sign in to comment.