Skip to content

Commit

Permalink
Add bindings detection to bin targets
Browse files Browse the repository at this point in the history
  • Loading branch information
@messense
messense committed May 27, 2022
1 parent 890494c commit 0a91da6
Show file tree
Hide file tree
Showing 10 changed files with 432 additions and 56 deletions.
1 change: 1 addition & 0 deletions Changelog.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Add network proxy support to upload command in [#939](https://github.com/PyO3/maturin/pull/939)
* Fix python interpreter detection on arm64 Windows in [#940](https://github.com/PyO3/maturin/pull/940)
* Fallback to `py -X.Y` when `pythonX.Y` cannot be found on Windows in [#943](https://github.com/PyO3/maturin/pull/943)
* Add bindings detection to bin targets in [#938](https://github.com/PyO3/maturin/pull/938)

## [0.12.17] - 2022-05-18

Expand Down
61 changes: 51 additions & 10 deletions src/build_context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ pub enum BridgeModel {
/// A native module with c bindings, i.e. `#[no_mangle] extern "C" <some item>`
Cffi,
/// A rust binary to be shipped a python package
Bin,
/// The String is the name of the bindings
/// providing crate, e.g. pyo3, the number is the minimum minor python version
Bin(Option<(String, usize)>),
/// A native module with pyo3 or rust-cpython bindings. The String is the name of the bindings
/// providing crate, e.g. pyo3, the number is the minimum minor python version
Bindings(String, usize),
Expand All @@ -46,17 +48,24 @@ impl BridgeModel {
/// Test whether this is using a specific bindings crate
pub fn is_bindings(&self, name: &str) -> bool {
match self {
BridgeModel::Bin(Some((value, _))) => value == name,
BridgeModel::Bindings(value, _) => value == name,
_ => false,
}
}

/// Test whether this is bin bindings
pub fn is_bin(&self) -> bool {
matches!(self, BridgeModel::Bin(_))
}
}

impl Display for BridgeModel {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
BridgeModel::Cffi => write!(f, "cffi"),
BridgeModel::Bin => write!(f, "bin"),
BridgeModel::Bin(Some((name, _))) => write!(f, "{} bin", name),
BridgeModel::Bin(None) => write!(f, "bin"),
BridgeModel::Bindings(name, _) => write!(f, "{}", name),
BridgeModel::BindingsAbi3(..) => write!(f, "pyo3"),
}
Expand Down Expand Up @@ -213,7 +222,8 @@ impl BuildContext {

let wheels = match &self.bridge {
BridgeModel::Cffi => self.build_cffi_wheel()?,
BridgeModel::Bin => self.build_bin_wheel()?,
BridgeModel::Bin(None) => self.build_bin_wheel(None)?,
BridgeModel::Bin(Some(..)) => self.build_bin_wheels(&self.interpreter)?,
BridgeModel::Bindings(..) => self.build_binding_wheels(&self.interpreter)?,
BridgeModel::BindingsAbi3(major, minor) => {
let cpythons: Vec<_> = self
Expand Down Expand Up @@ -309,7 +319,7 @@ impl BuildContext {
.collect();
others.sort();

if matches!(self.bridge, BridgeModel::Bin) && !musllinux.is_empty() {
if self.bridge.is_bin() && !musllinux.is_empty() {
return get_policy_and_libs(artifact, Some(musllinux[0]), &self.target);
}

Expand Down Expand Up @@ -669,13 +679,22 @@ impl BuildContext {

fn write_bin_wheel(
&self,
python_interpreter: Option<&PythonInterpreter>,
artifact: &Path,
platform_tags: &[PlatformTag],
ext_libs: &[Library],
) -> Result<BuiltWheelMetadata> {
let (tag, tags) = self
.target
.get_universal_tags(platform_tags, self.universal2)?;
let (tag, tags) = match (&self.bridge, python_interpreter) {
(BridgeModel::Bin(None), _) => self
.target
.get_universal_tags(platform_tags, self.universal2)?,
(BridgeModel::Bin(Some(..)), Some(python_interpreter)) => {
let tag =
python_interpreter.get_tag(&self.target, platform_tags, self.universal2)?;
(tag.clone(), vec![tag])
}
_ => unreachable!(),
};

if !self.metadata21.scripts.is_empty() {
bail!("Defining entrypoints and working with a binary doesn't mix well");
Expand Down Expand Up @@ -708,9 +727,12 @@ impl BuildContext {
/// Builds a wheel that contains a binary
///
/// Runs [auditwheel_rs()] if not deactivated
pub fn build_bin_wheel(&self) -> Result<Vec<BuiltWheelMetadata>> {
pub fn build_bin_wheel(
&self,
python_interpreter: Option<&PythonInterpreter>,
) -> Result<Vec<BuiltWheelMetadata>> {
let mut wheels = Vec::new();
let artifacts = compile(self, None, &self.bridge)
let artifacts = compile(self, python_interpreter, &self.bridge)
.context("Failed to build a native library through cargo")?;

let artifact = artifacts
Expand All @@ -725,12 +747,31 @@ impl BuildContext {
self.platform_tag.clone()
};

let (wheel_path, tag) = self.write_bin_wheel(&artifact, &platform_tags, &external_libs)?;
let (wheel_path, tag) = self.write_bin_wheel(
python_interpreter,
&artifact,
&platform_tags,
&external_libs,
)?;
println!("📦 Built wheel to {}", wheel_path.display());
wheels.push((wheel_path, tag));

Ok(wheels)
}

/// Builds a wheel that contains a binary
///
/// Runs [auditwheel_rs()] if not deactivated
pub fn build_bin_wheels(
&self,
interpreters: &[PythonInterpreter],
) -> Result<Vec<BuiltWheelMetadata>> {
let mut wheels = Vec::new();
for python_interpreter in interpreters {
wheels.extend(self.build_bin_wheel(Some(python_interpreter))?);
}
Ok(wheels)
}
}

/// Calculate the sha256 of a file
Expand Down
111 changes: 71 additions & 40 deletions src/build_options.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ impl BuildOptions {
}),
)?;

if bridge != BridgeModel::Bin && module_name.contains('-') {
if !bridge.is_bin() && module_name.contains('-') {
bail!(
"The module name must not contains a minus \
(Make sure you have set an appropriate [lib] name in your Cargo.toml)"
Expand Down Expand Up @@ -349,7 +349,7 @@ impl BuildOptions {
}
} else {
// Defaults to musllinux_1_2 for musl target if it's not bin bindings
if target.is_musl_target() && !matches!(bridge, BridgeModel::Bin) {
if target.is_musl_target() && !bridge.is_bin() {
Some(PlatformTag::Musllinux { x: 1, y: 2 })
} else {
None
Expand All @@ -371,8 +371,8 @@ impl BuildOptions {
}

match bridge {
BridgeModel::Bin => {
// Only support two different kind of platform tags when compiling to musl target
BridgeModel::Bin(None) => {
// Only support two different kind of platform tags when compiling to musl target without any binding crates
if platform_tags.iter().any(|tag| tag.is_musllinux()) && !target.is_musl_target() {
bail!(
"Cannot mix musllinux and manylinux platform tags when compiling to {}",
Expand Down Expand Up @@ -550,6 +550,28 @@ fn is_generating_import_lib(cargo_metadata: &Metadata) -> Result<bool> {
Ok(false)
}

/// Tries to determine the bindings type from dependency
fn find_bindings(
deps: &HashMap<&str, &Node>,
packages: &HashMap<&str, &cargo_metadata::Package>,
) -> Option<(String, usize)> {
if deps.get("pyo3").is_some() {
let ver = &packages["pyo3"].version;
let minor =
pyo3_minimum_python_minor_version(ver.major, ver.minor).unwrap_or(MINIMUM_PYTHON_MINOR);
Some(("pyo3".to_string(), minor))
} else if deps.get("pyo3-ffi").is_some() {
let ver = &packages["pyo3-ffi"].version;
let minor = pyo3_ffi_minimum_python_minor_version(ver.major, ver.minor)
.unwrap_or(MINIMUM_PYTHON_MINOR);
Some(("pyo3-ffi".to_string(), minor))
} else if deps.contains_key("cpython") {
Some(("rust-cpython".to_string(), MINIMUM_PYTHON_MINOR))
} else {
None
}
}

/// Tries to determine the [BridgeModel] for the target crate
pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<BridgeModel> {
let resolve = cargo_metadata
Expand All @@ -574,13 +596,21 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br
}
})
.collect();
let root_package = cargo_metadata
.root_package()
.context("Expected cargo to return metadata with root_package")?;
let targets: Vec<_> = root_package
.targets
.iter()
.flat_map(|target| target.crate_types.iter())
.map(String::as_str)
.collect();

let bridge = if let Some(bindings) = bridge {
if bindings == "cffi" {
BridgeModel::Cffi
} else if bindings == "bin" {
println!("🔗 Found bin bindings");
BridgeModel::Bin
BridgeModel::Bin(find_bindings(&deps, &packages))
} else {
if !deps.contains_key(bindings) {
bail!(
Expand All @@ -591,41 +621,26 @@ pub fn find_bridge(cargo_metadata: &Metadata, bridge: Option<&str>) -> Result<Br

BridgeModel::Bindings(bindings.to_string(), MINIMUM_PYTHON_MINOR)
}
} else if deps.get("pyo3").is_some() {
let ver = &packages["pyo3"].version;
let minor =
pyo3_minimum_python_minor_version(ver.major, ver.minor).unwrap_or(MINIMUM_PYTHON_MINOR);
BridgeModel::Bindings("pyo3".to_string(), minor)
} else if deps.get("pyo3-ffi").is_some() {
let ver = &packages["pyo3-ffi"].version;
let minor = pyo3_ffi_minimum_python_minor_version(ver.major, ver.minor)
.unwrap_or(MINIMUM_PYTHON_MINOR);
BridgeModel::Bindings("pyo3-ffi".to_string(), minor)
} else if deps.contains_key("cpython") {
println!("🔗 Found rust-cpython bindings");
BridgeModel::Bindings("rust_cpython".to_string(), MINIMUM_PYTHON_MINOR)
} else {
let package = cargo_metadata
.root_package()
.context("Expected cargo to return metadata with root_package")?;
let targets: Vec<_> = package
.targets
.iter()
.flat_map(|target| target.crate_types.iter())
.map(String::as_str)
.collect();

if targets.contains(&"cdylib") {
BridgeModel::Cffi
} else if targets.contains(&"bin") {
BridgeModel::Bin
} else if let Some((bindings, minor)) = find_bindings(&deps, &packages) {
if !targets.contains(&"cdylib") && targets.contains(&"bin") {
BridgeModel::Bin(Some((bindings, minor)))
} else {
bail!("Couldn't detect the binding type; Please specify them with --bindings/-b")
BridgeModel::Bindings(bindings, minor)
}
} else if targets.contains(&"cdylib") {
BridgeModel::Cffi
} else if targets.contains(&"bin") {
BridgeModel::Bin(find_bindings(&deps, &packages))
} else {
bail!("Couldn't detect the binding type; Please specify them with --bindings/-b")
};

if !(bridge.is_bindings("pyo3") || bridge.is_bindings("pyo3-ffi")) {
println!("🔗 Found {} bindings", bridge);
}

for &lib in PYO3_BINDING_CRATES.iter() {
if bridge.is_bindings(lib) {
if !bridge.is_bin() && bridge.is_bindings(lib) {
let pyo3_node = deps[lib];
if !pyo3_node.features.contains(&"extension-module".to_string()) {
let version = cargo_metadata[&pyo3_node.id].version.to_string();
Expand Down Expand Up @@ -771,7 +786,7 @@ pub fn find_interpreter(
generate_import_lib: bool,
) -> Result<Vec<PythonInterpreter>> {
match bridge {
BridgeModel::Bindings(binding_name, _) => {
BridgeModel::Bindings(binding_name, _) | BridgeModel::Bin(Some((binding_name, _))) => {
let mut native_interpreters = false;
let mut interpreters = Vec::new();
if let Some(config_file) = env::var_os("PYO3_CONFIG_FILE") {
Expand Down Expand Up @@ -893,7 +908,7 @@ pub fn find_interpreter(
println!("🐍 Using {} to generate the cffi bindings", interpreter);
Ok(vec![interpreter])
}
BridgeModel::Bin => Ok(vec![]),
BridgeModel::Bin(None) => Ok(vec![]),
BridgeModel::BindingsAbi3(major, minor) => {
let interpreter = if !interpreter.is_empty() {
PythonInterpreter::check_executables(interpreter, target, bridge)
Expand Down Expand Up @@ -1125,12 +1140,28 @@ mod test {

assert_eq!(
find_bridge(&hello_world, Some("bin")).unwrap(),
BridgeModel::Bin
BridgeModel::Bin(None)
);
assert_eq!(
find_bridge(&hello_world, None).unwrap(),
BridgeModel::Bin(None)
);
assert_eq!(find_bridge(&hello_world, None).unwrap(), BridgeModel::Bin);

assert!(find_bridge(&hello_world, Some("rust-cpython")).is_err());
assert!(find_bridge(&hello_world, Some("pyo3")).is_err());

let pyo3_bin = MetadataCommand::new()
.manifest_path(&Path::new("test-crates/pyo3-bin").join("Cargo.toml"))
.exec()
.unwrap();
assert!(matches!(
find_bridge(&pyo3_bin, Some("bin")).unwrap(),
BridgeModel::Bin(Some((..)))
));
assert!(matches!(
find_bridge(&pyo3_bin, None).unwrap(),
BridgeModel::Bin(Some(..))
));
}

#[test]
Expand Down
9 changes: 5 additions & 4 deletions src/compile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ fn compile_universal2(
python_interpreter: Option<&PythonInterpreter>,
bindings_crate: &BridgeModel,
) -> Result<HashMap<String, PathBuf>> {
let build_type = match bindings_crate {
BridgeModel::Bin => "bin",
_ => "cdylib",
let build_type = if bindings_crate.is_bin() {
"bin"
} else {
"cdylib"
};
let mut aarch64_context = context.clone();
aarch64_context.cargo_extra_args.extend(vec![
Expand Down Expand Up @@ -137,7 +138,7 @@ fn compile_target(
// We need to pass --bins / --lib to set the rustc extra args later
// TODO: What do we do when there are multiple bin targets?
match bindings_crate {
BridgeModel::Bin => shared_args.push("--bins"),
BridgeModel::Bin(..) => shared_args.push("--bins"),
BridgeModel::Cffi | BridgeModel::Bindings(..) | BridgeModel::BindingsAbi3(..) => {
shared_args.push("--lib");
// https://github.com/rust-lang/rust/issues/59302#issue-422994250
Expand Down
4 changes: 2 additions & 2 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {

// Since afaik all other PEP 517 backends also return linux tagged wheels, we do so too
let tags = match context.bridge {
BridgeModel::Bindings(..) => {
BridgeModel::Bindings(..) | BridgeModel::Bin(Some(..)) => {
vec![context.interpreter[0].get_tag(
&context.target,
&[PlatformTag::Linux],
Expand All @@ -274,7 +274,7 @@ fn pep517(subcommand: Pep517Command) -> Result<()> {
.get_platform_tag(&[PlatformTag::Linux], context.universal2)?;
vec![format!("cp{}{}-abi3-{}", major, minor, platform)]
}
BridgeModel::Bin | BridgeModel::Cffi => {
BridgeModel::Bin(None) | BridgeModel::Cffi => {
context
.target
.get_universal_tags(&[PlatformTag::Linux], context.universal2)?
Expand Down
Loading

0 comments on commit 0a91da6

Please sign in to comment.