Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Accept non utf-8 characters in compiler and archiver #998

Merged
merged 5 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 34 additions & 16 deletions src/command_helpers.rs
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
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
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