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..9f553b4b80d0 100644 --- a/src/runtime/cli/test/ChangedFilesFilter.rs +++ b/src/runtime/cli/test/ChangedFilesFilter.rs @@ -472,23 +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]> = '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() { - Output::err_generic( - "--changed: {s}", - (BStr::new(strings::trim(&result.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); - } - 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 +495,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 +551,33 @@ 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 - /// "not a git repo" style message. - pub spawn_failed: bool, - pub stdout: Vec, - pub stderr: Vec, +pub(crate) enum GitResult { + /// `run_git` has already reported the spawn error. + SpawnFailed, + ExitError { + stderr: Vec, + }, + Ok { + stdout: Vec, + }, } fn run_git(git_path: &[u8], cwd: &[u8], args: &[&[u8]]) -> GitResult { @@ -622,12 +611,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 +621,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, - }, } }