From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-qk1-f179.google.com (mail-qk1-f179.google.com [209.85.222.179]) (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 C43B3171CD for ; Mon, 6 Jul 2026 05:02:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.222.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783314125; cv=none; b=tr2/Ew15qictPO5x4NHipfR/61gM5LMl1Mgi9BrogYFFNXaGGjzxFGCDxJ0r49VfghGMfo6Nny9w+XOomPBB4saTqcPJ3DY/MbjVvhl16V32dk9CSMysRtaox0CySgFur1XfU0lYtBB0KPOkWJBC/p+mt6PwTkMc4eHhx+7znoI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783314125; c=relaxed/simple; bh=ogvAXMeUuJdsbVFovXm4WdhWDO4b4f8QcI5we51356g=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=E3c35c6PY02zpd6hr9Gtw2gPHAtAsEfW79HQOnbJv26IcBwLluV9A4ykvmNaRJ54uqUC8B38lAwnTLK5+owJudfTdPEKVoadnPNT2cze/6k0QzfjULKzYZ+9/KpHRDS+TW39jkHp6RynfzgQafLHT1Gi2N7Pa4wJsVQMhvINhq0= 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=fhRnAr/I; arc=none smtp.client-ip=209.85.222.179 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="fhRnAr/I" Received: by mail-qk1-f179.google.com with SMTP id af79cd13be357-92e51d3d83cso120298985a.2 for ; Sun, 05 Jul 2026 22:02:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1783314122; x=1783918922; 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=2VTKnrjcITTUBSgCDksByoe6ZYu8bkzt6A3DvfBWTe4=; b=fhRnAr/I3Ke5KLCvRkJ2UXA4cwmFIaWeftp1pGSmW5bspNjiUYrNXPtagq86n1GQXv 2Vz+3CbXeAMRY0eM/OfHCqAGPu8immc9+k2qlZRQHd7dirWXeErLqG83E+0jQijF5NAj tic4zxT/sj2rUU7KWaSU6Dd9ao6OYQ8H2I5e8O+TWfkFLYXH5smEyrS6qufP8Eu17tga 94BhOKNP0KKNXl7odfE76gCEV0ljV+3bxhH3vL050GNz0MNssxdvBE1wM2Es1zTDtkGH eiYTeEbA7z1naXGbn+wyWAVCK17j/H9if1uqw7e4M+vJakq8lkgieutctHQfArdF5Sh0 YGCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783314122; x=1783918922; 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=2VTKnrjcITTUBSgCDksByoe6ZYu8bkzt6A3DvfBWTe4=; b=MUNc+A3twGPcYQhA54LwN8Lu5qsaan8aHZ2r9Fiu/IuuTEMTKnT3crguSsZNeR35LK BUB2waF7PpXa3ja7q5MYjsdsxsu04oI+pKJbZqPVHQfSWZ/xRCape26LDL76LULcD4m5 gjQJte/3CqO5isQiAMhqqWDJETCgk8xxF8KDF5dM/frPhAOCCGzZ0BxfaxXpeE75Zu5e KHRci13hvO+V18qcOC4iUdZx3qklMhXnQd1nP8NKDhB3Bu4UYuGJa6v2lHFWJ7nw6WaJ pPCT+4PiG6sHh5zrV07S1ExAkpzrG+37Euw7IWhBXY3kMx2453ppHwU39opCGfHkWCnb 0JgA== X-Gm-Message-State: AOJu0YwzoiOaGW2EttVUtNVW7Mr8N3JeQaWtyZxcXIeMK/zoailhFNcs 04Vue4cDDBNc6wWP6xr0U/EgkRBaaQpr+uvINuEiDPSbfdre3PRZMIelIvXQPttC X-Gm-Gg: AfdE7clM+SzBKeyAMbKN0tzf9T64asVrkKIuFc3ombHaItY01wMR0hjL94J2YFWw8qU 14EcdDIj7xA3rcYuVtJwZ2Bs1q7QP5BLhDEnNfDwCYiqcrfhvFAR1vQy8Zk2QNHITot30jhwPLN FumCteOBrmwTid5DYu+dhn9HOMdbBOb/I8Wt1cAbIHO/wwj1vwXav1X8Rl3CmrsJ9AzeppuSIvZ RKmG9vVaDZgbwSEsOzL2cEZZ5QFbCFJUglzfaG6fnwdQ+skM1vjpUCQZ31SPBewj0K9tRbFLRth c+epMtI8jKV8+G0OVYyduxGz38b+ICYccbNCI4ku5YJaPvmGbpEqhuvmodGPbr9c0WGtdoMX7R/ mzi6mXJl3y5OmDKp0ZTGFJKn4wG5637HyiHbb6gDaPgH4Yfosw8tKGWSIRE1l1DRyUrTWzdyvTj vuTZ2wCMegJUAz4u4SG1yoUS7A X-Received: by 2002:a05:620a:4621:b0:92b:6805:eae7 with SMTP id af79cd13be357-92e9a51eedcmr1117639585a.68.1783314121327; Sun, 05 Jul 2026 22:02:01 -0700 (PDT) Received: from [127.0.0.1] ([68.154.38.16]) by smtp.gmail.com with ESMTPSA id af79cd13be357-92e90ba4209sm881026185a.12.2026.07.05.22.02.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 05 Jul 2026 22:02:00 -0700 (PDT) Message-Id: In-Reply-To: References: From: "Michael Montalbo via GitGitGadget" Date: Mon, 06 Jul 2026 05:01:52 +0000 Subject: [PATCH v4 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 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 /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_grep requires to exist and will BUG otherwise, so ++ use it only where the file is guaranteed to exist at that point. ++ 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), guard the assertion on that condition (a ++ prerequisite, or a 'test -e' on the path) and use test_grep ++ inside the guard: ++ ++ if test_have_prereq REFFILES ++ then ++ test_grep ! "$refname" .git/packed-refs ++ fi + - test_path_is_file test_path_is_dir 2: 7a837b77cd = 2: 44ba6d1f1a t: fix grep assertions missing file arguments 3: 5894ca1724 = 3: 8f3ebe6426 t: extract chainlint's parser into shared module 4: 416c95fee5 = 4: 5689d2074a t: fix Lexer line count for $() inside double-quoted strings 5: 7518445733 ! 5: 53fedba4d4 t: convert grep assertions to test_grep @@ Commit message 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 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 @@ Commit message # 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. + # 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) @@ 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 -+ # 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 ++ # git~2 is an 8.3 short name, present only when 8.3 name ++ # generation is enabled. The "directory not empty" check ++ # above is the primary assertion. ++ if test -f squatting-clone/d/a/git~2 ++ then ++ test_grep ! gitdir squatting-clone/d/a/git~2 ++ fi fi ' 6: 3b12a959fd ! 6: 5cda64fc26 t: add greplint to detect bare grep assertions @@ t/greplint.pl (new) + return 1; +} + -+# Some bare greps are intentional (e.g. file may not exist, -+# data filter). A '# lint-ok' annotation on the source line -+# suppresses the warning. ++# lint_ok() reports whether a bare grep carries a trailing ++# '# lint-ok' comment telling this linter to skip it. ++# ++# In practice this is needed for just one case: a grep acting ++# as a data filter whose output is consumed by a redirect or ++# pipe on an enclosing compound command (such as a subshell or ++# brace group) rather than by grep's own pipeline, e.g. ++# ++# ( grep ... && # lint-ok ++# sed ... ) >out ++# ++# { grep ... || : # lint-ok ++# } >out ++# ++# is_filter() only scans grep's own pipeline: it stops at the ++# separator before the compound command closes and never sees ++# the outer redirect, so it would flag such a grep as an ++# assertion. A grep that really is an assertion is better ++# written as test_grep (or a guarded test_grep when the file's ++# presence is conditional) than annotated with lint-ok. +sub lint_ok { + my ($raw_lines, $ln) = @_; + if ($ln < 1 || $ln > @$raw_lines) { -- gitgitgadget