Skip to content

Commit 461b37f

Browse files
committed
fix: Use arg files in compiletest
This allows compiletest to support the new Cargo `build-dir` layout which passes more `-L` flags as the `deps` dir has been split per build unit. This can be an issue on Windows as the max command size is fairly small.
1 parent deab78d commit 461b37f

10 files changed

Lines changed: 159 additions & 45 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -872,6 +872,7 @@ dependencies = [
872872
"semver",
873873
"serde",
874874
"serde_json",
875+
"tempfile",
875876
"tracing",
876877
"tracing-subscriber",
877878
"unified-diff",

src/tools/compiletest/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ rustfix = "0.8.1"
3131
semver = { version = "1.0.23", features = ["serde"] }
3232
serde = { version = "1.0", features = ["derive"] }
3333
serde_json = "1.0"
34+
tempfile = "3.23.0"
3435
tracing = "0.1"
3536
tracing-subscriber = { version = "0.3.3", default-features = false, features = ["ansi", "env-filter", "fmt", "parking_lot", "smallvec"] }
3637
unified-diff = "0.2.1"

src/tools/compiletest/src/runtest.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ use crate::errors::{Error, ErrorKind, load_errors};
2323
use crate::output_capture::ConsoleOut;
2424
use crate::read2::{Truncated, read2_abbreviated};
2525
use crate::runtest::compute_diff::{DiffLine, diff_by_lines, make_diff, write_diff};
26-
use crate::util::{Utf8PathBufExt, add_dylib_path, static_regex};
26+
use crate::util::{ArgFileCommand, Utf8PathBufExt, add_dylib_path, static_regex};
2727
use crate::{json, stamp_file_path};
2828

2929
// Helper modules that implement test running logic for each test suite.
@@ -383,7 +383,7 @@ impl<'test> TestCx<'test> {
383383
}
384384
}
385385

386-
/// Runs a [`Command`] and waits for it to finish, then converts its exit
386+
/// Runs a [`ArgFileCommand`] and waits for it to finish, then converts its exit
387387
/// status and output streams into a [`ProcRes`].
388388
///
389389
/// The command might have succeeded or failed; it is the caller's
@@ -393,7 +393,8 @@ impl<'test> TestCx<'test> {
393393
/// Panics if the command couldn't be executed at all
394394
/// (e.g. because the executable could not be found).
395395
#[must_use = "caller should check whether the command succeeded"]
396-
fn run_command_to_procres(&self, cmd: &mut Command) -> ProcRes {
396+
fn run_command_to_procres(&self, cmd: ArgFileCommand) -> ProcRes {
397+
let (mut cmd, _arg_file) = cmd.build().unwrap();
397398
let output = cmd
398399
.output()
399400
.unwrap_or_else(|e| self.fatal(&format!("failed to exec `{cmd:?}` because: {e}")));

src/tools/compiletest/src/runtest/coverage.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use glob::glob;
88

99
use crate::common::{TestSuite, UI_COVERAGE, UI_COVERAGE_MAP};
1010
use crate::runtest::{Emit, ProcRes, TestCx, WillExecute};
11-
use crate::util::static_regex;
11+
use crate::util::{ArgFileCommand, static_regex};
1212

1313
impl<'test> TestCx<'test> {
1414
fn coverage_dump_path(&self) -> &Utf8Path {
@@ -27,9 +27,9 @@ impl<'test> TestCx<'test> {
2727
}
2828
drop(proc_res);
2929

30-
let mut dump_command = Command::new(coverage_dump_path);
30+
let mut dump_command = ArgFileCommand::new(coverage_dump_path);
3131
dump_command.arg(llvm_ir_path);
32-
let proc_res = self.run_command_to_procres(&mut dump_command);
32+
let proc_res = self.run_command_to_procres(dump_command);
3333
if !proc_res.status.success() {
3434
self.fatal_proc_rec("coverage-dump failed!", &proc_res);
3535
}
@@ -227,18 +227,22 @@ impl<'test> TestCx<'test> {
227227
}
228228
}
229229

230-
fn run_llvm_tool(&self, name: &str, configure_cmd_fn: impl FnOnce(&mut Command)) -> ProcRes {
230+
fn run_llvm_tool(
231+
&self,
232+
name: &str,
233+
configure_cmd_fn: impl FnOnce(&mut ArgFileCommand),
234+
) -> ProcRes {
231235
let tool_path = self
232236
.config
233237
.llvm_bin_dir
234238
.as_ref()
235239
.expect("this test expects the LLVM bin dir to be available")
236240
.join(name);
237241

238-
let mut cmd = Command::new(tool_path);
242+
let mut cmd = ArgFileCommand::new(tool_path);
239243
configure_cmd_fn(&mut cmd);
240244

241-
self.run_command_to_procres(&mut cmd)
245+
self.run_command_to_procres(cmd)
242246
}
243247

244248
fn normalize_coverage_output(&self, coverage: &str) -> Result<String, String> {

src/tools/compiletest/src/runtest/debuginfo.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use tracing::debug;
99
use super::debugger::DebuggerCommands;
1010
use super::{Debugger, Emit, ProcRes, TestCx, Truncated, WillExecute};
1111
use crate::debuggers::extract_gdb_version;
12+
use crate::util::ArgFileCommand;
1213

1314
impl TestCx<'_> {
1415
pub(super) fn run_debuginfo_test(&self) {
@@ -458,7 +459,7 @@ impl TestCx<'_> {
458459
// make sure `PATH` points to all the dlls necessary to run the debugee
459460
let path = prepend_to_path(&self.config.target_run_lib_path);
460461

461-
let mut cmd = Command::new(lldb);
462+
let mut cmd = ArgFileCommand::new(lldb);
462463
cmd.arg("--one-line")
463464
.arg("script --language python -- import lldb_batchmode; lldb_batchmode.main()")
464465
.env("LLDB_BATCHMODE_TARGET_PATH", test_executable)
@@ -467,7 +468,7 @@ impl TestCx<'_> {
467468
.env("PYTHONPATH", pythonpath)
468469
.env("PATH", path);
469470

470-
self.run_command_to_procres(&mut cmd)
471+
self.run_command_to_procres(cmd)
471472
}
472473
}
473474

src/tools/compiletest/src/runtest/js_doc.rs

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
use std::process::Command;
2-
31
use super::{DocKind, TestCx};
2+
use crate::util::ArgFileCommand;
43

54
impl TestCx<'_> {
65
pub(super) fn run_rustdoc_js_test(&self) {
@@ -10,18 +9,18 @@ impl TestCx<'_> {
109
self.document(&out_dir, DocKind::Html);
1110

1211
let file_stem = self.testpaths.file.file_stem().expect("no file stem");
13-
let res = self.run_command_to_procres(
14-
Command::new(&nodejs)
15-
.arg(self.config.src_root.join("src/tools/rustdoc-js/tester.js"))
16-
.arg("--doc-folder")
17-
.arg(out_dir)
18-
.arg("--crate-name")
19-
.arg(file_stem.replace("-", "_"))
20-
.arg("--test-file")
21-
.arg(self.testpaths.file.with_extension("js"))
22-
.arg("--revision")
23-
.arg(self.revision.unwrap_or_default()),
24-
);
12+
13+
let mut cmd = ArgFileCommand::new(&nodejs);
14+
cmd.arg(self.config.src_root.join("src/tools/rustdoc-js/tester.js"))
15+
.arg("--doc-folder")
16+
.arg(out_dir)
17+
.arg("--crate-name")
18+
.arg(file_stem.replace("-", "_"))
19+
.arg("--test-file")
20+
.arg(self.testpaths.file.with_extension("js"))
21+
.arg("--revision")
22+
.arg(self.revision.unwrap_or_default());
23+
let res = self.run_command_to_procres(cmd);
2524
if !res.status.success() {
2625
self.fatal_proc_rec("rustdoc-js test failed!", &res);
2726
}

src/tools/compiletest/src/runtest/run_make.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use camino::{Utf8Path, Utf8PathBuf};
77

88
use super::{ProcRes, TestCx, disable_error_reporting};
99
use crate::common::TestSuite;
10-
use crate::util::{copy_dir_all, dylib_env_var};
10+
use crate::util::{ArgFileCommand, copy_dir_all, dylib_env_var};
1111

1212
impl TestCx<'_> {
1313
pub(super) fn run_rmake_test(&self) {
@@ -113,7 +113,7 @@ impl TestCx<'_> {
113113
.stage0_rustc_path
114114
.as_ref()
115115
.expect("stage0 rustc is required to run run-make tests");
116-
let mut rustc = Command::new(&stage0_rustc);
116+
let mut rustc = ArgFileCommand::new(&stage0_rustc);
117117
rustc
118118
// `rmake.rs` **must** be buildable by a stable compiler, it may not use *any* unstable
119119
// library or compiler features. Here, we force the stage 0 rustc to consider itself as
@@ -139,7 +139,7 @@ impl TestCx<'_> {
139139
rustc.arg("-Dunused_must_use");
140140

141141
// Now run rustc to build the recipe.
142-
let res = self.run_command_to_procres(&mut rustc);
142+
let res = self.run_command_to_procres(rustc);
143143
if !res.status.success() {
144144
self.fatal_proc_rec("run-make test failed: could not build `rmake.rs` recipe", &res);
145145
}

src/tools/compiletest/src/runtest/rustdoc.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
use std::process::Command;
2-
31
use super::{DocKind, TestCx, remove_and_create_dir_all};
2+
use crate::util::ArgFileCommand;
43

54
impl TestCx<'_> {
65
pub(super) fn run_rustdoc_html_test(&self) {
@@ -19,14 +18,14 @@ impl TestCx<'_> {
1918
if self.props.check_test_line_numbers_match {
2019
self.check_rustdoc_test_option(proc_res);
2120
} else {
22-
let mut cmd = Command::new(&self.config.python);
21+
let mut cmd = ArgFileCommand::new(&self.config.python);
2322
cmd.arg(self.config.src_root.join("src/etc/htmldocck.py"))
2423
.arg(&out_dir)
2524
.arg(&self.testpaths.file);
2625
if self.config.bless {
2726
cmd.arg("--bless");
2827
}
29-
let res = self.run_command_to_procres(&mut cmd);
28+
let res = self.run_command_to_procres(cmd);
3029
if !res.status.success() {
3130
self.fatal_proc_rec("htmldocck failed!", &res);
3231
}

src/tools/compiletest/src/runtest/rustdoc_json.rs

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
1-
use std::process::Command;
2-
31
use super::{DocKind, TestCx, remove_and_create_dir_all};
2+
use crate::util::ArgFileCommand;
43

54
impl TestCx<'_> {
65
pub(super) fn run_rustdoc_json_test(&self) {
@@ -23,13 +22,9 @@ impl TestCx<'_> {
2322
self.fatal_proc_rec("rustdoc failed!", &proc_res);
2423
}
2524

26-
let res = self.run_command_to_procres(
27-
Command::new(self.config.jsondocck_path.as_ref().unwrap())
28-
.arg("--doc-dir")
29-
.arg(&out_dir)
30-
.arg("--template")
31-
.arg(&self.testpaths.file),
32-
);
25+
let mut cmd = ArgFileCommand::new(self.config.jsondocck_path.as_ref().unwrap());
26+
cmd.arg("--doc-dir").arg(&out_dir).arg("--template").arg(&self.testpaths.file);
27+
let res = self.run_command_to_procres(cmd);
3328

3429
if !res.status.success() {
3530
self.fatal_proc_rec_general("jsondocck failed!", None, &res, || {
@@ -41,9 +36,9 @@ impl TestCx<'_> {
4136
let mut json_out = out_dir.join(self.testpaths.file.file_stem().unwrap());
4237
json_out.set_extension("json");
4338

44-
let res = self.run_command_to_procres(
45-
Command::new(self.config.jsondoclint_path.as_ref().unwrap()).arg(&json_out),
46-
);
39+
let mut cmd = ArgFileCommand::new(self.config.jsondoclint_path.as_ref().unwrap());
40+
cmd.arg(&json_out);
41+
let res = self.run_command_to_procres(cmd);
4742

4843
if !res.status.success() {
4944
self.fatal_proc_rec("jsondoclint failed!", &res);

src/tools/compiletest/src/util.rs

Lines changed: 114 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
use std::env;
2-
use std::process::Command;
2+
use std::ffi::{OsStr, OsString};
3+
use std::io::Write;
4+
use std::path::Path;
5+
use std::process::{Command, CommandEnvs};
36

47
use camino::{Utf8Path, Utf8PathBuf};
8+
use tempfile::NamedTempFile;
59

610
#[cfg(test)]
711
mod tests;
@@ -149,3 +153,112 @@ macro_rules! string_enum {
149153
}
150154

151155
pub(crate) use string_enum;
156+
157+
/// A wrapper around [`Command`] that adds support for arg files.
158+
/// This is useful as we have some commands that can get very long and at times
159+
/// hit the OS limit (usually Windows)
160+
///
161+
/// This implementation is based off the of `ProcessBuilder` implementation in Cargo
162+
/// but simplified.
163+
///
164+
/// NOTE: In most scenarios we want to avoid arg files as it makes debugging more complicated
165+
/// so we try to avoid it if the command is not close the the OS limit.
166+
#[derive(Debug)]
167+
pub(crate) struct ArgFileCommand {
168+
command: Command,
169+
args: Vec<OsString>,
170+
}
171+
172+
#[allow(dead_code)] // Roughly match the `std::process::Command` API
173+
impl ArgFileCommand {
174+
#[track_caller]
175+
pub(crate) fn new<S: AsRef<OsStr>>(program: S) -> Self {
176+
let command = Command::new(program);
177+
Self { command, args: Vec::new() }
178+
}
179+
pub(crate) fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Self {
180+
self.args.push(arg.as_ref().to_os_string());
181+
self
182+
}
183+
184+
pub(crate) fn args<I, S>(&mut self, args: I) -> &mut Self
185+
where
186+
I: IntoIterator<Item = S>,
187+
S: AsRef<OsStr>,
188+
{
189+
self.args.extend(args.into_iter().map(|s| s.as_ref().to_os_string()));
190+
self
191+
}
192+
193+
pub(crate) fn env<K, V>(&mut self, key: K, val: V) -> &mut Self
194+
where
195+
K: AsRef<OsStr>,
196+
V: AsRef<OsStr>,
197+
{
198+
self.command.env(key, val);
199+
self
200+
}
201+
202+
pub(crate) fn get_envs(&self) -> CommandEnvs<'_> {
203+
self.command.get_envs()
204+
}
205+
206+
pub(crate) fn env_remove<K: AsRef<OsStr>>(&mut self, key: K) -> &mut Self {
207+
self.command.env_remove(key);
208+
self
209+
}
210+
211+
pub(crate) fn current_dir<P: AsRef<Path>>(&mut self, dir: P) -> &mut Self {
212+
self.command.current_dir(dir);
213+
self
214+
}
215+
216+
pub(crate) fn stdin(&mut self, stdin: std::process::Stdio) -> &mut Self {
217+
self.command.stdin(stdin);
218+
self
219+
}
220+
221+
pub(crate) fn build(mut self) -> std::io::Result<(Command, NamedTempFile)> {
222+
let mut tmp = tempfile::Builder::new().prefix("compiletest-argfile.").tempfile()?;
223+
224+
// On Windows there is a hard limit of ~32KB, so we cut off at 30KB to
225+
// give some buffer just incase.
226+
#[cfg(windows)]
227+
let threshold: usize = 30 * 1024;
228+
// On unix the limit is defined by ARG_MAX. If its not explicitly set we set it to 1MB
229+
// which is fairly large but lower than the ~2MB that it defaults to on most systems.
230+
#[cfg(unix)]
231+
let threshold: usize =
232+
std::env::var("ARG_MAX").ok().and_then(|v| v.parse().ok()).unwrap_or(1024 * 1024);
233+
234+
let total_arg_len: usize = self.args.iter().map(|a| a.len() + 1).sum();
235+
if total_arg_len <= threshold {
236+
self.command.args(self.args);
237+
return Ok((self.command, tmp));
238+
}
239+
240+
let mut arg = OsString::from("@");
241+
arg.push(tmp.path());
242+
self.command.arg(arg);
243+
244+
let cap = self.args.iter().map(|arg| arg.len() + 1).sum::<usize>();
245+
let mut buf = Vec::with_capacity(cap);
246+
for arg in &self.args {
247+
let arg = arg.to_str().ok_or_else(|| {
248+
std::io::Error::other(format!(
249+
"argument for argfile contains invalid UTF-8 characters: `{}`",
250+
arg.to_string_lossy()
251+
))
252+
})?;
253+
if arg.contains('\n') {
254+
return Err(std::io::Error::other(format!(
255+
"argument for argfile contains newlines: `{arg}`"
256+
)));
257+
}
258+
writeln!(buf, "{arg}")?;
259+
}
260+
tmp.write_all(&buf)?;
261+
262+
Ok((self.command, tmp))
263+
}
264+
}

0 commit comments

Comments
 (0)