Skip to content

cli: collapse filter_run visit flags and GitResult bool pair into enums #36760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jarred-Sumner merged 4 commits into from
Aug 2, 2026
Merged

cli: collapse filter_run visit flags and GitResult bool pair into enums #36760

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
628f4bb
cli: collapse filter_run visit flags and GitResult bool pair into enums
robobun Aug 2, 2026
52849f2
[autofix.ci] apply automated fixes
autofix-ci[bot] Aug 2, 2026
9329c01
shorten SpawnFailed doc comment
robobun Aug 2, 2026
35af1c2
Merge branch 'main' into farm/15dcb190/cli-bool-pair-enums
Jarred-Sumner Aug 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions src/runtime/cli/filter_run.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 &current.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
}
186 changes: 86 additions & 100 deletions src/runtime/cli/test/ChangedFilesFilter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,84 +472,77 @@ 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();

if since.is_empty() {
// 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
Expand All @@ -558,37 +551,35 @@ 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
Comment thread
robobun marked this conversation as resolved.
Outdated
/// "not a git repo" style message.
pub spawn_failed: bool,
pub stdout: Vec<u8>,
pub stderr: Vec<u8>,
SpawnFailed,
ExitError {
stderr: Vec<u8>,
},
Ok {
stdout: Vec<u8>,
},
}

fn run_git(git_path: &[u8], cwd: &[u8], args: &[&[u8]]) -> GitResult {
Expand Down Expand Up @@ -622,12 +613,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;
}
};

Expand All @@ -637,19 +623,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,
},
}
}

Expand Down
Loading