Merge #1327

1327: Warn about `patchelf` version requirement r=messense a=messense See pypa/auditwheel#403 Co-authored-by: messense <[email protected]>
PyO3 · Dec 4, 2022 · a9d9260 · a9d9260
2 parents ef97b73 + 193cc74
commit a9d9260
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 13 deletions.
diff --git a/src/auditwheel/patchelf.rs b/src/auditwheel/patchelf.rs
@@ -3,6 +3,33 @@ use std::ffi::OsStr;
 use std::path::Path;
 use std::process::Command;
 
+static MISSING_PATCHELF_ERROR: &str = "Failed to execute 'patchelf', did you install it? Hint: Try `pip install maturin[patchelf]` (or just `pip install patchelf`)";
+
+/// Verify patchelf version
+pub fn verify_patchelf() -> Result<()> {
+    let output = Command::new("patchelf")
+        .arg("--version")
+        .output()
+        .context(MISSING_PATCHELF_ERROR)?;
+    let version = String::from_utf8(output.stdout)
+        .context("Failed to parse patchelf version")?
+        .trim()
+        .to_string();
+    let version = version.strip_prefix("patchelf").unwrap_or(&version).trim();
+    let semver = version
+        .parse::<semver::Version>()
+        .context("Failed to parse patchelf version")?;
+    println!("{:?}", semver);
+    if semver < semver::Version::new(0, 14, 0) {
+        // TODO: turn it into an error in 1.0
+        eprintln!(
+            "⚠️  Warning: patchelf {} found. auditwheel repair requires patchelf >= 0.14.",
+            version
+        );
+    }
+    Ok(())
+}
+
 /// Replace a declared dependency on a dynamic library with another one (`DT_NEEDED`)
 pub fn replace_needed<O: AsRef<OsStr>, N: AsRef<OsStr>>(
     file: impl AsRef<Path>,
@@ -13,9 +40,7 @@ pub fn replace_needed<O: AsRef<OsStr>, N: AsRef<OsStr>>(
         cmd.arg("--replace-needed").arg(old).arg(new);
     }
     cmd.arg(file.as_ref());
-    let output = cmd
-        .output()
-        .context("Failed to execute 'patchelf', did you install it? Hint: Try `pip install maturin[patchelf]` (or just `pip install patchelf`)")?;
+    let output = cmd.output().context(MISSING_PATCHELF_ERROR)?;
     if !output.status.success() {
         bail!(
             "patchelf --replace-needed failed: {}",
@@ -29,9 +54,7 @@ pub fn replace_needed<O: AsRef<OsStr>, N: AsRef<OsStr>>(
 pub fn set_soname<S: AsRef<OsStr>>(file: impl AsRef<Path>, soname: &S) -> Result<()> {
     let mut cmd = Command::new("patchelf");
     cmd.arg("--set-soname").arg(soname).arg(file.as_ref());
-    let output = cmd
-        .output()
-        .context("Failed to execute 'patchelf', did you install it?")?;
+    let output = cmd.output().context(MISSING_PATCHELF_ERROR)?;
     if !output.status.success() {
         bail!(
             "patchelf --set-soname failed: {}",
@@ -45,9 +68,7 @@ pub fn set_soname<S: AsRef<OsStr>>(file: impl AsRef<Path>, soname: &S) -> Result
 pub fn remove_rpath(file: impl AsRef<Path>) -> Result<()> {
     let mut cmd = Command::new("patchelf");
     cmd.arg("--remove-rpath").arg(file.as_ref());
-    let output = cmd
-        .output()
-        .context("Failed to execute 'patchelf', did you install it?")?;
+    let output = cmd.output().context(MISSING_PATCHELF_ERROR)?;
     if !output.status.success() {
         bail!(
             "patchelf --remove-rpath failed: {}",
@@ -65,9 +86,7 @@ pub fn set_rpath<S: AsRef<OsStr>>(file: impl AsRef<Path>, rpath: &S) -> Result<(
         .arg("--set-rpath")
         .arg(rpath)
         .arg(file.as_ref());
-    let output = cmd
-        .output()
-        .context("Failed to execute 'patchelf', did you install it?")?;
+    let output = cmd.output().context(MISSING_PATCHELF_ERROR)?;
     if !output.status.success() {
         bail!(
             "patchelf --set-rpath failed: {}",

diff --git a/src/build_context.rs b/src/build_context.rs
@@ -318,7 +318,7 @@ impl BuildContext {
 
     /// 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() {
+        if self.editable && self.target.is_linux() && !artifacts.is_empty() {
             for artifact in artifacts {
                 if artifact.linked_paths.is_empty() {
                     continue;
@@ -355,6 +355,9 @@ impl BuildContext {
         if ext_libs.iter().all(|libs| libs.is_empty()) {
             return Ok(());
         }
+
+        patchelf::verify_patchelf()?;
+
         // Put external libs to ${module_name}.libs directory
         // See https://github.com/pypa/auditwheel/issues/89
         let mut libs_dir = self