t: add greplint.pl and convert grep to test_grep by mmontalbo · Pull Request #2135 · gitgitgadget/git

mmontalbo · 2026-06-02T21:21:30Z

test_grep is a wrapper around grep for test assertions that prints
the file contents on failure for easier debugging. Bare grep fails
silently, making it hard to diagnose what went wrong.

This series converts existing bare grep assertions to test_grep and
adds greplint.pl to prevent new ones from being introduced.

Patch 1 documents test_grep in t/README.

Patch 2 fixes three greps missing file arguments (t2402, t7507,
t7700). They were reading empty stdin and passing vacuously.

Patch 3 extracts chainlint's Lexer, ShellParser, and ScriptParser
into a shared module (lib-shell-parser.pl) so greplint.pl can
reuse the same tokenizer. No functional change to chainlint.

Patch 4 fixes a latent line-counting bug in scan_dqstring where
newlines from $() bodies inside double-quoted strings were counted
twice. This does not affect chainlint (which uses byte offsets)
but matters for greplint.pl's line-number reporting.

Patch 5 converts existing assertion greps to test_grep, including
sourced test helpers. Greps used as data filters are left
unconverted with lint-ok annotations; an assertion on a
conditionally-present file is guarded on that condition (a
prerequisite, or a 'test -e') and uses test_grep inside the guard.

Patch 6 adds greplint.pl with test fixtures (modeled on chainlint/)
and wires it into the Makefile as test-greplint and check-greplint.

Changes since v3:

t/README: reworded the guidance to encourage explicit file
existence / pre-req checks. Instead of "keep a plain guarded
'grep'", it now says to guard the assertion on the
condition that governs the file's presence (a prerequisite, or a
'test -e' on the path) and use test_grep inside the guard, with a
worked example:
```
if test_have_prereq REFFILES
then
        test_grep ! "$refname" .git/packed-refs
fi
```
t7450 (squatting-clone/d/a/git~2): converted the last
missing-file-tolerant '! grep' to that guarded form:
```
if test -f squatting-clone/d/a/git\~2
then
        test_grep ! gitdir squatting-clone/d/a/git\~2
fi
```
'git~2' is the NTFS 8.3 short name of the planted '..git' file and
exists only when 8.3 short-name generation is enabled. Use
'test -f' so the conditional presence is now explicit instead of
relying on grep's tolerance for a missing file, and the assertion
gains test_grep's diagnostics where git~2 does materialize. This
removes the only '# lint-ok' in the series that guarded an assertion
rather than a data filter.
greplint.pl: updated the lint_ok() comment to match. A '# lint-ok'
annotation now documents a single case: a grep acting as a data
filter whose output is consumed by a redirect or pipe on an
enclosing compound command (a subshell or brace group), which the
filter heuristic cannot detect locally. The "file may not exist"
rationale is gone; the two remaining annotated greps (t5326, t5702)
are exactly this shape.

Note on ordering: this series leaves the t3420 '! grep dirty file3'
line untouched (per the request to omit it), so it depends on
sg/t3420-do-not-grep-in-missing-file, which replaces that line with
'test_path_is_missing file3' and is already in 'next'. Applied on top
of that topic the series is lint-clean with no edits. On a plain
'master' that does not yet contain it, the greplint check added by the
final patch will flag exactly that one line by design (the t3420 test
itself still passes; only the static lint fires).

Known limitation / follow-up:

Assertions like grep pattern file >/dev/null and
grep pattern <file are not converted because greplint.pl
treats any redirect as a filter, so it does not flag them.
The former could be converted, but test_grep prints the
matching line on success just as grep does, so the >/dev/null
would have to be kept or dropped as a judgment call. The
latter requires turning the <file redirect into a positional
file argument, since test_grep reads a named file rather than
stdin. Both are left as bare grep with no annotation:
unlike the genuine data-filter greps, they carry no
'# lint-ok' marker, because greplint silently classifies
any redirect as a filter and never flags them. A follow-up
series can address these once a convention is established.

Cc: "D. Ben Knoble" ben.knoble@gmail.com, Eric Sunshine sunshine@sunshineco.com, SZEDER Gábor szeder.dev@gmail.com

gitgitgadget · 2026-06-16T21:46:39Z

This patch series was integrated into seen via git@80b9fc2.

mmontalbo · 2026-07-03T04:48:55Z

/preview

gitgitgadget · 2026-07-03T04:50:19Z

Preview email sent as pull.2135.v3.git.1783054200.gitgitgadget@gmail.com

mmontalbo · 2026-07-03T04:53:22Z

/submit

gitgitgadget · 2026-07-03T04:54:56Z

Submitted as pull.2135.v3.git.1783054466.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2135/mmontalbo/mm/test-grep-docs-v3

To fetch this version to local tag pr-2135/mmontalbo/mm/test-grep-docs-v3:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2135/mmontalbo/mm/test-grep-docs-v3

test_grep is a wrapper around grep for test assertions that prints the file contents on failure for easier debugging. It also accepts '!' as its first argument for negation, which preserves the diagnostic output that '! test_grep' would suppress. Despite being widely used (and the preferred replacement for bare grep in assertions), test_grep has no entry in t/README alongside the other documented helpers like test_cmp and test_line_count. Add one. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>

Three grep assertions were missing their file arguments, causing them to read from empty stdin instead of the intended file: - t2402: '! grep ...' should read from 'out', matching the grep on the preceding line. - t7507: the closing quote is in the wrong place, making the entire 'diff --git actual' a single pattern with no file argument instead of pattern 'diff --git' and file 'actual'. - t7700: '! grep ...' should read from 'packlist', matching the redirect on the preceding line. Without file arguments these greps always succeed (empty stdin matches nothing), so the assertions were not actually checking anything. All three tests pass with the corrected file arguments, confirming the intended behavior is sound. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>

Move chainlint.pl's Lexer, ShellParser, and ScriptParser into a shared module (lib-shell-parser.pl) so other lint tools can reuse the same shell parsing infrastructure. A subsequent commit adds greplint.pl, which needs the same tokenizer to correctly identify command boundaries. ScriptParser's check_test() becomes a no-op in the shared module. chainlint.pl defines ChainlintParser (extending ScriptParser) with the &&-chain check_test() implementation. No functional change: chainlint produces the same output and check-chainlint self-tests pass. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>

scan_dqstring's post-loop newline counter re-counts newlines that were already counted during recursive parsing of $() bodies. This happens because scan_dollar returns text containing newlines (from multi-line command substitutions), and the catch-all counter at the end of scan_dqstring counts all of them again. Fix this by counting newlines inline as non-special characters are consumed, and removing the post-loop catch-all. Each newline is now counted exactly once: literal newlines at the inline match, line splices at the backslash handler, and $() newlines by scan_token during the recursive parse. This is a latent bug: any consumer that relies on token line numbers rather than byte offsets would get incorrect results for tokens following a multi-line $() inside a double-quoted string. chainlint is not affected because it annotates the original body text using byte offsets, not token line numbers. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>

Replace bare grep with test_grep in test assertions across the suite, including sourced test helpers (lib-*.sh, *-tests.sh). test_grep prints the contents of the file being searched on failure, making debugging easier than a bare grep which fails silently. Only assertion-style greps are converted: grep used as a filter in pipelines, command substitutions, conditionals, or with redirected I/O is left as-is with a "# lint-ok" annotation. Existing '! test_grep' calls are rewritten to 'test_grep !' so that the diagnostic output is preserved on failure. test_grep requires the file it reads to exist, so '! grep' assertions that inspect a file whose presence is conditional need care. In t5537 the '.git/shallow' file is still present after the repack (the client remains shallow), so the assertion is converted like any other. In t1400 the '.git/packed-refs' file exists only with the files backend, so its check is guarded with a REFFILES prerequisite; the backend-agnostic 'git show-ref' check that follows still runs under every backend. In t7450 'git~2' is the NTFS 8.3 short name of a '..git' file and only exists when 8.3 short-name generation is enabled, so its check is guarded with a 'test -f' on the path and uses test_grep inside the guard, the same shape as t1400 (a plain test_grep would BUG when the short name is absent). The conversion was generated using a grep-assertion linter (greplint.pl, added in the following commit) to identify bare grep calls at command position. To reproduce, from the t/ directory: # Step 1: annotate the two data-filter greps (grep produces # data, not a verdict) so the linter skips them. sed -i '/grep -vf before commits\.raw/s/$/ # lint-ok: data filter/' \ t5326-multi-pack-bitmaps.sh sed -i '/grep -E "^\[0-9a-f\].*|| :/s/$/ # lint-ok: data filter/' \ t5702-protocol-v2.sh # Step 1b: two '! grep' assertions need more than a mechanical # conversion; handle them by hand before the linter-driven steps # below so it leaves them alone. # # t1400: '.git/packed-refs' is absent under reftable, so guard the # check with REFFILES (a plain test_grep would BUG on the missing # file): # # git update-ref -d HEAD $B && # - ! grep "$m" .git/packed-refs && # + if test_have_prereq REFFILES # + then # + test_grep ! "$m" .git/packed-refs # + fi && # test_must_fail git show-ref --verify -q $m # # t7450: git~2 is an NTFS 8.3 short name that exists only when # short-name generation is enabled, so guard the check on its # presence with 'test -f' and note in a comment why the path can # be absent (a plain test_grep would BUG when it is): # # - ! grep gitdir squatting-clone/d/a/git~2 # + if test -f squatting-clone/d/a/git~2 # + then # + test_grep ! gitdir squatting-clone/d/a/git~2 # + fi # Step 2: reorder pre-existing '! test_grep' to 'test_grep !' # (must come before steps 3-4 so greplint does not see them) sed -i 's/! test_grep/test_grep !/' t0031-lockfile-pid.sh sed -i 's/! test_grep/test_grep !/' t5300-pack-object.sh sed -i 's/! test_grep/test_grep !/' t5319-multi-pack-index.sh # Step 3: convert '! grep' -> 'test_grep !' perl greplint.pl *.sh 2>&1 | cut -d: -f1,2 | while IFS=: read f l; do sed -i "${l}s/! *grep/test_grep !/" "$f" done # Step 4: convert remaining 'grep' -> 'test_grep' perl greplint.pl *.sh 2>&1 | cut -d: -f1,2 | while IFS=: read f l; do sed -i "${l}s/grep/test_grep/" "$f" done To verify, run: make -C t test-greplint Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>

Without a lint guard, bare grep assertions will creep back into tests over time, defeating the previous commit's conversion. Add greplint.pl to catch bare 'grep' used as a test assertion (where 'test_grep' should be used) and '! test_grep' (where 'test_grep !' should be used). greplint.pl reuses the shared shell parser from lib-shell-parser.pl to tokenize test bodies. The Lexer collapses heredocs, command substitutions, and quoted strings into single tokens, so 'grep' appearing inside these contexts is not flagged. A flat walk over the token stream tracks command position and pipeline state to distinguish assertion greps from filter greps. For double-quoted test bodies, a source-line walk counts backslash-continuation lines that the Lexer consumes without emitting into the body text, adjusting the reported line number accordingly. Add test fixtures in greplint/ (modeled on chainlint/) covering detection of bare grep assertions, correct skipping of filters, pipelines, redirects, command substitutions, and lint-ok annotations. Wire into the Makefile as: - test-greplint: runs greplint.pl on $(T) $(THELPERS) $(TPERF) - check-greplint: runs greplint.pl on fixtures, diffs against expected - clean-greplint: removes temp dir Add eol=lf entries in t/.gitattributes for greplint fixtures, matching chainlint, so that check-greplint passes on Windows where core.autocrlf would otherwise cause CRLF mismatches between expected and actual output. Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>

mmontalbo · 2026-07-06T04:53:18Z

/preview

gitgitgadget · 2026-07-06T04:54:30Z

Preview email sent as pull.2135.v4.git.1783313654.gitgitgadget@gmail.com

mmontalbo · 2026-07-06T05:01:02Z

/submit

gitgitgadget · 2026-07-06T05:02:30Z

Submitted as pull.2135.v4.git.1783314119.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2135/mmontalbo/mm/test-grep-docs-v4

To fetch this version to local tag pr-2135/mmontalbo/mm/test-grep-docs-v4:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2135/mmontalbo/mm/test-grep-docs-v4

gitgitgadget · 2026-07-06T11:46:45Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

Needs review.
source: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-06T11:46:54Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

Needs review.
source: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-06T11:52:54Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

Needs review.
source: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-06T12:12:39Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

Needs review.
source: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-06T12:19:02Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

Needs review.
source: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-06T12:38:25Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

Needs review.
source: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-06T12:43:08Z

@@ -523,7 +523,7 @@ test_expect_success 'Verify descending sort' '



SZEDER Gábor wrote on the Git mailing list (how to reply to this email):

On Sat, Jun 13, 2026 at 04:06:14AM +0000, Michael Montalbo via GitGitGadget wrote: > From: Michael Montalbo <mmontalbo@gmail.com> > > Replace bare grep with test_grep in test assertions across the > suite, including sourced test helpers (lib-*.sh, *-tests.sh). > test_grep prints the contents of the file being searched on > failure, making debugging easier than a bare grep which fails > silently. > > Only assertion-style greps are converted: grep used as a filter > in pipelines, command substitutions, conditionals, or with > redirected I/O is left as-is with a "# lint-ok" annotation. > Existing '! test_grep' calls are rewritten to 'test_grep !' so > that the diagnostic output is preserved on failure. Thanks for taking the effort for cleaning up all those negated '! grep' and '! test_grep' callsites and turning them into 'test_grep !'. > The conversion was generated using a grep-assertion linter > (greplint.pl, added in the following commit) to identify bare > grep calls at command position. To reproduce: > > # Step 1: mark bare greps that should not be converted > sed -i '/! grep "$m" \.git\/packed-refs/s/$/ # lint-ok: file may not exist (reftable)/' \ > t/t1400-update-ref.sh > sed -i '/! grep dirty file3 &&/{/lint-ok/!s/$/ # lint-ok: file may not exist after --quit/}' \ > t/t3420-rebase-autostash.sh I think in this case checking the file3's contents is wrong, because at this point file3 should not exist in the first place. I've sent a patch to fix this long ago, but apparently didn't manage to follow through back then. https://lore.kernel.org/git/20211010172809.1472914-1-szeder.dev@gmail.com/ > diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh > index f0bbc476ff..f6cb3dd72e 100755 > --- a/t/t3420-rebase-autostash.sh > +++ b/t/t3420-rebase-autostash.sh > @@ -141,8 +141,8 @@ testrebase () { > git checkout -b rebased-feature-branch feature-branch && > echo dirty >>file3 && > git rebase$type unrelated-onto-branch >actual 2>&1 && > - grep unrelated file4 && > - grep dirty file3 && > + test_grep unrelated file4 && > + test_grep dirty file3 && > git checkout feature-branch > ' > > @@ -165,8 +165,8 @@ testrebase () { > echo dirty >>file3 && > git add file3 && > git rebase$type unrelated-onto-branch && > - grep unrelated file4 && > - grep dirty file3 && > + test_grep unrelated file4 && > + test_grep dirty file3 && > git checkout feature-branch > ' > > @@ -197,7 +197,7 @@ testrebase () { > git add file2 && > git rebase --continue && > test_path_is_missing $dotest/autostash && > - grep dirty file3 && > + test_grep dirty file3 && > git checkout feature-branch > ' > > @@ -212,7 +212,7 @@ testrebase () { > test_path_is_missing file3 && > git rebase --skip && > test_path_is_missing $dotest/autostash && > - grep dirty file3 && > + test_grep dirty file3 && > git checkout feature-branch > ' > > @@ -227,7 +227,7 @@ testrebase () { > test_path_is_missing file3 && > git rebase --abort && > test_path_is_missing $dotest/autostash && > - grep dirty file3 && > + test_grep dirty file3 && > git checkout feature-branch > ' > > @@ -244,7 +244,7 @@ testrebase () { > git rebase --quit && > test_when_finished git stash drop && > test_path_is_missing $dotest/autostash && > - ! grep dirty file3 && > + ! grep dirty file3 && # lint-ok: file may not exist after --quit > git stash show -p >actual && > test_cmp expect actual && > git reset --hard && > @@ -260,11 +260,11 @@ testrebase () { > git rebase$type unrelated-onto-branch >actual 2>&1 && > test_path_is_missing $dotest && > git reset --hard && > - grep unrelated file4 && > - ! grep dirty file4 && > + test_grep unrelated file4 && > + test_grep ! dirty file4 && > git checkout feature-branch && > git stash pop && > - grep dirty file4 > + test_grep dirty file4 > ' > > test_expect_success "rebase$type: check output with conflicting stash" ' > @@ -286,7 +286,7 @@ test_expect_success "rebase: fast-forward rebase" ' > test_when_finished git branch -D behind-feature-branch && > echo dirty >>file1 && > git rebase feature-branch && > - grep dirty file1 && > + test_grep dirty file1 && > git checkout feature-branch > ' > > @@ -297,7 +297,7 @@ test_expect_success "rebase: noop rebase" ' > test_when_finished git branch -D same-feature-branch && > echo dirty >>file1 && > git rebase feature-branch && > - grep dirty file1 && > + test_grep dirty file1 && > git checkout feature-branch > ' >

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

SZEDER Gábor <szeder.dev@gmail.com> writes: >> The conversion was generated using a grep-assertion linter >> (greplint.pl, added in the following commit) to identify bare >> grep calls at command position. To reproduce: >> >> # Step 1: mark bare greps that should not be converted >> sed -i '/! grep "$m" \.git\/packed-refs/s/$/ # lint-ok: file may not exist (reftable)/' \ >> t/t1400-update-ref.sh >> sed -i '/! grep dirty file3 &&/{/lint-ok/!s/$/ # lint-ok: file may not exist after --quit/}' \ >> t/t3420-rebase-autostash.sh > > I think in this case checking the file3's contents is wrong, because > at this point file3 should not exist in the first place. I've sent a > patch to fix this long ago, but apparently didn't manage to follow > through back then. > > https://lore.kernel.org/git/20211010172809.1472914-1-szeder.dev@gmail.com/ Thanks. I guess the test_grep can be extended to catch this case, where test_grep ! -e pattern1 -e pattern2 file does not find any hits, but only because 'file' is missing, as an error, just like "test_must_fail git foo" that segfaults is flagged as "yes, it fails but that is not the kind of failure we expect".

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Junio C Hamano <gitster@pobox.com> writes: > SZEDER Gábor <szeder.dev@gmail.com> writes: > >> I think in this case checking the file3's contents is wrong, because >> at this point file3 should not exist in the first place. I've sent a >> patch to fix this long ago, but apparently didn't manage to follow >> through back then. >> >> https://lore.kernel.org/git/20211010172809.1472914-1-szeder.dev@gmail.com/ > > Thanks. I guess the test_grep can be extended to catch this case, > where > > test_grep ! -e pattern1 -e pattern2 file > > does not find any hits, but only because 'file' is missing, as an > error, ... Wait. The necessary check is already there, isn't it? test_grep () { eval "last_arg=\${$#}" test -f "$last_arg" || BUG "test_grep requires a file to read as the last parameter" So why don't we see it every time we run that test that inspects file3's contents with Michael's series merged in? Puzzled...

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Junio C Hamano <gitster@pobox.com> writes: > Junio C Hamano <gitster@pobox.com> writes: > >> SZEDER Gábor <szeder.dev@gmail.com> writes: >> >>> I think in this case checking the file3's contents is wrong, because >>> at this point file3 should not exist in the first place. I've sent a >>> patch to fix this long ago, but apparently didn't manage to follow >>> through back then. >>> >>> https://lore.kernel.org/git/20211010172809.1472914-1-szeder.dev@gmail.com/ >> >> Thanks. I guess the test_grep can be extended to catch this case, >> where >> >> test_grep ! -e pattern1 -e pattern2 file >> >> does not find any hits, but only because 'file' is missing, as an >> error, ... > > Wait. The necessary check is already there, isn't it? > > test_grep () { > eval "last_arg=\${$#}" > > test -f "$last_arg" || > BUG "test_grep requires a file to read as the last parameter" > > So why don't we see it every time we run that test that inspects > file3's contents with Michael's series merged in? Puzzled... Ah, of course. Michael sidesteps this mechanism by not using "test_grep !", with ! grep dirty file3 && # lint-ok: file may not exist after --quit and if we realize that "may not exist" is actually "never exists", then your other patch from 5 years ago would become the most sensible fix for this line. It may not be a bad idea to go through "# lint-ok:" introduced by Michael's series with finer toothed comb (there are only a handful of them) and see if there are similar "look, the file we are grepping in never exists with correctly running Git" gotchas. Thanks.

gitgitgadget · 2026-07-06T14:51:14Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

Waiting for response(s) to review comment(s).
cf. <aj93BE8MYatQAjoy@szeder.dev>
source: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-06T14:52:47Z

@@ -523,7 +523,7 @@ test_expect_success 'Verify descending sort' '



Junio C Hamano wrote on the Git mailing list (how to reply to this email):

Junio C Hamano <gitster@pobox.com> writes: > Ah, of course. Michael sidesteps this mechanism by not using > "test_grep !", with > > ! grep dirty file3 && # lint-ok: file may not exist after --quit > > and if we realize that "may not exist" is actually "never exists", > then your other patch from 5 years ago would become the most > sensible fix for this line. > > It may not be a bad idea to go through "# lint-ok:" introduced by > Michael's series with finer toothed comb (there are only a handful > of them) and see if there are similar "look, the file we are > grepping in never exists with correctly running Git" gotchas. In any case, I think SZEDER's fix to stop grepping in the file but instead insisting on its absense does make sense and it is now in 'next'. So perhaps this topic can have a small and final reroll v3 that omits change to this particular line (and possibly fix other lines that punts with "# lint-ok" if needed) and we can declare victory after that? Thanks, all.

Michael Montalbo wrote on the Git mailing list (how to reply to this email):

On Mon, Jun 29, 2026 at 2:21 PM Junio C Hamano <gitster@pobox.com> wrote: > > > It may not be a bad idea to go through "# lint-ok:" introduced by > > Michael's series with finer toothed comb (there are only a handful > > of them) and see if there are similar "look, the file we are > > grepping in never exists with correctly running Git" gotchas. > > In any case, I think SZEDER's fix to stop grepping in the file but > instead insisting on its absense does make sense and it is now in > 'next'. So perhaps this topic can have a small and final reroll v3 > that omits change to this particular line (and possibly fix other > lines that punts with "# lint-ok" if needed) and we can declare > victory after that? > > Thanks, all. Thank you, SZEDER, for the nice catch. I will apply the suggested fix to the series locally, and go through the other #lint-ok's with a fine toothed comb as Junio suggests. Appreciate the eyes on the series, will send a reroll soon.

gitgitgadget · 2026-07-06T15:01:23Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

Waiting for response(s) to review comment(s).
cf. <aj93BE8MYatQAjoy@szeder.dev>
cf. <xmqqqzlpt543.fsf@gitster.g>
source: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-06T15:07:25Z

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Changes since v2:
>
>  * t3420-rebase-autostash: dropped the change to the '! grep dirty file3'
>    line under 'rebase --quit'. As SZEDER pointed out, file3 never exists in
>    the conflicted state, so that grep was passing only because it could not
>    open the file. SZEDER's fix (sg/t3420-do-not-grep-in-missing-file, now in
>    'next') replaces the line with 'test_path_is_missing file3', which is the
>    right check; this series simply leaves that line to his fix.
>
>  * Audited the remaining '# lint-ok' annotations for the same "grep a file
>    that never exists with correctly running Git" gotcha, as Junio suggested.
>    The rule the audit applies: 'grep' becomes 'test_grep' only where its
>    exit code is the assertion; grep that produces data (a filter) or that
>    reads a file whose presence is conditional stays a plain 'grep', because
>    test_grep BUGs on a missing file.
>    
>    * t5537 (.git/shallow): the file is still present after the repack (the
>      client stays shallow), so the assertion is converted to 'test_grep !'
>      like any other; the "may not exist" note was wrong.
>    
>    * t1400 (.git/packed-refs): the file exists only with the files backend.
>      Guarded the packed-refs check with a REFFILES prerequisite; the
>      backend-agnostic 'git show-ref' check that follows still runs under
>      every backend.
>    
>    * t7450 (squatting-clone/d/a/git~2): kept as '! grep' with an improved '#
>      lint-ok'. 'git~2' is the NTFS 8.3 short name of a planted '..git' decoy
>      and only exists when 8.3 short-name generation is enabled. Verified on
>      a Windows VM: with 8.3 disabled (the modern default) the short name is
>      absent, the '! grep' correctly tolerates it, and a plain test_grep
>      would BUG. So this one deliberately stays a missing-file-tolerant grep.
>    
>    * t5326 and t5702 remain annotated: these are genuine data filters (grep
>      produces data that is redirected/captured, not an assertion).

Great.

>      ++   test_grep requires <file> to exist and will BUG otherwise.
>      ++   When a file's presence is conditional (a backend-specific
>      ++   file, or a path that only exists on some platforms, such as
>      ++   an NTFS 8.3 short name), keep a plain guarded 'grep' instead.

It is not quite clear if I can follow this instruction myself,
without knowing what a "plain guarded 'grep'" is, unfortunately.  Is
it different from bog-standard grep?

>      @@ t/t1400-update-ref.sh: test_expect_success "move $m (by HEAD)" '
>        	test_when_finished "git update-ref -d $m" &&
>        	git update-ref -d HEAD $B &&
>       -	! grep "$m" .git/packed-refs &&
>      -+	! grep "$m" .git/packed-refs && # lint-ok: file may not exist (reftable)
>      ++	if test_have_prereq REFFILES
>      ++	then
>      ++		test_grep ! "$m" .git/packed-refs
>      ++	fi &&

The intent is shown very well in this version (admittedly, the
lint-ok comment is readable but only by humans and LLMs).  Here, we
expect .git/packed-refs only while REFFILES prerequiste is active.

Thanks.

gitgitgadget · 2026-07-06T15:07:46Z

Michael Montalbo wrote on the Git mailing list (how to reply to this email):

On Sat, Jul 4, 2026 at 6:38 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> It is not quite clear if I can follow this instruction myself,
> without knowing what a "plain guarded 'grep'" is, unfortunately.  Is
> it different from bog-standard grep?
>

I agree with you that the word "guarded" is really confusing here
after reading it
again. The suggestion should just be to use "plain `grep`" with no mention of a
guard. Will remove it.

My original intent was something along the lines of "guarded against BUG'ing
on missing file", but the wording is confusing and, worse, could encourage the
anti-pattern of using grep to test for file existence.

gitgitgadget · 2026-07-06T22:42:30Z

This patch series is no longer integrated into seen.

gitgitgadget · 2026-07-07T18:10:50Z

There was a status update in the "Cooking" section about the branch mm/test-grep-lint on the Git mailing list:

The test suite has been updated to use the 'test_grep' helper instead
of bare 'grep' for test assertions, allowing file contents to be
printed on failure for easier debugging. A new 'greplint' linter has
been introduced to detect and prevent new bare 'grep' assertions from
being added to the test suite.

Needs review.
source: <pull.2135.v4.git.1783314119.gitgitgadget@gmail.com>

gitgitgadget · 2026-07-08T22:07:41Z

This patch series was integrated into seen via git@0c14868.

mmontalbo force-pushed the mm/test-grep-docs branch 30 times, most recently from 5d5366a to 9354adf Compare June 3, 2026 18:05

mmontalbo added 6 commits July 5, 2026 16:39

gitgitgadget Bot reviewed Jul 6, 2026

View reviewed changes

		@@ -523,7 +523,7 @@ test_expect_success 'Verify descending sort' '

Uh oh!

Conversation

mmontalbo commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gitgitgadget Bot commented Jun 16, 2026

Uh oh!

mmontalbo commented Jul 3, 2026

Uh oh!

gitgitgadget Bot commented Jul 3, 2026

Uh oh!

mmontalbo commented Jul 3, 2026

Uh oh!

gitgitgadget Bot commented Jul 3, 2026

Uh oh!

mmontalbo commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

mmontalbo commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 6, 2026

Uh oh!

gitgitgadget Bot commented Jul 7, 2026

Uh oh!

gitgitgadget Bot commented Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mmontalbo commented Jun 2, 2026 •

edited

Loading