diff options
| author | bors <bors@rust-lang.org> | 2024-03-03 14:02:57 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2024-03-03 14:02:57 +0000 |
| commit | f47732b55e74774595f5b12bdbfc8e8e0ec95508 (patch) | |
| tree | 49f056af4f13d39dc07682d5527d3ba439e9be71 | |
| parent | f51f9236f9e708dded92bb40b929f0da7eb93357 (diff) | |
| parent | 83e2e2d89dae346d38909f5590d9231cf2a5d218 (diff) | |
| download | rust-f47732b55e74774595f5b12bdbfc8e8e0ec95508.tar.gz rust-f47732b55e74774595f5b12bdbfc8e8e0ec95508.zip | |
Auto merge of #3330 - RossSmyth:win-fmt, r=RalfJung
Fix .\miri fmt on Windows This allows .\miri fmt to work on Windows. Closes #3317. To reiterate, the problem with using `miri fmt` on Windows is that the CLI arguments to rustfmt are too long. Currently over 65,000 characters are used in the call to rustfmt, [which is incompatible with Windows](https://devblogs.microsoft.com/oldnewthing/20031210-00/?p=41553) as it is limited to (2^15 - 1) for all arguments plus all env vars. Two things are done do get around this limit: 1. Call out to cargo-fmt for the crates that exist. 2. Batch rustfmt calls by length Another alternative would be to just use rustfmt for everything and don't use cargo-fmt for the crates. I don't know how much you guys may care about `miri fmt` time to run. I don't know the diff as it did not work before on my computer. [I have another branch that solves this, but in a less permanent way](RossSmyth/miri/tree/windows-fmt). That was my initial attempt, and I learned that even with cargo-fmt and relative paths, the rustfmt call still has 27k characters. This is closer to the limit than I expected, so it would not be a permanent solution. So I went back to absolute paths & batching.
| -rw-r--r-- | src/tools/miri/miri-script/src/commands.rs | 46 | ||||
| -rw-r--r-- | src/tools/miri/miri-script/src/util.rs | 48 |
2 files changed, 64 insertions, 30 deletions
diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 88ee62fc18a..dc6ef58c520 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -526,37 +526,27 @@ impl Command { } fn fmt(flags: Vec<OsString>) -> Result<()> { + use itertools::Itertools; + let e = MiriEnv::new()?; - let toolchain = &e.toolchain; let config_path = path!(e.miri_dir / "rustfmt.toml"); - let mut cmd = cmd!( - e.sh, - "rustfmt +{toolchain} --edition=2021 --config-path {config_path} --unstable-features --skip-children {flags...}" - ); - eprintln!("$ {cmd} ..."); - - // Add all the filenames to the command. - // FIXME: `rustfmt` will follow the `mod` statements in these files, so we get a bunch of - // duplicate diffs. - for item in WalkDir::new(&e.miri_dir).into_iter().filter_entry(|entry| { - let name = entry.file_name().to_string_lossy(); - let ty = entry.file_type(); - if ty.is_file() { - name.ends_with(".rs") - } else { - // dir or symlink. skip `target` and `.git`. - &name != "target" && &name != ".git" - } - }) { - let item = item?; - if item.file_type().is_file() { - cmd = cmd.arg(item.into_path()); - } - } + // Collect each rust file in the miri repo. + let files = WalkDir::new(&e.miri_dir) + .into_iter() + .filter_entry(|entry| { + let name = entry.file_name().to_string_lossy(); + let ty = entry.file_type(); + if ty.is_file() { + name.ends_with(".rs") + } else { + // dir or symlink. skip `target` and `.git`. + &name != "target" && &name != ".git" + } + }) + .filter_ok(|item| item.file_type().is_file()) + .map_ok(|item| item.into_path()); - // We want our own error message, repeating the command is too much. - cmd.quiet().run().map_err(|_| anyhow!("`rustfmt` failed"))?; - Ok(()) + e.format_files(files, &e.toolchain[..], &config_path, &flags[..]) } } diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index 420ecdab63c..361a4ca0cf7 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -1,7 +1,7 @@ use std::ffi::{OsStr, OsString}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use dunce::canonicalize; use path_macro::path; use xshell::{cmd, Shell}; @@ -145,4 +145,48 @@ impl MiriEnv { .run()?; Ok(()) } + + /// Receives an iterator of files. + /// Will format each file with the miri rustfmt config. + /// Does not recursively format modules. + pub fn format_files( + &self, + files: impl Iterator<Item = Result<PathBuf, walkdir::Error>>, + toolchain: &str, + config_path: &Path, + flags: &[OsString], + ) -> anyhow::Result<()> { + use itertools::Itertools; + + let mut first = true; + + // Format in batches as not all our files fit into Windows' command argument limit. + for batch in &files.chunks(256) { + // Build base command. + let mut cmd = cmd!( + self.sh, + "rustfmt +{toolchain} --edition=2021 --config-path {config_path} --unstable-features --skip-children {flags...}" + ); + if first { + // Log an abbreviating command, and only once. + eprintln!("$ {cmd} ..."); + first = false; + } + // Add files. + for file in batch { + // Make it a relative path so that on platforms with extremely tight argument + // limits (like Windows), we become immune to someone cloning the repo + // 50 directories deep. + let file = file?; + let file = file.strip_prefix(&self.miri_dir)?; + cmd = cmd.arg(file); + } + + // Run rustfmt. + // We want our own error message, repeating the command is too much. + cmd.quiet().run().map_err(|_| anyhow!("`rustfmt` failed"))?; + } + + Ok(()) + } } |
