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.

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

Add library search paths in Cargo target directory to rpath in editable mode on Linux #1094

Merged
merged 1 commit into from
Sep 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* 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)
* Add library search paths in Cargo target directory to rpath in editable mode on Linux in [#1094](https://github.com/PyO3/maturin/pull/1094)

## [0.13.2] - 2022-08-14

Expand Down
9 changes: 2 additions & 7 deletions src/auditwheel/audit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,13 +429,8 @@ 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.path,
&policy,
sysroot,
artifact.linked_paths.clone(),
)
.with_context(|| {
let ld_paths = artifact.linked_paths.iter().map(PathBuf::from).collect();
find_external_libs(&artifact.path, &policy, sysroot, ld_paths).with_context(|| {
if let Some(platform_tag) = platform_tag {
format!("Error repairing wheel for {} compliance", platform_tag)
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/auditwheel/patchelf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub fn set_soname<S: AsRef<OsStr>>(file: impl AsRef<Path>, soname: &S) -> Result
Ok(())
}

/// /// Remove a `RPATH` from executables and libraries
/// Remove a `RPATH` from executables and libraries
pub fn remove_rpath(file: impl AsRef<Path>) -> Result<()> {
let mut cmd = Command::new("patchelf");
cmd.arg("--remove-rpath").arg(file.as_ref());
Expand Down
36 changes: 34 additions & 2 deletions src/build_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ impl BuildContext {
platform_tag: &[PlatformTag],
python_interpreter: Option<&PythonInterpreter>,
) -> Result<(Policy, Vec<Library>)> {
if self.skip_auditwheel || self.editable {
if self.skip_auditwheel {
return Ok((Policy::default(), Vec::new()));
}

Expand Down Expand Up @@ -239,12 +239,43 @@ impl BuildContext {
get_policy_and_libs(artifact, tag, &self.target)
}

/// Add library search paths in Cargo target directory rpath when building in editable mode
fn add_rpath(&self, artifacts: &[&BuildArtifact]) -> Result<()> {
if self.editable && self.target.is_linux() {
for artifact in artifacts {
if artifact.linked_paths.is_empty() {
continue;
}
let old_rpaths = patchelf::get_rpath(&artifact.path)?;
let mut new_rpaths: Vec<&str> =
old_rpaths.split(':').filter(|r| !r.is_empty()).collect();
for path in &artifact.linked_paths {
if !old_rpaths.contains(path) {
new_rpaths.push(path);
}
}
let new_rpath = new_rpaths.join(":");
if let Err(err) = patchelf::set_rpath(&artifact.path, &new_rpath) {
println!(
"⚠️ Warning: Failed to set rpath for {}: {}",
artifact.path.display(),
err
);
}
}
}
Ok(())
}

fn add_external_libs(
&self,
writer: &mut WheelWriter,
artifacts: &[&BuildArtifact],
ext_libs: &[Vec<Library>],
) -> Result<()> {
if self.editable {
return self.add_rpath(artifacts);
}
if ext_libs.iter().all(|libs| libs.is_empty()) {
return Ok(());
}
Expand Down Expand Up @@ -343,7 +374,8 @@ impl BuildContext {
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 mut new_rpaths: Vec<&str> =
old_rpaths.split(':').filter(|r| !r.is_empty()).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(":");
Expand Down
6 changes: 3 additions & 3 deletions src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct BuildArtifact {
pub path: PathBuf,
/// Array of paths to include in the library search path, as indicated by
/// the `cargo:rustc-link-search` instruction.
pub linked_paths: Vec<PathBuf>,
pub linked_paths: Vec<String>,
}

/// Builds the rust crate into a native module (i.e. an .so or .dll) for a
Expand Down Expand Up @@ -445,9 +445,9 @@ fn compile_target(
for path in msg.linked_paths.iter().map(|p| p.as_str()) {
// `linked_paths` may include a "KIND=" prefix in the string where KIND is the library kind
if let Some(index) = path.find('=') {
linked_paths.push(PathBuf::from(&path[index + 1..]));
linked_paths.push(path[index + 1..].to_string());
} else {
linked_paths.push(PathBuf::from(path));
linked_paths.push(path.to_string());
}
}
}
Expand Down
5 changes: 1 addition & 4 deletions src/module_writer.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
//! The wheel format is (mostly) specified in PEP 427

use crate::project_layout::ProjectLayout;
use crate::PythonInterpreter;
use crate::Target;
use crate::{BridgeModel, Metadata21};
use crate::{BridgeModel, Metadata21, PythonInterpreter, Target};
use anyhow::{anyhow, bail, Context, Result};
use flate2::write::GzEncoder;
use flate2::Compression;
Expand Down