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.

Sign up for GitHub

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

Don't resolve python interpreter when building sdist only #2292

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Don't resolve python interpreter when building sdist only #2292

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e79141e
Don't resolve python interpreter when building sdist only
messense Nov 6, 2024
0337141
Update src/main.rs
messense Nov 13, 2024
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
2 changes: 2 additions & 0 deletions deny.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ feature-depth = 1
ignore = [
# multipart unmaintained
"RUSTSEC-2023-0050",
# derivative unmaintained
"RUSTSEC-2024-0388",
#{ id = "RUSTSEC-0000-0000", reason = "you can specify a reason the advisory is ignored" },
#"[email protected]", # you can also ignore yanked crate versions if you wish
#{ crate = "[email protected]", reason = "you can specify why you are ignoring the yanked crate" },
Expand Down
152 changes: 112 additions & 40 deletions src/build_options.rs
View file
Open in desktop
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 && env::var_os("MATURIN_TEST_PYTHON").is_none() {
// 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
Open in desktop
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
Open in desktop
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)
messense marked this conversation as resolved.
Show resolved Hide resolved
.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
Loading
Loading