fix(vcs): chunk path arguments so large rollbacks fit the Windows command line (#468)

Rollback, stage, unstage, clean, reset and shelve passed every path to one git invocation, so a few thousand files exceeded the 32,767-character CreateProcess limit and failed with os error 206 before git ran. Path-taking commands now run in batches sized to the platform limit, with room reserved for the quotes Windows adds around arguments containing spaces. Commit and stash are not chunked, since splitting them would change their result.

Covers the sibling call sites in patch.rs and shelf.rs. Not verified on a real Windows machine.

Closes #457

Co-authored-by: a-rookie-of-C-language <[email protected]>

crates/ide-vcs/src/cli.rs

@@ -128,6 +128,78 @@ pub fn git_version(executable: Option<&Path>) -> IdeResult<String> {
     Ok(text)
 }
 
+/// `CreateProcessW` takes at most 32767 chars including the NUL; leave slack for quoting.
+/// (The same limits as `ide_run::java_cmdline`'s shortened java command lines.)
+pub(crate) const WINDOWS_LIMIT: usize = 32_000;
+/// Linux caps one argument at 128 KiB (`MAX_ARG_STRLEN`); macOS the whole `ARG_MAX` (1 MiB, env included).
+pub(crate) const UNIX_LIMIT: usize = 120_000;
+
+/// The longest command line this platform's process spawning accepts.
+pub(crate) fn command_line_limit() -> usize {
+    if cfg!(windows) { WINDOWS_LIMIT } else { UNIX_LIMIT }
+}
+
+/// Slack for the program path and the `-c` options [`command`] adds.
+const COMMAND_LINE_SLACK: usize = 512;
+
+/// Budget for `base` (two quotes and one separator per argument) plus [`COMMAND_LINE_SLACK`].
+pub(crate) fn base_command_line_len(base: &[&str]) -> usize {
+    base.iter().map(|a| a.len() + 3).sum::<usize>() + COMMAND_LINE_SLACK
+}
+
+/// Splits `paths` into runs whose length (`base_len` plus each path, two quotes, and a separator)
+/// stays within `limit`. A path that alone exceeds the remaining room still gets a batch
+/// of its own, so every call makes progress; an empty list yields no batches.
+pub(crate) fn batch_paths(base_len: usize, paths: &[String], limit: usize) -> Vec<&[String]> {
+    let mut batches = Vec::new();
+    let mut start = 0;
+    while start < paths.len() {
+        let mut len = base_len;
+        let mut end = start;
+        while end < paths.len() {
+            // Windows quotes arguments containing spaces or tabs; reserve both quotes on every platform.
+            let next = len + paths[end].len() + 3;
+            if next > limit && end > start {
+                break;
+            }
+            len = next;
+            end += 1;
+        }
+        batches.push(&paths[start..end]);
+        start = end;
+    }
+    batches
+}
+
+/// [`run`] with a list of paths appended after `--`, split into as many invocations as
+/// the platform's command-line limit needs (a large rollback would otherwise fail with
+/// `os error 206` on Windows). `args` must already carry `--literal-pathspecs` and
+/// end with `--`; the first error stops the remaining batches.
+pub(crate) fn run_paths_env(
+    root: &Path,
+    args: &[&str],
+    paths: &[String],
+    stdin: Option<&[u8]>,
+    env: &[(&str, &std::ffi::OsStr)],
+) -> IdeResult<Output> {
+    let mut out = Output { stdout: Vec::new(), stderr: Vec::new(), code: Some(0) };
+    if paths.is_empty() {
+        return Ok(out);
+    }
+    let base_len = base_command_line_len(args);
+    for batch in batch_paths(base_len, paths, command_line_limit()) {
+        let mut full: Vec<&str> = args.to_vec();
+        full.extend(batch.iter().map(String::as_str));
+        out = run_env(root, &full, stdin, env)?;
+    }
+    Ok(out)
+}
+
+/// [`run_paths_env`] without extra environment variables.
+pub(crate) fn run_paths(root: &Path, args: &[&str], paths: &[String]) -> IdeResult<Output> {
+    run_paths_env(root, args, paths, None, &[])
+}
+
 /// Builds a `git` command with the environment every invocation needs.
 pub(crate) fn command(root: &Path) -> IdeResult<Command> {
     let mut cmd = Command::new(git_binary()?);
@@ -500,3 +572,70 @@ mod command_session_tests {
         assert_eq!(ours, vec![(second_id, false, String::new()), (first_id, true, "done".into())]);
     }
 }
+
+#[cfg(test)]
+mod batching_tests {
+    use super::{WINDOWS_LIMIT, base_command_line_len, batch_paths, command_line_limit};
+
+    fn paths(n: usize, len: usize) -> Vec<String> {
+        (0..n).map(|i| format!("src/file-{i}.rs").chars().cycle().take(len).collect()).collect()
+    }
+
+    fn len_of(base_len: usize, batch: &[String]) -> usize {
+        base_len + batch.iter().map(|p| p.len() + 3).sum::<usize>()
+    }
+
+    #[test]
+    fn empty_input_yields_no_batches() {
+        let empty: Vec<String> = Vec::new();
+        assert!(batch_paths(10, &empty, 100).is_empty());
+    }
+
+    #[test]
+    fn every_path_is_batched_exactly_once_and_in_order() {
+        let all = paths(100, 12);
+        let batches = batch_paths(50, &all, 200);
+        let joined: Vec<&String> = batches.iter().flat_map(|b| b.iter()).collect();
+        assert_eq!(joined, all.iter().collect::<Vec<_>>());
+        assert!(batches.len() > 1);
+    }
+
+    #[test]
+    fn batches_stay_within_the_limit() {
+        let all = paths(5_000, 40);
+        let base = 40;
+        let limit = 1_000;
+        for batch in batch_paths(base, &all, limit) {
+            assert!(len_of(base, batch) <= limit || batch.len() == 1);
+        }
+        // The whole list would not have fit one invocation.
+        assert!(len_of(base, &all) > limit);
+    }
+
+    #[test]
+    fn paths_with_spaces_fit_the_windows_limit_including_quotes() {
+        let all = vec![format!("directory with spaces/{}.rs", "x".repeat(15)); 5_000];
+        assert_eq!(all[0].len(), 40);
+        let base = base_command_line_len(&["--literal-pathspecs", "add", "-A", "--"]);
+        let batches = batch_paths(base, &all, WINDOWS_LIMIT);
+        assert!(batches.len() > 1);
+        for batch in batches {
+            assert!(len_of(base, batch) <= WINDOWS_LIMIT);
+        }
+    }
+
+    #[test]
+    fn an_oversized_path_gets_its_own_batch() {
+        let all = vec!["a".to_string(), "x".repeat(10_000), "b".to_string()];
+        let batches = batch_paths(0, &all, 100);
+        assert_eq!(batches.len(), 3);
+        assert_eq!(batches[0].to_vec(), vec!["a".to_string()]);
+        assert_eq!(batches[1].to_vec(), vec!["x".repeat(10_000)]);
+        assert_eq!(batches[2].to_vec(), vec!["b".to_string()]);
+    }
+
+    #[test]
+    fn the_platform_limit_is_a_positive_budget() {
+        assert!(command_line_limit() > 1_000);
+    }
+}

crates/ide-vcs/src/patch.rs

@@ -47,9 +47,7 @@ pub(crate) fn working_patch(r: &Repo, rel_paths: &[String], reverse: bool) -> Id
     } else {
         run_env(r.root(), &["read-tree", "--empty"], None, &env)?;
     }
-    let mut add: Vec<&str> = vec!["--literal-pathspecs", "add", "-A", "--"];
-    add.extend(rel_paths.iter().map(String::as_str));
-    run_env(r.root(), &add, None, &env)?;
+    cli::run_paths_env(r.root(), &["--literal-pathspecs", "add", "-A", "--"], rel_paths, None, &env)?;
     let mut diff =
         vec!["diff", "--cached", "--binary", "--full-index", "-M", "--no-color", "--no-ext-diff", "--no-textconv"];
     if reverse {
@@ -155,9 +153,7 @@ pub(crate) fn apply(r: &Repo, patch: &str) -> IdeResult<ApplyOutcome> {
     // `--3way` stages what it applied; keep the index as it was for the clean files.
     let clean: Vec<String> = paths.iter().filter(|p| !conflicts.contains(p)).cloned().collect();
     if !clean.is_empty() && r.has_head()? {
-        let mut args = vec!["--literal-pathspecs", "reset", "-q", "--"];
-        args.extend(clean.iter().map(String::as_str));
-        run(r.root(), &args, None)?;
+        cli::run_paths(r.root(), &["--literal-pathspecs", "reset", "-q", "--"], &clean)?;
     }
     Ok(ApplyOutcome { applied: clean, conflicts })
 }

crates/ide-vcs/src/shelf.rs

@@ -2,7 +2,7 @@
 //! `.idea/shelf`. Each shelved change list is `<shelf>/<id>/shelved.patch` plus
 //! `<shelf>/<id>/shelf.json` with its name, date and files.
 
-use crate::cli::run;
+use crate::cli::run_paths;
 use crate::{ApplyOutcome, ChangeKind, Repo, ShelvedChangeList};
 use ide_core::{IdeError, IdeResult};
 use serde::{Deserialize, Serialize};
@@ -152,15 +152,14 @@ pub(crate) fn shelve(r: &Repo, shelf: &Path, name: &str, rel_paths: &[String]) -
 fn revert_files(r: &Repo, files: &[crate::ChangedFile]) -> IdeResult<()> {
     let has_head = r.has_head()?;
     let (in_head, new): (Vec<_>, Vec<_>) = files.iter().partition(|f| has_head && f.kind != ChangeKind::Added);
+    let paths = |files: &[&crate::ChangedFile]| -> Vec<String> { files.iter().map(|f| f.path.clone()).collect() };
     if !in_head.is_empty() {
-        let mut args = vec!["--literal-pathspecs", "restore", "--source=HEAD", "--staged", "--worktree", "--"];
-        args.extend(in_head.iter().map(|f| f.path.as_str()));
-        run(r.root(), &args, None)?;
+        let args = ["--literal-pathspecs", "restore", "--source=HEAD", "--staged", "--worktree", "--"];
+        run_paths(r.root(), &args, &paths(&in_head))?;
     }
     if !new.is_empty() {
-        let mut args = vec!["--literal-pathspecs", "rm", "--cached", "-q", "-r", "--ignore-unmatch", "--"];
-        args.extend(new.iter().map(|f| f.path.as_str()));
-        run(r.root(), &args, None)?;
+        let args = ["--literal-pathspecs", "rm", "--cached", "-q", "-r", "--ignore-unmatch", "--"];
+        run_paths(r.root(), &args, &paths(&new))?;
         for f in new {
             let abs = r.abs_path(&f.path);
             if abs.is_file() || abs.is_symlink() {

crates/ide-vcs/src/write.rs

@@ -31,16 +31,14 @@ impl Repo {
     }
 }
 
-/// Runs a path-taking git command with literal pathspecs.
+/// Runs a path-taking git command with literal pathspecs, in as many invocations as
+/// the platform's command-line limit needs (a large rollback would otherwise fail with
+/// `os error 206` on Windows).
 fn run_paths(r: &Repo, args: &[&str], paths: &[String]) -> IdeResult<()> {
-    if paths.is_empty() {
-        return Ok(());
-    }
-    let mut full: Vec<&str> = vec!["--literal-pathspecs"];
-    full.extend_from_slice(args);
-    full.push("--");
-    full.extend(paths.iter().map(String::as_str));
-    run(r.root(), &full, None).map(|_| ())
+    let mut base: Vec<&str> = vec!["--literal-pathspecs"];
+    base.extend_from_slice(args);
+    base.push("--");
+    cli::run_paths(r.root(), &base, paths).map(|_| ())
 }
 
 pub(crate) fn blame(