From 628f4bb90baf956c4f7c697dfb8a0445099db7f1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 07:09:39 +0000 Subject: [PATCH 1/3] cli: collapse filter_run visit flags and GitResult bool pair into enums ProcessHandle tracked DFS cycle-detection state as two dependent bools (visited, visiting) where visiting && !visited was unreachable. Replace with a tri-state VisitState enum and an exhaustive match in has_cycle. GitResult carried (ok, spawn_failed) where ok && spawn_failed was impossible; every caller checked spawn_failed then ok. Replace with a three-variant enum whose Ok and ExitError arms carry only the output stream the caller reads on that path. No behavior change. --- src/runtime/cli/filter_run.rs | 30 ++-- src/runtime/cli/test/ChangedFilesFilter.rs | 169 ++++++++++----------- 2 files changed, 95 insertions(+), 104 deletions(-) diff --git a/src/runtime/cli/filter_run.rs b/src/runtime/cli/filter_run.rs index 73fffdea20d5..0c1b58a5fd62 100644 --- a/src/runtime/cli/filter_run.rs +++ b/src/runtime/cli/filter_run.rs @@ -66,8 +66,14 @@ pub(crate) struct ProcessHandle<'a> { remaining_dependencies: usize, dependents: Vec<*mut ProcessHandle<'a>>, - visited: bool, - visiting: bool, + visit_state: VisitState, +} + +#[derive(Clone, Copy)] +enum VisitState { + Unvisited, + Visiting, + Visited, } impl<'a> ProcessHandle<'a> { @@ -978,8 +984,7 @@ pub(crate) fn run_scripts_with_filter( end_time: None, remaining_dependencies: 0, dependents: Vec::new(), - visited: false, - visiting: false, + visit_state: VisitState::Unvisited, }); } state.handles = handles_vec.into_boxed_slice(); @@ -1068,19 +1073,20 @@ pub(crate) fn run_scripts_with_filter( } fn has_cycle(current: &mut ProcessHandle) -> bool { - current.visited = true; - current.visiting = true; + current.visit_state = VisitState::Visiting; for &dep in ¤t.dependents { // SAFETY: dep points into state.handles, valid for the run loop lifetime. let dep = unsafe { &mut *dep }; - if dep.visiting { - return true; - } else if !dep.visited { - if has_cycle(dep) { - return true; + match dep.visit_state { + VisitState::Visiting => return true, + VisitState::Unvisited => { + if has_cycle(dep) { + return true; + } } + VisitState::Visited => {} } } - current.visiting = false; + current.visit_state = VisitState::Visited; false } diff --git a/src/runtime/cli/test/ChangedFilesFilter.rs b/src/runtime/cli/test/ChangedFilesFilter.rs index 349e5213350d..edbfddfb31c1 100644 --- a/src/runtime/cli/test/ChangedFilesFilter.rs +++ b/src/runtime/cli/test/ChangedFilesFilter.rs @@ -472,22 +472,24 @@ fn get_changed_files( // Find the git repository root so we can make the paths git prints // absolute (git prints paths relative to the repo toplevel with these // commands). - let git_root: Box<[u8]> = 'blk: { - let result = run_git(git_path, top_level_dir, &[b"rev-parse", b"--show-toplevel"]); - if !result.ok { - if result.spawn_failed { - // run_git already printed the spawn error. - } else if !result.stderr.is_empty() { + let git_root: Box<[u8]> = match run_git( + git_path, + top_level_dir, + &[b"rev-parse", b"--show-toplevel"], + ) { + GitResult::SpawnFailed => return Err(GitError::GitFailed), + GitResult::ExitError { stderr } => { + if !stderr.is_empty() { Output::err_generic( "--changed: {s}", - (BStr::new(strings::trim(&result.stderr, b" \r\n\t")),), + (BStr::new(strings::trim(&stderr, b" \r\n\t")),), ); } else { Output::err_generic("--changed requires running inside a git repository", ()); } return Err(GitError::GitFailed); } - break 'blk Box::<[u8]>::from(strings::trim(&result.stdout, b" \r\n\t")); + GitResult::Ok { stdout } => Box::<[u8]>::from(strings::trim(&stdout, b" \r\n\t")), }; let mut set = StringSet::new(); @@ -496,60 +498,54 @@ fn get_changed_files( // Uncommitted (unstaged + staged). `git diff HEAD` covers both. // On a repo with no commits, `HEAD` is unresolved; fall back to just // `git diff` (unstaged) + staged. - let diff = run_git( + match run_git( git_path, top_level_dir, &[b"diff", b"--name-only", b"HEAD", b"--"], - ); - if diff.spawn_failed { - return Err(GitError::GitFailed); - } - if diff.ok { - append_paths(&mut set, &git_root, &diff.stdout); - } else { - let unstaged = run_git(git_path, top_level_dir, &[b"diff", b"--name-only", b"--"]); - if unstaged.spawn_failed { - return Err(GitError::GitFailed); - } - if unstaged.ok { - append_paths(&mut set, &git_root, &unstaged.stdout); - } + ) { + GitResult::SpawnFailed => return Err(GitError::GitFailed), + GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), + GitResult::ExitError { .. } => { + match run_git(git_path, top_level_dir, &[b"diff", b"--name-only", b"--"]) { + GitResult::SpawnFailed => return Err(GitError::GitFailed), + GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), + GitResult::ExitError { .. } => {} + } - let staged = run_git( - git_path, - top_level_dir, - &[b"diff", b"--name-only", b"--cached", b"--"], - ); - if staged.spawn_failed { - return Err(GitError::GitFailed); - } - if staged.ok { - append_paths(&mut set, &git_root, &staged.stdout); + match run_git( + git_path, + top_level_dir, + &[b"diff", b"--name-only", b"--cached", b"--"], + ) { + GitResult::SpawnFailed => return Err(GitError::GitFailed), + GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), + GitResult::ExitError { .. } => {} + } } } } else { - let diff = run_git( + match run_git( git_path, top_level_dir, &[b"diff", b"--name-only", since, b"--"], - ); - if !diff.ok { - if diff.spawn_failed { - // run_git already printed the spawn error. - } else if !diff.stderr.is_empty() { - Output::err_generic( - "--changed: {s}", - (BStr::new(strings::trim(&diff.stderr, b" \r\n\t")),), - ); - } else { - Output::err_generic( - "--changed: git diff against {f} failed", - (bun_fmt::quote(since),), - ); + ) { + GitResult::SpawnFailed => return Err(GitError::GitFailed), + GitResult::ExitError { stderr } => { + if !stderr.is_empty() { + Output::err_generic( + "--changed: {s}", + (BStr::new(strings::trim(&stderr, b" \r\n\t")),), + ); + } else { + Output::err_generic( + "--changed: git diff against {f} failed", + (bun_fmt::quote(since),), + ); + } + return Err(GitError::GitFailed); } - return Err(GitError::GitFailed); + GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), } - append_paths(&mut set, &git_root, &diff.stdout); } // Untracked files are always considered changed — a brand-new file @@ -558,37 +554,31 @@ fn get_changed_files( // supplement with ls-files in both branches above. `--full-name` // forces repo-root-relative output regardless of our cwd, matching // `git diff --name-only`. - { - let untracked = run_git( - git_path, - top_level_dir, - &[ - b"ls-files", - b"--others", - b"--exclude-standard", - b"--full-name", - ], - ); - if untracked.spawn_failed { - return Err(GitError::GitFailed); - } - if untracked.ok { - append_paths(&mut set, &git_root, &untracked.stdout); - } + match run_git( + git_path, + top_level_dir, + &[ + b"ls-files", + b"--others", + b"--exclude-standard", + b"--full-name", + ], + ) { + GitResult::SpawnFailed => return Err(GitError::GitFailed), + GitResult::Ok { stdout } => append_paths(&mut set, &git_root, &stdout), + GitResult::ExitError { .. } => {} } Ok(set) } -#[derive(Default)] -pub(crate) struct GitResult { - pub ok: bool, - /// Set when the git process could not be spawned at all. The failure - /// has already been reported; callers should not print a second +pub(crate) enum GitResult { + /// The git process could not be spawned at all. The failure has + /// already been reported; callers should not print a second /// "not a git repo" style message. - pub spawn_failed: bool, - pub stdout: Vec, - pub stderr: Vec, + SpawnFailed, + ExitError { stderr: Vec }, + Ok { stdout: Vec }, } fn run_git(git_path: &[u8], cwd: &[u8], args: &[&[u8]]) -> GitResult { @@ -622,12 +612,7 @@ fn run_git(git_path: &[u8], cwd: &[u8], args: &[&[u8]]) -> GitResult { Ok(p) => p, Err(err) => { Output::err_generic("--changed: failed to spawn git: {s}", (err.name(),)); - return GitResult { - ok: false, - spawn_failed: true, - stdout: Vec::new(), - stderr: Vec::new(), - }; + return GitResult::SpawnFailed; } }; @@ -637,19 +622,19 @@ fn run_git(git_path: &[u8], cwd: &[u8], args: &[&[u8]]) -> GitResult { "--changed: failed to spawn git: {f}", format_args!("{}", err), ); - GitResult { - ok: false, - spawn_failed: true, - stdout: Vec::new(), - stderr: Vec::new(), + GitResult::SpawnFailed + } + sys::Result::Ok(result) => { + if result.is_ok() { + GitResult::Ok { + stdout: result.stdout, + } + } else { + GitResult::ExitError { + stderr: result.stderr, + } } } - sys::Result::Ok(result) => GitResult { - ok: result.is_ok(), - spawn_failed: false, - stdout: result.stdout, - stderr: result.stderr, - }, } } From 52849f26a559b91a2da98841d7868dca6041169c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 2 Aug 2026 07:12:09 +0000 Subject: [PATCH 2/3] [autofix.ci] apply automated fixes --- src/runtime/cli/test/ChangedFilesFilter.rs | 41 +++++++++++----------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/runtime/cli/test/ChangedFilesFilter.rs b/src/runtime/cli/test/ChangedFilesFilter.rs index edbfddfb31c1..cb10fe71b91d 100644 --- a/src/runtime/cli/test/ChangedFilesFilter.rs +++ b/src/runtime/cli/test/ChangedFilesFilter.rs @@ -472,25 +472,22 @@ fn get_changed_files( // Find the git repository root so we can make the paths git prints // absolute (git prints paths relative to the repo toplevel with these // commands). - let git_root: Box<[u8]> = match run_git( - git_path, - top_level_dir, - &[b"rev-parse", b"--show-toplevel"], - ) { - GitResult::SpawnFailed => return Err(GitError::GitFailed), - GitResult::ExitError { stderr } => { - if !stderr.is_empty() { - Output::err_generic( - "--changed: {s}", - (BStr::new(strings::trim(&stderr, b" \r\n\t")),), - ); - } else { - Output::err_generic("--changed requires running inside a git repository", ()); + let git_root: Box<[u8]> = + match run_git(git_path, top_level_dir, &[b"rev-parse", b"--show-toplevel"]) { + GitResult::SpawnFailed => return Err(GitError::GitFailed), + GitResult::ExitError { stderr } => { + if !stderr.is_empty() { + Output::err_generic( + "--changed: {s}", + (BStr::new(strings::trim(&stderr, b" \r\n\t")),), + ); + } else { + Output::err_generic("--changed requires running inside a git repository", ()); + } + return Err(GitError::GitFailed); } - return Err(GitError::GitFailed); - } - GitResult::Ok { stdout } => Box::<[u8]>::from(strings::trim(&stdout, b" \r\n\t")), - }; + GitResult::Ok { stdout } => Box::<[u8]>::from(strings::trim(&stdout, b" \r\n\t")), + }; let mut set = StringSet::new(); @@ -577,8 +574,12 @@ pub(crate) enum GitResult { /// already been reported; callers should not print a second /// "not a git repo" style message. SpawnFailed, - ExitError { stderr: Vec }, - Ok { stdout: Vec }, + ExitError { + stderr: Vec, + }, + Ok { + stdout: Vec, + }, } fn run_git(git_path: &[u8], cwd: &[u8], args: &[&[u8]]) -> GitResult { From 9329c017fe7aa56f1bd66549ea96dcd48e9f8333 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 2 Aug 2026 07:15:12 +0000 Subject: [PATCH 3/3] shorten SpawnFailed doc comment --- src/runtime/cli/test/ChangedFilesFilter.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/runtime/cli/test/ChangedFilesFilter.rs b/src/runtime/cli/test/ChangedFilesFilter.rs index cb10fe71b91d..9f553b4b80d0 100644 --- a/src/runtime/cli/test/ChangedFilesFilter.rs +++ b/src/runtime/cli/test/ChangedFilesFilter.rs @@ -570,9 +570,7 @@ fn get_changed_files( } pub(crate) enum GitResult { - /// The git process could not be spawned at all. The failure has - /// already been reported; callers should not print a second - /// "not a git repo" style message. + /// `run_git` has already reported the spawn error. SpawnFailed, ExitError { stderr: Vec,