Skip to content

Commit

Permalink
Merge pull request #1092 from messense/bundle-dylibs-in-target
Browse files Browse the repository at this point in the history
auditwheel: find dylibs in Cargo target directory
  • Loading branch information
messense authored Sep 9, 2022
2 parents 3432382 + 82e15c0 commit 25788bf
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 60 deletions.
28 changes: 10 additions & 18 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ ignore = "0.4.18"
dialoguer = { version = "0.10.2", default-features = false }
console = "0.15.0"
minijinja = "0.20.0"
lddtree = "0.2.9"
lddtree = "0.3.0"
cc = "1.0.72"
clap = { version = "3.2.19", features = ["derive", "env", "wrap_help"] }
clap_complete = "3.1.3"
Expand Down
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Fix abi3 wheel build when no Python interpreters found in [#1072](https://github.com/PyO3/maturin/pull/1072)
* Add `zig ar` support in [#1073](https://github.com/PyO3/maturin/pull/1073)
* Fix sdist build for optional path dependencies in [#1084](https://github.com/PyO3/maturin/pull/1084)
* auditwheel: find dylibs in Cargo target directory in [#1092](https://github.com/PyO3/maturin/pull/1092)

## [0.13.2] - 2022-08-14

Expand Down
14 changes: 11 additions & 3 deletions src/auditwheel/audit.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use super::musllinux::{find_musl_libc, get_musl_version};
use super::policy::{Policy, MANYLINUX_POLICIES, MUSLLINUX_POLICIES};
use crate::auditwheel::{find_external_libs, PlatformTag};
use crate::compile::BuildArtifact;
use crate::target::Target;
use anyhow::{bail, Context, Result};
use fs_err::File;
Expand Down Expand Up @@ -253,13 +254,14 @@ fn get_default_platform_policies() -> Vec<Policy> {
///
/// Does nothing for `platform_tag` set to `Off`/`Linux` or non-linux platforms.
pub fn auditwheel_rs(
path: &Path,
artifact: &BuildArtifact,
target: &Target,
platform_tag: Option<PlatformTag>,
) -> Result<(Policy, bool), AuditWheelError> {
if !target.is_linux() || platform_tag == Some(PlatformTag::Linux) {
return Ok((Policy::default(), false));
}
let path = &artifact.path;
let arch = target.target_arch().to_string();
let mut file = File::open(path).map_err(AuditWheelError::IoError)?;
let mut buffer = Vec::new();
Expand Down Expand Up @@ -413,7 +415,7 @@ pub fn get_sysroot_path(target: &Target) -> Result<PathBuf> {
/// For the given compilation result, return the manylinux platform and the external libs
/// we need to add to repair it
pub fn get_policy_and_libs(
artifact: &Path,
artifact: &BuildArtifact,
platform_tag: Option<PlatformTag>,
target: &Target,
) -> Result<(Policy, Vec<Library>)> {
Expand All @@ -427,7 +429,13 @@ pub fn get_policy_and_libs(
})?;
let external_libs = if should_repair {
let sysroot = get_sysroot_path(target).unwrap_or_else(|_| PathBuf::from("/"));
find_external_libs(&artifact, &policy, sysroot).with_context(|| {
find_external_libs(
&artifact.path,
&policy,
sysroot,
artifact.linked_paths.clone(),
)
.with_context(|| {
if let Some(platform_tag) = platform_tag {
format!("Error repairing wheel for {} compliance", platform_tag)
} else {
Expand Down
3 changes: 2 additions & 1 deletion src/auditwheel/repair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ pub fn find_external_libs(
artifact: impl AsRef<Path>,
policy: &Policy,
sysroot: PathBuf,
ld_paths: Vec<PathBuf>,
) -> Result<Vec<lddtree::Library>, AuditWheelError> {
let dep_analyzer = DependencyAnalyzer::new(sysroot);
let dep_analyzer = DependencyAnalyzer::new(sysroot).library_paths(ld_paths);
let deps = dep_analyzer
.analyze(artifact)
.map_err(AuditWheelError::DependencyAnalysisError)?;
Expand Down
63 changes: 35 additions & 28 deletions src/build_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use crate::module_writer::{
};
use crate::project_layout::ProjectLayout;
use crate::source_distribution::source_distribution;
use crate::{compile, Metadata21, ModuleWriter, PyProjectToml, PythonInterpreter, Target};
use crate::{
compile, BuildArtifact, Metadata21, ModuleWriter, PyProjectToml, PythonInterpreter, Target,
};
use anyhow::{anyhow, bail, Context, Result};
use cargo_metadata::Metadata;
use fs_err as fs;
Expand Down Expand Up @@ -195,7 +197,7 @@ impl BuildContext {

fn auditwheel(
&self,
artifact: &Path,
artifact: &BuildArtifact,
platform_tag: &[PlatformTag],
python_interpreter: Option<&PythonInterpreter>,
) -> Result<(Policy, Vec<Library>)> {
Expand Down Expand Up @@ -240,7 +242,7 @@ impl BuildContext {
fn add_external_libs(
&self,
writer: &mut WheelWriter,
artifacts: &[PathBuf],
artifacts: &[&BuildArtifact],
ext_libs: &[Vec<Library>],
) -> Result<()> {
if ext_libs.iter().all(|libs| libs.is_empty()) {
Expand Down Expand Up @@ -306,7 +308,7 @@ impl BuildContext {
})
.collect::<Vec<_>>();
if !replacements.is_empty() {
patchelf::replace_needed(artifact, &replacements[..])?;
patchelf::replace_needed(&artifact.path, &replacements[..])?;
}
}

Expand Down Expand Up @@ -338,14 +340,14 @@ impl BuildContext {
// Currently artifact .so file always resides at ${module_name}/${module_name}.so
let artifact_dir = Path::new(&self.module_name);
for artifact in artifacts {
let old_rpaths = patchelf::get_rpath(artifact)?;
let old_rpaths = patchelf::get_rpath(&artifact.path)?;
// TODO: clean existing rpath entries if it's not pointed to a location within the wheel
// See https://github.com/pypa/auditwheel/blob/353c24250d66951d5ac7e60b97471a6da76c123f/src/auditwheel/repair.py#L160
let mut new_rpaths: Vec<&str> = old_rpaths.split(':').collect();
let new_rpath = Path::new("$ORIGIN").join(relpath(&libs_dir, artifact_dir));
new_rpaths.push(new_rpath.to_str().unwrap());
let new_rpath = new_rpaths.join(":");
patchelf::set_rpath(artifact, &new_rpath)?;
patchelf::set_rpath(&artifact.path, &new_rpath)?;
}
Ok(())
}
Expand All @@ -359,7 +361,7 @@ impl BuildContext {

fn write_binding_wheel_abi3(
&self,
artifact: &Path,
artifact: BuildArtifact,
platform_tags: &[PlatformTag],
ext_libs: Vec<Library>,
major: u8,
Expand All @@ -371,13 +373,13 @@ impl BuildContext {
let tag = format!("cp{}{}-abi3-{}", major, min_minor, platform);

let mut writer = WheelWriter::new(&tag, &self.out, &self.metadata21, &[tag.clone()])?;
self.add_external_libs(&mut writer, &[artifact.to_path_buf()], &[ext_libs])?;
self.add_external_libs(&mut writer, &[&artifact], &[ext_libs])?;

write_bindings_module(
&mut writer,
&self.project_layout,
&self.module_name,
artifact,
&artifact.path,
None,
&self.target,
self.editable,
Expand Down Expand Up @@ -414,7 +416,7 @@ impl BuildContext {
self.platform_tag.clone()
};
let (wheel_path, tag) = self.write_binding_wheel_abi3(
&artifact,
artifact,
&platform_tags,
external_libs,
major,
Expand All @@ -435,20 +437,20 @@ impl BuildContext {
fn write_binding_wheel(
&self,
python_interpreter: &PythonInterpreter,
artifact: &Path,
artifact: BuildArtifact,
platform_tags: &[PlatformTag],
ext_libs: Vec<Library>,
) -> Result<BuiltWheelMetadata> {
let tag = python_interpreter.get_tag(&self.target, platform_tags, self.universal2)?;

let mut writer = WheelWriter::new(&tag, &self.out, &self.metadata21, &[tag.clone()])?;
self.add_external_libs(&mut writer, &[artifact.to_path_buf()], &[ext_libs])?;
self.add_external_libs(&mut writer, &[&artifact], &[ext_libs])?;

write_bindings_module(
&mut writer,
&self.project_layout,
&self.module_name,
artifact,
&artifact.path,
Some(python_interpreter),
&self.target,
self.editable,
Expand Down Expand Up @@ -490,7 +492,7 @@ impl BuildContext {
};
let (wheel_path, tag) = self.write_binding_wheel(
python_interpreter,
&artifact,
artifact,
&platform_tags,
external_libs,
)?;
Expand All @@ -517,38 +519,40 @@ impl BuildContext {
&self,
python_interpreter: Option<&PythonInterpreter>,
extension_name: Option<&str>,
) -> Result<PathBuf> {
) -> Result<BuildArtifact> {
let artifacts = compile(self, python_interpreter, &self.bridge)
.context("Failed to build a native library through cargo")?;
let error_msg = "Cargo didn't build a cdylib. Did you miss crate-type = [\"cdylib\"] \
in the lib section of your Cargo.toml?";
let artifacts = artifacts.get(0).context(error_msg)?;

let artifact = artifacts
let mut artifact = artifacts
.get("cdylib")
.cloned()
.ok_or_else(|| anyhow!(error_msg,))?;

if let Some(extension_name) = extension_name {
// globin has an issue parsing MIPS64 ELF, see https://github.com/m4b/goblin/issues/274
// But don't fail the build just because we can't emit a warning
let _ = warn_missing_py_init(&artifact, extension_name);
let _ = warn_missing_py_init(&artifact.path, extension_name);
}

if self.editable || self.skip_auditwheel {
return Ok(artifact);
}
// auditwheel repair will edit the file, so we need to copy it to avoid errors in reruns
let maturin_build = artifact.parent().unwrap().join("maturin");
let artifact_path = &artifact.path;
let maturin_build = artifact_path.parent().unwrap().join("maturin");
fs::create_dir_all(&maturin_build)?;
let new_artifact = maturin_build.join(artifact.file_name().unwrap());
fs::copy(&artifact, &new_artifact)?;
Ok(new_artifact)
let new_artifact_path = maturin_build.join(artifact_path.file_name().unwrap());
fs::copy(artifact_path, &new_artifact_path)?;
artifact.path = new_artifact_path;
Ok(artifact)
}

fn write_cffi_wheel(
&self,
artifact: &Path,
artifact: BuildArtifact,
platform_tags: &[PlatformTag],
ext_libs: Vec<Library>,
) -> Result<BuiltWheelMetadata> {
Expand All @@ -557,15 +561,15 @@ impl BuildContext {
.get_universal_tags(platform_tags, self.universal2)?;

let mut writer = WheelWriter::new(&tag, &self.out, &self.metadata21, &tags)?;
self.add_external_libs(&mut writer, &[artifact.to_path_buf()], &[ext_libs])?;
self.add_external_libs(&mut writer, &[&artifact], &[ext_libs])?;

write_cffi_module(
&mut writer,
&self.project_layout,
self.manifest_path.parent().unwrap(),
&self.target_dir,
&self.module_name,
artifact,
&artifact.path,
&self.interpreter[0].executable,
self.editable,
)?;
Expand All @@ -586,7 +590,7 @@ impl BuildContext {
} else {
self.platform_tag.clone()
};
let (wheel_path, tag) = self.write_cffi_wheel(&artifact, &platform_tags, external_libs)?;
let (wheel_path, tag) = self.write_cffi_wheel(artifact, &platform_tags, external_libs)?;

// Warn if cffi isn't specified in the requirements
if !self
Expand All @@ -610,7 +614,7 @@ impl BuildContext {
fn write_bin_wheel(
&self,
python_interpreter: Option<&PythonInterpreter>,
artifacts: &[PathBuf],
artifacts: &[BuildArtifact],
platform_tags: &[PlatformTag],
ext_libs: &[Vec<Library>],
) -> Result<BuiltWheelMetadata> {
Expand Down Expand Up @@ -639,15 +643,18 @@ impl BuildContext {
}
}

let mut artifacts_ref = Vec::with_capacity(artifacts.len());
for artifact in artifacts {
artifacts_ref.push(artifact);
// I wouldn't know of any case where this would be the wrong (and neither do
// I know a better alternative)
let bin_name = artifact
.path
.file_name()
.expect("Couldn't get the filename from the binary produced by cargo");
write_bin(&mut writer, artifact, &self.metadata21, bin_name)?;
write_bin(&mut writer, &artifact.path, &self.metadata21, bin_name)?;
}
self.add_external_libs(&mut writer, artifacts, ext_libs)?;
self.add_external_libs(&mut writer, &artifacts_ref, ext_libs)?;

self.add_pth(&mut writer)?;
add_data(&mut writer, self.project_layout.data.as_deref())?;
Expand Down
Loading

0 comments on commit 25788bf

Please sign in to comment.