From b6965389624b53f7c1093a26b6f2a3685a24c284 Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 6 Jan 2025 14:20:48 +0100 Subject: [PATCH 1/2] exclude packages not in the dependency tree when finding bindings, fix #2423 --- src/build_options.rs | 47 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/src/build_options.rs b/src/build_options.rs index be8e5b5f0..628905b2b 100644 --- a/src/build_options.rs +++ b/src/build_options.rs @@ -6,7 +6,7 @@ use crate::pyproject_toml::ToolMaturin; use crate::python_interpreter::{InterpreterConfig, InterpreterKind}; use crate::{Bindings, BridgeModel, BuildContext, PythonInterpreter, Target}; use anyhow::{bail, format_err, Context, Result}; -use cargo_metadata::{CrateType, TargetKind}; +use cargo_metadata::{CrateType, PackageId, TargetKind}; use cargo_metadata::{Metadata, Node}; use cargo_options::heading; use pep440_rs::VersionSpecifiers; @@ -1034,18 +1034,47 @@ fn find_bindings( } } -/// Tries to determine the [BridgeModel] for the target crate -pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result { +/// Return a map with all (transitive) dependencies of the *current* crate. +/// This is different from `metadata.resolve`, which also includes packages +/// that are used in the same workspace, but on which the current crate does not depend. +fn current_crate_dependencies(cargo_metadata: &Metadata) -> Result> { let resolve = cargo_metadata .resolve .as_ref() - .ok_or_else(|| format_err!("Expected to get a dependency graph from cargo"))?; + .context("Expected to get a dependency graph from cargo")?; + let root = resolve + .root + .as_ref() + .context("expected to get a root package")?; + let nodes: HashMap<&PackageId, &Node> = + resolve.nodes.iter().map(|node| (&node.id, node)).collect(); + + // Walk the dependency tree to get all (in)direct children. + let mut dep_ids = HashSet::with_capacity(nodes.len()); + let mut todo = Vec::from([root]); + while let Some(id) = todo.pop() { + for dep in nodes[id].deps.iter() { + if dep_ids.contains(&dep.pkg) { + continue; + } + dep_ids.insert(&dep.pkg); + todo.push(&dep.pkg); + } + } - let deps: HashMap<&str, &Node> = resolve - .nodes - .iter() - .map(|node| (cargo_metadata[&node.id].name.as_ref(), node)) - .collect(); + Ok(nodes + .into_iter() + .filter_map(|(id, node)| { + dep_ids + .contains(&id) + .then_some((cargo_metadata[id].name.as_ref(), node)) + }) + .collect()) +} + +/// Tries to determine the [BridgeModel] for the target crate +pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result { + let deps = current_crate_dependencies(cargo_metadata)?; let packages: HashMap<&str, &cargo_metadata::Package> = cargo_metadata .packages .iter() From 0c1ebf44cde97786af541826aeb472ce89de512c Mon Sep 17 00:00:00 2001 From: Maximilian Hils Date: Mon, 6 Jan 2025 14:21:21 +0100 Subject: [PATCH 2/2] make `has_abi3` use existing dependency map --- src/build_options.rs | 58 ++++++++++++++++++-------------------------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/src/build_options.rs b/src/build_options.rs index 628905b2b..97889d6b7 100644 --- a/src/build_options.rs +++ b/src/build_options.rs @@ -934,46 +934,34 @@ fn filter_cargo_targets( } /// pyo3 supports building abi3 wheels if the unstable-api feature is not selected -fn has_abi3(cargo_metadata: &Metadata) -> Result> { - let resolve = cargo_metadata - .resolve - .as_ref() - .context("Expected cargo to return metadata with resolve")?; +fn has_abi3(deps: &HashMap<&str, &Node>) -> Result> { for &lib in PYO3_BINDING_CRATES.iter() { - let pyo3_packages = resolve - .nodes - .iter() - .filter(|package| cargo_metadata[&package.id].name.as_str() == lib) - .collect::>(); - match pyo3_packages.as_slice() { - [pyo3_crate] => { - // Find the minimal abi3 python version. If there is none, abi3 hasn't been selected - // This parser abi3-py{major}{minor} and returns the minimal (major, minor) tuple - let abi3_selected = pyo3_crate.features.iter().any(|x| x == "abi3"); + if let Some(pyo3_crate) = deps.get(lib) { + // Find the minimal abi3 python version. If there is none, abi3 hasn't been selected + // This parser abi3-py{major}{minor} and returns the minimal (major, minor) tuple + let abi3_selected = pyo3_crate.features.iter().any(|x| x == "abi3"); - let min_abi3_version = pyo3_crate - .features - .iter() - .filter(|x| x.starts_with("abi3-py") && x.len() >= "abi3-pyxx".len()) - .map(|x| { - Ok(( - (x.as_bytes()[7] as char).to_string().parse::()?, - x[8..].parse::()?, - )) - }) - .collect::>>() - .context(format!("Bogus {lib} cargo features"))? - .into_iter() - .min(); - if abi3_selected && min_abi3_version.is_none() { - bail!( + let min_abi3_version = pyo3_crate + .features + .iter() + .filter(|x| x.starts_with("abi3-py") && x.len() >= "abi3-pyxx".len()) + .map(|x| { + Ok(( + (x.as_bytes()[7] as char).to_string().parse::()?, + x[8..].parse::()?, + )) + }) + .collect::>>() + .context(format!("Bogus {lib} cargo features"))? + .into_iter() + .min(); + if abi3_selected && min_abi3_version.is_none() { + bail!( "You have selected the `abi3` feature but not a minimum version (e.g. the `abi3-py36` feature). \ maturin needs a minimum version feature to build abi3 wheels." ) - } - return Ok(min_abi3_version); } - _ => continue, + return Ok(min_abi3_version); } } Ok(None) @@ -1170,7 +1158,7 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result