Merge pull request #5 from tux3/error_streams_strings

Use String in MergeError::ExternalToolError stdout/stderr, instead of Vec<u8>

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "armerge"
-version = "1.5.1"
+version = "2.0.0"
 authors = ["tux3 <[email protected]>"]
 edition = "2021"
 rust-version = "1.56"

src/arbuilder/mac.rs

@@ -77,8 +77,8 @@ impl MacArBuilder {
                 reason: "Failed to merge object files with `libtool`".to_string(),
                 tool: "libtool".to_string(),
                 args,
-                stdout: output.stdout,
-                stderr: output.stderr,
+                stdout: String::from_utf8_lossy(&output.stdout).to_string(),
+                stderr: String::from_utf8_lossy(&output.stderr).to_string(),
             })
         }
     }

src/archives.rs

@@ -165,8 +165,8 @@ pub fn create_index(archive_path: &std::path::Path) -> Result<(), MergeError> {
             reason: "Failed to create archive index with `ranlib`".to_string(),
             tool: "ranlib".to_string(),
             args: archive_path.iter().map(|p| p.to_owned()).collect(),
-            stdout: output.stdout,
-            stderr: output.stderr,
+            stdout: String::from_utf8_lossy(&output.stdout).to_string(),
+            stderr: String::from_utf8_lossy(&output.stderr).to_string(),
         })
     }
 }

src/merge_error.rs

@@ -7,16 +7,14 @@ use thiserror::Error;
 #[derive(Debug, Error)]
 pub enum MergeError {
     #[error(
-        "{reason}: {tool:?} {args:?})\nstdout: {}\nstderr: {}",
-        String::from_utf8_lossy(stdout),
-        String::from_utf8_lossy(stderr)
+        "{reason}: {tool:?} {args:?})\nstdout: {stdout}\nstderr: {stderr}"
     )]
     ExternalToolError {
         reason: String,
         tool: String,
         args: Vec<OsString>,
-        stdout: Vec<u8>,
-        stderr: Vec<u8>,
+        stdout: String,
+        stderr: String,
     },
     #[error("failed to launch external tool `{tool}`: {inner})")]
     ExternalToolLaunchError { tool: String, inner: io::Error },

src/objects.rs

@@ -30,7 +30,13 @@ pub fn merge_required_objects(
     #[allow(clippy::if_same_then_else)] // Clippy can't see both [cfg] at once
     if contents_type == ArchiveContents::Elf {
         #[cfg(feature = "objpoke_symbols")]
-        builtin_filter::merge_required_objects(obj_dir, merged_path, objs, keep_or_remove, keeps)?;
+        builtin_filter::merge_required_objects(
+            obj_dir,
+            merged_path,
+            objs,
+            keep_or_remove,
+            regexes,
+        )?;
         #[cfg(not(feature = "objpoke_symbols"))]
         system_filter::merge_required_objects(obj_dir, merged_path, objs, keep_or_remove, regexes)?;
     } else if contents_type == ArchiveContents::MachO {

src/objects/builtin_filter.rs

@@ -1,4 +1,4 @@
-use crate::MergeError;
+use crate::{ArmergeKeepOrRemove, MergeError};
 use regex::Regex;
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
@@ -10,20 +10,25 @@ pub fn merge_required_objects(
     _obj_dir: &Path,
     merged_path: &Path,
     objects: &HashMap<PathBuf, ObjectSyms>,
-    keep_regexes: &[Regex],
+    keep_or_remove: ArmergeKeepOrRemove,
+    regexes: &[Regex],
 ) -> Result<(), MergeError> {
+    if keep_or_remove == ArmergeKeepOrRemove::RemoveSymbols {
+        unimplemented!("--remove-symbols not yet supported with builtin filter")
+    }
+
     // The merging part is still not builtin, it has to be done by a real linker
     merge::create_merged_object(merged_path, &[], objects.keys(), false)?;
 
     // Filtering the symbols is faster in pure Rust, compared to calling the system's objcopy
     let merged_elf = std::fs::read(merged_path)?;
-    let filtered_elf = objpoke::elf::localize_elf_symbols(merged_elf, keep_regexes)
+    let filtered_elf = objpoke::elf::localize_elf_symbols(merged_elf, regexes)
         .map_err(|e| MergeError::InternalError(e.into()))?;
 
     // If a symbol we localize is in a COMDAT section group, we also want to turn it into a regular
     // section group. Otherwise the local symbol is not really local, because the containing section
     // could later get COMDAT-folded with other (potentially incompatible) object files.
-    let filtered_elf = objpoke::elf::demote_comdat_groups(filtered_elf, keep_regexes)
+    let filtered_elf = objpoke::elf::demote_comdat_groups(filtered_elf, regexes)
         .map_err(|e| MergeError::InternalError(e.into()))?;
 
     std::fs::write(merged_path, filtered_elf)?;

src/objects/filter_deps.rs

@@ -2,7 +2,6 @@ use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
 
 use crate::{ArmergeKeepOrRemove, MergeError};
-use rayon::iter::IntoParallelIterator;
 use rayon::prelude::*;
 use regex::Regex;
 use tracing::{event_enabled, info, Level};

src/objects/merge.rs

@@ -73,8 +73,8 @@ pub fn create_merged_object(
             reason: "Failed to merged object files".to_string(),
             tool: ld_path.to_string_lossy().to_string(),
             args,
-            stdout: output.stdout,
-            stderr: output.stderr,
+            stdout: String::from_utf8_lossy(&output.stdout).to_string(),
+            stderr: String::from_utf8_lossy(&output.stderr).to_string(),
         })
     }
 }

src/objects/system_filter.rs

@@ -33,7 +33,7 @@ fn create_filtered_merged_macho_object(
     let extra_args = &["-unexported_symbols_list".as_ref(), filter_list.as_os_str()];
     let merged_firstpass_path = merged_path.parent().unwrap().join("merged_firstpass.o");
     create_merged_object(&merged_firstpass_path, extra_args, objects, false)?;
-    create_merged_object(merged_path, &[], &[&merged_firstpass_path], true)?;
+    create_merged_object(merged_path, &[], [&merged_firstpass_path], true)?;
 
     Ok(())
 }
@@ -135,8 +135,8 @@ fn filter_symbols(object_path: &Path, filter_list_path: &Path) -> Result<(), Mer
             reason: "Failed to filter symbols".to_string(),
             tool: objcopy_path.to_string_lossy().to_string(),
             args,
-            stdout: output.stdout,
-            stderr: output.stderr,
+            stdout: String::from_utf8_lossy(&output.stdout).to_string(),
+            stderr: String::from_utf8_lossy(&output.stderr).to_string(),
         })
     }
 }