rust
/
cargo
mirror of https://github.com/rust-lang/cargo
0
0
Fork 0
Code Issues Releases Wiki Activity

Auto merge of #10546 - weihanglo:issue-10381-rustc-argfile, r=epage 

Retry command invocation with argfile

### What does this PR try to resolve?

Fixes #10381

The goal is to invoke `rustc` and `rustdoc` with ``@path`` arg file as a fallback.

Since the `-Zcheck-cfg-features`[^1] has already been implemented, in the foreseeable future cargo will probably hit the “**command line too big**” more often on Windows. `rustdoc` and `rustc` both support ``@path`` argfile [^2][^3], so we can leverage the feature for the fallback logic.

The idea behind this PR is that we put the **retry logic** in `ProcessBuilder::exec*` functions and reuse them as much as possible.

### How should we test and review this PR?

Review it commit by commit is recommended.

Argfile fallback is hard to tested with integration tests, so I added some unit tests for `cargo_util::ProcessBuilder` and `cargo::ops::fix::FixArgs`.

If you do want to test the support of argfile manually, a new environment variable `__CARGO_TEST_FORCE_ARGFILE` is added for the sake of forcing cargo to use argfile for rustc invocations. For example, `__CARGO_TEST_FORCE_ARGFILE cargo test --test testsuite -- fix::` is usually a good start to test this feature.

[^1]: https://doc.rust-lang.org/beta/cargo/reference/unstable.html?highlight=check#check-cfg-features
[^2]: https://doc.rust-lang.org/rustc/command-line-arguments.html#path-load-command-line-flags-from-a-path
[^3]: https://doc.rust-lang.org/rustdoc/command-line-arguments.html#path-load-command-line-flags-from-a-path
This commit is contained in:
bors 2022-04-11 14:57:46 +00:00
parent f51e799636 275b0c68d4
commit 1073915930
13 changed files with 467 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
.github/workflows/main.yml vendored
View File

@ -68,6 +68,13 @@ jobs:
# Deny warnings on CI to avoid warnings getting into the codebase.
- run: cargo test --features 'deny-warnings'
- name: Check operability of rustc invocation with argfile
env:
__CARGO_TEST_FORCE_ARGFILE: 1
run: |
# This only tests `cargo fix` because fix-proxy-mode is one of the most
# complicated subprocess management in Cargo.
cargo test --test testsuite --features 'deny-warnings' -- fix::
- run: cargo test --features 'deny-warnings' --manifest-path crates/cargo-test-support/Cargo.toml
env:
CARGO_TARGET_DIR: target

2
Cargo.toml
View File

@ -19,7 +19,7 @@ path = "src/cargo/lib.rs"
atty = "0.2"
bytesize = "1.0"
cargo-platform = { path = "crates/cargo-platform", version = "0.1.2" }
cargo-util = { path = "crates/cargo-util", version = "0.1.2" }
cargo-util = { path = "crates/cargo-util", version = "0.1.3" }
crates-io = { path = "crates/crates-io", version = "0.34.0" }
crossbeam-utils = "0.8"
curl = { version = "0.4.41", features = ["http2"] }

11
crates/cargo-test-support/src/tools.rs
View File

@ -24,8 +24,17 @@ pub fn echo_wrapper() -> PathBuf {
.file(
"src/main.rs",
r#"
use std::fs::read_to_string;
use std::path::PathBuf;
fn main() {
let args = std::env::args().collect::<Vec<_>>();
// Handle args from `@path` argfile for rustc
let args = std::env::args()
.flat_map(|p| if let Some(p) = p.strip_prefix("@") {
read_to_string(p).unwrap().lines().map(String::from).collect()
} else {
vec![p]
})
.collect::<Vec<_>>();
eprintln!("WRAPPER CALLED: {}", args[1..].join(" "));
let status = std::process::Command::new(&args[1])
.args(&args[2..]).status().unwrap();

2
crates/cargo-util/Cargo.toml
View File

@ -1,6 +1,6 @@
[package]
name = "cargo-util"
version = "0.1.2"
version = "0.1.3"
edition = "2021"
license = "MIT OR Apache-2.0"
homepage = "https://github.com/rust-lang/cargo"

345
crates/cargo-util/src/process_builder.rs
View File

@ -1,15 +1,19 @@
use crate::process_error::ProcessError;
use crate::read2;
use anyhow::{bail, Context, Result};
use jobserver::Client;
use shell_escape::escape;
use tempfile::NamedTempFile;
use std::collections::BTreeMap;
use std::env;
use std::ffi::{OsStr, OsString};
use std::fmt;
use std::io;
use std::iter::once;
use std::path::Path;
use std::process::{Command, Output, Stdio};
use std::process::{Command, ExitStatus, Output, Stdio};
/// A builder object for an external process, similar to [`std::process::Command`].
#[derive(Clone, Debug)]
@ -22,6 +26,9 @@ pub struct ProcessBuilder {
env: BTreeMap<String, Option<OsString>>,
/// The directory to run the program from.
cwd: Option<OsString>,
/// A list of wrappers that wrap the original program when calling
/// [`ProcessBuilder::wrapped`]. The last one is the outermost one.
wrappers: Vec<OsString>,
/// The `make` jobserver. See the [jobserver crate] for
/// more information.
///
@ -29,6 +36,9 @@ pub struct ProcessBuilder {
jobserver: Option<Client>,
/// `true` to include environment variable in display.
display_env_vars: bool,
/// `true` to retry with an argfile if hitting "command line too big" error.
/// See [`ProcessBuilder::retry_with_argfile`] for more information.
retry_with_argfile: bool,
}
impl fmt::Display for ProcessBuilder {
@ -48,9 +58,9 @@ impl fmt::Display for ProcessBuilder {
}
}
write!(f, "{}", self.program.to_string_lossy())?;
write!(f, "{}", self.get_program().to_string_lossy())?;
for arg in &self.args {
for arg in self.get_args() {
write!(f, " {}", escape(arg.to_string_lossy()))?;
}
@ -66,8 +76,10 @@ impl ProcessBuilder {
args: Vec::new(),
cwd: None,
env: BTreeMap::new(),
wrappers: Vec::new(),
jobserver: None,
display_env_vars: false,
retry_with_argfile: false,
}
}
@ -92,6 +104,13 @@ impl ProcessBuilder {
/// (chainable) Replaces the args list with the given `args`.
pub fn args_replace<T: AsRef<OsStr>>(&mut self, args: &[T]) -> &mut ProcessBuilder {
if let Some(program) = self.wrappers.pop() {
// User intend to replace all args, so we
// - use the outermost wrapper as the main program, and
// - cleanup other inner wrappers.
self.program = program;
self.wrappers = Vec::new();
}
self.args = args.iter().map(|t| t.as_ref().to_os_string()).collect();
self
}
@ -117,12 +136,17 @@ impl ProcessBuilder {
/// Gets the executable name.
pub fn get_program(&self) -> &OsString {
&self.program
self.wrappers.last().unwrap_or(&self.program)
}
/// Gets the program arguments.
pub fn get_args(&self) -> &[OsString] {
&self.args
pub fn get_args(&self) -> impl Iterator<Item = &OsString> {
self.wrappers
.iter()
.rev()
.chain(once(&self.program))
.chain(self.args.iter())
.skip(1) // Skip the main `program
}
/// Gets the current working directory for the process.
@ -161,13 +185,56 @@ impl ProcessBuilder {
self
}
/// Enables retrying with an argfile if hitting "command line too big" error
///
/// This is primarily for the `@path` arg of rustc and rustdoc, which treat
/// each line as an command-line argument, so `LF` and `CRLF` bytes are not
/// valid as an argument for argfile at this moment.
/// For example, `RUSTDOCFLAGS="--crate-version foo\nbar" cargo doc` is
/// valid when invoking from command-line but not from argfile.
///
/// To sum up, the limitations of the argfile are:
///
/// - Must be valid UTF-8 encoded.
/// - Must not contain any newlines in each argument.
///
/// Ref:
///
/// - https://doc.rust-lang.org/rustdoc/command-line-arguments.html#path-load-command-line-flags-from-a-path
/// - https://doc.rust-lang.org/rustc/command-line-arguments.html#path-load-command-line-flags-from-a-path>
pub fn retry_with_argfile(&mut self, enabled: bool) -> &mut Self {
self.retry_with_argfile = enabled;
self
}
fn should_retry_with_argfile(&self, err: &io::Error) -> bool {
self.retry_with_argfile && imp::command_line_too_big(err)
}
/// Like [`Command::status`] but with a better error message.
pub fn status(&self) -> Result<ExitStatus> {
self._status()
.with_context(|| ProcessError::could_not_execute(self))
}
fn _status(&self) -> io::Result<ExitStatus> {
if !debug_force_argfile(self.retry_with_argfile) {
let mut cmd = self.build_command();
match cmd.spawn() {
Err(ref e) if self.should_retry_with_argfile(e) => {}
Err(e) => return Err(e),
Ok(mut child) => return child.wait(),
}
}
let (mut cmd, argfile) = self.build_command_with_argfile()?;
let status = cmd.spawn()?.wait();
close_tempfile_and_log_error(argfile);
status
}
/// Runs the process, waiting for completion, and mapping non-success exit codes to an error.
pub fn exec(&self) -> Result<()> {
let mut command = self.build_command();
let exit = command.status().with_context(|| {
ProcessError::new(&format!("could not execute process {}", self), None, None)
})?;
let exit = self.status()?;
if exit.success() {
Ok(())
} else {
@ -199,14 +266,30 @@ impl ProcessBuilder {
imp::exec_replace(self)
}
/// Like [`Command::output`] but with a better error message.
pub fn output(&self) -> Result<Output> {
self._output()
.with_context(|| ProcessError::could_not_execute(self))
}
fn _output(&self) -> io::Result<Output> {
if !debug_force_argfile(self.retry_with_argfile) {
let mut cmd = self.build_command();
match piped(&mut cmd).spawn() {
Err(ref e) if self.should_retry_with_argfile(e) => {}
Err(e) => return Err(e),
Ok(child) => return child.wait_with_output(),
}
}
let (mut cmd, argfile) = self.build_command_with_argfile()?;
let output = piped(&mut cmd).spawn()?.wait_with_output();
close_tempfile_and_log_error(argfile);
output
}
/// Executes the process, returning the stdio output, or an error if non-zero exit status.
pub fn exec_with_output(&self) -> Result<Output> {
let mut command = self.build_command();
let output = command.output().with_context(|| {
ProcessError::new(&format!("could not execute process {}", self), None, None)
})?;
let output = self.output()?;
if output.status.success() {
Ok(output)
} else {
@ -237,16 +320,25 @@ impl ProcessBuilder {
let mut stdout = Vec::new();
let mut stderr = Vec::new();
let mut cmd = self.build_command();
cmd.stdout(Stdio::piped())
.stderr(Stdio::piped())
.stdin(Stdio::null());
let mut callback_error = None;
let mut stdout_pos = 0;
let mut stderr_pos = 0;
let spawn = |mut cmd| {
if !debug_force_argfile(self.retry_with_argfile) {
match piped(&mut cmd).spawn() {
Err(ref e) if self.should_retry_with_argfile(e) => {}
Err(e) => return Err(e),
Ok(child) => return Ok((child, None)),
}
}
let (mut cmd, argfile) = self.build_command_with_argfile()?;
Ok((piped(&mut cmd).spawn()?, Some(argfile)))
};
let status = (|| {
let mut child = cmd.spawn()?;
let cmd = self.build_command();
let (mut child, argfile) = spawn(cmd)?;
let out = child.stdout.take().unwrap();
let err = child.stderr.take().unwrap();
read2(out, err, &mut |is_out, data, eof| {
@ -292,11 +384,13 @@ impl ProcessBuilder {
data.drain(..idx);
*pos = 0;
})?;
child.wait()
let status = child.wait();
if let Some(argfile) = argfile {
close_tempfile_and_log_error(argfile);
}
status
})()
.with_context(|| {
ProcessError::new(&format!("could not execute process {}", self), None, None)
})?;
.with_context(|| ProcessError::could_not_execute(self))?;
let output = Output {
status,
stdout,
@ -324,16 +418,56 @@ impl ProcessBuilder {
Ok(output)
}
/// Converts `ProcessBuilder` into a `std::process::Command`, and handles the jobserver, if
/// present.
pub fn build_command(&self) -> Command {
let mut command = Command::new(&self.program);
/// Builds the command with an `@<path>` argfile that contains all the
/// arguments. This is primarily served for rustc/rustdoc command family.
fn build_command_with_argfile(&self) -> io::Result<(Command, NamedTempFile)> {
use std::io::Write as _;
let mut tmp = tempfile::Builder::new()
.prefix("cargo-argfile.")
.tempfile()?;
let mut arg = OsString::from("@");
arg.push(tmp.path());
let mut cmd = self.build_command_without_args();
cmd.arg(arg);
log::debug!("created argfile at {} for {self}", tmp.path().display());
let cap = self.get_args().map(|arg| arg.len() + 1).sum::<usize>();
let mut buf = Vec::with_capacity(cap);
for arg in &self.args {
let arg = arg.to_str().ok_or_else(|| {
io::Error::new(
io::ErrorKind::Other,
format!(
"argument for argfile contains invalid UTF-8 characters: `{}`",
arg.to_string_lossy()
),
)
})?;
if arg.contains('\n') {
return Err(io::Error::new(
io::ErrorKind::Other,
format!("argument for argfile contains newlines: `{arg}`"),
));
}
writeln!(buf, "{arg}")?;
}
tmp.write_all(&mut buf)?;
Ok((cmd, tmp))
}
/// Builds a command from `ProcessBuilder` for everythings but not `args`.
fn build_command_without_args(&self) -> Command {
let mut command = {
let mut iter = self.wrappers.iter().rev().chain(once(&self.program));
let mut cmd = Command::new(iter.next().expect("at least one `program` exists"));
cmd.args(iter);
cmd
};
if let Some(cwd) = self.get_cwd() {
command.current_dir(cwd);
}
for arg in &self.args {
command.arg(arg);
}
for (k, v) in &self.env {
match *v {
Some(ref v) => {
@ -350,6 +484,19 @@ impl ProcessBuilder {
command
}
/// Converts `ProcessBuilder` into a `std::process::Command`, and handles
/// the jobserver, if present.
///
/// Note that this method doesn't take argfile fallback into account. The
/// caller should handle it by themselves.
pub fn build_command(&self) -> Command {
let mut command = self.build_command_without_args();
for arg in &self.args {
command.arg(arg);
}
command
}
/// Wraps an existing command with the provided wrapper, if it is present and valid.
///
/// # Examples
@ -363,46 +510,80 @@ impl ProcessBuilder {
/// let cmd = cmd.wrapped(Some("sccache"));
/// ```
pub fn wrapped(mut self, wrapper: Option<impl AsRef<OsStr>>) -> Self {
let wrapper = if let Some(wrapper) = wrapper.as_ref() {
wrapper.as_ref()
} else {
return self;
};
if wrapper.is_empty() {
return self;
if let Some(wrapper) = wrapper.as_ref() {
let wrapper = wrapper.as_ref();
if !wrapper.is_empty() {
self.wrappers.push(wrapper.to_os_string());
}
}
let args = once(self.program).chain(self.args.into_iter()).collect();
self.program = wrapper.to_os_string();
self.args = args;
self
}
}
/// Forces the command to use `@path` argfile.
///
/// You should set `__CARGO_TEST_FORCE_ARGFILE` to enable this.
fn debug_force_argfile(retry_enabled: bool) -> bool {
cfg!(debug_assertions) && env::var("__CARGO_TEST_FORCE_ARGFILE").is_ok() && retry_enabled
}
/// Creates new pipes for stderr and stdout. Ignores stdin.
fn piped(cmd: &mut Command) -> &mut Command {
cmd.stdout(Stdio::piped())
.stderr(Stdio::piped())
.stdin(Stdio::null())
}
fn close_tempfile_and_log_error(file: NamedTempFile) {
file.close().unwrap_or_else(|e| {
log::warn!("failed to close temporary file: {e}");
});
}
#[cfg(unix)]
mod imp {
use super::{ProcessBuilder, ProcessError};
use super::{close_tempfile_and_log_error, debug_force_argfile, ProcessBuilder, ProcessError};
use anyhow::Result;
use std::io;
use std::os::unix::process::CommandExt;
pub fn exec_replace(process_builder: &ProcessBuilder) -> Result<()> {
let mut command = process_builder.build_command();
let error = command.exec();
let mut error;
let mut file = None;
if debug_force_argfile(process_builder.retry_with_argfile) {
let (mut command, argfile) = process_builder.build_command_with_argfile()?;
file = Some(argfile);
error = command.exec()
} else {
let mut command = process_builder.build_command();
error = command.exec();
if process_builder.should_retry_with_argfile(&error) {
let (mut command, argfile) = process_builder.build_command_with_argfile()?;
file = Some(argfile);
error = command.exec()
}
}
if let Some(file) = file {
close_tempfile_and_log_error(file);
}
Err(anyhow::Error::from(error).context(ProcessError::new(
&format!("could not execute process {}", process_builder),
None,
None,
)))
}
pub fn command_line_too_big(err: &io::Error) -> bool {
err.raw_os_error() == Some(libc::E2BIG)
}
}
#[cfg(windows)]
mod imp {
use super::{ProcessBuilder, ProcessError};
use anyhow::Result;
use std::io;
use winapi::shared::minwindef::{BOOL, DWORD, FALSE, TRUE};
use winapi::um::consoleapi::SetConsoleCtrlHandler;
@ -421,4 +602,66 @@ mod imp {
// Just execute the process as normal.
process_builder.exec()
}
pub fn command_line_too_big(err: &io::Error) -> bool {
use winapi::shared::winerror::ERROR_FILENAME_EXCED_RANGE;
err.raw_os_error() == Some(ERROR_FILENAME_EXCED_RANGE as i32)
}
}
#[cfg(test)]
mod tests {
use super::ProcessBuilder;
use std::fs;
#[test]
fn argfile_build_succeeds() {
let mut cmd = ProcessBuilder::new("echo");
cmd.args(["foo", "bar"].as_slice());
let (cmd, argfile) = cmd.build_command_with_argfile().unwrap();
assert_eq!(cmd.get_program(), "echo");
let cmd_args: Vec<_> = cmd.get_args().map(|s| s.to_str().unwrap()).collect();
assert_eq!(cmd_args.len(), 1);
assert!(cmd_args[0].starts_with("@"));
assert!(cmd_args[0].contains("cargo-argfile."));
let buf = fs::read_to_string(argfile.path()).unwrap();
assert_eq!(buf, "foo\nbar\n");
}
#[test]
fn argfile_build_fails_if_arg_contains_newline() {
let mut cmd = ProcessBuilder::new("echo");
cmd.arg("foo\n");
let err = cmd.build_command_with_argfile().unwrap_err();
assert_eq!(
err.to_string(),
"argument for argfile contains newlines: `foo\n`"
);
}
#[test]
fn argfile_build_fails_if_arg_contains_invalid_utf8() {
let mut cmd = ProcessBuilder::new("echo");
#[cfg(windows)]
let invalid_arg = {
use std::os::windows::prelude::*;
std::ffi::OsString::from_wide(&[0x0066, 0x006f, 0xD800, 0x006f])
};
#[cfg(unix)]
let invalid_arg = {
use std::os::unix::ffi::OsStrExt;
std::ffi::OsStr::from_bytes(&[0x66, 0x6f, 0x80, 0x6f]).to_os_string()
};
cmd.arg(invalid_arg);
let err = cmd.build_command_with_argfile().unwrap_err();
assert_eq!(
err.to_string(),
"argument for argfile contains invalid UTF-8 characters: `fo<66>o`"
);
}
}

7
crates/cargo-util/src/process_error.rs
View File

@ -95,6 +95,13 @@ impl ProcessError {
stderr: stderr.map(|s| s.to_vec()),
}
}
/// Creates a [`ProcessError`] with "could not execute process {cmd}".
///
/// * `cmd` is usually but not limited to [`std::process::Command`].
pub fn could_not_execute(cmd: impl fmt::Display) -> ProcessError {
ProcessError::new(&format!("could not execute process {cmd}"), None, None)
}
}
/// Converts an [`ExitStatus`] to a human-readable string suitable for

2
src/cargo/core/compiler/build_plan.rs
View File

@ -78,7 +78,7 @@ impl Invocation {
.ok_or_else(|| anyhow::format_err!("unicode program string required"))?
.to_string();
self.cwd = Some(cmd.get_cwd().unwrap().to_path_buf());
for arg in cmd.get_args().iter() {
for arg in cmd.get_args() {
self.args.push(
arg.to_str()
.ok_or_else(|| anyhow::format_err!("unicode argument string required"))?

1
src/cargo/core/compiler/compilation.rs
View File

@ -194,6 +194,7 @@ impl<'cfg> Compilation<'cfg> {
let rustdoc = ProcessBuilder::new(&*self.config.rustdoc()?);
let cmd = fill_rustc_tool_env(rustdoc, unit);
let mut cmd = self.fill_env(cmd, &unit.pkg, script_meta, unit.kind, true)?;
cmd.retry_with_argfile(true);
unit.target.edition().cmd_edition_arg(&mut cmd);
for crate_type in unit.target.rustc_crate_types() {

2
src/cargo/core/compiler/mod.rs
View File

@ -754,7 +754,7 @@ fn rustdoc(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Work> {
// The --crate-version flag could have already been passed in RUSTDOCFLAGS
// or as an extra compiler argument for rustdoc
fn crate_version_flag_already_present(rustdoc: &ProcessBuilder) -> bool {
rustdoc.get_args().iter().any(|flag| {
rustdoc.get_args().any(|flag| {
flag.to_str()
.map_or(false, |flag| flag.starts_with(RUSTDOC_CRATE_VERSION_FLAG))
})

173
src/cargo/ops/fix.rs
View File

@ -39,13 +39,12 @@
//! show them to the user.
use std::collections::{BTreeSet, HashMap, HashSet};
use std::env;
use std::ffi::OsString;
use std::path::{Path, PathBuf};
use std::process::{self, Command, ExitStatus};
use std::str;
use std::process::{self, ExitStatus};
use std::{env, fs, str};
use anyhow::{bail, Context, Error};
use anyhow::{bail, Context as _};
use cargo_util::{exit_status_to_string, is_simple_exit_code, paths, ProcessBuilder};
use log::{debug, trace, warn};
use rustfix::diagnostics::Diagnostic;
@ -122,6 +121,9 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions) -> CargoResult<()> {
let rustc = ws.config().load_global_rustc(Some(ws))?;
wrapper.arg(&rustc.path);
// This is calling rustc in cargo fix-proxy-mode, so it also need to retry.
// The argfile handling are located at `FixArgs::from_args`.
wrapper.retry_with_argfile(true);
// primary crates are compiled using a cargo subprocess to do extra work of applying fixes and
// repeating build until there are no more changes to be applied
@ -352,10 +354,17 @@ pub fn fix_maybe_exec_rustc(config: &Config) -> CargoResult<bool> {
.map(PathBuf::from)
.ok();
let mut rustc = ProcessBuilder::new(&args.rustc).wrapped(workspace_rustc.as_ref());
rustc.retry_with_argfile(true);
rustc.env_remove(FIX_ENV);
args.apply(&mut rustc);
trace!("start rustfixing {:?}", args.file);
let fixes = rustfix_crate(&lock_addr, &rustc, &args.file, &args, config)?;
let json_error_rustc = {
let mut cmd = rustc.clone();
cmd.arg("--error-format=json");
cmd
};
let fixes = rustfix_crate(&lock_addr, &json_error_rustc, &args.file, &args, config)?;
// Ok now we have our final goal of testing out the changes that we applied.
// If these changes went awry and actually started to cause the crate to
@ -366,11 +375,8 @@ pub fn fix_maybe_exec_rustc(config: &Config) -> CargoResult<bool> {
// new rustc, and otherwise we capture the output to hide it in the scenario
// that we have to back it all out.
if !fixes.files.is_empty() {
let mut cmd = rustc.build_command();
args.apply(&mut cmd);
cmd.arg("--error-format=json");
debug!("calling rustc for final verification: {:?}", cmd);
let output = cmd.output().context("failed to spawn rustc")?;
debug!("calling rustc for final verification: {json_error_rustc}");
let output = json_error_rustc.output()?;
if output.status.success() {
for (path, file) in fixes.files.iter() {
@ -399,7 +405,18 @@ pub fn fix_maybe_exec_rustc(config: &Config) -> CargoResult<bool> {
paths::write(path, &file.original_code)?;
}
}
log_failed_fix(&output.stderr, output.status)?;
let krate = {
let mut iter = json_error_rustc.get_args();
let mut krate = None;
while let Some(arg) = iter.next() {
if arg == "--crate-name" {
krate = iter.next().and_then(|s| s.to_owned().into_string().ok());
}
}
krate
};
log_failed_fix(krate, &output.stderr, output.status)?;
}
}
@ -407,15 +424,13 @@ pub fn fix_maybe_exec_rustc(config: &Config) -> CargoResult<bool> {
// - If the fix failed, show the original warnings and suggestions.
// - If `--broken-code`, show the error messages.
// - If the fix succeeded, show any remaining warnings.
let mut cmd = rustc.build_command();
args.apply(&mut cmd);
for arg in args.format_args {
// Add any json/error format arguments that Cargo wants. This allows
// things like colored output to work correctly.
cmd.arg(arg);
rustc.arg(arg);
}
debug!("calling rustc to display remaining diagnostics: {:?}", cmd);
exit_with(cmd.status().context("failed to spawn rustc")?);
debug!("calling rustc to display remaining diagnostics: {rustc}");
exit_with(rustc.status()?);
}
#[derive(Default)]
@ -439,7 +454,7 @@ fn rustfix_crate(
filename: &Path,
args: &FixArgs,
config: &Config,
) -> Result<FixedCrate, Error> {
) -> CargoResult<FixedCrate> {
if !args.can_run_rustfix(config)? {
// This fix should not be run. Skipping...
return Ok(FixedCrate::default());
@ -502,7 +517,7 @@ fn rustfix_crate(
// We'll generate new errors below.
file.errors_applying_fixes.clear();
}
rustfix_and_fix(&mut fixes, rustc, filename, args, config)?;
rustfix_and_fix(&mut fixes, rustc, filename, config)?;
let mut progress_yet_to_be_made = false;
for (path, file) in fixes.files.iter_mut() {
if file.errors_applying_fixes.is_empty() {
@ -543,26 +558,14 @@ fn rustfix_and_fix(
fixes: &mut FixedCrate,
rustc: &ProcessBuilder,
filename: &Path,
args: &FixArgs,
config: &Config,
) -> Result<(), Error> {
) -> CargoResult<()> {
// If not empty, filter by these lints.
// TODO: implement a way to specify this.
let only = HashSet::new();
let mut cmd = rustc.build_command();
cmd.arg("--error-format=json");
args.apply(&mut cmd);
debug!(
"calling rustc to collect suggestions and validate previous fixes: {:?}",
cmd
);
let output = cmd.output().with_context(|| {
format!(
"failed to execute `{}`",
rustc.get_program().to_string_lossy()
)
})?;
debug!("calling rustc to collect suggestions and validate previous fixes: {rustc}");
let output = rustc.output()?;
// If rustc didn't succeed for whatever reasons then we're very likely to be
// looking at otherwise broken code. Let's not make things accidentally
@ -703,7 +706,7 @@ fn exit_with(status: ExitStatus) -> ! {
process::exit(status.code().unwrap_or(3));
}
fn log_failed_fix(stderr: &[u8], status: ExitStatus) -> Result<(), Error> {
fn log_failed_fix(krate: Option<String>, stderr: &[u8], status: ExitStatus) -> CargoResult<()> {
let stderr = str::from_utf8(stderr).context("failed to parse rustc stderr as utf-8")?;
let diagnostics = stderr
@ -725,19 +728,6 @@ fn log_failed_fix(stderr: &[u8], status: ExitStatus) -> Result<(), Error> {
.filter(|x| !x.starts_with('{'))
.map(|x| x.to_string()),
);
let mut krate = None;
let mut prev_dash_dash_krate_name = false;
for arg in env::args() {
if prev_dash_dash_krate_name {
krate = Some(arg.clone());
}
if arg == "--crate-name" {
prev_dash_dash_krate_name = true;
} else {
prev_dash_dash_krate_name = false;
}
}
let files = files.into_iter().collect();
let abnormal_exit = if status.code().map_or(false, is_simple_exit_code) {
@ -784,36 +774,66 @@ struct FixArgs {
}
impl FixArgs {
fn get() -> Result<FixArgs, Error> {
let rustc = env::args_os()
fn get() -> CargoResult<FixArgs> {
Self::from_args(env::args_os())
}
// This is a separate function so that we can use it in tests.
fn from_args(argv: impl IntoIterator<Item = OsString>) -> CargoResult<Self> {
let mut argv = argv.into_iter();
let mut rustc = argv
.nth(1)
.map(PathBuf::from)
.ok_or_else(|| anyhow::anyhow!("expected rustc as first argument"))?;
.ok_or_else(|| anyhow::anyhow!("expected rustc or `@path` as first argument"))?;
let mut file = None;
let mut enabled_edition = None;
let mut other = Vec::new();
let mut format_args = Vec::new();
for arg in env::args_os().skip(2) {
let mut handle_arg = |arg: OsString| -> CargoResult<()> {
let path = PathBuf::from(arg);
if path.extension().and_then(|s| s.to_str()) == Some("rs") && path.exists() {
file = Some(path);
continue;
return Ok(());
}
if let Some(s) = path.to_str() {
if let Some(edition) = s.strip_prefix("--edition=") {
enabled_edition = Some(edition.parse()?);
continue;
return Ok(());
}
if s.starts_with("--error-format=") || s.starts_with("--json=") {
// Cargo may add error-format in some cases, but `cargo
// fix` wants to add its own.
format_args.push(s.to_string());
continue;
return Ok(());
}
}
other.push(path.into());
Ok(())
};
if let Some(argfile_path) = rustc.to_str().unwrap_or_default().strip_prefix("@") {
// Because cargo in fix-proxy-mode might hit the command line size limit,
// cargo fix need handle `@path` argfile for this special case.
if argv.next().is_some() {
bail!("argfile `@path` cannot be combined with other arguments");
}
let contents = fs::read_to_string(argfile_path)
.with_context(|| format!("failed to read argfile at `{argfile_path}`"))?;
let mut iter = contents.lines().map(OsString::from);
rustc = iter
.next()
.map(PathBuf::from)
.ok_or_else(|| anyhow::anyhow!("expected rustc as first argument"))?;
for arg in iter {
handle_arg(arg)?;
}
} else {
for arg in argv {
handle_arg(arg)?;
}
}
let file = file.ok_or_else(|| anyhow::anyhow!("could not find .rs file in rustc args"))?;
let idioms = env::var(IDIOMS_ENV).is_ok();
@ -834,7 +854,7 @@ impl FixArgs {
})
}
fn apply(&self, cmd: &mut Command) {
fn apply(&self, cmd: &mut ProcessBuilder) {
cmd.arg(&self.file);
cmd.args(&self.other);
if self.prepare_for_edition.is_some() {
@ -925,3 +945,46 @@ impl FixArgs {
.and(Ok(true))
}
}
#[cfg(test)]
mod tests {
use super::FixArgs;
use std::ffi::OsString;
use std::io::Write as _;
use std::path::PathBuf;
#[test]
fn get_fix_args_from_argfile() {
let mut temp = tempfile::Builder::new().tempfile().unwrap();
let main_rs = tempfile::Builder::new().suffix(".rs").tempfile().unwrap();
let content = format!("/path/to/rustc\n{}\nfoobar\n", main_rs.path().display());
temp.write_all(content.as_bytes()).unwrap();
let argfile = format!("@{}", temp.path().display());
let args = ["cargo", &argfile];
let fix_args = FixArgs::from_args(args.map(|x| x.into())).unwrap();
assert_eq!(fix_args.rustc, PathBuf::from("/path/to/rustc"));
assert_eq!(fix_args.file, main_rs.path());
assert_eq!(fix_args.other, vec![OsString::from("foobar")]);
}
#[test]
fn get_fix_args_from_argfile_with_extra_arg() {
let mut temp = tempfile::Builder::new().tempfile().unwrap();
let main_rs = tempfile::Builder::new().suffix(".rs").tempfile().unwrap();
let content = format!("/path/to/rustc\n{}\nfoobar\n", main_rs.path().display());
temp.write_all(content.as_bytes()).unwrap();
let argfile = format!("@{}", temp.path().display());
let args = ["cargo", &argfile, "boo!"];
match FixArgs::from_args(args.map(|x| x.into())) {
Err(e) => assert_eq!(
e.to_string(),
"argfile `@path` cannot be combined with other arguments"
),
Ok(_) => panic!("should fail"),
}
}
}

21
src/cargo/util/rustc.rs
View File

@ -93,18 +93,24 @@ impl Rustc {
/// Gets a process builder set up to use the found rustc version, with a wrapper if `Some`.
pub fn process(&self) -> ProcessBuilder {
ProcessBuilder::new(self.path.as_path()).wrapped(self.wrapper.as_ref())
let mut cmd = ProcessBuilder::new(self.path.as_path()).wrapped(self.wrapper.as_ref());
cmd.retry_with_argfile(true);
cmd
}
/// Gets a process builder set up to use the found rustc version, with a wrapper if `Some`.
pub fn workspace_process(&self) -> ProcessBuilder {
ProcessBuilder::new(self.path.as_path())
let mut cmd = ProcessBuilder::new(self.path.as_path())
.wrapped(self.workspace_wrapper.as_ref())
.wrapped(self.wrapper.as_ref())
.wrapped(self.wrapper.as_ref());
cmd.retry_with_argfile(true);
cmd
}
pub fn process_no_wrapper(&self) -> ProcessBuilder {
ProcessBuilder::new(&self.path)
let mut cmd = ProcessBuilder::new(&self.path);
cmd.retry_with_argfile(true);
cmd
}
/// Gets the output for the given command.
@ -231,10 +237,7 @@ impl Cache {
} else {
debug!("rustc info cache miss");
debug!("running {}", cmd);
let output = cmd
.build_command()
.output()
.with_context(|| format!("could not execute process {} (never executed)", cmd))?;
let output = cmd.output()?;
let stdout = String::from_utf8(output.stdout)
.map_err(|e| anyhow::anyhow!("{}: {:?}", e, e.as_bytes()))
.with_context(|| format!("`{}` didn't return utf8 output", cmd))?;
@ -351,7 +354,7 @@ fn rustc_fingerprint(
fn process_fingerprint(cmd: &ProcessBuilder, extra_fingerprint: u64) -> u64 {
let mut hasher = StableHasher::new();
extra_fingerprint.hash(&mut hasher);
cmd.get_args().hash(&mut hasher);
cmd.get_args().for_each(|arg| arg.hash(&mut hasher));
let mut env = cmd.get_envs().iter().collect::<Vec<_>>();
env.sort_unstable();
env.hash(&mut hasher);

13
tests/testsuite/fix.rs
View File

@ -89,8 +89,14 @@ fn broken_fixes_backed_out() {
fn main() {
// Ignore calls to things like --print=file-names and compiling build.rs.
// Also compatible for rustc invocations with `@path` argfile.
let is_lib_rs = env::args_os()
.map(PathBuf::from)
.flat_map(|p| if let Some(p) = p.to_str().unwrap_or_default().strip_prefix("@") {
fs::read_to_string(p).unwrap().lines().map(PathBuf::from).collect()
} else {
vec![p]
})
.any(|l| l == Path::new("src/lib.rs"));
if is_lib_rs {
let path = PathBuf::from(env::var_os("OUT_DIR").unwrap());
@ -1319,8 +1325,15 @@ fn fix_to_broken_code() {
use std::process::{self, Command};
fn main() {
// Ignore calls to things like --print=file-names and compiling build.rs.
// Also compatible for rustc invocations with `@path` argfile.
let is_lib_rs = env::args_os()
.map(PathBuf::from)
.flat_map(|p| if let Some(p) = p.to_str().unwrap_or_default().strip_prefix("@") {
fs::read_to_string(p).unwrap().lines().map(PathBuf::from).collect()
} else {
vec![p]
})
.any(|l| l == Path::new("src/lib.rs"));
if is_lib_rs {
let path = PathBuf::from(env::var_os("OUT_DIR").unwrap());

1
tests/testsuite/rustdocflags.rs
View File

@ -112,6 +112,7 @@ fn whitespace() {
const SPACED_VERSION: &str = "a\nb\tc\u{00a0}d";
p.cargo("doc")
.env_remove("__CARGO_TEST_FORCE_ARGFILE") // Not applicable for argfile.
.env(
"RUSTDOCFLAGS",
format!("--crate-version {}", SPACED_VERSION),