From eedb9bcdadd52efa4dec9713cdf3e24863e7efab Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 7 Mar 2024 18:38:52 +1100 Subject: [PATCH 1/5] Accept `impl AsRef` in fn `run*`, `spawn` and `try_wait_on_child` Signed-off-by: Jiahao XU --- src/command_helpers.rs | 63 ++++++++++++++++++++++++++++++------------ src/tool.rs | 2 +- 2 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/command_helpers.rs b/src/command_helpers.rs index 919d276c..a50f2767 100644 --- a/src/command_helpers.rs +++ b/src/command_helpers.rs @@ -2,6 +2,7 @@ use std::{ collections::hash_map, + convert::AsRef, ffi::OsString, fmt::Display, fs, @@ -215,7 +216,7 @@ fn write_warning(line: &[u8]) { fn wait_on_child( cmd: &Command, - program: &str, + program: &Path, child: &mut Child, cargo_output: &CargoOutput, ) -> Result<(), Error> { @@ -227,8 +228,10 @@ fn wait_on_child( return Err(Error::new( ErrorKind::ToolExecError, format!( - "Failed to wait on spawned child process, command {:?} with args {:?}: {}.", - cmd, program, e + "Failed to wait on spawned child process, command {:?} with args {}: {}.", + cmd, + program.display(), + e ), )); } @@ -242,8 +245,10 @@ fn wait_on_child( Err(Error::new( ErrorKind::ToolExecError, format!( - "Command {:?} with args {:?} did not execute successfully (status code {}).", - cmd, program, status + "Command {:?} with args {} did not execute successfully (status code {}).", + cmd, + program.display(), + status ), )) } @@ -299,18 +304,22 @@ pub(crate) fn objects_from_files(files: &[Arc], dst: &Path) -> Result, cargo_output: &CargoOutput, ) -> Result<(), Error> { + let program = program.as_ref(); + let mut child = spawn(cmd, program, cargo_output)?; wait_on_child(cmd, program, &mut child, cargo_output) } pub(crate) fn run_output( cmd: &mut Command, - program: &str, + program: impl AsRef, cargo_output: &CargoOutput, ) -> Result, Error> { + let program = program.as_ref(); + cmd.stdout(Stdio::piped()); let mut child = spawn(cmd, program, cargo_output)?; @@ -328,9 +337,9 @@ pub(crate) fn run_output( Ok(stdout) } -pub(crate) fn spawn( +fn spawn_inner( cmd: &mut Command, - program: &str, + program: &Path, cargo_output: &CargoOutput, ) -> Result { struct ResetStderr<'cmd>(&'cmd mut Command); @@ -358,19 +367,33 @@ for help)" }; Err(Error::new( ErrorKind::ToolNotFound, - format!("Failed to find tool. Is `{}` installed?{}", program, extra), + format!( + "Failed to find tool. Is `{}` installed?{}", + program.display(), + extra + ), )) } Err(e) => Err(Error::new( ErrorKind::ToolExecError, format!( - "Command {:?} with args {:?} failed to start: {:?}", - cmd.0, program, e + "Command {:?} with args {} failed to start: {:?}", + cmd.0, + program.display(), + e ), )), } } +pub(crate) fn spawn( + cmd: &mut Command, + program: impl AsRef, + cargo_output: &CargoOutput, +) -> Result { + spawn_inner(cmd, program.as_ref(), cargo_output) +} + pub(crate) fn command_add_output_file( cmd: &mut Command, dst: &Path, @@ -393,11 +416,13 @@ pub(crate) fn command_add_output_file( #[cfg(feature = "parallel")] pub(crate) fn try_wait_on_child( cmd: &Command, - program: &str, + program: impl AsRef, child: &mut Child, stdout: &mut dyn io::Write, stderr_forwarder: &mut StderrForwarder, ) -> Result, Error> { + let program = program.as_ref(); + stderr_forwarder.forward_available(); match child.try_wait() { @@ -412,8 +437,10 @@ pub(crate) fn try_wait_on_child( Err(Error::new( ErrorKind::ToolExecError, format!( - "Command {:?} with args {:?} did not execute successfully (status code {}).", - cmd, program, status + "Command {:?} with args {} did not execute successfully (status code {}).", + cmd, + program.display(), + status ), )) } @@ -424,8 +451,10 @@ pub(crate) fn try_wait_on_child( Err(Error::new( ErrorKind::ToolExecError, format!( - "Failed to wait on spawned child process, command {:?} with args {:?}: {}.", - cmd, program, e + "Failed to wait on spawned child process, command {:?} with args {}: {}.", + cmd, + program.display(), + e ), )) } diff --git a/src/tool.rs b/src/tool.rs index a193a90f..bb7de512 100644 --- a/src/tool.rs +++ b/src/tool.rs @@ -81,7 +81,7 @@ impl Tool { let stdout = match run_output( &mut cmd, - &path.to_string_lossy(), + path, // tool detection issues should always be shown as warnings cargo_output, ) From 6d72bc17cf00681f8a04d146e1e142d739f8016d Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 7 Mar 2024 18:45:45 +1100 Subject: [PATCH 2/5] Accept non utf-8 path in compiler in `create_compile_object_cmd` Signed-off-by: Jiahao XU --- src/lib.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 92b8eff6..e6a4835d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1439,7 +1439,7 @@ impl Build { let pendings = Cell::new(Vec::<( Command, - String, + Cow<'static, Path>, KillOnDrop, parallel::job_token::JobToken, )>::new()); @@ -1563,7 +1563,10 @@ impl Build { Ok(()) } - fn create_compile_object_cmd(&self, obj: &Object) -> Result<(Command, String), Error> { + fn create_compile_object_cmd( + &self, + obj: &Object, + ) -> Result<(Command, Cow<'static, Path>), Error> { let asm_ext = AsmFileExt::from_path(&obj.src); let is_asm = asm_ext.is_some(); let target = self.get_target()?; @@ -1574,7 +1577,8 @@ impl Build { let is_assembler_msvc = msvc && asm_ext == Some(AsmFileExt::DotAsm); let (mut cmd, name) = if is_assembler_msvc { - self.msvc_macro_assembler()? + let (cmd, name) = self.msvc_macro_assembler()?; + (cmd, Cow::Borrowed(Path::new(name))) } else { let mut cmd = compiler.to_command(); for (a, b) in self.env.iter() { @@ -1585,9 +1589,9 @@ impl Build { compiler .path .file_name() - .ok_or_else(|| Error::new(ErrorKind::IOError, "Failed to get compiler path."))? - .to_string_lossy() - .into_owned(), + .ok_or_else(|| Error::new(ErrorKind::IOError, "Failed to get compiler path.")) + .map(PathBuf::from) + .map(Cow::Owned)?, ) }; let is_arm = target.contains("aarch64") || target.contains("arm"); @@ -2321,7 +2325,7 @@ impl Build { } } - fn msvc_macro_assembler(&self) -> Result<(Command, String), Error> { + fn msvc_macro_assembler(&self) -> Result<(Command, &'static str), Error> { let target = self.get_target()?; let tool = if target.contains("x86_64") { "ml64.exe" @@ -2374,7 +2378,7 @@ impl Build { cmd.arg("-safeseh"); } - Ok((cmd, tool.to_string())) + Ok((cmd, tool)) } fn assemble(&self, lib_name: &str, dst: &Path, objs: &[Object]) -> Result<(), Error> { From f0ad4d694c74fbc0c0590f028e4265410b194fb1 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 7 Mar 2024 18:52:45 +1100 Subject: [PATCH 3/5] Accept non utf-8 characters in archiver path Signed-off-by: Jiahao XU --- src/lib.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e6a4835d..699b9279 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3030,7 +3030,7 @@ impl Build { } } - fn get_ar(&self) -> Result<(Command, String, bool), Error> { + fn get_ar(&self) -> Result<(Command, PathBuf, bool), Error> { self.try_get_archiver_and_flags() } @@ -3063,7 +3063,7 @@ impl Build { Ok(self.try_get_archiver_and_flags()?.0) } - fn try_get_archiver_and_flags(&self) -> Result<(Command, String, bool), Error> { + fn try_get_archiver_and_flags(&self) -> Result<(Command, PathBuf, bool), Error> { let (mut cmd, name) = self.get_base_archiver()?; let mut any_flags = false; if let Ok(flags) = self.envflags("ARFLAGS") { @@ -3077,12 +3077,14 @@ impl Build { Ok((cmd, name, any_flags)) } - fn get_base_archiver(&self) -> Result<(Command, String), Error> { + fn get_base_archiver(&self) -> Result<(Command, PathBuf), Error> { if let Some(ref a) = self.archiver { - return Ok((self.cmd(&**a), a.to_string_lossy().into_owned())); + let archiver = &**a; + return Ok((self.cmd(archiver), archiver.into())); } self.get_base_archiver_variant("AR", "ar") + .map(|(cmd, archiver)| (cmd, archiver.into())) } /// Get the ranlib that's in use for this configuration. From 77429966b26e6653b281812a23afa389f68af117 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 7 Mar 2024 18:57:59 +1100 Subject: [PATCH 4/5] Remove generics from `spawn` and `try_wait_on_child` They are only used in `Build::compile_objects` with `&Path`, so there is no need to use generics. Signed-off-by: Jiahao XU --- src/command_helpers.rs | 14 ++------------ src/lib.rs | 4 +--- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/command_helpers.rs b/src/command_helpers.rs index a50f2767..589d034a 100644 --- a/src/command_helpers.rs +++ b/src/command_helpers.rs @@ -337,7 +337,7 @@ pub(crate) fn run_output( Ok(stdout) } -fn spawn_inner( +pub(crate) fn spawn( cmd: &mut Command, program: &Path, cargo_output: &CargoOutput, @@ -386,14 +386,6 @@ for help)" } } -pub(crate) fn spawn( - cmd: &mut Command, - program: impl AsRef, - cargo_output: &CargoOutput, -) -> Result { - spawn_inner(cmd, program.as_ref(), cargo_output) -} - pub(crate) fn command_add_output_file( cmd: &mut Command, dst: &Path, @@ -416,13 +408,11 @@ pub(crate) fn command_add_output_file( #[cfg(feature = "parallel")] pub(crate) fn try_wait_on_child( cmd: &Command, - program: impl AsRef, + program: &Path, child: &mut Child, stdout: &mut dyn io::Write, stderr_forwarder: &mut StderrForwarder, ) -> Result, Error> { - let program = program.as_ref(); - stderr_forwarder.forward_available(); match child.try_wait() { diff --git a/src/lib.rs b/src/lib.rs index 699b9279..8ef48305 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1657,9 +1657,7 @@ impl Build { let name = compiler .path .file_name() - .ok_or_else(|| Error::new(ErrorKind::IOError, "Failed to get compiler path."))? - .to_string_lossy() - .into_owned(); + .ok_or_else(|| Error::new(ErrorKind::IOError, "Failed to get compiler path."))?; Ok(run_output(&mut cmd, &name, &self.cargo_output)?) } From 0d50061d364733c052150be05e91e606856279d8 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Thu, 7 Mar 2024 19:01:33 +1100 Subject: [PATCH 5/5] Remove rebundant import of `std::convert::AsRef` Signed-off-by: Jiahao XU --- src/command_helpers.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/command_helpers.rs b/src/command_helpers.rs index 589d034a..10dafeb1 100644 --- a/src/command_helpers.rs +++ b/src/command_helpers.rs @@ -2,7 +2,6 @@ use std::{ collections::hash_map, - convert::AsRef, ffi::OsString, fmt::Display, fs,