Merge pull request #1390 from axodotdev/fix-linkage

fix: make linkage truly infallible
axodotdev · Sep 3, 2024 · bc0f80f · bc0f80f
2 parents 7fd4855 + ec8ee30
commit bc0f80f
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 40 deletions.
diff --git a/cargo-dist/src/build/mod.rs b/cargo-dist/src/build/mod.rs
@@ -173,7 +173,7 @@ impl BuildExpectations {
                 let bin = dist.binary(result_bin.idx);
 
                 // compute linkage for the binary
-                self.compute_linkage(dist, manifest, result_bin, &bin.target)?;
+                self.compute_linkage_and_sign(dist, manifest, result_bin, &bin.target)?;
 
                 // copy files to their final homes
                 self.copy_assets(result_bin, bin)?;
@@ -189,8 +189,8 @@ impl BuildExpectations {
         Ok(())
     }
 
-    // Compute the linkage info for this binary
-    fn compute_linkage(
+    // Compute the linkage info for this binary and sign it
+    fn compute_linkage_and_sign(
         &self,
         dist: &DistGraph,
         manifest: &mut DistManifest,
@@ -215,7 +215,7 @@ impl BuildExpectations {
             });
             linkage
         } else {
-            determine_linkage(src_path, target)?
+            determine_linkage(src_path, target)
         };
 
         let bin = dist.binary(src.idx);

diff --git a/cargo-dist/src/errors.rs b/cargo-dist/src/errors.rs
@@ -303,11 +303,8 @@ pub enum DistError {
     },
 
     /// Linkage report can't be run for this target
-    #[error("unable to run linkage report for this type of binary; file was: {file_name:?}")]
-    LinkageCheckUnsupportedBinary {
-        /// The object's file name
-        file_name: Option<String>,
-    },
+    #[error("unable to run linkage report for this type of binary")]
+    LinkageCheckUnsupportedBinary,
 
     /// Error parsing a string containing an environment variable
     /// in VAR=value syntax

diff --git a/cargo-dist/src/linkage.rs b/cargo-dist/src/linkage.rs
@@ -82,28 +82,7 @@ fn compute_linkage_assuming_local_build(
                 if !bin_path.exists() {
                     eprintln!("Binary {bin_path} missing; skipping check");
                 } else {
-                    let linkage = match determine_linkage(&bin_path, target) {
-                        Ok(linkage) => linkage,
-                        // We don't want to treat the primary linkage
-                        // failure check as a failure - instead, print
-                        // a warning and provide a default.
-                        Err(DistError::LinkageCheckUnsupportedBinary { file_name }) => {
-                            if let Some(file_name) = file_name {
-                                warn!("Unable to fetch linkage from {file_name}");
-                            } else {
-                                warn!("Unable to fetch linkage");
-                            }
-                            Linkage::default()
-                        }
-                        Err(e) => {
-                            warn!(
-                                "Encountered an issue trying to determine linkage: {:?}",
-                                miette::Report::new(e)
-                            );
-                            Linkage::default()
-                        }
-                    };
-
+                    let linkage = determine_linkage(&bin_path, target);
                     manifest.assets.insert(
                         bin.id.clone(),
                         AssetInfo {
@@ -394,14 +373,25 @@ fn do_pe(path: &Utf8PathBuf) -> DistResult<Vec<String>> {
         Object::PE(pe) => Ok(pe.libraries.into_iter().map(|s| s.to_owned()).collect()),
         // Static libraries link against nothing
         Object::Archive(_) => Ok(vec![]),
-        _ => Err(DistError::LinkageCheckUnsupportedBinary {
-            file_name: path.file_name().map(String::from),
-        }),
+        _ => Err(DistError::LinkageCheckUnsupportedBinary),
     }
 }
 
 /// Get the linkage for a single binary
-pub fn determine_linkage(path: &Utf8PathBuf, target: &str) -> DistResult<Linkage> {
+///
+/// If linkage fails for any reason we warn and return the default empty linkage
+pub fn determine_linkage(path: &Utf8PathBuf, target: &str) -> Linkage {
+    match try_determine_linkage(path, target) {
+        Ok(linkage) => linkage,
+        Err(e) => {
+            warn!("Skipping linkage for {path}:\n{:?}", miette::Report::new(e));
+            Linkage::default()
+        }
+    }
+}
+
+/// Get the linkage for a single binary
+fn try_determine_linkage(path: &Utf8PathBuf, target: &str) -> DistResult<Linkage> {
     let libraries = match target {
         // Can be run on any OS
         "i686-apple-darwin" | "x86_64-apple-darwin" | "aarch64-apple-darwin" => do_otool(path)?,
@@ -427,11 +417,7 @@ pub fn determine_linkage(path: &Utf8PathBuf, target: &str) -> DistResult<Linkage
         | "i686-pc-windows-gnu"
         | "x86_64-pc-windows-gnu"
         | "aarch64-pc-windows-gnu" => do_pe(path)?,
-        _ => {
-            return Err(DistError::LinkageCheckUnsupportedBinary {
-                file_name: path.file_name().map(String::from),
-            })
-        }
+        _ => return Err(DistError::LinkageCheckUnsupportedBinary),
     };
 
     let mut linkage = Linkage {