fix(vcs): keep history revision lists off the git command line (#2075)

Refs #1538

Show History on a repository with many branches and tags failed on Windows with os error 206 ("文件名或扩展名过长"): default-scope history passed HEAD and every branch/tag tip as argv revisions, and the command line overflowed the ~32K limit. Revisions now count toward the same argv budget as pathspecs; when they overflow, revisions, `--` and paths go through `git log --stdin`. The same bound applies to every history walk in ide-vcs, including the shallow-clone `rev-list`. Small inputs produce the same command as before. Verified on macOS with 1500 tags on a deeply nested file and folder; Windows wasn't run natively.

Co-authored-by: Inf1mum <[email protected]>

crates/ide-vcs/src/history.rs

@@ -80,17 +80,8 @@ fn git_select(
             args.push("--first-parent".into());
         }
         args.extend(filter.iter().cloned());
-        // Past this many bytes of paths the command line risks Windows' ~32K limit.
-        let stdin = (paths.iter().map(|p| p.len() + 12).sum::<usize>() > 8 * 1024).then(|| paths_stdin(paths));
-        if stdin.is_some() {
-            args.push("--stdin".into());
-        }
-        args.push("--end-of-options".into());
-        args.extend(scope.split_whitespace().map(str::to_string));
-        if stdin.is_none() {
-            args.push("--".into());
-            args.extend(paths.iter().map(|p| format!(":(literal){p}")));
-        }
+        let revs: Vec<String> = scope.split_whitespace().map(str::to_string).collect();
+        let stdin = revision_args(&mut args, &revs, &[], Some(paths));
         let output = run_log(r, args, stdin.as_deref(), cancel)?;
         for line in output.stdout_str().lines() {
             ids.insert(ObjectId::from_hex(line.as_bytes()).map_err(gx)?);
@@ -99,13 +90,41 @@ fn git_select(
     Ok(ids)
 }
 
-/// Stdin for `git log --stdin` that limits the walk to `paths`, however many there are.
-fn paths_stdin(paths: &[String]) -> String {
-    let mut text = String::from("--\n");
-    for p in paths {
-        text.push_str(&format!(":(literal){p}\n"));
+/// Keep revision walks below a conservative argv budget on every platform. Exclusions
+/// always use stdin; large walks move revisions and literal pathspecs there together.
+fn revision_args(
+    args: &mut Vec<String>,
+    revs: &[String],
+    excluded: &[String],
+    paths: Option<&[String]>,
+) -> Option<String> {
+    let long = revs.iter().chain(excluded).map(|rev| rev.len() + 12).sum::<usize>()
+        + paths.into_iter().flatten().map(|path| path.len() + 12).sum::<usize>()
+        > 8 * 1024;
+    let stdin = if long {
+        let mut text = revs.iter().chain(excluded).map(|rev| format!("{rev}\n")).collect::<String>();
+        if let Some(paths) = paths {
+            text.push_str("--\n");
+            for path in paths {
+                text.push_str(&format!(":(literal){path}\n"));
+            }
+        }
+        Some(text)
+    } else {
+        (!excluded.is_empty()).then(|| excluded.join("\n") + "\n")
+    };
+    if stdin.is_some() {
+        args.push("--stdin".into());
     }
-    text
+    args.push("--end-of-options".into());
+    if !long {
+        args.extend(revs.iter().cloned());
+        if let Some(paths) = paths {
+            args.push("--".into());
+            args.extend(paths.iter().map(|path| format!(":(literal){path}")));
+        }
+    }
+    stdin
 }
 
 fn run_log(
@@ -149,17 +168,9 @@ fn file_history(
         if first_parent {
             opts.push("--first-parent".into());
         }
-        let pathspec = ["--".to_string(), format!(":(literal){path}")];
-        // `excluded` can name a revision per created file, so it goes through stdin.
         let run = |excluded: &[String]| -> IdeResult<Vec<(ObjectId, Option<char>)>> {
             let mut args = opts.clone();
-            if !excluded.is_empty() {
-                args.push("--stdin".into());
-            }
-            args.push("--end-of-options".into());
-            args.extend(revs.iter().cloned());
-            args.extend(pathspec.iter().cloned());
-            let stdin = (!excluded.is_empty()).then(|| excluded.join("\n") + "\n");
+            let stdin = revision_args(&mut args, &revs, excluded, Some(std::slice::from_ref(&path)));
             Ok(parse_name_status_log(&run_log(r, args, stdin.as_deref(), cancel)?.stdout_str()))
         };
         let records = run(&[])?;
@@ -448,10 +459,9 @@ fn log_indexed(
         {
             args.push(format!("--max-count={}", u64::from(q.skip) + u64::from(q.limit)));
         }
-        args.push("--end-of-options".into());
-        args.extend(tips.iter().map(ToString::to_string));
-        let result =
-            crate::cli::run_cancellable(r.root(), &args.iter().map(String::as_str).collect::<Vec<_>>(), cancel)?;
+        let revs: Vec<_> = tips.iter().map(ToString::to_string).collect();
+        let stdin = revision_args(&mut args, &revs, &[], None);
+        let result = run_log(r, args, stdin.as_deref(), cancel)?;
         let ids = result
             .stdout_str()
             .lines()
@@ -460,10 +470,9 @@ fn log_indexed(
         if repo.is_shallow().unwrap_or(false) {
             // A separately bounded walk provides the domain; pagination must never
             // turn a shallow boundary into a connection through a locally present object.
-            let mut boundary_args = vec!["rev-list", "--end-of-options"];
-            let ids_as_text: Vec<_> = tips.iter().map(ToString::to_string).collect();
-            boundary_args.extend(ids_as_text.iter().map(String::as_str));
-            let boundaries = crate::cli::run_cancellable(r.root(), &boundary_args, cancel)?;
+            let mut boundary_args = vec!["rev-list".to_string()];
+            let stdin = revision_args(&mut boundary_args, &revs, &[], None);
+            let boundaries = run_log(r, boundary_args, stdin.as_deref(), cancel)?;
             visibility.range_ids = Some(
                 boundaries
                     .stdout_str()
@@ -865,18 +874,20 @@ fn range_ids(
     first_parent: bool,
     cancel: &CancellationToken,
 ) -> IdeResult<Vec<ObjectId>> {
-    let mut args: Vec<&str> = vec!["rev-list", if order == LogOrder::Date { "--date-order" } else { "--topo-order" }];
+    let mut args: Vec<String> =
+        ["rev-list", if order == LogOrder::Date { "--date-order" } else { "--topo-order" }].map(String::from).into();
     if first_parent {
-        args.push("--first-parent");
+        args.push("--first-parent".into());
     }
-    args.push("--end-of-options");
+    let mut revs = Vec::new();
     for part in range.split_whitespace() {
         if part.starts_with('-') {
             return Err(ide_core::IdeError::invalid(format!("invalid revision range '{range}'")));
         }
-        args.push(part);
+        revs.push(part.to_string());
     }
-    let out = crate::cli::run_cancellable(r.root(), &args, cancel)?;
+    let stdin = revision_args(&mut args, &revs, &[], None);
+    let out = run_log(r, args, stdin.as_deref(), cancel)?;
     out.stdout_str()
         .lines()
         .filter(|l| !l.trim().is_empty())
@@ -1113,3 +1124,59 @@ pub(crate) fn commits_changes(r: &Repo, commits: &[String]) -> IdeResult<Vec<Rev
     out.sort_by(|a, b| a.path.cmp(&b.path));
     Ok(out)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::revision_args;
+
+    #[test]
+    fn small_revision_walks_keep_their_arguments() {
+        let revs = vec!["HEAD".into(), "^HEAD~2".into()];
+        let paths = vec!["src/[literal] file.rs".into()];
+        for paths in [None, Some(paths.as_slice())] {
+            for excluded in [vec![], vec!["^HEAD~1^@".into()]] {
+                let mut args = vec!["log".into()];
+                let stdin = revision_args(&mut args, &revs, &excluded, paths);
+                let mut expected = vec!["log"];
+                if !excluded.is_empty() {
+                    expected.push("--stdin");
+                }
+                expected.extend(["--end-of-options", "HEAD", "^HEAD~2"]);
+                if paths.is_some() {
+                    expected.extend(["--", ":(literal)src/[literal] file.rs"]);
+                }
+                assert_eq!(args, expected);
+                assert_eq!(stdin, (!excluded.is_empty()).then(|| "^HEAD~1^@\n".to_string()));
+            }
+        }
+    }
+
+    #[test]
+    fn revisions_and_paths_share_one_budget() {
+        let revs = vec!["a".repeat(4084)];
+        let mut paths = vec!["b".repeat(4084)];
+        let mut args = vec!["log".into()];
+        assert_eq!(revision_args(&mut args, &revs, &[], Some(&paths)), None);
+        paths[0].push('b');
+        let mut args = vec!["log".into()];
+        assert_eq!(
+            revision_args(&mut args, &revs, &[], Some(&paths)),
+            Some(format!("{}\n--\n:(literal){}\n", revs[0], paths[0]))
+        );
+        let excluded = vec!["^HEAD".into()];
+        let mut args = vec!["log".into()];
+        let stdin = revision_args(&mut args, &revs, &excluded, Some(&paths));
+        assert_eq!(args, ["log", "--stdin", "--end-of-options"]);
+        assert_eq!(stdin, Some(format!("{}\n^HEAD\n--\n:(literal){}\n", revs[0], paths[0])));
+    }
+
+    #[test]
+    fn large_revision_walks_preserve_input_order_and_exclusions() {
+        let revs = vec!["HEAD".into(); 600];
+        let excluded = vec!["^HEAD~2".into()];
+        let mut args = vec!["rev-list".into(), "--date-order".into()];
+        let stdin = revision_args(&mut args, &revs, &excluded, None);
+        assert_eq!(args, ["rev-list", "--date-order", "--stdin", "--end-of-options"]);
+        assert_eq!(stdin, Some("HEAD\n".repeat(600) + "^HEAD~2\n"));
+    }
+}

crates/ide-vcs/tests/log_projection.rs

@@ -700,3 +700,161 @@ fn many_selected_paths_filter_history_without_a_huge_command_line() {
     let log = repo.log(&LogQuery { paths, text: Some("c3".into()), ..Default::default() }).unwrap();
     assert_eq!(log.len(), 50);
 }
+
+/// One commit per revision of a single file, plus one lightweight tag per commit, made
+/// with fast-import and `update-ref --stdin` so hundreds stay quick.
+fn commit_one_file_with_a_tag_per_commit(repo_dir: &Path, count: usize, file: &str) {
+    let mut stream = String::new();
+    for i in 0..count {
+        stream.push_str(&format!(
+            "commit refs/heads/main\ncommitter T <[email protected]> {} +0000\ndata 3\nc{}\n",
+            1_700_000_000 + i,
+            i % 10
+        ));
+        let content = format!("x{i}");
+        stream.push_str(&format!("M 100644 inline {file}\ndata {}\n{content}\n\n", content.len()));
+    }
+    let mut child = Command::new("git")
+        .current_dir(repo_dir)
+        .args(["fast-import", "--quiet"])
+        .stdin(Stdio::piped())
+        .spawn()
+        .unwrap();
+    child.stdin.take().unwrap().write_all(stream.as_bytes()).unwrap();
+    assert!(child.wait().unwrap().success());
+    git(repo_dir, &["symbolic-ref", "HEAD", "refs/heads/main"]);
+    git(repo_dir, &["reset", "-q", "--hard"]);
+    let log = git(repo_dir, &["log", "--reverse", "--format=%H"]);
+    let mut input = String::new();
+    for (i, sha) in log.lines().take(count).enumerate() {
+        input.push_str(&format!("update refs/tags/t{i:04} {sha}\n"));
+    }
+    let mut child = Command::new("git")
+        .current_dir(repo_dir)
+        .args(["update-ref", "--stdin"])
+        .stdin(Stdio::piped())
+        .spawn()
+        .unwrap();
+    child.stdin.take().unwrap().write_all(input.as_bytes()).unwrap();
+    assert!(child.wait().unwrap().success());
+}
+
+#[test]
+fn many_branch_tips_with_a_path_filter_stay_off_the_command_line() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    git(dir, &["init", "-q"]);
+    let file = "module/src/main/java/com/example/Big.java";
+    commit_one_file_with_a_tag_per_commit(dir, 800, file);
+    let repo = Repo::discover(dir).unwrap().unwrap();
+    let log = repo.log(&LogQuery { path: Some(file.into()), ..Default::default() }).unwrap();
+    assert_eq!(log.len(), 800);
+}
+
+#[test]
+fn followed_file_history_with_many_branch_tips_stays_off_the_command_line() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    git(dir, &["init", "-q"]);
+    let file = "module/src/main/java/com/example/Big.java";
+    commit_one_file_with_a_tag_per_commit(dir, 800, file);
+    let repo = Repo::discover(dir).unwrap().unwrap();
+    let log = repo.log(&LogQuery { path: Some(file.into()), follow: true, ..Default::default() }).unwrap();
+    assert_eq!(log.len(), 800);
+    assert!(log.iter().all(|c| c.history_path.as_deref() == Some(file)));
+}
+
+#[test]
+fn followed_file_history_survives_a_long_path() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    git(dir, &["init", "-q"]);
+    git(dir, &["config", "core.longpaths", "true"]);
+    let mut path = String::from("src");
+    while path.len() < 260 {
+        path.push_str("/deep-history-directory-segment");
+    }
+    let file = format!("{path}/DeepHistory.java");
+    std::fs::create_dir_all(dir.join(&path)).unwrap();
+    let first = commit(dir, &file, "first");
+    let second = commit(dir, &file, "second");
+    let repo = Repo::discover(dir).unwrap().unwrap();
+    let log = repo.log(&LogQuery { path: Some(file.clone().into()), follow: true, ..Default::default() }).unwrap();
+    let ids: std::collections::HashSet<_> = log.iter().map(|c| c.id.clone()).collect();
+    assert_eq!(ids, [first, second].into());
+    assert!(log.iter().all(|c| c.history_path.as_deref() == Some(file.as_str())));
+}
+
+#[test]
+fn many_tips_preserve_cancellable_shallow_and_range_walks() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    git(dir, &["init", "-q"]);
+    commit_one_file_with_a_tag_per_commit(dir, 200, "file.txt");
+    let expected = git(dir, &["rev-list", "--topo-order", "HEAD"]);
+    let expected: Vec<_> = expected.lines().map(str::to_string).collect();
+    let range = format!("{} ^{}", expected[0], expected[50]);
+    for shallow in [false, true] {
+        if shallow {
+            std::fs::write(dir.join(".git/shallow"), format!("{}\n", expected[100])).unwrap();
+        }
+        let repo = Repo::discover(dir).unwrap().unwrap();
+        for order in [LogOrder::Topo, LogOrder::Date, LogOrder::Standard] {
+            for first_parent in [false, true] {
+                let query = LogQuery { order, first_parent, ..Default::default() };
+                let log = repo.log(&LogQuery { request: Some(ide_core::RequestId(1)), ..query.clone() }).unwrap();
+                assert_eq!(log.iter().map(|c| c.id.clone()).collect::<Vec<_>>(), expected);
+                if shallow {
+                    let boundary = log.iter().find(|c| c.id == expected[100]).unwrap();
+                    assert_eq!(boundary.graph_parents, Some(vec![]));
+                }
+                let mixed = repo
+                    .log(&LogQuery { branches: expected.clone(), ranges: vec![range.clone()], ..query.clone() })
+                    .unwrap();
+                assert_eq!(mixed.iter().map(|c| c.id.clone()).collect::<Vec<_>>(), expected);
+                let long_range = format!("{} ^{}", vec![expected[0].as_str(); 200].join(" "), expected[50]);
+                let ranged = repo.log(&LogQuery { range: Some(long_range), ..query }).unwrap();
+                assert_eq!(ranged.iter().map(|c| c.id.clone()).collect::<Vec<_>>(), expected[..50]);
+            }
+        }
+    }
+}
+
+#[test]
+fn many_tips_preserve_followed_renames_and_recreation_exclusions() {
+    let tmp = tempfile::tempdir().unwrap();
+    let dir = tmp.path();
+    git(dir, &["init", "-q"]);
+    commit_one_file_with_a_tag_per_commit(dir, 200, "old [literal] file.txt");
+    git(dir, &["mv", "old [literal] file.txt", "renamed file.txt"]);
+    git(dir, &["commit", "-qm", "renamed"]);
+    let renamed = git(dir, &["rev-parse", "HEAD"]);
+    git(dir, &["rm", "renamed file.txt"]);
+    git(dir, &["commit", "-qm", "deleted"]);
+    let recreated = commit(dir, "renamed file.txt", "recreated");
+    let repo = Repo::discover(dir).unwrap().unwrap();
+    for order in [LogOrder::Topo, LogOrder::Date, LogOrder::Standard] {
+        for first_parent in [false, true] {
+            let query = LogQuery {
+                path: Some("renamed file.txt".into()),
+                follow: true,
+                order,
+                first_parent,
+                ..Default::default()
+            };
+            let log = repo.log(&query).unwrap();
+            assert_eq!(log.len(), 1);
+            assert_eq!(log[0].id, recreated);
+            let query = LogQuery { range: Some(renamed.clone()), ..query };
+            let small = repo.log(&query).unwrap();
+            let large = repo.log(&LogQuery { range: Some(vec![renamed.as_str(); 200].join(" ")), ..query }).unwrap();
+            assert_eq!(small.len(), 201);
+            assert_eq!(
+                large.iter().map(|c| (&c.id, &c.history_path, &c.graph_parents)).collect::<Vec<_>>(),
+                small.iter().map(|c| (&c.id, &c.history_path, &c.graph_parents)).collect::<Vec<_>>()
+            );
+            assert_eq!(large[0].history_path.as_deref(), Some("renamed file.txt"));
+            assert!(large[1..].iter().all(|c| c.history_path.as_deref() == Some("old [literal] file.txt")));
+        }
+    }
+}