From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qv1-f47.google.com (mail-qv1-f47.google.com [209.85.219.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5BF543254B8 for ; Fri, 3 Jul 2026 04:54:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783054485; cv=none; b=dFXEvWuKNdL960hGbXx3XlsjLmapURyOYmOnezl4+bexagJT6eg9JmgbeWuECBYs2ALO68ac1I4Ulh9LmF++dh98BkNDkqD6hlY3M1ICwJ48h8wwte42V9jFTlx44Fmt83RhLnNLWr1x5lN2hkTNss7Zvpz9a7VHxhgioy/wpEs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783054485; c=relaxed/simple; bh=xCENxBiaC4cjtL7YwgRVL/ftm0voVjtnMKxRouqfMu0=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=q5n8avUwCrVLfxJoMt42L4XhzPjGLmE8nzBug7iDPxP3G79nJCW87gl4TxZl4J9Ac96b3r6RMVhxCjffDAGkhB5s4aNszOsECWH7gcWcNvA06nfbmjc/5h9xUe9UMEFv+lOaNQUnP5ZpA7r5mJB1laF7ge9obI8f837na+bRELY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=Q0BVDGDA; arc=none smtp.client-ip=209.85.219.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Q0BVDGDA" Received: by mail-qv1-f47.google.com with SMTP id 6a1803df08f44-8f2509d6becso1384856d6.2 for ; Thu, 02 Jul 2026 21:54:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1783054469; x=1783659269; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=fpaJDrRXD1d9Ub2Sz1aRl6otLw1TwVhpWQyNqlDrfWo=; b=Q0BVDGDAvpg+3eIkhl2h+HGzjymD4tTZjCKNeI9vbkDWGgpJce2GmbrEpvzFNt6igK Yd1yQfe+3U9bcA2jUy/NDvCEQQu/UQxwoHiv3GRCtuxIznUNs4wyQdugSCvJGpxg/mqH 0FOK+SWtERylTqWxDMTky/vW6kaEomj5mXVDt+jc4dxzzligm9KB1qLYtHvUlxq9g6dh okqGtpRIqdzeWZjj2ikccyrACbENJgmIlj23lWNOmEA498gBM7OO1IldE00E+yrr9rJV XMOZOCoMLfl551a3qo66C7nlGr9stqp+5WyjebkCMPoikhOXu9oNXPyIoGMwt2egluiu ZkrQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783054469; x=1783659269; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=fpaJDrRXD1d9Ub2Sz1aRl6otLw1TwVhpWQyNqlDrfWo=; b=C/4KTiIrSzq51bJ/eOob5ePykRtHsDvwFknVvvMUHi64z7dWLGuYa8Cyep5pimdhS3 RGJqUl+BUllWIkEWoXPtoI0j/X6MVyn6gxXUUMGzK09udSnBJVdbHYfE3YUwayalAdDp f+4ObV8ijoaGLPrEQS8xnkx6To+x7gaofCt/DqJNYEDmyxvyUn7L7oGyzJb6MrN6O8eY JCNd+gZFzFafp7BvKxmrc5TJsD/D9IuRUN3rvcCixXIchRw54uaJh1uVP9qWpl9kLSrZ GRX0Sc91N8kuemegoJpwNC9GYvDtArJRpkJcxRBwjU+DFBsafsaDuC/T5ibiRBG+j/Wr lSBw== X-Gm-Message-State: AOJu0YzjIHyCiTy0i2+JeQsMevfO/WbVFIcRXeJBA/4496E+WWPCg0WO jpTMKik2CQ+nZ0etdm3aBEVO8kGz1Sfa7VPCQ7NYfrI68zMllcJLEuqMunQBJw== X-Gm-Gg: AfdE7cnCC1ONOEU+ca1SSBpr+DZ5+zV+XRmE8269sPrgiAPwCd9X+qKXjEDU0blWR3x 8+XPDtwOqKesixFKzgy2y3CpA/D3CG1JIV6nxJr5jFDPYKMEEIFhEIpfR2ex/r4rqNYS+Mc+Cw1 6SdMHJnzi8v3zEgTkc8xgZaelk0BuYFfYnZgyNVtdSakPdJaK4S9hN9YfeiJk0RHQhm5Z8KE+9M Jc3l8vS4ICnAZ9l9WgTzjg9hxWGC80iHzlFhSojrFsqYq2MxpQGrvrMaSK5dQX4kAjDwV+wSX0O zcOruAEPeNsZCh+w1UFLjvAZhF/acphMD2jnUGmKMhKRKo5xlKTRE6FLNoLBnM/Lg1D+KPJlACb LDWbxu9Fvx3uZUyqo+2gaBxWdSG1kUZJvm46XWhnueu80rJtURuq+sqPUTaKdnMh6fJcbLOHPKB 2DdGdJj4r3SIBFgTk= X-Received: by 2002:a05:6214:e6a:b0:8ef:db4:d84a with SMTP id 6a1803df08f44-8f3c301f0damr130046386d6.0.1783054468491; Thu, 02 Jul 2026 21:54:28 -0700 (PDT) Received: from [127.0.0.1] ([48.217.251.96]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-8f471814324sm45543676d6.28.2026.07.02.21.54.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 02 Jul 2026 21:54:27 -0700 (PDT) Message-Id: In-Reply-To: References: From: "Michael Montalbo via GitGitGadget" Date: Fri, 03 Jul 2026 04:54:19 +0000 Subject: [PATCH v3 0/6] t: add greplint.pl and convert grep to test_grep Fcc: Sent Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: "D. Ben Knoble" , Eric Sunshine , SZEDER =?UTF-8?Q?G=C3=A1bor?= , Michael Montalbo 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 or on files that may not exist are left unconverted with lint-ok annotations. 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 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). * Rebased onto master. The only conflict was a one-line codespell typo fix that upstream applied to chainlint.pl in the meantime; it now lives in the extracted lib-shell-parser.pl. Known limitation / follow-up: Assertions like grep pattern file >/dev/null and grep pattern /dev/null would have to be kept or dropped as a judgment call. The latter requires turning the 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. + - test_path_is_file test_path_is_dir 2: f3e8e19e6e = 2: 7a837b77cd t: fix grep assertions missing file arguments 3: 460461b5fe ! 3: 5894ca1724 t: extract chainlint's parser into shared module @@ t/chainlint.pl: my $jobs = -1; -# -# In other languages, `1+2` would typically be scanned as three tokens -# (`1`, `+`, and `2`), but in shell it is a single token. However, the similar --# `1 + 2`, which embeds whitepace, is scanned as three token in shell, as well. +-# `1 + 2`, which embeds whitespace, is scanned as three token in shell, as well. -# In shell, several characters with special meaning lose that meaning when not -# surrounded by whitespace. For instance, the negation operator `!` is special -# when standing alone surrounded by whitespace; whereas in `foo!uucp` it is @@ t/lib-shell-parser.pl (new) +# +# In other languages, `1+2` would typically be scanned as three tokens +# (`1`, `+`, and `2`), but in shell it is a single token. However, the similar -+# `1 + 2`, which embeds whitepace, is scanned as three token in shell, as well. ++# `1 + 2`, which embeds whitespace, is scanned as three token in shell, as well. +# In shell, several characters with special meaning lose that meaning when not +# surrounded by whitespace. For instance, the negation operator `!` is special +# when standing alone surrounded by whitespace; whereas in `foo!uucp` it is 4: c1b86748d1 = 4: 416c95fee5 t: fix Lexer line count for $() inside double-quoted strings 5: 3a589ef738 ! 5: 7518445733 t: convert grep assertions to test_grep @@ Commit message 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 decoy '..git' file and only exists + when 8.3 short-name generation is enabled; that '! grep' tolerates + the missing file on purpose, so it is left as-is with a + "# lint-ok" annotation rather than converted (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: + grep calls at command position. To reproduce, from the t/ + directory: - # 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 + # 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/' \ - t/t5326-multi-pack-bitmaps.sh - sed -i '/! grep $d shallow-client\/\.git\/shallow/s/$/ # lint-ok: file may not exist after repack/' \ - t/t5537-fetch-shallow.sh + t5326-multi-pack-bitmaps.sh sed -i '/grep -E "^\[0-9a-f\].*|| :/s/$/ # lint-ok: data filter/' \ - t/t5702-protocol-v2.sh - sed -i '/! grep gitdir squatting-clone/s/$/ # lint-ok: file may not exist after failed clone/' \ - t/t7450-bad-git-dotfiles.sh + 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 this stays a missing-file- + # tolerant '! grep'; add a comment plus "# lint-ok" so the linter + # skips it. # 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 !/' t/t0031-lockfile-pid.sh - sed -i 's/! test_grep/test_grep !/' t/t5300-pack-object.sh - sed -i 's/! test_grep/test_grep !/' t/t5319-multi-pack-index.sh + 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 t/greplint.pl t/*.sh 2>&1 | cut -d: -f1,2 | + 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 t/greplint.pl t/*.sh 2>&1 | cut -d: -f1,2 | + 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 @@ 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 && test_must_fail git show-ref --verify -q $m ' @@ t/t3420-rebase-autostash.sh: testrebase () { git checkout feature-branch ' -@@ t/t3420-rebase-autostash.sh: 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 && @@ t/t3420-rebase-autostash.sh: testrebase () { git rebase$type unrelated-onto-branch >actual 2>&1 && test_path_is_missing $dotest && @@ t/t4211-line-log.sh: test_expect_success '-L with -G filters to diff-text matche + test_grep "F2 + 2" actual ' - test_done + test_expect_success '-L with --diff-filter=M excludes root commit' ' ## t/t4216-log-bloom.sh ## @@ t/t4216-log-bloom.sh: test_expect_success 'persist filter settings' ' @@ t/t5537-fetch-shallow.sh: test_expect_success '.git/shallow is edited by repack' git -C shallow-client repack -adfl && test_must_fail git -C shallow-client rev-parse --verify $d^0 && - ! grep $d shallow-client/.git/shallow && -+ ! grep $d shallow-client/.git/shallow && # lint-ok: file may not exist after repack ++ test_grep ! $d shallow-client/.git/shallow && git -C shallow-server branch branch-orig $d && git -C shallow-client fetch --prune --depth=2 \ @@ t/t7450-bad-git-dotfiles.sh: test_expect_success WINDOWS 'prevent git~1 squattin clone --recurse-submodules squatting squatting-clone 2>err && test_grep -e "directory not empty" -e "not an empty directory" err && - ! grep gitdir squatting-clone/d/a/git~2 -+ ! grep gitdir squatting-clone/d/a/git~2 # lint-ok: file may not exist after failed clone ++ # git~2 is the 8.3 short name of the ..git decoy, present ++ # only when 8.3 name generation is on; a missing git~2 is ++ # fine because the "directory not empty" check above is the ++ # real assertion. ++ ! grep gitdir squatting-clone/d/a/git~2 # lint-ok: 8.3 short name git~2 may not exist fi ' 6: e5ecb37401 = 6: 3b12a959fd t: add greplint to detect bare grep assertions -- gitgitgadget