Skip to content

Commit

Permalink
Fix: Accept non utf-8 characters in compiler and archiver (#998)
Browse files Browse the repository at this point in the history 
* Accept `impl AsRef<Path>` in fn `run*`, `spawn` and `try_wait_on_child`

Signed-off-by: Jiahao XU <[email protected]>

* Accept non utf-8 path in compiler in `create_compile_object_cmd`

Signed-off-by: Jiahao XU <[email protected]>

* Accept non utf-8 characters in archiver path

Signed-off-by: Jiahao XU <[email protected]>

* 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 <[email protected]>

* Remove rebundant import of `std::convert::AsRef`

Signed-off-by: Jiahao XU <[email protected]>

---------

Signed-off-by: Jiahao XU <[email protected]>
  • Loading branch information
@NobodyXu
NobodyXu authored Mar 7, 2024
1 parent f6d81ef commit 2a0bf2b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 32 deletions.
50 changes: 34 additions & 16 deletions src/command_helpers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ fn write_warning(line: &[u8]) {

fn wait_on_child(
cmd: &Command,
program: &str,
program: &Path,
child: &mut Child,
cargo_output: &CargoOutput,
) -> Result<(), Error> {
Expand All @@ -227,8 +227,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
),
));
}
Expand All @@ -242,8 +244,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
),
))
}
Expand Down Expand Up @@ -299,18 +303,22 @@ pub(crate) fn objects_from_files(files: &[Arc<Path>], dst: &Path) -> Result<Vec<

pub(crate) fn run(
cmd: &mut Command,
program: &str,
program: impl AsRef<Path>,
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<Path>,
cargo_output: &CargoOutput,
) -> Result<Vec<u8>, Error> {
let program = program.as_ref();

cmd.stdout(Stdio::piped());

let mut child = spawn(cmd, program, cargo_output)?;
Expand All @@ -330,7 +338,7 @@ pub(crate) fn run_output(

pub(crate) fn spawn(
cmd: &mut Command,
program: &str,
program: &Path,
cargo_output: &CargoOutput,
) -> Result<Child, Error> {
struct ResetStderr<'cmd>(&'cmd mut Command);
Expand Down Expand Up @@ -358,14 +366,20 @@ 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
),
)),
}
Expand Down Expand Up @@ -393,7 +407,7 @@ pub(crate) fn command_add_output_file(
#[cfg(feature = "parallel")]
pub(crate) fn try_wait_on_child(
cmd: &Command,
program: &str,
program: &Path,
child: &mut Child,
stdout: &mut dyn io::Write,
stderr_forwarder: &mut StderrForwarder,
Expand All @@ -412,8 +426,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
),
))
}
Expand All @@ -424,8 +440,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
),
))
}
Expand Down
34 changes: 19 additions & 15 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,7 @@ impl Build {

let pendings = Cell::new(Vec::<(
Command,
String,
Cow<'static, Path>,
KillOnDrop,
parallel::job_token::JobToken,
)>::new());
Expand Down Expand Up @@ -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()?;
Expand All @@ -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() {
Expand All @@ -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");
Expand Down Expand Up @@ -1653,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)?)
}
Expand Down Expand Up @@ -2321,7 +2323,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"
Expand Down Expand Up @@ -2374,7 +2376,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> {
Expand Down Expand Up @@ -3026,7 +3028,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()
}

Expand Down Expand Up @@ -3059,7 +3061,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") {
Expand All @@ -3073,12 +3075,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.
Expand Down
2 changes: 1 addition & 1 deletion src/tool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down

0 comments on commit 2a0bf2b

Please sign in to comment.