From 634fdda22f4e38905e19db99202b96aba7bd0b81 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 29 Sep 2026 11:28:40 +0000 Subject: [PATCH] fix(grep): apply -m per file, as GNU does GNU's -m NUM stops reading *each file* after NUM matching lines; the budget is per operand. Bashkit accumulated matches across operands and broke out of the file loop once the total was spent, so a file that used up the budget silenced every file after it. Verified against GNU grep 3.11: $ grep -m1 foo p.txt q.txt p.txt:foo1 q.txt:foo4 # bashkit stopped after p.txt $ grep -c -m1 foo p.txt q.txt p.txt:1 q.txt:1 # bashkit omitted this row entirely Same shape for -v. The per-file counter that drives output already existed (match_count), so the budget checks now read it instead of the cross-file total, and the total is gone along with the cross-file break. This removes the last divergence found while reviewing #2485. The 275-case differential sweep (-m0/-m1/-m2/-m5 x -q/-c/-l/-L/-v/-n/-o/-i/-H/-h x present, multiple, missing and mixed operands) now matches GNU grep 3.11 byte for byte: 33 divergent lines before #2485, 6 after it, 0 now. Two max_reached assignments became dead once the budget was per-file, since their break already leaves the loop. Removed, with a note that the flag now exists only so the -o closure can signal out of itself. Claude-Session: https://claude.ai/code/session_01DyKfW3rVZUrUKzrCLpurco --- crates/bashkit/src/builtins/grep.rs | 32 ++++++--------- .../tests/spec_cases/grep/grep.test.sh | 39 +++++++++++++++++++ 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/crates/bashkit/src/builtins/grep.rs b/crates/bashkit/src/builtins/grep.rs index 25378ccb6..de7829a01 100644 --- a/crates/bashkit/src/builtins/grep.rs +++ b/crates/bashkit/src/builtins/grep.rs @@ -609,7 +609,6 @@ impl Builtin for Grep { let mut read_failed = false; let mut any_match = false; let mut exit_code = 1; // 1 = no match - let mut total_matches = 0usize; // Determine input sources // GNU grep names stdin "(standard input)" everywhere it names it at @@ -726,20 +725,15 @@ impl Builtin for Grep { }; let has_context = opts.before_context > 0 || opts.after_context > 0; - let mut max_reached = false; - 'file_loop: for (filename, content) in &inputs { - // Check if we already reached max count from previous files. - // `-L` is exempt: its output is driven by the files that *didn't* - // match, so a spent match budget must not stop the enumeration. - // GNU `grep -L -m1 foo match.txt other.txt` still lists other.txt. - if let Some(max) = opts.max_count - && total_matches >= max - && !opts.files_without_matches - { - break 'file_loop; - } - + // GNU `-m NUM` stops reading *each file* after NUM matching lines; + // the budget is per operand, not shared across them. So + // `grep -m1 foo a b` prints one match from a *and* one from b, and + // `grep -c -m1 foo a b` reports a count for every operand. + // Only the `-o` closure needs this: it cannot break the enclosing + // line loop itself, so it signals the budget out. Every other exit + // path breaks directly. + let mut max_reached = false; let mut match_count = 0; let mut file_matched = false; @@ -776,9 +770,8 @@ impl Builtin for Grep { for (line_num, line) in lines.iter().enumerate() { // Check max count limit before adding more matches if let Some(max) = opts.max_count - && total_matches >= max + && match_count >= max { - max_reached = true; break; // Break inner loop, continue to output phase } @@ -789,14 +782,13 @@ impl Builtin for Grep { file_matched = true; any_match = true; match_count += 1; - total_matches += 1; if opts.files_with_matches || opts.files_without_matches || opts.quiet { return false; } if let Some(max) = opts.max_count - && total_matches >= max + && match_count >= max { max_reached = true; return false; @@ -821,7 +813,6 @@ impl Builtin for Grep { file_matched = true; any_match = true; match_count += 1; - total_matches += 1; match_lines.push(line_num); if opts.files_with_matches || opts.files_without_matches { @@ -833,9 +824,8 @@ impl Builtin for Grep { // Check max after recording this match if let Some(max) = opts.max_count - && total_matches >= max + && match_count >= max { - max_reached = true; break; } } diff --git a/crates/bashkit/tests/spec_cases/grep/grep.test.sh b/crates/bashkit/tests/spec_cases/grep/grep.test.sh index 9942043e5..f78db9764 100644 --- a/crates/bashkit/tests/spec_cases/grep/grep.test.sh +++ b/crates/bashkit/tests/spec_cases/grep/grep.test.sh @@ -661,6 +661,45 @@ echo $? 0 ### end +### grep_max_count_is_per_file_not_cumulative +# GNU -m NUM stops reading *each file* after NUM matching lines. The budget is +# per operand, so a file that spends it does not silence the next one. +printf 'foo1\nfoo2\nfoo3\n' > /tmp/grep_mm_p.txt +printf 'foo4\nfoo5\n' > /tmp/grep_mm_q.txt +grep -m1 foo /tmp/grep_mm_p.txt /tmp/grep_mm_q.txt +echo "rc=$?" +grep -m2 foo /tmp/grep_mm_p.txt /tmp/grep_mm_q.txt +echo "rc=$?" +### expect +/tmp/grep_mm_p.txt:foo1 +/tmp/grep_mm_q.txt:foo4 +rc=0 +/tmp/grep_mm_p.txt:foo1 +/tmp/grep_mm_p.txt:foo2 +/tmp/grep_mm_q.txt:foo4 +/tmp/grep_mm_q.txt:foo5 +rc=0 +### end + +### grep_max_count_per_file_with_count_and_invert +# -c reports a count for every operand, not just until a shared budget runs +# out, and -v applies the same per-file budget to non-matching lines. +# Each spec block gets a fresh VFS, so this recreates its own inputs. +printf 'foo1\nfoo2\nfoo3\n' > /tmp/grep_mm_p.txt +printf 'foo4\nfoo5\n' > /tmp/grep_mm_q.txt +printf 'foo\nbar\nbaz\n' > /tmp/grep_mm_v.txt +grep -c -m1 foo /tmp/grep_mm_p.txt /tmp/grep_mm_q.txt +echo "rc=$?" +grep -v -m1 foo /tmp/grep_mm_v.txt /tmp/grep_mm_q.txt +echo "rc=$?" +### expect +/tmp/grep_mm_p.txt:1 +/tmp/grep_mm_q.txt:1 +rc=0 +/tmp/grep_mm_v.txt:bar +rc=0 +### end + ### grep_files_without_match # -L prints files that have no matches printf 'foo\n' > /tmp/grep_l_a.txt && printf 'bar\n' > /tmp/grep_l_b.txt && grep -L foo /tmp/grep_l_a.txt /tmp/grep_l_b.txt