* [GSoC Patch v3 3/4] repo: add path.commondir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-12 18:28 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kristofferhaugsbakk,
kumarayushjha123, lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260612182847.562816-1-jayatheerthkulkarni2005@gmail.com>
Scripts working with worktree setups need a reliable way to discover
the common directory, which diverges from the git directory when
multiple worktrees are in use. There is no way to retrieve this path
from git repo info today.
Introduce path.commondir.absolute and path.commondir.relative keys.
Exposing explicit format variants rather than a single key with a
default avoids ambiguity for scripts that require predictable output.
Add a test helper test_repo_info_path that creates isolated
repositories per test case to prevent state leaks, captures the repo
root before changing directories to avoid eval, and accepts an optional
init_command to cover environment variable overrides such as
GIT_COMMON_DIR and GIT_DIR.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 9 ++++++
builtin/repo.c | 26 ++++++++++++++++
t/t1900-repo-info.sh | 61 +++++++++++++++++++++++++++++++++++++
3 files changed, 96 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 42262c1983..890c34051d 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -104,6 +104,15 @@ values that they return:
`object.format`::
The object format (hash algorithm) used in the repository.
+`path.commondir.absolute`::
+ The canonical absolute path to the Git repository's common
+ directory (the shared `.git` directory containing objects,
+ refs, and global configuration).
+
+`path.commondir.relative`::
+ The path to the Git repository's common directory relative to
+ the current working directory.
+
`references.format`::
The reference storage format. The valid values are:
+
diff --git a/builtin/repo.c b/builtin/repo.c
index 71a5c1c29c..c4cc3bf3fc 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -7,12 +7,14 @@
#include "hex.h"
#include "odb.h"
#include "parse-options.h"
+#include "path.h"
#include "path-walk.h"
#include "progress.h"
#include "quote.h"
#include "ref-filter.h"
#include "refs.h"
#include "revision.h"
+#include "setup.h"
#include "strbuf.h"
#include "string-list.h"
#include "shallow.h"
@@ -75,6 +77,28 @@ static int get_object_format(struct repository *repo, struct strbuf *buf)
return 0;
}
+static int get_path_commondir_absolute(struct repository *repo, struct strbuf *buf)
+{
+ const char *common_dir = repo_get_common_dir(repo);
+
+ if (!common_dir)
+ return error(_("unable to get common directory"));
+
+ append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
+static int get_path_commondir_relative(struct repository *repo, struct strbuf *buf)
+{
+ const char *common_dir = repo_get_common_dir(repo);
+
+ if (!common_dir)
+ return error(_("unable to get common directory"));
+
+ append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
static int get_references_format(struct repository *repo, struct strbuf *buf)
{
strbuf_addstr(buf,
@@ -87,6 +111,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "layout.bare", get_layout_bare },
{ "layout.shallow", get_layout_shallow },
{ "object.format", get_object_format },
+ { "path.commondir.absolute", get_path_commondir_absolute },
+ { "path.commondir.relative", get_path_commondir_relative },
{ "references.format", get_references_format },
};
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 39bb77dda0..28fe76e25b 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -155,4 +155,65 @@ test_expect_success 'git repo info -h shows only repo info usage' '
test_grep ! "git repo structure" actual
'
+# Helper function to test path keys in both absolute and relative formats.
+# $1: label for the test
+# $2: field_name (e.g., commondir)
+# $3: unique repo name for isolation
+# $4: expect_absolute (suffix appended to repo root)
+# $5: expect_relative (the relative path string expected)
+# $6: init_command (extra setup like exporting env vars)
+test_repo_info_path () {
+ label=$1
+ field_name=$2
+ repo_name=$3
+ expect_absolute_suffix=$4
+ expect_relative=$5
+ init_command=$6
+
+ absolute_root="$repo_name-absolute"
+ relative_root="$repo_name-relative"
+
+ test_expect_success "setup: $label" '
+ git init "$absolute_root" &&
+ git init "$relative_root" &&
+ mkdir -p "$absolute_root/sub" "$relative_root/sub"
+ '
+
+ test_expect_success "absolute: $label" '
+ (
+ cd "$absolute_root/sub" &&
+ ROOT="$(test-tool path-utils real_path "..")" && export ROOT &&
+ eval "$init_command" &&
+ expect_path="$ROOT${expect_absolute_suffix:+/$expect_absolute_suffix}" &&
+ echo "path.$field_name.absolute=$expect_path" >expect &&
+ git repo info "path.$field_name.absolute" >actual &&
+ test_cmp expect actual
+ )
+ '
+
+ test_expect_success "relative: $label" '
+ (
+ cd "$relative_root/sub" &&
+ ROOT="$(test-tool path-utils real_path "..")" && export ROOT &&
+ eval "$init_command" &&
+ echo "path.$field_name.relative=$expect_relative" >expect &&
+ git repo info "path.$field_name.relative" >actual &&
+ test_cmp expect actual
+ )
+ '
+}
+
+test_repo_info_path 'commondir standard' 'commondir' 'commondir-std' \
+ '.git' '../.git'
+
+test_repo_info_path 'commondir with GIT_COMMON_DIR and GIT_DIR' 'commondir' \
+ 'commondir-envs' 'custom-common' '../custom-common' \
+ 'GIT_COMMON_DIR="$ROOT/custom-common" && export GIT_COMMON_DIR &&
+ GIT_DIR="../.git" && export GIT_DIR &&
+ git init --bare "$ROOT/custom-common"'
+
+test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
+ 'commondir-only-gitdir' '.git' '../.git' \
+ 'GIT_DIR="../.git" && export GIT_DIR'
+
test_done
--
2.54.0
^ permalink raw reply related
* [GSoC Patch v3 4/4] repo: add path.gitdir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-12 18:28 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kristofferhaugsbakk,
kumarayushjha123, lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260612182847.562816-1-jayatheerthkulkarni2005@gmail.com>
Scripts need a stable way to locate the git directory without
parsing rev-parse output or relying on its flag-driven path format
selection. There is no way to retrieve this path from git repo info
today.
Introduce path.gitdir.absolute and path.gitdir.relative keys,
consistent with the path.commondir keys added in the previous patch.
Reuse the test_repo_info_path helper introduced there to validate
both variants.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 6 ++++++
builtin/repo.c | 24 ++++++++++++++++++++++++
t/t1900-repo-info.sh | 7 +++++++
3 files changed, 37 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 890c34051d..ed7d80c690 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -113,6 +113,12 @@ values that they return:
The path to the Git repository's common directory relative to
the current working directory.
+`path.gitdir.absolute`::
+ The canonical absolute path to the Git repository directory (the `.git` directory).
+
+`path.gitdir.relative`::
+ The path to the Git repository directory relative to the current working directory.
+
`references.format`::
The reference storage format. The valid values are:
+
diff --git a/builtin/repo.c b/builtin/repo.c
index c4cc3bf3fc..9a312d127a 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -99,6 +99,28 @@ static int get_path_commondir_relative(struct repository *repo, struct strbuf *b
return 0;
}
+static int get_path_gitdir_absolute(struct repository *repo, struct strbuf *buf)
+{
+ const char *git_dir = repo_get_git_dir(repo);
+
+ if (!git_dir)
+ return error(_("unable to get git directory"));
+
+ append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
+static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
+{
+ const char *git_dir = repo_get_git_dir(repo);
+
+ if (!git_dir)
+ return error(_("unable to get git directory"));
+
+ append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
static int get_references_format(struct repository *repo, struct strbuf *buf)
{
strbuf_addstr(buf,
@@ -113,6 +135,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "object.format", get_object_format },
{ "path.commondir.absolute", get_path_commondir_absolute },
{ "path.commondir.relative", get_path_commondir_relative },
+ { "path.gitdir.absolute", get_path_gitdir_absolute },
+ { "path.gitdir.relative", get_path_gitdir_relative },
{ "references.format", get_references_format },
};
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 28fe76e25b..26acb5fe82 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -216,4 +216,11 @@ test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
'commondir-only-gitdir' '.git' '../.git' \
'GIT_DIR="../.git" && export GIT_DIR'
+test_repo_info_path 'gitdir standard' 'gitdir' 'gitdir-std' \
+ '.git' '../.git'
+
+test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
+ 'gitdir-env' '.git' '../.git' \
+ 'GIT_DIR="../.git" && export GIT_DIR'
+
test_done
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v3 2/2] compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
From: Dominik Loidolt @ 2026-06-12 19:04 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, gitster, asedeno, asedeno, avarab
In-Reply-To: <aiwJSBfRbUFZ70gP@pks.im>
Thanks again for taking the time to review my contribution.
On Fri, Jun 12, 2026 at 03:27:36PM +0200, Patrick Steinhardt wrote:
> > It is also more future-proof, as it no longer assumes that GCC version
> > components stay below 65536.
>
> I feel like all the message needs to say is "let's do it for
> consistency, and it's easier to read". That would've been sufficient,
> whereas this argument here feels a bit thin.
Agreed. I'll simplify the commit message. The "future-proof" bit was a joke I
just couldn't resist, but it may cause more confusion than it is worth.
I'll drop it.
> It would've been nice to either move these changes into a preparatory
> commit or at least mention them
Agreed. I'll split the cleanup into a separate commit.
> I'm not sure myself whether this could use another reroll. It's all just
> nits, and the intent is clear enough.
I think it's worth rerolling.
Thanks,
Dominik
^ permalink raw reply
* Re: Automated reviews by AI (was Re: [PATCH 0/5] Duplicate entry hardening)
From: Junio C Hamano @ 2026-06-12 19:32 UTC (permalink / raw)
To: Christian Couder
Cc: Patrick Steinhardt, Elijah Newren via GitGitGadget, git,
Elijah Newren, Konstantin Ryabitsev, Taylor Blau
In-Reply-To: <CAP8UFD35cLP6FcEuPr+SghKae1ew4JWLWYAoMQ-fuEOu-JmZdg@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> On Tue, Jun 2, 2026 at 8:16 AM Patrick Steinhardt <ps@pks.im> wrote:
>>
>> Overall I agree that everyone who is a core contributor should also make
>> reviews part of their regular worflow. At least for corporate
>> contributors that might also make it easier to communicate this to their
>> respective employers. Regardless of that, my expectation is that there
>> will be times where it works well, and other times where it works less
>> well.
>
> Sashiko (https://github.com/sashiko-dev/sashiko) is used these days by
> Linux kernel developers and seems to work well for them.
>
> At GitLab and probably in other companies, some of us also use AI to
> review our work before sending it to the mailing list. And yeah, it
> helps find issues before our patches reach the mailing list.
>
> In the same way as we require that patches must pass CI, do we want to
> require that patches "pass" an AI review before they get accepted?
I do not think so. You (figuratively, not limited to Christian
Couder) are welcome to use whatever tool available to you to help
you polish your submission, and the higher quality your patches are
(e.g., fewer typos and jumps in logic flow that interferes the
thought process of human reviewers), the more helpful you are being
to the community. The use of GitHub PR initiated CI run falls into
the same category, I think, in that we do not require you to have an
account and trigger the CI there, but you are doing a good service
if you made sure you caught breakages on macOS you do not have
access to otherwise before sending your patches to the list.
But I do not think we should require you to bring your own token
budget to be able to contribute.
> The benefit would be that it would hopefully catch a lot of trivial
> things like indentation, typos/grammos, etc, and a lot of things a bit
> more difficult to spot like memory issues. Perhaps with some amount of
> prompting/configuration (for example pointing it at our
> CodingGuidelines and SubmittingPatches) it could also catch issues
> like style issues, commits that do too many things, refactoring
> opportunities, etc.
Yes.
Similarly, you are welcome to use tools including AI tools to help
you review others' patches, or help sanity check your reviews of
others' patches before you send them out. The reason why such an
effort is valuable to the community is the same.
But I personally consider that the use of the tools (not limited to
AI tools) is up to each developer. What counts a lot more is the
quality of the output. Just like PR driven CI at GitHub is offered
to everybody who wants to participate and is willing to have an
account there, it may help those aspiring developers if automated
review services are made easily available, but it is a different
story to _require_ use of such service.
^ permalink raw reply
* [PATCH 0/3] midx: honor custom bases for incremental writes
From: Taylor Blau @ 2026-06-12 20:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt,
SZEDER Gábor
SZEDER noticed[1] that t5334 was trying to call `nth_line()`, despite
that helper living only in t5335.
Fixing that should have made the test exercise `git multi-pack-index
write --incremental --base=...`. Instead, it uncovered another wrinkle,
which is that the normal MIDX write path parsed "--bases" without
actually passing it down to the MIDX writer.
This short series fixes both issues. It is structured as follows:
* The first patch moves `nth_line()` to lib-midx.sh so that t5334 and
t5335 use the same helper.
* The second patch threads the parsed `--base` value through
`write_midx_file()`, and consequently marks two t5334 cases as known
breakages.
* The final patch fixes the pack inclusion check and marks the tests
successful again.
The result is that `--base=none` and `--base=<hash>` now correctly
produce detached incremental layers that include any packs above the
selected base, preserving reachability closure for bitmaps.
Thanks in advance for your review!
[1]: https://lore.kernel.org/git/aiuaf3fKJ6kIITrf@szeder.dev/
Taylor Blau (3):
t5334: expose shared `nth_line()` helper
midx: pass custom '--base' through incremental writes
midx-write: include packs above custom incremental base
builtin/multi-pack-index.c | 3 ++-
builtin/repack.c | 2 +-
midx-write.c | 18 +++++++++++++-----
midx.h | 2 +-
t/lib-midx.sh | 6 ++++++
t/t5334-incremental-multi-pack-index.sh | 20 +++++++++++++++++---
t/t5335-compact-multi-pack-index.sh | 7 +------
7 files changed, 41 insertions(+), 17 deletions(-)
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
--
2.55.0.rc0.3.g7bf7c87b605
^ permalink raw reply
* [PATCH 1/3] t5334: expose shared `nth_line()` helper
From: Taylor Blau @ 2026-06-12 20:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt,
SZEDER Gábor
In-Reply-To: <cover.1781294771.git.me@ttaylorr.com>
Since commit 0cd2255e64b (midx: support custom `--base` for incremental
MIDX writes, 2026-05-19), t5334 has referred to a non-existent helper
function 'nth_line', which is defined in t5335, but not here.
Move the helper to lib-midx.sh so that both tests can use the same
implementation. Ensure likewise that `nth_line()` remains visible from
within t5335 by sourcing lib-midx.sh there appropriately.
Curiously, t5334 passes both before and after this change. Before this
change, the failed command substitution leaves '--base' with an empty
value, and after this change, the custom base value is still ignored by
the normal incremental write path. The following commits will explain
and address that behavior.
Noticed-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
t/lib-midx.sh | 6 ++++++
t/t5335-compact-multi-pack-index.sh | 7 +------
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/t/lib-midx.sh b/t/lib-midx.sh
index e38c609604c..b522dbdb0f4 100644
--- a/t/lib-midx.sh
+++ b/t/lib-midx.sh
@@ -34,3 +34,9 @@ compare_results_with_midx () {
midx_git_two_modes "cat-file --batch-all-objects --batch-check --unordered" sorted
'
}
+
+nth_line() {
+ local n="$1"
+ shift
+ awk "NR==$n" "$@"
+}
diff --git a/t/t5335-compact-multi-pack-index.sh b/t/t5335-compact-multi-pack-index.sh
index ec1dafe89fc..6a4b799b9c9 100755
--- a/t/t5335-compact-multi-pack-index.sh
+++ b/t/t5335-compact-multi-pack-index.sh
@@ -3,6 +3,7 @@
test_description='multi-pack-index compaction'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-midx.sh
GIT_TEST_MULTI_PACK_INDEX=0
GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0
@@ -13,12 +14,6 @@ packdir=$objdir/pack
midxdir=$packdir/multi-pack-index.d
midx_chain=$midxdir/multi-pack-index-chain
-nth_line() {
- local n="$1"
- shift
- awk "NR==$n" "$@"
-}
-
write_packs () {
for c in "$@"
do
--
2.55.0.rc0.3.g7bf7c87b605
^ permalink raw reply related
* [PATCH 2/3] midx: pass custom '--base' through incremental writes
From: Taylor Blau @ 2026-06-12 20:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt,
SZEDER Gábor
In-Reply-To: <cover.1781294771.git.me@ttaylorr.com>
The 'multi-pack-index' builtin parses '--base' for incremental writes,
but the normal write path does not pass that value through to
`write_midx_file()`.
As a result, something like:
$ git multi-pack-index write --incremental --base=<base>
behaves as if no custom base had been given (unless the caller used the
'--stdin-packs' path).
Thread the parsed base through `write_midx_file()`, and update the
repack caller to pass NULL for the new argument where no custom base
selection is needed.
This exposes a pre-existing problem in incremental writes with custom
bases: the writer skips packs from the full existing MIDX chain, even
when the caller selected an older base or no base at all.
The affected t5334 cases fail while trying to write MIDX bitmaps. The
detached layer omits packs above the selected base, and thus the
resulting MIDX does not have a reachability closure, making it
impossible to generate reachability bitmaps.
Mark those tests as expected failures accordingly. The following commit
will fix the broken behavior and restore these tests.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/multi-pack-index.c | 3 ++-
builtin/repack.c | 2 +-
midx-write.c | 2 ++
midx.h | 2 +-
t/t5334-incremental-multi-pack-index.sh | 24 +++++++++++++++++++-----
5 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 00ffb36394d..949bfa796b2 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -224,7 +224,8 @@ static int cmd_multi_pack_index_write(int argc, const char **argv,
}
ret = write_midx_file(source, opts.preferred_pack,
- opts.refs_snapshot, opts.flags);
+ opts.refs_snapshot, opts.incremental_base,
+ opts.flags);
free(opts.refs_snapshot);
return ret;
diff --git a/builtin/repack.c b/builtin/repack.c
index 1524a9c13ad..0092a72a996 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -629,7 +629,7 @@ int cmd_repack(int argc,
unsigned flags = 0;
if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL, 0))
flags |= MIDX_WRITE_INCREMENTAL;
- write_midx_file(existing.source, NULL, NULL, flags);
+ write_midx_file(existing.source, NULL, NULL, NULL, flags);
}
cleanup:
diff --git a/midx-write.c b/midx-write.c
index 561e9eedc0e..aa438775ebd 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -1850,12 +1850,14 @@ static int write_midx_internal(struct write_midx_opts *opts)
int write_midx_file(struct odb_source *source,
const char *preferred_pack_name,
const char *refs_snapshot,
+ const char *incremental_base,
unsigned flags)
{
struct write_midx_opts opts = {
.source = source,
.preferred_pack_name = preferred_pack_name,
.refs_snapshot = refs_snapshot,
+ .incremental_base = incremental_base,
.flags = flags,
};
diff --git a/midx.h b/midx.h
index 63853a03a47..92ed29d913d 100644
--- a/midx.h
+++ b/midx.h
@@ -131,7 +131,7 @@ int prepare_multi_pack_index_one(struct odb_source *source);
*/
int write_midx_file(struct odb_source *source,
const char *preferred_pack_name, const char *refs_snapshot,
- unsigned flags);
+ const char *incremental_base, unsigned flags);
int write_midx_file_only(struct odb_source *source,
struct string_list *packs_to_include,
const char *preferred_pack_name,
diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index 68a103d13d2..69e96bf8d93 100755
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -119,7 +119,7 @@ test_expect_success 'write MIDX layer with --base without --no-write-chain-file'
test_grep "cannot use --base without --no-write-chain-file" err
'
-test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file' '
+test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file' '
test_commit base-none &&
git repack -d &&
@@ -128,19 +128,33 @@ test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file
--no-write-chain-file --base=none)" &&
test_cmp "$midx_chain.bak" "$midx_chain" &&
- test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
+ test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
+
+ echo "$layer" >"$midx_chain" &&
+ test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
+ test_grep "^$(git rev-parse 2.2) " midx.objects &&
+ cp "$midx_chain.bak" "$midx_chain"
'
-test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
+test_expect_failure 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
test_commit base-hash &&
git repack -d &&
cp "$midx_chain" "$midx_chain.bak" &&
+ base="$(nth_line 1 "$midx_chain")" &&
layer="$(git multi-pack-index write --bitmap --incremental \
- --no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
+ --no-write-chain-file --base="$base")" &&
test_cmp "$midx_chain.bak" "$midx_chain" &&
- test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
+ test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
+
+ {
+ echo "$base" &&
+ echo "$layer"
+ } >"$midx_chain" &&
+ test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
+ test_grep "^$(git rev-parse 2.2) " midx.objects &&
+ cp "$midx_chain.bak" "$midx_chain"
'
for reuse in false single multi
--
2.55.0.rc0.3.g7bf7c87b605
^ permalink raw reply related
* [PATCH 3/3] midx-write: include packs above custom incremental base
From: Taylor Blau @ 2026-06-12 20:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt,
SZEDER Gábor
In-Reply-To: <cover.1781294771.git.me@ttaylorr.com>
The previous commit made '--base' take effect on the normal incremental
write path, which exposed an existing assumption in our helper function
`should_include_pack()`, which is that any pack already present in
`ctx->m` was skipped.
That is only correct for non-incremental writes. For incremental writes,
`ctx->base_midx` is the boundary that should be excluded from the new
layer. If the caller selects an older base, or no base at all, then
packs from layers above that base have to be included in the detached
layer so that its bitmap has reachability closure.
Teach `should_include_pack()` to choose the MIDX used for pack exclusion
based on whether or not we are performing an incremental write. When
doing so, use `ctx->base_midx`, and use `ctx->m` otherwise.
The t5334 cases from the previous commit can now be marked as
successful.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
midx-write.c | 16 +++++++++++-----
t/t5334-incremental-multi-pack-index.sh | 4 ++--
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/midx-write.c b/midx-write.c
index aa438775ebd..c50fdb5c6d1 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -133,8 +133,17 @@ static uint32_t midx_pack_perm(struct write_midx_context *ctx,
static int should_include_pack(const struct write_midx_context *ctx,
const char *file_name)
{
+ struct multi_pack_index *m = ctx->m;
/*
- * Note that at most one of ctx->m and ctx->to_include are set,
+ * When writing incrementally, ctx->m may contain layers above
+ * the selected base MIDX, which must be included in the new
+ * layer.
+ */
+ if (ctx->incremental)
+ m = ctx->base_midx;
+
+ /*
+ * Note that at most one of m and ctx->to_include are set,
* so we are testing midx_contains_pack() and
* string_list_has_string() independently (guarded by the
* appropriate NULL checks).
@@ -148,10 +157,7 @@ static int should_include_pack(const struct write_midx_context *ctx,
* should be performed independently (likely checking
* to_include before the existing MIDX).
*/
- if (ctx->m && midx_contains_pack(ctx->m, file_name))
- return 0;
- else if (ctx->base_midx && midx_contains_pack(ctx->base_midx,
- file_name))
+ if (m && midx_contains_pack(m, file_name))
return 0;
else if (ctx->to_include &&
!string_list_has_string(ctx->to_include, file_name))
diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index 69e96bf8d93..84ff6120978 100755
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -119,7 +119,7 @@ test_expect_success 'write MIDX layer with --base without --no-write-chain-file'
test_grep "cannot use --base without --no-write-chain-file" err
'
-test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file' '
+test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file' '
test_commit base-none &&
git repack -d &&
@@ -136,7 +136,7 @@ test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file
cp "$midx_chain.bak" "$midx_chain"
'
-test_expect_failure 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
+test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
test_commit base-hash &&
git repack -d &&
--
2.55.0.rc0.3.g7bf7c87b605
^ permalink raw reply related
* Re: [PATCH v4 06/16] midx: support custom `--base` for incremental MIDX writes
From: Taylor Blau @ 2026-06-12 20:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: SZEDER Gábor, git, Jeff King, Elijah Newren,
Patrick Steinhardt
In-Reply-To: <aixNXOxfPZnAVLgK@nand.local>
On Fri, Jun 12, 2026 at 02:18:04PM -0400, Taylor Blau wrote:
> On Fri, Jun 12, 2026 at 06:21:48AM -0700, Junio C Hamano wrote:
> > SZEDER Gábor <szeder.dev@gmail.com> writes:
> >
> > >> + layer="$(git multi-pack-index write --bitmap --incremental \
> > >> + --no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
> > >
> > > There is no 'nth_line' helper function in this test script.
> >
> > Good eyes. It has been there in the file next door t5335 since
> > February, but not available here in t5334.
>
> Good spotting indeed. Fortunately or unfortunately for us, pulling on
> this thread revealed a bit of a rabbit hole. Patches forthcoming..
https://lore.kernel.org/git/cover.1781294771.git.me@ttaylorr.com/
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v2] log: improve --follow following renames for non-linear history
From: Junio C Hamano @ 2026-06-12 20:10 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jeff King, git
In-Reply-To: <aipTOsH8LKTSwglj@collabora.com>
Miklos Vajna <vmiklos@collabora.com> writes:
> Documentation/config/log.adoc | 3 +-
> log-tree.c | 133 ++++++++++++++++++++++++++++++++++
> log-tree.h | 1 +
> revision.c | 2 +
> revision.h | 4 +
> t/meson.build | 1 +
> t/t4218-log-follow-merge.sh | 119 ++++++++++++++++++++++++++++++
t4218 seems to be taken by another topic in-flight, so this needs
renumbering.
^ permalink raw reply
* Re: [PATCH v3] ref-filter: restore prefix-scoped iteration
From: Tamir Duberstein @ 2026-06-12 21:24 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, ZheNing Hu
In-Reply-To: <aivx-7VOKE_TC50R@pks.im>
On Fri, Jun 12, 2026 at 7:48 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Wed, Jun 10, 2026 at 05:29:49AM -0700, Tamir Duberstein wrote:
> > dabecb9db2 (for-each-ref: introduce a '--start-after' option,
> > 2025-07-15) changed branch, remote-tracking branch, and tag enumeration
> > from constructing an iterator with the namespace prefix to constructing
> > an unscoped iterator and seeking to the prefix.
> >
> > The files backend constructs its loose-ref iterator with cache priming
> > enabled. cache_ref_iterator_begin() immediately applies the construction
> > prefix through cache_ref_iterator_set_prefix(), reading loose refs
> > beneath it before packed refs are opened. An empty prefix therefore
> > reads every loose ref, and a later seek cannot undo that I/O.
> >
> > For these single-kind filters, construct the iterator with the namespace
> > prefix when start_after is not set. Keep the existing unscoped
> > construction for start_after, whose seek position may differ from the
> > namespace prefix.
> >
> > With 10,000 unrelated loose refs, the p6300 tests improve as follows:
> >
> > before after
> > branch 2.74 s 0.11 s
> > branch --remotes 2.81 s 0.12 s
> > tag 3.01 s 0.11 s
> >
> > Link: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
> > Link: https://lore.kernel.org/git/xmqqikjq7s16.fsf@gitster.g/
> > Link: https://lore.kernel.org/r/CAOLa=ZRHKNNymXGk31YgECjUmF9nZ8GsPUdQb7aKBH5DKMz7=w@mail.gmail.com
>
> I honestly have no idea what you want to say with these links, as they
> seem to just link to random reviews mails when the above mentioned
> commit was reviewed. In general, we typically try to embed references
> like this into the explanation, like:
>
> In [1], we discussed... and this is relevant because of ...
>
> [1]: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
>
> Just dropping the links as-is without much of an explanation isn't
> helpful.
Will be numbered references in next spin.
>
> > diff --git a/ref-filter.c b/ref-filter.c
> > index 1da4c0e60d..9b04e3af85 100644
> > --- a/ref-filter.c
> > +++ b/ref-filter.c
> > @@ -3316,15 +3316,14 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
> >
> > if (prefix) {
> > struct ref_iterator *iter;
> > + struct ref_store *store = get_main_ref_store(the_repository);
> >
> > - iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
> > - "", NULL, 0, 0);
> > -
> > - if (filter->start_after)
> > + if (filter->start_after) {
> > + iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
> > ret = start_ref_iterator_after(iter, filter->start_after);
> > - else
> > - ret = ref_iterator_seek(iter, prefix,
> > - REF_ITERATOR_SEEK_SET_PREFIX);
> > + } else {
> > + iter = refs_ref_iterator_begin(store, prefix, NULL, 0, 0);
> > + }
> >
> > if (!ret)
> > ret = do_for_each_ref_iterator(iter, fn, cb_data);
>
> The patch itself seems sensible to me.
>
> > diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh
> > index fa7289c752..ed9c1c6a19 100755
> > --- a/t/perf/p6300-for-each-ref.sh
> > +++ b/t/perf/p6300-for-each-ref.sh
> > @@ -1,6 +1,6 @@
> > #!/bin/sh
> >
> > -test_description='performance of for-each-ref'
> > +test_description='performance of ref-filter users'
> > . ./perf-lib.sh
> >
> > test_perf_fresh_repo
> > @@ -84,4 +84,41 @@ test_expect_success 'pack refs' '
> > '
> > run_tests "packed"
> >
> > +test_expect_success REFFILES 'setup many unrelated loose refs' '
> > + git init scoped &&
> > + test_commit -C scoped --no-tag base &&
> > + test_seq $ref_count_per_type |
> > + sed "s,.*,update refs/custom/unrelated_& HEAD," |
> > + git -C scoped update-ref --stdin &&
> > + git -C scoped update-ref refs/remotes/origin/main HEAD &&
> > + git -C scoped update-ref refs/tags/only HEAD
> > +'
>
> I've already called this out before on other patches, but the REFFILES
> prerequisite just doesn't make any sense here as this test logic is
> generic.
You're right. Removed in v4.
^ permalink raw reply
* Re: [PATCH v3 1/3] commit-reach: handle cycles in contains walk
From: Tamir Duberstein @ 2026-06-12 21:26 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: git, Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren
In-Reply-To: <CAL71e4PRqN9iPCzvgwC1Vtj-kzn4Udv+v1LTFSUXtGnC5KGrpA@mail.gmail.com>
On Fri, Jun 12, 2026 at 2:53 AM Kristofer Karlsson <krka@spotify.com> wrote:
>
> On Fri, 12 Jun 2026 at 05:00, Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > The memoized contains traversal used by git tag assumes that commit
> > ancestry is acyclic. Replacement refs can violate that assumption,
> > causing it to keep pushing an already active commit until memory is
> > exhausted.
> >
>
> The cycle detection itself makes sense, but would it be simpler to
> just die() when a cycle is found rather than falling back to a
> second reachability walk?
>
> A cycle in the commit graph means replacement refs are
> misconfigured. The existing code already loops forever when it
> hits one, so detecting and dying is strictly an improvement. The
> fallback adds a second codepath through the function, discards all
> cached results (so later candidates redo work), and papers over
> what is really a broken invariant.
>
> do_lookup_replace_object() already dies when replacement refs
> chain deeper than MAXREPLACEDEPTH (which covers cycles), so the
> existing contract treats this as a fatal configuration error.
> parse_commit_or_die() sets the same precedent within the walk
> itself.
Yes. The test creates an ancestry cycle through replacement commit
parents, so MAXREPLACEDEPTH does not catch this particular cycle. But I
agree with the design conclusion: the history is malformed and the
fallback only adds complexity.
Done in v4.
Thanks!
^ permalink raw reply
* [PATCH v4] ref-filter: restore prefix-scoped iteration
From: Tamir Duberstein @ 2026-06-12 21:27 UTC (permalink / raw)
To: git
Cc: Karthik Nayak, Patrick Steinhardt, Junio C Hamano, Victoria Dye,
ZheNing Hu, Tamir Duberstein
In-Reply-To: <20260610-fix-git-branch-regression-v3-1-6fd48fad7a53@gmail.com>
dabecb9db2 (for-each-ref: introduce a '--start-after' option,
2025-07-15) changed branch, remote-tracking branch, and tag enumeration
from constructing an iterator with the namespace prefix to constructing
an unscoped iterator and seeking to the prefix.
Review of --start-after noted that the construction prefix and seek
position represent different state and are easy to conflate [1]. It also
noted that future branch or tag support would need to retain the
namespace prefix while moving the cursor [2].
The files backend constructs its loose-ref iterator with cache priming
enabled. cache_ref_iterator_begin() immediately applies the construction
prefix through cache_ref_iterator_set_prefix(), reading loose refs
beneath it before packed refs are opened. An empty prefix therefore
reads every loose ref, and a later seek cannot undo that I/O.
For the current single-kind filters, construct the iterator with the
namespace prefix when start_after is not set. Leave the existing
start_after path unchanged; no current command combines it with these
filters, and future support must carry the prefix separately from the
cursor.
With 10,000 unrelated loose refs in the files backend, the p6300 tests
improve as follows:
before after
branch 2.74 s 0.11 s
branch --remotes 2.81 s 0.12 s
tag 3.01 s 0.11 s
[1] https://lore.kernel.org/r/aGZidwwlToWThkn8@pks.im/
[2] https://lore.kernel.org/r/xmqqikjq7s16.fsf@gitster.g/
Fixes: dabecb9db2b2 ("for-each-ref: introduce a '--start-after' option")
Suggested-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
The series is based on a89346e34a (maint) because the regression has
been present in released versions since Git 2.51.0.
---
Changes in v4:
- Explain the historical references in the commit message.
- Run the new performance cases with both ref backends.
- Drop the Assisted-by trailer.
- Link to v3: https://patch.msgid.link/20260610-fix-git-branch-regression-v3-1-6fd48fad7a53@gmail.com
Changes in v3:
- Construct the iterator directly with the namespace prefix.
- Explain when the files backend primes its loose-ref cache.
- Condense the commit message and performance results.
- Link to v2: https://patch.msgid.link/20260608-fix-git-branch-regression-v2-1-fd82075a8520@gmail.com
Changes in v2:
- Extract local variable `store`.
- Link to v1: https://patch.msgid.link/20260605-fix-git-branch-regression-v1-1-02f40ad40929@gmail.com
---
ref-filter.c | 13 ++++++-------
t/perf/p6300-for-each-ref.sh | 39 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1da4c0e60d..9b04e3af85 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3316,15 +3316,14 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
if (prefix) {
struct ref_iterator *iter;
+ struct ref_store *store = get_main_ref_store(the_repository);
- iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
- "", NULL, 0, 0);
-
- if (filter->start_after)
+ if (filter->start_after) {
+ iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
ret = start_ref_iterator_after(iter, filter->start_after);
- else
- ret = ref_iterator_seek(iter, prefix,
- REF_ITERATOR_SEEK_SET_PREFIX);
+ } else {
+ iter = refs_ref_iterator_begin(store, prefix, NULL, 0, 0);
+ }
if (!ret)
ret = do_for_each_ref_iterator(iter, fn, cb_data);
diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh
index fa7289c752..25ffa5e84c 100755
--- a/t/perf/p6300-for-each-ref.sh
+++ b/t/perf/p6300-for-each-ref.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='performance of for-each-ref'
+test_description='performance of ref-filter users'
. ./perf-lib.sh
test_perf_fresh_repo
@@ -84,4 +84,41 @@ test_expect_success 'pack refs' '
'
run_tests "packed"
+test_expect_success 'setup many unrelated refs' '
+ git init scoped &&
+ test_commit -C scoped --no-tag base &&
+ test_seq $ref_count_per_type |
+ sed "s,.*,update refs/custom/unrelated_& HEAD," |
+ git -C scoped update-ref --stdin &&
+ git -C scoped update-ref refs/remotes/origin/main HEAD &&
+ git -C scoped update-ref refs/tags/only HEAD
+'
+
+test_perf "branch (many unrelated refs)" "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git branch --format='%(refname)' >/dev/null
+ done
+ )
+"
+
+test_perf "branch --remotes (many unrelated refs)" "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git branch --remotes --format='%(refname)' >/dev/null
+ done
+ )
+"
+
+test_perf "tag (many unrelated refs)" "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git tag --format='%(refname)' >/dev/null
+ done
+ )
+"
+
test_done
---
base-commit: a89346e34a937f001e5d397ee62224e3e9852040
change-id: 20260605-fix-git-branch-regression-9e4236f18091
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply related
* [PATCH v4 0/3] Reuse --contains traversal results
From: Tamir Duberstein @ 2026-06-12 21:49 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260611-ref-filter-memoized-contains-v3-0-b26af3dba285@gmail.com>
git tag uses a memoized traversal for --contains, while git branch
and git for-each-ref repeat a reachability walk for each ref. Reuse
the memoized traversal when generation numbers can bound the walk.
The first patch makes the memoized traversal reject cyclic replacement
histories. The last makes the non-memoized path report reachability
errors.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v4:
- Die on cyclic ancestry instead of retrying another reachability walk.
- Update the cycle test and credit Kristofer Karlsson.
- Remove unexplained links to review messages.
- Link to v3: https://patch.msgid.link/20260611-ref-filter-memoized-contains-v3-0-b26af3dba285@gmail.com
Changes in v3:
- Split missing-ancestor error handling into its own patch.
- Use die() for reachability errors, remove redundant cache setup, and
chain cycle-test cleanup.
- Drop the unrelated empty-target-list behavior change.
- Explain why git tag retains memoization without generation numbers.
- Add p1500 coverage for all three frontends and a shared-history
case.
- Remove correctness checks from p1500 and drop output hashes.
- Link to v2: https://patch.msgid.link/20260608-ref-filter-memoized-contains-v2-0-e72720344a7c@gmail.com
Changes in v2:
- Split cycle handling into a preparatory patch.
- Exercise cycle handling through the existing git tag path.
- Move perf result verification out of setup.
- Link to v1: https://patch.msgid.link/20260607-ref-filter-memoized-contains-v1-1-a1972dde9c76@gmail.com
---
Tamir Duberstein (3):
commit-reach: reject cycles in contains walk
ref-filter: memoize --contains with generations
commit-reach: die on contains walk errors
commit-reach.c | 23 ++++++++++++++++++-----
commit-reach.h | 3 ++-
t/perf/p1500-graph-walks.sh | 28 +++++++++++++++++++++++++++-
t/t6301-for-each-ref-errors.sh | 22 ++++++++++++++++++++++
t/t7004-tag.sh | 18 ++++++++++++++++++
5 files changed, 87 insertions(+), 7 deletions(-)
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260607-ref-filter-memoized-contains-7cb6b3bccad1
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply
* [PATCH v4 1/3] commit-reach: reject cycles in contains walk
From: Tamir Duberstein @ 2026-06-12 21:49 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260612-ref-filter-memoized-contains-v4-0-5ed39fd001dd@gmail.com>
The memoized contains traversal used by git tag assumes that commit
ancestry is acyclic. Replacement refs can violate that assumption,
causing it to keep pushing an already active commit until memory is
exhausted.
Mark commits while they are active and die if the traversal encounters
an active commit. Other failures in this walk already die through
parse_commit_or_die(); using a second reachability walk would only add
a separate policy for malformed history.
Suggested-by: Kristofer Karlsson <krka@spotify.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 12 +++++++++---
commit-reach.h | 3 ++-
t/t7004-tag.sh | 18 ++++++++++++++++++
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 9b3ea46d6f..e1bedc596d 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -708,7 +708,8 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)
/*
* Test whether the candidate is contained in the list.
- * Do not recurse to find out, though, but return -1 if inconclusive.
+ * Do not recurse to find out, though, but return CONTAINS_UNKNOWN if
+ * inconclusive.
*/
static enum contains_result contains_test(struct commit *candidate,
const struct commit_list *want,
@@ -765,6 +766,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
if (result != CONTAINS_UNKNOWN)
return result;
+ *contains_cache_at(cache, candidate) = CONTAINS_IN_PROGRESS;
push_to_contains_stack(candidate, &contains_stack);
while (contains_stack.nr) {
struct contains_stack_entry *entry = &contains_stack.contains_stack[contains_stack.nr - 1];
@@ -776,8 +778,8 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
contains_stack.nr--;
}
/*
- * If we just popped the stack, parents->item has been marked,
- * therefore contains_test will return a meaningful yes/no.
+ * A parent may have just been popped and marked, or may still
+ * be active when replacement refs create a cycle.
*/
else switch (contains_test(parents->item, want, cache, cutoff)) {
case CONTAINS_YES:
@@ -787,7 +789,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
case CONTAINS_NO:
entry->parents = parents->next;
break;
+ case CONTAINS_IN_PROGRESS:
+ die(_("commit ancestry contains a cycle"));
case CONTAINS_UNKNOWN:
+ *contains_cache_at(cache, parents->item) =
+ CONTAINS_IN_PROGRESS;
push_to_contains_stack(parents->item, &contains_stack);
break;
}
diff --git a/commit-reach.h b/commit-reach.h
index 3f3a563d8a..f908d305b1 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -73,7 +73,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid);
enum contains_result {
CONTAINS_UNKNOWN = 0,
CONTAINS_NO,
- CONTAINS_YES
+ CONTAINS_YES,
+ CONTAINS_IN_PROGRESS
};
define_commit_slab(contains_cache, enum contains_result);
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index d918005dd9..67309494d2 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1611,6 +1611,24 @@ test_expect_success 'checking that first commit is in all tags (hash)' '
test_cmp expected actual
'
+test_expect_success 'tag --contains rejects cyclic replacement histories' '
+ first=$(git rev-parse HEAD~2) &&
+ second=$(git rev-parse HEAD~) &&
+ third=$(git rev-parse HEAD) &&
+ test_when_finished "
+ git replace -d $first &&
+ git replace -d $third &&
+ git tag -d cycle-a cycle-b
+ " &&
+ git tag cycle-a "$first" &&
+ git tag cycle-b "$third" &&
+ git replace --graft "$first" "$third" "$second" &&
+ git replace --graft "$third" "$first" &&
+ test_must_fail git tag --contains="$second" --list "cycle-*" \
+ >/dev/null 2>err &&
+ test_grep "fatal: commit ancestry contains a cycle" err
+'
+
# other ways of specifying the commit
test_expect_success 'checking that first commit is in all tags (tag)' '
cat >expected <<-\EOF &&
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH v4 2/3] ref-filter: memoize --contains with generations
From: Tamir Duberstein @ 2026-06-12 21:49 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260612-ref-filter-memoized-contains-v4-0-5ed39fd001dd@gmail.com>
git branch and git for-each-ref run a separate reachability walk for
each ref considered by --contains and --no-contains. Refs with shared
history therefore traverse the same commits repeatedly.
git tag instead uses a depth-first walk that caches results across
refs. That walk can perform poorly without generation numbers: a
negative check may walk to the root instead of stopping at a nearby
divergence. Generation numbers let it stop below the oldest target.
Use the memoized walk for all ref-filter callers when generation
numbers are available. Keep git tag on its existing path without
generations. Caching still helps when many tags share deep history:
ffc4b8012d (tag: speed up --contains calculation, 2011-06-11) reduced
git tag --contains HEAD~200 in linux-2.6 from 15.417 to 5.329 seconds.
The new shared-history perf test improves from 0.72 to 0.03 seconds. In
a repository with 62,174 remote-tracking refs, running:
git branch -r --contains c78ae85f3ce7e
improves from 104.365 seconds to 468 milliseconds.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 3 ++-
t/perf/p1500-graph-walks.sh | 28 +++++++++++++++++++++++++++-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index e1bedc596d..18fcd69113 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -805,7 +805,8 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
int commit_contains(struct ref_filter *filter, struct commit *commit,
struct commit_list *list, struct contains_cache *cache)
{
- if (filter->with_commit_tag_algo)
+ if (filter->with_commit_tag_algo ||
+ generation_numbers_enabled(the_repository))
return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
return repo_is_descendant_of(the_repository, commit, list);
}
diff --git a/t/perf/p1500-graph-walks.sh b/t/perf/p1500-graph-walks.sh
index 5b23ce5db9..d167b4f7e1 100755
--- a/t/perf/p1500-graph-walks.sh
+++ b/t/perf/p1500-graph-walks.sh
@@ -32,7 +32,16 @@ test_expect_success 'setup' '
echo "X:$line" >>test-tool-tags || return 1
done &&
- commit=$(git commit-tree $(git rev-parse HEAD^{tree})) &&
+ git rev-list --first-parent --max-count=8192 HEAD >contains-commits &&
+ test_file_not_empty contains-commits &&
+ git update-ref refs/contains-perf-base "$(tail -n 1 contains-commits)" &&
+ awk "{
+ printf \"update refs/contains-perf/%04d %s\\n\", NR, \$1
+ }" contains-commits |
+ git update-ref --stdin &&
+ git pack-refs --include "refs/contains-perf/*" &&
+
+ commit=$(git commit-tree HEAD^{tree}) &&
git update-ref refs/heads/disjoint-base $commit &&
git commit-graph write --reachable
@@ -62,6 +71,23 @@ test_perf 'contains: git tag --merged' '
xargs git tag --merged=HEAD <tags
'
+test_perf 'contains: git for-each-ref' '
+ git for-each-ref --contains=refs/contains-perf-base --stdin <refs
+'
+
+test_perf 'contains: git branch' '
+ xargs git branch --contains=refs/contains-perf-base <branches
+'
+
+test_perf 'contains: git tag' '
+ xargs git tag --contains=refs/contains-perf-base <tags
+'
+
+test_perf 'contains: synthetic shared history' '
+ git for-each-ref --contains=refs/contains-perf-base \
+ refs/contains-perf/ >/dev/null
+'
+
test_perf 'is-base check: test-tool reach (refs)' '
test-tool reach get_branch_base_for_tip <test-tool-refs
'
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH v4 3/3] commit-reach: die on contains walk errors
From: Tamir Duberstein @ 2026-06-12 21:49 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260612-ref-filter-memoized-contains-v4-0-5ed39fd001dd@gmail.com>
Without generation numbers, repo_is_descendant_of() can return -1 when
it cannot read commit ancestry. commit_contains() exposes that result
through a Boolean interface, so ref-filter treats it as true. This can
include a ref for --contains or exclude it for --no-contains without
failing the command.
Die when repo_is_descendant_of() reports an error. The memoized walk
already dies when it cannot parse a commit, so callers of the
non-memoized path no longer turn a failed walk into a match.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 8 +++++++-
t/t6301-for-each-ref-errors.sh | 22 ++++++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/commit-reach.c b/commit-reach.c
index 18fcd69113..37b66b6b21 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -805,10 +805,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
int commit_contains(struct ref_filter *filter, struct commit *commit,
struct commit_list *list, struct contains_cache *cache)
{
+ int result;
+
if (filter->with_commit_tag_algo ||
generation_numbers_enabled(the_repository))
return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
- return repo_is_descendant_of(the_repository, commit, list);
+
+ result = repo_is_descendant_of(the_repository, commit, list);
+ if (result < 0)
+ die(_("failed to check reachability"));
+ return result;
}
int can_all_from_reach_with_flag(struct object_array *from,
diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh
index e06feb06e9..72b27c8be3 100755
--- a/t/t6301-for-each-ref-errors.sh
+++ b/t/t6301-for-each-ref-errors.sh
@@ -52,6 +52,28 @@ test_expect_success 'Missing objects are reported correctly' '
test_must_be_empty brief-err
'
+test_expect_success 'missing ancestors are reported by contains filters' '
+ test_when_finished "git update-ref -d refs/heads/missing-parent" &&
+ {
+ echo "tree $(git rev-parse HEAD^{tree})" &&
+ echo "parent $MISSING" &&
+ git cat-file commit HEAD |
+ sed -n -e "/^author /p" -e "/^committer /p" &&
+ echo &&
+ echo "missing parent"
+ } >commit &&
+ broken=$(git hash-object -t commit -w commit) &&
+ git update-ref refs/heads/missing-parent "$broken" &&
+ for option in --contains --no-contains
+ do
+ test_must_fail git for-each-ref "$option=HEAD" \
+ refs/heads/missing-parent >out 2>err &&
+ test_must_be_empty out &&
+ test_grep "parse commit $MISSING" err ||
+ return 1
+ done
+'
+
test_expect_success 'ahead-behind requires an argument' '
test_must_fail git for-each-ref \
--format="%(ahead-behind)" 2>err &&
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH] clone: accept DEPTH env var as fallback for --depth
From: h8d13 via GitGitGadget @ 2026-06-13 1:39 UTC (permalink / raw)
To: git; +Cc: h8d13, h8d13
From: h8d13 <hadean-eon-dev@proton.me>
When git clone is run by a tool the user does not control directly
(CI runners, package build scripts such as makepkg, or any wrapper
that spawns nested clones), there is no way to request a shallow
clone: --depth only exists as a command-line option on the process
that invokes git clone, and unlike url.*.insteadOf there is no
configuration key that could be injected via GIT_CONFIG_* to achieve
the same effect.
Teach git clone to read a DEPTH environment variable when --depth is
not given on the command line. Since environment variables propagate
to child processes, exporting DEPTH=1 once makes every nested clone
underneath shallow, which is useful in CI pipelines and recursive
build tools. An explicit --depth on the command line still takes
precedence, and the value goes through the existing validation, so a
non-positive DEPTH dies with the same error as a non-positive
--depth.
Signed-off-by: h8d13 <hadean-eon-dev@proton.me>
---
clone: accept DEPTH env var as fallback for --depth
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2333%2Fh8d13%2Fdepth-env-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2333/h8d13/depth-env-v1
Pull-Request: https://github.com/git/git/pull/2333
builtin/clone.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/builtin/clone.c b/builtin/clone.c
index d60d1b60bc..549506f672 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1022,6 +1022,12 @@ int cmd_clone(int argc,
usage_msg_opt(_("You must specify a repository to clone."),
builtin_clone_usage, builtin_clone_options);
+ if (!option_depth) {
+ const char *env_depth = getenv("DEPTH");
+ if (env_depth && *env_depth)
+ option_depth = xstrdup(env_depth);
+ }
+
if (option_depth || option_since || option_not.nr)
deepen = 1;
if (option_single_branch == -1)
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
--
gitgitgadget
^ permalink raw reply related
* RE: [ANNOUNCE] Git v2.55.0-rc0
From: rsbecker @ 2026-06-13 1:56 UTC (permalink / raw)
To: 'Junio C Hamano'; +Cc: git
In-Reply-To: <xmqqzf0zhjuq.fsf@gitster.g>
On June 12, 2026 11:14 AM, Junio C Hamano wrote:
> <rsbecker@nexbridge.com> writes:
>
> > On June 11, 2026 11:32 AM, Junio wrote:
> >> An early preview release Git v2.55.0-rc0 is now available for testing
> >> at the usual places. It is comprised of 397 non-merge commits since
> >> v2.54.0, contributed by
> >> 70 people, 22 of which are new faces [*].
> >
> > Cargo is not available everywhere. Build is not possible on NonStop.
> >
> > cargo build --release
> > /usr/coreutils/bin/bash: cargo: command not found
> > Makefile:3021: recipe for target 'target/release/libgitcore.a' failed
> > make: *** [target/release/libgitcore.a] Error 127
> >
> > Is there a way around this?
>
> I see this in the Makefile that you may or may not have read. Does it
work?
>
> # Define NO_RUST if you want to disable features and subsystems written in
Rust #
> from being compiled into Git. For now, Rust is still an optional feature
of # the build
> process. With Git 3.0 though, Rust will always be enabled.
Thanks. Adding NO_RUST=yes to the command line for make worked properly.
^ permalink raw reply
* Re: [PATCH 6/7] fetch: add configuration option fetch.followRemoteHEAD
From: Matt Hunter @ 2026-06-13 2:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqik7nj11i.fsf@gitster.g>
On Fri Jun 12, 2026 at 10:17 AM EDT, Junio C Hamano wrote:
>
> By the way, do not call a "configuration variable" a "configuration option".
> Let's keep the vocabulary forcused without using random synonyms.
Noted. I can appreciate that the term "option" may be better reserved
for describing command-line options, to avoid confusion.
Is it safe to assume "setting" may be an appropriate alternative to
"configuration variable" in some contexts?
>
> I think these uses of strcasecmp() are unnecessary and actively
> harms end-user experience. This is especially true because the
> value given to remote.<name>.followRemoteHEAD is case sensitive.
>
> [...]
>
> Admittedly values to some existing configuration variables may be
> parsed case insensitively but we should aim to fix the mistake in
> the longer term, and we should certainly not add more of them.
Thanks for clarifying the correct form here. The use of strcasecmp()
was largely to match surrounding context as I assumed it would meet most
people's expectations.
I think a detail like this can be especially confusing since it seems
like the parsing for config variable **names** generally is
case-insensitive.
>
> Is it sensible to die() here? If you are fetching from somewhere
> without keeping a set of remote-tracking branches for it (i.e., a
> single shot "git fetch https://github.com/gitster/git master"), you
> do not care what garbage value is in fetch.followRemoteHEAD.
> Perhaps the version of Git that is slightly newer than the version
> that ships with this patch defined new valid values that this patch
> does not know about, and such a user who is doing a single-shot
> fetch may have that setting to help them working with their usual
> non-single shot repositories, but they use a newer version of Git
> for such regular work, and they are using slightly old version of
> Git to perform this single-shot fetch. The point is that the
> configured value will *NOT* be used for such a user, and dying only
> because this piece of code does not understand the configuration that
> will not be used is of dubious value.
Very good point about forward compatibility. Agreed that die() is the
wrong call here.
The most sensible thing is probably to leave fetch.followRemoteHEAD
UNCONFIGURED if the value is unrecognized, so we fall back to the
"create" behavior unless the remote in question defines its own
followRemoteHEAD policy.
Will incorporate each of these in the next round, thanks!
^ permalink raw reply
* Re: [PATCH v4 0/2] graph: indent visual roots in graph
From: Junio C Hamano @ 2026-06-13 3:01 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, ayu.chandekar, chandrapratap3519, christian.couder, jltobler,
karthik.188, peff, phillip.wood, siddharthasthana31
In-Reply-To: <20260612-ps-pre-commit-indent-v4-0-e8492037ebae@gmail.com>
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> When rendering a graph, if the history contains multiple "visual roots",
> actual roots or commits that look like roots (i.e. have their parents
> filtered out) can end up being vertically adjacent to unrelated commits,
> falsely appearing to be related.
> ...
> [1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
>
> V3 DIFF:
>
> - Completly changes the approach to indent the visual roots instead of the
> commits after the visual roots.
>
> Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
> ---
> Pablo Sabater (2):
> lib-log-graph: move check_graph function
> graph: indent visual root in graph
The new tests added here does not seem to play well with
linux-TEST-vars CI job when merged to 'seen' or 'jch' with other
topics in flight.
https://github.com/git/git/actions/runs/27445164550/job/81128391150#step:10:1779
https://github.com/git/git/actions/runs/27445164037/job/81128386244#step:10:1778
I suspect that these tests are failing the same way even standalone.
https://github.com/git/git/actions/runs/27447146727/job/81134594264#step:9:2136
^ permalink raw reply
* [PATCH v2 0/6] t: add greplint.pl and convert grep to test_grep
From: Michael Montalbo via GitGitGadget @ 2026-06-13 4:06 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Eric Sunshine, Michael Montalbo
In-Reply-To: <pull.2135.git.1780559158.gitgitgadget@gmail.com>
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 v1:
* Dropped lint-style.pl and the --fix mode concept. Replaced with
greplint.pl, which more closely follows chainlint's conventions, and
reuses its parser logic via a shared module.
* A regex approach to grep linting was prototyped in an attempt to reduce
the number of patches in the series, but this approach produced false
positives from grep inside heredoc bodies (e.g. write_script) and
cross-line pipelines where the pipe or redirect is on a different line
from the grep. The shared module's Lexer already collapses these into
single tokens, giving zero false positives with less code than the regex
heuristics would need, which is why it was retained in the current
version.
* Reverted incorrect conversions where grep was used as a data filter
inside redirected compound commands, not as a test assertion.
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. The
former is ambiguous because >/dev/null becomes dead code under test_grep
(which already suppresses matching-line output). The latter requires
removing the redirect and passing the file as a positional argument,
since test_grep does not support stdin redirects. Both are left as bare
grep. A follow-up series can address these once a convention is
established.
Michael Montalbo (6):
t/README: document test_grep helper
t: fix grep assertions missing file arguments
t: extract chainlint's parser into shared module
t: fix Lexer line count for $() inside double-quoted strings
t: convert grep assertions to test_grep
t: add greplint to detect bare grep assertions
t/.gitattributes | 2 +
t/Makefile | 29 +-
t/README | 21 +
t/chainlint.pl | 529 +----------------
t/for-each-ref-tests.sh | 12 +-
t/greplint-cat.pl | 27 +
t/greplint.pl | 241 ++++++++
t/greplint/bare-grep-after-and.expect | 1 +
t/greplint/bare-grep-after-and.test | 4 +
t/greplint/bare-grep-after-semicolon.expect | 1 +
t/greplint/bare-grep-after-semicolon.test | 4 +
t/greplint/bare-grep-compound-body.expect | 3 +
t/greplint/bare-grep-compound-body.test | 17 +
t/greplint/bare-grep-count-mode.expect | 1 +
t/greplint/bare-grep-count-mode.test | 3 +
t/greplint/bare-grep-explicit-pattern.expect | 1 +
t/greplint/bare-grep-explicit-pattern.test | 3 +
t/greplint/bare-grep-flags.expect | 1 +
t/greplint/bare-grep-flags.test | 3 +
t/greplint/bare-grep-lint-ok.expect | 0
t/greplint/bare-grep-lint-ok.test | 4 +
t/greplint/bare-grep-negated.expect | 1 +
t/greplint/bare-grep-negated.test | 3 +
t/greplint/bare-grep-pattern-file.expect | 1 +
t/greplint/bare-grep-pattern-file.test | 3 +
t/greplint/bare-grep-simple.expect | 1 +
t/greplint/bare-grep-simple.test | 3 +
t/greplint/bare-grep-subshell.expect | 1 +
t/greplint/bare-grep-subshell.test | 5 +
.../dqstring-continuation-offset.expect | 1 +
t/greplint/dqstring-continuation-offset.test | 11 +
t/greplint/filter-command-substitution.expect | 0
t/greplint/filter-command-substitution.test | 3 +
t/greplint/filter-pipe-input.expect | 0
t/greplint/filter-pipe-input.test | 3 +
t/greplint/filter-pipe-output.expect | 0
t/greplint/filter-pipe-output.test | 3 +
t/greplint/filter-redirect-output.expect | 0
t/greplint/filter-redirect-output.test | 3 +
t/greplint/filter-stdin-redirect.expect | 0
t/greplint/filter-stdin-redirect.test | 3 +
t/greplint/grep-as-argument.expect | 0
t/greplint/grep-as-argument.test | 3 +
t/greplint/grep-as-value.expect | 0
t/greplint/grep-as-value.test | 6 +
t/greplint/wrong-negation.expect | 1 +
t/greplint/wrong-negation.test | 3 +
t/lib-bitmap.sh | 12 +-
t/lib-bundle-uri-protocol.sh | 26 +-
t/lib-httpd.sh | 2 +-
t/lib-shell-parser.pl | 534 ++++++++++++++++++
t/pack-refs-tests.sh | 2 +-
t/show-ref-exists-tests.sh | 2 +-
t/t0000-basic.sh | 16 +-
t/t0001-init.sh | 18 +-
t/t0008-ignores.sh | 8 +-
t/t0009-git-dir-validation.sh | 6 +-
t/t0012-help.sh | 4 +-
t/t0013-sha1dc.sh | 2 +-
t/t0017-env-helper.sh | 4 +-
t/t0021-conversion.sh | 18 +-
t/t0029-core-unsetenvvars.sh | 4 +-
t/t0030-stripspace.sh | 4 +-
t/t0031-lockfile-pid.sh | 2 +-
t/t0040-parse-options.sh | 52 +-
t/t0041-usage.sh | 2 +-
t/t0052-simple-ipc.sh | 10 +-
t/t0061-run-command.sh | 2 +-
t/t0066-dir-iterator.sh | 2 +-
t/t0068-for-each-repo.sh | 16 +-
t/t0070-fundamental.sh | 6 +-
t/t0081-find-pack.sh | 12 +-
t/t0091-bugreport.sh | 18 +-
t/t0092-diagnose.sh | 12 +-
t/t0100-previous.sh | 2 +-
t/t0200-gettext-basic.sh | 14 +-
t/t0203-gettext-setlocale-sanity.sh | 4 +-
t/t0204-gettext-reencode-sanity.sh | 8 +-
t/t0210-trace2-normal.sh | 6 +-
t/t0211-trace2-perf.sh | 80 +--
t/t0212-trace2-event.sh | 8 +-
t/t0300-credentials.sh | 4 +-
t/t0410-partial-clone.sh | 82 +--
t/t0450-txt-doc-vs-help.sh | 2 +-
t/t0500-progress-display.sh | 18 +-
t/t0610-reftable-basics.sh | 8 +-
t/t1004-read-tree-m-u-wf.sh | 8 +-
t/t1006-cat-file.sh | 18 +-
t/t1007-hash-object.sh | 8 +-
t/t1011-read-tree-sparse-checkout.sh | 10 +-
t/t1050-large.sh | 6 +-
t/t1091-sparse-checkout-builtin.sh | 24 +-
t/t1092-sparse-checkout-compatibility.sh | 44 +-
t/t1300-config.sh | 16 +-
t/t1305-config-include.sh | 2 +-
t/t1308-config-set.sh | 6 +-
t/t1400-update-ref.sh | 170 +++---
t/t1403-show-ref.sh | 18 +-
t/t1410-reflog.sh | 4 +-
t/t1415-worktree-refs.sh | 4 +-
t/t1430-bad-ref-name.sh | 56 +-
t/t1450-fsck.sh | 12 +-
t/t1451-fsck-buffer.sh | 6 +-
t/t1460-refs-migrate.sh | 2 +-
t/t1500-rev-parse.sh | 6 +-
t/t1502-rev-parse-parseopt.sh | 2 +-
t/t1503-rev-parse-verify.sh | 10 +-
t/t1510-repo-setup.sh | 10 +-
t/t1512-rev-parse-disambiguation.sh | 4 +-
t/t1515-rev-parse-outside-repo.sh | 2 +-
t/t1800-hook.sh | 18 +-
t/t2004-checkout-cache-temp.sh | 4 +-
t/t2019-checkout-ambiguous-ref.sh | 4 +-
t/t2024-checkout-dwim.sh | 8 +-
t/t2030-unresolve-info.sh | 6 +-
t/t2060-switch.sh | 6 +-
t/t2070-restore.sh | 2 +-
t/t2080-parallel-checkout-basics.sh | 14 +-
t/t2081-parallel-checkout-collisions.sh | 24 +-
t/t2082-parallel-checkout-attributes.sh | 12 +-
t/t2103-update-index-ignore-missing.sh | 6 +-
t/t2200-add-update.sh | 2 +-
t/t2203-add-intent.sh | 6 +-
t/t2400-worktree-add.sh | 24 +-
t/t2402-worktree-list.sh | 16 +-
t/t2403-worktree-move.sh | 6 +-
t/t2405-worktree-submodule.sh | 6 +-
t/t2407-worktree-heads.sh | 26 +-
t/t2500-untracked-overwriting.sh | 8 +-
t/t2501-cwd-empty.sh | 4 +-
t/t3001-ls-files-others-exclude.sh | 6 +-
t/t3007-ls-files-recurse-submodules.sh | 6 +-
t/t3200-branch.sh | 12 +-
t/t3202-show-branch.sh | 10 +-
t/t3203-branch-output.sh | 4 +-
t/t3206-range-diff.sh | 78 +--
t/t3207-branch-submodule.sh | 4 +-
t/t3301-notes.sh | 32 +-
t/t3310-notes-merge-manual-resolve.sh | 16 +-
t/t3320-notes-merge-worktrees.sh | 2 +-
t/t3400-rebase.sh | 16 +-
t/t3402-rebase-merge.sh | 16 +-
t/t3404-rebase-interactive.sh | 72 +--
t/t3406-rebase-message.sh | 6 +-
t/t3415-rebase-autosquash.sh | 10 +-
t/t3416-rebase-onto-threedots.sh | 4 +-
t/t3418-rebase-continue.sh | 10 +-
t/t3420-rebase-autostash.sh | 26 +-
t/t3422-rebase-incompatible-options.sh | 4 +-
t/t3429-rebase-edit-todo.sh | 2 +-
t/t3430-rebase-merges.sh | 32 +-
t/t3500-cherry.sh | 4 +-
t/t3501-revert-cherry-pick.sh | 6 +-
t/t3504-cherry-pick-rerere.sh | 6 +-
t/t3510-cherry-pick-sequence.sh | 24 +-
t/t3602-rm-sparse-checkout.sh | 4 +-
t/t3705-add-sparse-checkout.sh | 10 +-
t/t3800-mktag.sh | 4 +-
t/t3901-i18n-patch.sh | 16 +-
t/t3903-stash.sh | 28 +-
t/t3904-stash-patch.sh | 4 +-
t/t3908-stash-in-worktree.sh | 2 +-
t/t4000-diff-format.sh | 2 +-
t/t4001-diff-rename.sh | 4 +-
t/t4011-diff-symlink.sh | 2 +-
t/t4013-diff-various.sh | 2 +-
t/t4014-format-patch.sh | 344 +++++------
t/t4015-diff-whitespace.sh | 16 +-
t/t4017-diff-retval.sh | 2 +-
t/t4018-diff-funcname.sh | 2 +-
t/t4019-diff-wserror.sh | 8 +-
t/t4020-diff-external.sh | 18 +-
t/t4021-format-patch-numbered.sh | 4 +-
t/t4022-diff-rewrite.sh | 14 +-
t/t4028-format-patch-mime-headers.sh | 6 +-
t/t4031-diff-rewrite-binary.sh | 18 +-
t/t4033-diff-patience.sh | 2 +-
t/t4036-format-patch-signer-mime.sh | 6 +-
t/t4038-diff-combined.sh | 6 +-
t/t4051-diff-function-context.sh | 38 +-
t/t4053-diff-no-index.sh | 4 +-
t/t4063-diff-blobs.sh | 2 +-
t/t4065-diff-anchored.sh | 26 +-
t/t4067-diff-partial-clone.sh | 12 +-
t/t4073-diff-stat-name-width.sh | 24 +-
t/t4103-apply-binary.sh | 2 +-
t/t4120-apply-popt.sh | 2 +-
t/t4124-apply-ws-rule.sh | 10 +-
t/t4128-apply-root.sh | 2 +-
t/t4140-apply-ita.sh | 4 +-
t/t4141-apply-too-large.sh | 2 +-
t/t4150-am.sh | 48 +-
t/t4200-rerere.sh | 6 +-
t/t4201-shortlog.sh | 2 +-
t/t4202-log.sh | 84 +--
t/t4204-patch-id.sh | 2 +-
t/t4205-log-pretty-formats.sh | 2 +-
t/t4209-log-pickaxe.sh | 10 +-
t/t4211-line-log.sh | 72 +--
t/t4216-log-bloom.sh | 18 +-
t/t4252-am-options.sh | 22 +-
t/t4254-am-corrupt.sh | 6 +-
t/t4258-am-quoted-cr.sh | 2 +-
t/t4301-merge-tree-write-tree.sh | 18 +-
t/t5000-tar-tree.sh | 10 +-
t/t5004-archive-corner-cases.sh | 2 +-
t/t5100-mailinfo.sh | 2 +-
t/t5150-request-pull.sh | 18 +-
t/t5300-pack-object.sh | 22 +-
t/t5302-pack-index.sh | 6 +-
t/t5304-prune.sh | 8 +-
t/t5310-pack-bitmaps.sh | 14 +-
t/t5317-pack-objects-filter-objects.sh | 12 +-
t/t5318-commit-graph.sh | 8 +-
t/t5319-multi-pack-index.sh | 16 +-
t/t5324-split-commit-graph.sh | 10 +-
t/t5325-reverse-index.sh | 2 +-
t/t5326-multi-pack-bitmaps.sh | 28 +-
t/t5328-commit-graph-64bit-time.sh | 2 +-
t/t5329-pack-objects-cruft.sh | 8 +-
t/t5334-incremental-multi-pack-index.sh | 2 +-
t/t5335-compact-multi-pack-index.sh | 4 +-
t/t5351-unpack-large-objects.sh | 2 +-
t/t5402-post-merge-hook.sh | 4 +-
t/t5403-post-checkout-hook.sh | 2 +-
t/t5404-tracking-branches.sh | 2 +-
t/t5406-remote-rejects.sh | 2 +-
t/t5407-post-rewrite-hook.sh | 8 +-
t/t5409-colorize-remote-messages.sh | 36 +-
t/t5500-fetch-pack.sh | 38 +-
t/t5504-fetch-receive-strict.sh | 14 +-
t/t5505-remote.sh | 20 +-
t/t5510-fetch.sh | 10 +-
t/t5512-ls-remote.sh | 8 +-
t/t5514-fetch-multiple.sh | 2 +-
t/t5516-fetch-push.sh | 20 +-
t/t5520-pull.sh | 4 +-
t/t5524-pull-msg.sh | 6 +-
t/t5526-fetch-submodules.sh | 16 +-
t/t5529-push-errors.sh | 4 +-
t/t5530-upload-pack-error.sh | 18 +-
t/t5531-deep-submodule-push.sh | 2 +-
t/t5532-fetch-proxy.sh | 2 +-
t/t5533-push-cas.sh | 12 +-
t/t5534-push-signed.sh | 22 +-
t/t5537-fetch-shallow.sh | 2 +-
t/t5538-push-shallow.sh | 2 +-
t/t5539-fetch-http-shallow.sh | 4 +-
t/t5541-http-push-smart.sh | 32 +-
t/t5544-pack-objects-hook.sh | 12 +-
t/t5550-http-fetch-dumb.sh | 4 +-
t/t5551-http-fetch-smart.sh | 46 +-
t/t5552-skipping-fetch-negotiator.sh | 6 +-
t/t5554-noop-fetch-negotiator.sh | 4 +-
t/t5557-http-get.sh | 2 +-
t/t5558-clone-bundle-uri.sh | 38 +-
t/t5562-http-backend-content-length.sh | 2 +-
t/t5564-http-proxy.sh | 10 +-
t/t5581-http-curl-verbose.sh | 2 +-
t/t5583-push-branches.sh | 8 +-
t/t5601-clone.sh | 28 +-
t/t5604-clone-reference.sh | 8 +-
t/t5605-clone-local.sh | 2 +-
t/t5606-clone-options.sh | 6 +-
t/t5612-clone-refspec.sh | 2 +-
t/t5616-partial-clone.sh | 60 +-
t/t5619-clone-local-ambiguous-transport.sh | 2 +-
t/t5620-backfill.sh | 12 +-
t/t5700-protocol-v1.sh | 46 +-
t/t5701-git-serve.sh | 14 +-
t/t5702-protocol-v2.sh | 154 ++---
t/t5703-upload-pack-ref-in-want.sh | 22 +-
t/t5705-session-id-in-capabilities.sh | 12 +-
t/t5750-bundle-uri-parse.sh | 8 +-
t/t5801-remote-helpers.sh | 4 +-
t/t5810-proto-disable-local.sh | 2 +-
t/t5813-proto-disable-ssh.sh | 4 +-
t/t6000-rev-list-misc.sh | 26 +-
t/t6005-rev-list-count.sh | 8 +-
t/t6006-rev-list-format.sh | 4 +-
t/t6009-rev-list-parent.sh | 4 +-
t/t6020-bundle-misc.sh | 12 +-
t/t6022-rev-list-missing.sh | 4 +-
t/t6030-bisect-porcelain.sh | 150 ++---
t/t6040-tracking-info.sh | 2 +-
t/t6112-rev-list-filters-objects.sh | 24 +-
t/t6115-rev-list-du.sh | 4 +-
t/t6120-describe.sh | 14 +-
t/t6200-fmt-merge-msg.sh | 82 +--
t/t6402-merge-rename.sh | 4 +-
t/t6403-merge-file.sh | 6 +-
t/t6404-recursive-merge.sh | 2 +-
t/t6406-merge-attr.sh | 20 +-
t/t6417-merge-ours-theirs.sh | 30 +-
t/t6418-merge-text-auto.sh | 2 +-
t/t6422-merge-rename-corner-cases.sh | 8 +-
t/t6423-merge-rename-directories.sh | 72 +--
t/t6424-merge-unrelated-index-changes.sh | 6 +-
t/t6427-diff3-conflict-markers.sh | 10 +-
t/t6432-merge-recursive-space-options.sh | 4 +-
t/t6436-merge-overwrite.sh | 6 +-
t/t6437-submodule-merge.sh | 10 +-
t/t6500-gc.sh | 8 +-
t/t6600-test-reach.sh | 4 +-
t/t7001-mv.sh | 16 +-
t/t7002-mv-sparse-checkout.sh | 38 +-
t/t7003-filter-branch.sh | 16 +-
t/t7004-tag.sh | 2 +-
t/t7006-pager.sh | 16 +-
t/t7012-skip-worktree-writing.sh | 6 +-
t/t7030-verify-tag.sh | 52 +-
t/t7031-verify-tag-signed-ssh.sh | 46 +-
t/t7102-reset.sh | 2 +-
t/t7110-reset-merge.sh | 40 +-
t/t7201-co.sh | 6 +-
t/t7300-clean.sh | 2 +-
t/t7301-clean-interactive.sh | 2 +-
t/t7400-submodule-basic.sh | 32 +-
t/t7402-submodule-rebase.sh | 2 +-
t/t7406-submodule-update.sh | 26 +-
t/t7416-submodule-dash-url.sh | 20 +-
t/t7417-submodule-path-url.sh | 2 +-
t/t7450-bad-git-dotfiles.sh | 14 +-
t/t7501-commit-basic-functionality.sh | 16 +-
t/t7502-commit-porcelain.sh | 2 +-
t/t7507-commit-verbose.sh | 6 +-
t/t7508-status.sh | 6 +-
t/t7510-signed-commit.sh | 68 +--
t/t7516-commit-races.sh | 4 +-
t/t7519-status-fsmonitor.sh | 14 +-
t/t7527-builtin-fsmonitor.sh | 82 +--
t/t7528-signed-commit-ssh.sh | 68 +--
t/t7600-merge.sh | 10 +-
t/t7603-merge-reduce-heads.sh | 20 +-
t/t7606-merge-custom.sh | 2 +-
t/t7607-merge-state.sh | 4 +-
t/t7610-mergetool.sh | 18 +-
t/t7700-repack.sh | 14 +-
t/t7703-repack-geometric.sh | 4 +-
t/t7704-repack-cruft.sh | 12 +-
t/t7800-difftool.sh | 26 +-
t/t7810-grep.sh | 22 +-
t/t7814-grep-recurse-submodules.sh | 2 +-
t/t7900-maintenance.sh | 34 +-
t/t8008-blame-formats.sh | 2 +-
t/t8010-cat-file-filters.sh | 2 +-
t/t8012-blame-colors.sh | 2 +-
t/t9001-send-email.sh | 190 +++----
t/t9003-help-autocorrect.sh | 6 +-
t/t9106-git-svn-commit-diff-clobber.sh | 2 +-
t/t9107-git-svn-migrate.sh | 30 +-
t/t9110-git-svn-use-svm-props.sh | 20 +-
t/t9111-git-svn-use-svnsync-props.sh | 18 +-
t/t9114-git-svn-dcommit-merge.sh | 6 +-
t/t9116-git-svn-log.sh | 8 +-
t/t9117-git-svn-init-clone.sh | 12 +-
t/t9119-git-svn-info.sh | 16 +-
t/t9122-git-svn-author.sh | 8 +-
t/t9130-git-svn-authors-file.sh | 8 +-
t/t9138-git-svn-authors-prog.sh | 14 +-
t/t9140-git-svn-reset.sh | 4 +-
t/t9153-git-svn-rewrite-uuid.sh | 4 +-
t/t9200-git-cvsexportcommit.sh | 2 +-
t/t9210-scalar.sh | 34 +-
t/t9211-scalar-clone.sh | 16 +-
t/t9300-fast-import.sh | 10 +-
t/t9350-fast-export.sh | 54 +-
t/t9351-fast-export-anonymize.sh | 36 +-
t/t9400-git-cvsserver-server.sh | 4 +-
t/t9501-gitweb-standalone-http-status.sh | 58 +-
t/t9502-gitweb-standalone-parse-output.sh | 38 +-
t/t9800-git-p4-basic.sh | 10 +-
t/t9801-git-p4-branch.sh | 48 +-
t/t9806-git-p4-options.sh | 10 +-
t/t9807-git-p4-submit.sh | 2 +-
t/t9810-git-p4-rcs.sh | 8 +-
t/t9813-git-p4-preserve-users.sh | 8 +-
t/t9814-git-p4-rename.sh | 8 +-
t/t9827-git-p4-change-filetype.sh | 4 +-
t/t9832-unshelve.sh | 6 +-
t/t9833-errors.sh | 4 +-
t/t9835-git-p4-metadata-encoding-python2.sh | 36 +-
t/t9836-git-p4-metadata-encoding-python3.sh | 38 +-
t/t9850-shell.sh | 2 +-
t/t9902-completion.sh | 26 +-
385 files changed, 3772 insertions(+), 3329 deletions(-)
create mode 100644 t/greplint-cat.pl
create mode 100644 t/greplint.pl
create mode 100644 t/greplint/bare-grep-after-and.expect
create mode 100644 t/greplint/bare-grep-after-and.test
create mode 100644 t/greplint/bare-grep-after-semicolon.expect
create mode 100644 t/greplint/bare-grep-after-semicolon.test
create mode 100644 t/greplint/bare-grep-compound-body.expect
create mode 100644 t/greplint/bare-grep-compound-body.test
create mode 100644 t/greplint/bare-grep-count-mode.expect
create mode 100644 t/greplint/bare-grep-count-mode.test
create mode 100644 t/greplint/bare-grep-explicit-pattern.expect
create mode 100644 t/greplint/bare-grep-explicit-pattern.test
create mode 100644 t/greplint/bare-grep-flags.expect
create mode 100644 t/greplint/bare-grep-flags.test
create mode 100644 t/greplint/bare-grep-lint-ok.expect
create mode 100644 t/greplint/bare-grep-lint-ok.test
create mode 100644 t/greplint/bare-grep-negated.expect
create mode 100644 t/greplint/bare-grep-negated.test
create mode 100644 t/greplint/bare-grep-pattern-file.expect
create mode 100644 t/greplint/bare-grep-pattern-file.test
create mode 100644 t/greplint/bare-grep-simple.expect
create mode 100644 t/greplint/bare-grep-simple.test
create mode 100644 t/greplint/bare-grep-subshell.expect
create mode 100644 t/greplint/bare-grep-subshell.test
create mode 100644 t/greplint/dqstring-continuation-offset.expect
create mode 100644 t/greplint/dqstring-continuation-offset.test
create mode 100644 t/greplint/filter-command-substitution.expect
create mode 100644 t/greplint/filter-command-substitution.test
create mode 100644 t/greplint/filter-pipe-input.expect
create mode 100644 t/greplint/filter-pipe-input.test
create mode 100644 t/greplint/filter-pipe-output.expect
create mode 100644 t/greplint/filter-pipe-output.test
create mode 100644 t/greplint/filter-redirect-output.expect
create mode 100644 t/greplint/filter-redirect-output.test
create mode 100644 t/greplint/filter-stdin-redirect.expect
create mode 100644 t/greplint/filter-stdin-redirect.test
create mode 100644 t/greplint/grep-as-argument.expect
create mode 100644 t/greplint/grep-as-argument.test
create mode 100644 t/greplint/grep-as-value.expect
create mode 100644 t/greplint/grep-as-value.test
create mode 100644 t/greplint/wrong-negation.expect
create mode 100644 t/greplint/wrong-negation.test
create mode 100644 t/lib-shell-parser.pl
base-commit: 1ff279f3404a482a83fb04c7457e41ab26884aea
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2135%2Fmmontalbo%2Fmm%2Ftest-grep-docs-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2135/mmontalbo/mm/test-grep-docs-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2135
Range-diff vs v1:
1: 43402040bf = 1: 5959cab258 t/README: document test_grep helper
5: c0db9fdb5b = 2: f3e8e19e6e t: fix grep assertions missing file arguments
2: a1069efa8f ! 3: 460461b5fe t: extract chainlint's parser into shared module
@@ Metadata
## Commit message ##
t: extract chainlint's parser into shared module
- Move the Lexer, ShellParser, and ScriptParser packages from
- chainlint.pl into t/lib-shell-parser.pl so they can be reused by
- other tools. ScriptParser's check_test() is a no-op in the shared
- module; callers subclass ScriptParser and override it.
+ 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.
- chainlint.pl defines TestParser (&&-chain detection) and
- ChainlintParser (a ScriptParser subclass whose check_test runs
- TestParser and formats the results). The shared module is loaded
- via do() for portability with minimal Perl installations.
+ ScriptParser's check_test() becomes a no-op in the shared module.
+ chainlint.pl defines ChainlintParser (extending ScriptParser)
+ with the &&-chain check_test() implementation.
- A subsequent commit introduces lint-style.pl which needs the same
- shell parser to properly tokenize test scripts. Sharing the parser
- avoids reimplementing heredoc handling, $(...) nesting, pipe
- tracking, quoting, and test body extraction.
+ No functional change: chainlint produces the same output and
+ check-chainlint self-tests pass.
Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
@@ t/chainlint.pl: my $jobs = -1;
- return @tokens;
-}
+use File::Basename;
-+my $_lib = dirname($0) . "/lib-shell-parser.pl";
-+$_lib = "./$_lib" unless $_lib =~ m{^/};
-+do $_lib or die "failed to load $_lib: $@$!\n";
++do(dirname($0) . "/lib-shell-parser.pl")
++ or die "$0: failed to load lib-shell-parser.pl: $@$!\n";
# TestParser is a subclass of ShellParser which, beyond parsing shell script
# code, is also imbued with semantic knowledge of test construction, and checks
@@ t/chainlint.pl: DONE:
+ # the tests themselves or in behaviors being exercised by the tests. As such,
+ # TestParser is only called upon to parse test bodies, not the top-level
# scripts in which the tests are defined.
++
package TestParser;
-use base 'ShellParser';
@@ t/chainlint.pl: DONE:
-# at the top-level of test scripts but also within compound commands such as
-# loops and function definitions.
-package ScriptParser;
-+# ChainlintParser is a subclass of ScriptParser which checks each test
-+# body for broken &&-chains via TestParser, then formats and collects
-+# the results.
-+package ChainlintParser;
-
+-
-use base 'ShellParser';
-+our @ISA = ('ScriptParser');
-
- sub new {
- my $class = shift @_;
-@@ t/chainlint.pl: sub new {
- return $self;
- }
+-
+-sub new {
+- my $class = shift @_;
+- my $self = $class->SUPER::new(@_);
+- $self->{ntests} = 0;
+- $self->{nerrs} = 0;
+- return $self;
+-}
++# ChainlintParser extends ScriptParser with &&-chain checking
++package ChainlintParser;
-# extract the raw content of a token, which may be a single string or a
-# composition of multiple strings and non-string character runs; for instance,
@@ t/chainlint.pl: sub new {
- }
- return $s
-}
--
++our @ISA = ('ScriptParser');
+
sub format_problem {
local $_ = shift;
- /^AMP$/ && return "missing '&&'";
@@ t/chainlint.pl: sub format_problem {
sub check_test {
@@ t/chainlint.pl: sub check_test {
-}
-
# main contains high-level functionality for processing command-line switches,
- # feeding input test scripts to ScriptParser, and reporting results.
+-# feeding input test scripts to ScriptParser, and reporting results.
++# feeding input test scripts to ChainlintParser, and reporting results.
package main;
+
+ my $getnow = sub { return time(); };
@@ t/chainlint.pl: sub check_script {
}
my $s = do { local $/; <$fh> };
@@ t/chainlint.pl: sub check_script {
## t/lib-shell-parser.pl (new) ##
@@
++# Copyright (c) 2021-2022 Eric Sunshine <sunshine@sunshineco.com>
++#
++# Shared shell script parser for test lint tools. Provides Lexer,
++# ShellParser, and ScriptParser. Subclass ScriptParser and override
++# check_test() to implement lint checks.
++
+use strict;
+use warnings;
+
-+# Copyright (c) 2021-2022 Eric Sunshine <sunshine@sunshineco.com>
-+#
+# Lexer tokenizes POSIX shell scripts. It is roughly modeled after section 2.3
+# "Token Recognition" of POSIX chapter 2 "Shell Command Language". Although
+# similar to lexical analyzers for other languages, this one differs in a few
@@ t/lib-shell-parser.pl (new)
+# ShellParser parses POSIX shell scripts (with minor extensions for Bash). It
+# is a recursive descent parser very roughly modeled after section 2.10 "Shell
+# Grammar" of POSIX chapter 2 "Shell Command Language".
++
+package ShellParser;
+
+sub new {
@@ t/lib-shell-parser.pl (new)
+}
+
+# ScriptParser is a subclass of ShellParser which identifies individual test
-+# definitions within test scripts and calls check_test() for each test body
-+# found. Callers subclass ScriptParser and override check_test() to
-+# implement specific checks (e.g. chainlint checks &&-chains, lint-style
-+# checks grep usage).
++# definitions within test scripts and passes each test body to check_test().
++# ScriptParser detects test definitions not only at the top-level of test
++# scripts but also within compound commands such as loops and function
++# definitions.
++
+package ScriptParser;
+
+our @ISA = ('ShellParser');
+
++sub new {
++ my $class = shift @_;
++ my $self = $class->SUPER::new(@_);
++ $self->{ntests} = 0;
++ $self->{nerrs} = 0;
++ return $self;
++}
++
+# extract the raw content of a token, which may be a single string or a
+# composition of multiple strings and non-string character runs; for instance,
+# `"test body"` unwraps to `test body`; `word"a b"42'c d'` to `worda b42c d`
@@ t/lib-shell-parser.pl (new)
+}
+
+sub check_test {
-+ # no-op; subclasses override to implement specific checks
++ # no-op; subclass and override to implement lint checks
+}
+
+sub parse_cmd {
3: 93c2b29683 ! 4: c1b86748d1 t: fix Lexer line count for $() inside double-quoted strings
@@ Commit message
scan_dqstring's post-loop newline counter re-counts newlines that
were already counted during recursive parsing of $() bodies. This
- happens because scan_dollar's returned text can contain newlines
- (from token text of multi-line strings and from \n command separator
- tokens), and the catch-all counter at the end of scan_dqstring
- counts all of them again.
+ 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 \<newline> handler, and $() newlines by
+ line splices at the backslash handler, and $() newlines by
scan_token during the recursive parse.
- This does not affect chainlint's output because chainlint annotates
- the original body text using byte offsets, not token line numbers.
- It does matter for tools like lint-style.pl (introduced in a
- subsequent commit) that use token line numbers to locate and fix
- specific lines in the original file.
-
- Add check-shell-parser.pl to verify that the Lexer reports correct
- line numbers after multi-line $() in double-quoted strings.
+ 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>
- ## t/Makefile ##
-@@ t/Makefile: check-meson:
- test-lint: test-lint-duplicates test-lint-executable \
- test-lint-filenames
- ifneq ($(PERL_PATH),)
--test-lint: test-lint-shell-syntax
-+test-lint: test-lint-shell-syntax check-shell-parser
- else
- GIT_TEST_CHAIN_LINT = 0
- endif
-@@ t/Makefile: test-lint-executable:
- test-lint-shell-syntax:
- @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
-
-+check-shell-parser:
-+ @'$(PERL_PATH_SQ)' check-shell-parser.pl
- test-lint-filenames:
- @# We do *not* pass a glob to ls-files but use grep instead, to catch
- @# non-ASCII characters (which are quoted within double-quotes)
-@@ t/Makefile: perf:
- $(MAKE) -C perf/ all
-
- .PHONY: pre-clean $(T) aggregate-results clean valgrind perf \
-- check-chainlint clean-chainlint test-chainlint $(UNIT_TESTS)
-+ check-chainlint clean-chainlint test-chainlint \
-+ check-shell-parser $(UNIT_TESTS)
-
- .PHONY: libgit-sys-test libgit-rs-test
- libgit-sys-test:
-
- ## t/check-shell-parser.pl (new) ##
-@@
-+#!/usr/bin/perl
-+
-+# Tests for the shared shell parser (lib-shell-parser.pl).
-+
-+use strict;
-+use warnings;
-+use File::Basename;
-+
-+my $_lib = dirname($0) . "/lib-shell-parser.pl";
-+$_lib = "./$_lib" unless $_lib =~ m{^/};
-+do $_lib or die "$0: failed to load $_lib: $@$!\n";
-+
-+my $rc = 0;
-+
-+sub check {
-+ my ($desc, $body, $want_token, $want_line) = @_;
-+ my $parser = ShellParser->new(\$body);
-+ my @tokens = $parser->parse();
-+ for my $t (reverse @tokens) {
-+ next unless $t->[0] eq $want_token && defined $t->[3];
-+ if ($t->[3] != $want_line) {
-+ print STDERR "FAIL: $desc: " .
-+ "'$want_token' at line $t->[3], " .
-+ "expected line $want_line\n";
-+ $rc = 1;
-+ }
-+ return;
-+ }
-+ print STDERR "FAIL: $desc: token '$want_token' not found\n";
-+ $rc = 1;
-+}
-+
-+# Multi-line $() inside a dq-string: MARKER should be at line 3.
-+check('dq-string with multi-line $()', <<'BODY', 'MARKER', 3);
-+ x="$(echo one
-+ echo two)" &&
-+ MARKER here
-+BODY
-+
-+# Two multi-line $() substitutions: verifies drift does not accumulate.
-+# MARKER should be at line 5.
-+check('two dq-string $()', <<'BODY', 'MARKER', 5);
-+ x="$(echo a
-+ b)" &&
-+ y="$(echo c
-+ d)" &&
-+ MARKER here
-+BODY
-+
-+# $() outside a dq-string: no double-counting either way.
-+# MARKER should be at line 3.
-+check('bare $() spanning lines', <<'BODY', 'MARKER', 3);
-+ x=$(echo one
-+ echo two) &&
-+ MARKER here
-+BODY
-+
-+exit $rc;
-
## t/lib-shell-parser.pl ##
@@ t/lib-shell-parser.pl: sub scan_dqstring {
my $b = $self->{buff};
@@ t/lib-shell-parser.pl: sub scan_dqstring {
while (1) {
- # slurp up non-special characters
- $s .= $1 if $$b =~ /\G([^"\$\\]+)/gc;
-+ # slurp up non-special characters; count newlines
-+ # inline so we don't need a catch-all counter that
-+ # would miscount newlines from recursive $() parsing
++ # Slurp non-special characters; count newlines here because
++ # newlines inside $() are already counted by the recursive parse.
+ if ($$b =~ /\G([^"\$\\]+)/gc) {
-+ my $chunk = $1;
-+ $self->{lineno} += () = $chunk =~ /\n/sg;
-+ $s .= $chunk;
++ $s .= $1;
++ $self->{lineno} += $1 =~ tr/\n//;
+ }
# handle special characters
last unless $$b =~ /\G(.)/sgc;
6: 1527293f1c ! 5: 3a589ef738 t: lint and convert grep assertions to test_grep
@@ Metadata
Author: Michael Montalbo <mmontalbo@gmail.com>
## Commit message ##
- t: lint and convert grep assertions to test_grep
-
- Extend lint-style.pl with a rule that detects bare 'grep' used as a
- test assertion and converts it to test_grep. test_grep prints the
- file contents on failure, making test debugging significantly easier.
-
- parse_commands() is extended to split at shell structural tokens
- ({, }, (, ), |) and keywords (if, then, for, etc.), and each
- command gains a token_pos index so that rules can scan backward and
- forward in the token stream for context.
-
- Three new functions implement the grep-assertion rule:
-
- - is_filter_context() scans the surrounding tokens for pipes,
- control-flow keywords (if/elif/while/until), for-in value
- lists, and brace groups with output redirects.
-
- - is_grep_assertion() classifies a grep command: convertible
- assertion (pattern and file present), filter (not an assertion),
- or missing file argument (flagged as a likely bug).
-
- - check_bare_grep() ties them together and calls
- report_violation() with the appropriate fix.
-
- The --fix mode handles:
- - Replacing 'grep' with 'test_grep'
- - Moving negation from '! grep' to 'test_grep !'
- - Stripping the -q flag (test_grep inherently checks match status)
-
- Five files require '# lint-ok' annotations for intentional grep
- usage that cannot be mechanically converted: t1400 (packed-refs
- may not exist on reftable), t3901 (piped stdin via case block),
- t6437 (glob argument breaks test_grep's test -f check), t7450
- (file may not exist after failed MINGW clone), and t7527 ($?
- capture on the next line).
-
- The test-lint-style scope is extended to include sourced test
- fragments in subdirectories (t5411/*.sh and similar) via a new
- TSOURCED variable.
-
- Run '--fix' to convert all ~2800 grep assertions across ~340 files
- in the test suite. test-lib-functions.sh and lib-rebase.sh are
- excluded from linting since they implement test infrastructure
- rather than test assertions.
+ t: convert grep assertions to test_grep
+
+ 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.
+
+ 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
+ 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
+ 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
+
+ # 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
+
+ # Step 3: convert '! grep' -> 'test_grep !'
+ perl t/greplint.pl t/*.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 |
+ 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>
- ## t/Makefile ##
-@@ t/Makefile: test-lint-shell-syntax:
- check-shell-parser:
- @'$(PERL_PATH_SQ)' check-shell-parser.pl
-
-+TSOURCED = $(sort $(wildcard t[0-9]*/*.sh))
-+
- test-lint-style:
-- @'$(PERL_PATH_SQ)' lint-style.pl $(T) $(THELPERS) $(TPERF)
-+ @'$(PERL_PATH_SQ)' lint-style.pl $(T) $(THELPERS) $(TPERF) $(TSOURCED)
-
- check-lint-style:
- @rc=0; for t in $(LINT_STYLE_TESTS); do \
-
## t/for-each-ref-tests.sh ##
@@ t/for-each-ref-tests.sh: test_expect_success 'Verify descending sort' '
@@ t/lib-httpd.sh: test_http_push_nonff () {
test_expect_success 'non-fast-forward push shows help message' '
- ## t/lint-style.pl ##
-@@
- # Detection uses parsed tokens from the shared shell parser for
- # correct handling of heredocs, $(...), pipes, and quoting.
- # Fixes modify the original file text to preserve formatting.
-+#
-+# Architecture: the harness (LintParser, parse_commands) tokenizes
-+# test bodies and splits them into commands. Rules are independent
-+# functions that examine each command and its surrounding token
-+# context to decide if there is a violation.
-
- use strict;
- use warnings;
-@@ t/lint-style.pl: do $_lib or die "$0: failed to load $_lib: $@$!\n";
- # on each test body. Per-file state (file name, raw lines, dirty
- # flag) is stored on the instance before calling parse().
- #
--# Subroutines defined below (parse_commands, check_test_grep_negation,
--# etc.) are in package main and called with the main:: prefix.
--# File-scoped lexicals ($fix_mode, $has_fixable, etc.) are visible
--# across packages since 'package' does not introduce a new scope.
-+# Subroutines defined below are in package main and called with
-+# the main:: prefix. File-scoped lexicals ($fix_mode, etc.) are
-+# visible across packages since 'package' does not introduce a
-+# new scope.
- package LintParser;
- our @ISA = ('ScriptParser');
-
-@@ t/lint-style.pl: package main;
- my $exit_code = 0;
- my $has_fixable = 0;
-
-+my %skip_file = map { $_ => 1 }
-+ grep { m{(?:test-lib-functions|lib-rebase)\.sh$} } @ARGV;
-+
- sub err {
- my ($file, $lineno, $line, $msg, %opts) = @_;
- $line =~ s/^\s+//;
-@@ t/lint-style.pl: sub err {
- $exit_code = 1 unless $fix_mode && $opts{fixable};
- }
-
--# Report a lint violation found by a rule. In --fix mode, apply
--# the regex substitution on the raw line and report success.
--# Otherwise just report. Returns 1 if the line was modified.
-+# Report a lint violation. In --fix mode, apply the regex
-+# substitution on the raw line. Returns 1 if modified.
- sub report_violation {
- my ($file, $cmd, $line_ref, $match, $fix, $from) = @_;
- my $lineno = $cmd->{lineno};
-@@ t/lint-style.pl: sub report_violation {
- return 0;
- }
-
-+# --- Harness: tokenize and split into commands ---
-+#
- # Split a token stream into commands at &&, ||, ;;, and \n.
-+# Each command is {tokens => [...], lineno => N, token_pos => I}
-+# where token_pos is the index in @all_tokens where the command's
-+# first token appeared (so rules can look backward for context).
- sub parse_commands {
-- my ($content) = @_;
-- my $parser = ShellParser->new(\$content);
-- my @all_tokens = $parser->parse();
--
-+ my ($all_tokens) = @_;
- my @commands;
- my @current;
- my $lineno = 1;
-+ my $first_pos = 0;
-+
-+ my %shell_keyword;
-+ @shell_keyword{qw(if then else elif fi for do done
-+ while until case in esac)} = ();
-
-- for (my $ti = 0; $ti < @all_tokens; $ti++) {
-- my $text = $all_tokens[$ti]->[0];
-+ for (my $ti = 0; $ti < @$all_tokens; $ti++) {
-+ my $text = $all_tokens->[$ti]->[0];
- if ($text =~ /^(?:&&|\|\||;;|\n)$/) {
-+ # Command separators: flush current command
-+ if (@current) {
-+ push @commands, {
-+ tokens => [@current],
-+ lineno => $lineno,
-+ token_pos => $first_pos,
-+ };
-+ @current = ();
-+ }
-+ } elsif ($text =~ /^[{}()|]$/ || exists $shell_keyword{$text}) {
-+ # Shell structural tokens and keywords:
-+ # flush current command (these are boundaries,
-+ # not part of the command's arguments)
- if (@current) {
- push @commands, {
-- tokens => [@current],
-- lineno => $lineno,
-+ tokens => [@current],
-+ lineno => $lineno,
-+ token_pos => $first_pos,
- };
- @current = ();
- }
- } else {
-- $lineno = $all_tokens[$ti]->[3]
-- if !@current && defined $all_tokens[$ti]->[3];
-+ if (!@current) {
-+ # Record line number of the first token
-+ $lineno = $all_tokens->[$ti]->[3]
-+ if defined $all_tokens->[$ti]->[3];
-+ $first_pos = $ti;
-+ }
- push @current, $text;
- }
- }
- if (@current) {
- push @commands, {
-- tokens => [@current],
-- lineno => $lineno,
-+ tokens => [@current],
-+ lineno => $lineno,
-+ token_pos => $first_pos,
- };
- }
- return @commands;
- }
-
- # --- Rule: '! test_grep' should be 'test_grep !' ---
--# Shell-level negation suppresses test_grep's diagnostic output
--# on failure. Built-in negation preserves it.
- sub check_test_grep_negation {
-- my ($cmd, $file, $line_ref) = @_;
-+ my ($cmd, $file, $line_ref, $all_tokens) = @_;
- my @tokens = @{$cmd->{tokens}};
- return unless @tokens >= 2 && $tokens[0] eq '!' && $tokens[1] eq 'test_grep';
-
-@@ t/lint-style.pl: sub check_test_grep_negation {
- qr/!\s*test_grep/, 'test_grep !', '! test_grep');
- }
-
--# Map parsed commands back to raw file lines for --fix.
--# Detection uses parsed tokens (correct handling of quoting,
--# heredocs, pipes) but fixes must modify the original text
--# to preserve formatting.
-+# --- Rule: bare 'grep' should be 'test_grep' ---
-+
-+# Check if this command is in a filter context by looking at
-+# the surrounding tokens in the stream. This is grep-rule
-+# specific: it knows what contexts make a grep not an assertion.
-+sub is_filter_context {
-+ my ($all_tokens, $cmd) = @_;
-+ my $pos = $cmd->{token_pos};
-+
-+ # Scan backward to the previous command separator.
-+ # If we find '|', this command is part of a pipeline.
-+ # If we find if/elif/while/until, it's a condition.
-+ for (my $j = $pos - 1; $j >= 0; $j--) {
-+ my $t = $all_tokens->[$j]->[0];
-+ # Stop at command separators (but not \n after |)
-+ last if $t =~ /^(?:&&|\|\||;;)$/;
-+ if ($t eq "\n") {
-+ # \n after | is a line continuation, keep scanning
-+ next if $j > 0 && $all_tokens->[$j - 1]->[0] eq '|';
-+ last;
-+ }
-+ return 1 if $t eq '|';
-+ return 1 if $t =~ /^(?:if|elif|while|until)$/;
-+ # for ... in ITEMS ... do: if we're between 'in' and 'do',
-+ # we're in a value list, not a command
-+ return 1 if $t eq 'in';
-+ }
-+
-+ # Forward: pipe after command
-+ for (my $j = $pos + @{$cmd->{tokens}}; $j < @$all_tokens; $j++) {
-+ my $t = $all_tokens->[$j]->[0];
-+ last if $t =~ /^(?:&&|\|\||;;|\n)$/;
-+ return 1 if $t eq '|';
-+ }
-+
-+ # { cmd; } >output
-+ return 1 if is_in_redirected_brace($all_tokens, $pos);
-+
-+ return 0;
-+}
-+
-+# Check if position $pos is inside a brace group whose output is
-+# redirected: { grep ...; } >file. Scan backward for the enclosing
-+# '{', then forward for the matching '}', and check what follows it.
-+sub is_in_redirected_brace {
-+ my ($all_tokens, $pos) = @_;
-+ my $brace_depth = 0;
-+ for (my $j = $pos - 1; $j >= 0; $j--) {
-+ my $t = $all_tokens->[$j]->[0];
-+ $brace_depth++ if $t eq '}';
-+ if ($t eq '{') {
-+ return 0 if $brace_depth > 0;
-+ $brace_depth--;
-+ # Found our enclosing '{'. Find matching '}'
-+ my $depth = 1;
-+ for (my $k = $j + 1; $k < @$all_tokens; $k++) {
-+ $depth++ if $all_tokens->[$k]->[0] eq '{';
-+ $depth-- if $all_tokens->[$k]->[0] eq '}';
-+ if ($depth == 0) {
-+ my $after = $k + 1 < @$all_tokens ?
-+ $all_tokens->[$k + 1]->[0] : '';
-+ return $after =~ /^>{1,2}/;
-+ }
-+ }
-+ return 0;
-+ }
-+ }
-+ return 0;
-+}
-+
-+# Classify a grep command: assertion, filter, or bug.
-+#
-+# Returns:
-+# 1 assertion (PATTERN + FILE), can be converted to test_grep
-+# 0 not a grep, or grep used as a filter
-+# -1 likely bug (e.g., missing file argument)
-+sub is_grep_assertion {
-+ my ($cmd, $all_tokens) = @_;
-+ my @tokens = @{$cmd->{tokens}};
-+
-+ # Find grep, possibly after "!"
-+ my $i = 0;
-+ $i++ if $tokens[0] eq '!';
-+ return 0 unless defined $tokens[$i] && $tokens[$i] eq 'grep';
-+ return 0 if grep { $_ eq 'test_grep' } @tokens;
-+
-+ # Check surrounding context (pipes, control flow, brace redirects)
-+ return 0 if is_filter_context($all_tokens, $cmd);
-+
-+ $i++; # skip 'grep'
-+
-+ # Check grep's own flags and arguments
-+ my @positional;
-+ my $has_pattern_flag = 0;
-+ my $end_of_flags = 0;
-+ while ($i < @tokens) {
-+ my $tok = $tokens[$i];
-+ if ($tok eq '|' || $tok eq '<') {
-+ return 0;
-+ }
-+ if ($tok =~ /^>{1,2}$/) {
-+ # Stdout redirect means filter (grep ... >out).
-+ # Stderr redirect (2>err) is fine: skip the fd
-+ # and the target, and keep classifying.
-+ my $prev = $i > 0 ? $tokens[$i - 1] : '';
-+ return 0 unless $prev =~ /^\d+$/ && $prev >= 2;
-+ pop @positional if @positional && $positional[-1] eq $prev;
-+ $i += 2;
-+ next;
-+ }
-+ if (!$end_of_flags && $tok =~ /^-\w*[clLrR]/) {
-+ return 0;
-+ }
-+ if (!$end_of_flags && $tok eq '--') {
-+ $end_of_flags = 1;
-+ } elsif (!$end_of_flags && $tok =~ /^-\w*[ef]$/) {
-+ $has_pattern_flag = 1;
-+ $i++;
-+ } elsif (!$end_of_flags && $tok =~ /^-/) {
-+ # skip other flags
-+ } else {
-+ push @positional, $tok;
-+ }
-+ $i++;
-+ }
-+
-+ my $need = $has_pattern_flag ? 1 : 2;
-+ return 0 if !@positional && !$has_pattern_flag;
-+ return -1 if @positional < $need;
-+ return 0 if $positional[-1] =~ /^-/;
-+ return 1;
-+}
-+
-+sub check_bare_grep {
-+ my ($cmd, $file, $line_ref, $all_tokens) = @_;
-+ my @tokens = @{$cmd->{tokens}};
-+
-+ my $result = is_grep_assertion($cmd, $all_tokens);
-+ return unless $result;
-+
-+ if ($result == -1) {
-+ err $file, $cmd->{lineno}, join(' ', @tokens),
-+ "grep assertion appears to be missing a file argument";
-+ return 0;
-+ }
-+
-+ # Determine negation and -q flag
-+ my $negated = $tokens[0] eq '!';
-+ my $has_q = 0;
-+ my ($pre_q, $post_q) = ('', '');
-+ for my $tok (@tokens) {
-+ if ($tok =~ /^-(\w*)q(\w*)$/) {
-+ $has_q = 1;
-+ ($pre_q, $post_q) = ($1, $2);
-+ last;
-+ }
-+ last if $tok !~ /^-/ && $tok ne '!' && $tok ne 'grep';
-+ }
-+
-+ # Build the replacement
-+ my $fix = "test_grep";
-+ $fix .= " !" if $negated;
-+ if ($has_q) {
-+ my $rest = "$pre_q$post_q";
-+ $fix .= " -$rest" if $rest;
-+ }
-+
-+ # Build the match pattern
-+ my $neg_match = $negated ? '!\s*' : '\b';
-+ my $neg_from = $negated ? '! ' : '';
-+ my ($match, $from);
-+ if ($has_q) {
-+ $match = qr/${neg_match}grep\s+-\w*q\w*/;
-+ $from = "${neg_from}grep -${pre_q}q${post_q}";
-+ } else {
-+ $match = qr/${neg_match}grep\b/;
-+ $from = "${neg_from}grep";
-+ }
-+
-+ return report_violation($file, $cmd, $line_ref,
-+ $match, $fix, $from);
-+}
-+
-+# --- Harness: LintParser.check_test ---
-+#
-+# Called by ScriptParser::parse_cmd for each test_expect_success
-+# or test_expect_failure block. Extracts the body, tokenizes it,
-+# splits into commands, and runs each rule.
- package LintParser;
-
- sub check_test {
-- # Called by ScriptParser::parse_cmd for each test_expect_success
-- # or test_expect_failure block.
- my $self = shift @_;
- my $title = ScriptParser::unwrap(shift @_);
-
-@@ t/lint-style.pl: sub check_test {
- }
- return unless $body;
-
-+ # Tokenize the body once; commands and rules share the stream
-+ my $parser = ShellParser->new(\$body);
-+ my @all_tokens = $parser->parse();
-+ my @commands = main::parse_commands(\@all_tokens);
-+
- # Map each command back to its file line number.
- # $lineno_base is where the body starts in the file;
- # $cmd->{lineno} is relative to the body (starting at 1).
- my $raw_lines = $self->{raw_lines};
-- for my $cmd (main::parse_commands($body)) {
-+ for my $cmd (@commands) {
- my $ln = ($cmd->{lineno} || 0) + $lineno_base - 1;
- $cmd->{lineno} = $ln;
- next unless $ln >= 1 && $ln <= @$raw_lines;
- next if $raw_lines->[$ln - 1] =~ /#.*lint-ok/;
-
-- if (main::check_test_grep_negation($cmd, $self->{file}, \$raw_lines->[$ln - 1])) {
-- $self->{dirty} = 1;
-- }
-+ my $line_ref = \$raw_lines->[$ln - 1];
-+ # Stop after the first fix: later rules should not
-+ # re-match against already-modified text.
-+ my $modified = 0;
-+ $modified ||= main::check_test_grep_negation(
-+ $cmd, $self->{file}, $line_ref, \@all_tokens);
-+ $modified ||= main::check_bare_grep(
-+ $cmd, $self->{file}, $line_ref, \@all_tokens);
-+ $self->{dirty} = 1 if $modified;
- }
- }
-
- package main;
-
- for my $file (@ARGV) {
-+ next if $skip_file{$file};
- # :unix:crlf strips \r on Windows (same as chainlint.pl)
- open(my $fh, '<:unix:crlf', $file) or die "$0: $file: $!\n";
- my @raw_lines = <$fh>;
-
- ## t/lint-style/grep-assert.expect (new) ##
-@@
-+lint-style/grep-assert.test:2: error: replace 'grep' with 'test_grep': grep "pattern" actual
-+lint-style/grep-assert.test:3: error: replace 'grep' with 'test_grep': grep -E "extended" actual
-+lint-style/grep-assert.test:4: error: replace 'grep' with 'test_grep': grep -e "explicit" actual
-+lint-style/grep-assert.test:5: error: replace 'grep' with 'test_grep': grep -f patterns.txt actual
-+lint-style/grep-assert.test:6: error: replace 'grep' with 'test_grep': grep -Fe "fixed-explicit" actual
-+lint-style/grep-assert.test:7: error: replace 'grep' with 'test_grep': grep "^-looks-like-flag" actual
-+lint-style/grep-assert.test:8: error: replace 'grep' with 'test_grep': grep -v "inverted" actual
-+lint-style/grep-assert.test:9: error: replace 'grep' with 'test_grep': grep -- "-e" actual
-+lint-style/grep-assert.test:10: error: replace 'grep' with 'test_grep': grep "with-stderr-redirect" actual 2 > err
-+lint-style/grep-assert.test:12: error: replace 'grep' with 'test_grep': grep "after-or" actual
-+lint-style/grep-assert.test:17: error: replace 'grep' with 'test_grep': grep "after-pipe" actual
-+lint-style/grep-assert.test:22: error: replace 'grep' with 'test_grep': grep "inside-case" actual
-+hint: run with --fix to apply the suggested replacements.
-
- ## t/lint-style/grep-assert.test (new) ##
-@@
-+test_expect_success 'grep assertions' '
-+ grep "pattern" actual &&
-+ grep -E "extended" actual &&
-+ grep -e "explicit" actual &&
-+ grep -f patterns.txt actual &&
-+ grep -Fe "fixed-explicit" actual &&
-+ grep "^-looks-like-flag" actual &&
-+ grep -v "inverted" actual &&
-+ grep -- "-e" actual &&
-+ grep "with-stderr-redirect" actual 2>err &&
-+ cmd ||
-+ grep "after-or" actual
-+'
-+
-+test_expect_success 'pipe only suppresses first command' '
-+ cmd |
-+ grep "piped" && grep "after-pipe" actual
-+'
-+
-+test_expect_success 'case pattern does not hide assertion' '
-+ case foo in
-+ *) grep "inside-case" actual ;;
-+ esac
-+'
-
- ## t/lint-style/grep-fix.expect (new) ##
-@@
-+test_expect_success 'all fixable forms' '
-+ test_grep "pattern" actual &&
-+ test_grep -E "extended" actual &&
-+ test_grep "quiet" actual &&
-+ test_grep -F "combined" actual &&
-+ test_grep -i "quiet-insensitive" actual &&
-+ test_grep ! "negated" actual &&
-+ test_grep ! "neg-quiet" actual &&
-+ test_grep ! "shell-neg" actual &&
-+ test_grep -e "explicit" actual &&
-+ test_grep -Fe "fixed-explicit" actual &&
-+ test_grep -f patterns.txt actual &&
-+ test_grep -- "-e" actual &&
-+ test_grep "continuation" \
-+ actual
-+'
-
- ## t/lint-style/grep-fix.test (new) ##
-@@
-+test_expect_success 'all fixable forms' '
-+ grep "pattern" actual &&
-+ grep -E "extended" actual &&
-+ grep -q "quiet" actual &&
-+ grep -qF "combined" actual &&
-+ grep -qi "quiet-insensitive" actual &&
-+ ! grep "negated" actual &&
-+ ! grep -q "neg-quiet" actual &&
-+ ! test_grep "shell-neg" actual &&
-+ grep -e "explicit" actual &&
-+ grep -Fe "fixed-explicit" actual &&
-+ grep -f patterns.txt actual &&
-+ grep -- "-e" actual &&
-+ grep "continuation" \
-+ actual
-+'
-
- ## t/lint-style/grep-missing-file.expect (new) ##
-@@
-+lint-style/grep-missing-file.test:2: error: grep assertion appears to be missing a file argument: grep "pattern"
-+lint-style/grep-missing-file.test:3: error: grep assertion appears to be missing a file argument: ! grep "negated"
-+lint-style/grep-missing-file.test:4: error: grep assertion appears to be missing a file argument: grep -e "pattern-flag-no-file"
-+lint-style/grep-missing-file.test:5: error: grep assertion appears to be missing a file argument: grep -f patterns.txt
-
- ## t/lint-style/grep-missing-file.test (new) ##
-@@
-+test_expect_success 'grep missing file argument' '
-+ grep "pattern" &&
-+ ! grep "negated" &&
-+ grep -e "pattern-flag-no-file" &&
-+ grep -f patterns.txt
-+'
-
- ## t/lint-style/grep-negated.expect (new) ##
-@@
-+lint-style/grep-negated.test:2: error: replace '! grep' with 'test_grep !': ! grep "pattern" actual
-+lint-style/grep-negated.test:3: error: replace '! grep' with 'test_grep !': ! grep -i "insensitive" actual
-+lint-style/grep-negated.test:7: error: replace '! grep -q' with 'test_grep !': ! grep -q "pattern" actual
-+lint-style/grep-negated.test:8: error: replace '! grep -qF' with 'test_grep ! -F': ! grep -qF "combined" actual
-+hint: run with --fix to apply the suggested replacements.
-
- ## t/lint-style/grep-negated.test (new) ##
-@@
-+test_expect_success 'negated grep' '
-+ ! grep "pattern" actual &&
-+ ! grep -i "insensitive" actual
-+'
-+
-+test_expect_success 'negated grep -q' '
-+ ! grep -q "pattern" actual &&
-+ ! grep -qF "combined" actual
-+'
-
- ## t/lint-style/grep-not-assert.expect (new) ##
-
- ## t/lint-style/grep-not-assert.test (new) ##
-@@
-+test_expect_success 'grep used as filter (not assertion)' '
-+ grep "pattern" file | wc -l &&
-+ grep "pattern" file >output &&
-+ grep "pattern" file 1>output &&
-+ grep -c "count" file &&
-+ grep -ci "count-insensitive" file &&
-+ grep -l "list" file &&
-+ grep -rl "recursive-list" dir &&
-+ grep -L "list-without" file1 file2 &&
-+ result=$(grep "pattern" file) &&
-+ result=$(echo $(grep "nested-subshell" file)) &&
-+ grep "pattern" <stdin &&
-+ grep "pattern" file && # lint-ok
-+ cmd | grep "pattern-only" &&
-+ cmd |
-+ grep "cross-line-pipe" &&
-+ grep -r "recursive" dir
-+'
-+
-+test_expect_success 'grep in control flow (not assertion)' '
-+ if grep "condition" file
-+ then
-+ echo yes
-+ elif grep "other-condition" file
-+ then
-+ echo no
-+ fi
-+'
-+
-+test_expect_success 'grep in brace group with redirect' '
-+ { grep "captured" out; } >result
-+'
-+
-+test_expect_success 'grep in for-in value list' '
-+ for cmd in grep sed awk; do
-+ echo "$cmd"
-+ done
-+'
-+
-+test_expect_success 'grep in subshell' '
-+ (cd sub && grep "pattern" file >output) &&
-+ (cmd | grep "piped-in-subshell")
-+'
-
## t/pack-refs-tests.sh ##
@@ t/pack-refs-tests.sh: test_expect_success 'delete ref while another dangling packed ref' '
test_expect_success 'pack ref directly below refs/' '
@@ t/t0030-stripspace.sh: test_expect_success 'strip comments with changed comment
test_expect_success '-c with single line' '
+ ## t/t0031-lockfile-pid.sh ##
+@@ t/t0031-lockfile-pid.sh: test_expect_success 'PID info not shown by default' '
+ test_must_fail git add . 2>err &&
+ # Should not crash, just show normal error without PID
+ test_grep "Unable to create" err &&
+- ! test_grep "is held by process" err
++ test_grep ! "is held by process" err
+ )
+ '
+
+
## t/t0040-parse-options.sh ##
@@ t/t0040-parse-options.sh: test_expect_success 'non ambiguous option (after two options it abbreviates)' '
@@ t/t0052-simple-ipc.sh: test_expect_success 'servers cannot share the same path'
test-tool simple-ipc send --token=big >actual &&
test_line_count -ge 10000 actual &&
- grep -q "big: [0]*9999\$" actual
-+ test_grep "big: [0]*9999\$" actual
++ test_grep -q "big: [0]*9999\$" actual
'
test_expect_success 'chunk response' '
test-tool simple-ipc send --token=chunk >actual &&
test_line_count -ge 10000 actual &&
- grep -q "big: [0]*9999\$" actual
-+ test_grep "big: [0]*9999\$" actual
++ test_grep -q "big: [0]*9999\$" actual
'
test_expect_success 'slow response' '
test-tool simple-ipc send --token=slow >actual &&
test_line_count -ge 100 actual &&
- grep -q "big: [0]*99\$" actual
-+ test_grep "big: [0]*99\$" actual
++ test_grep -q "big: [0]*99\$" actual
'
# Send an IPC with n=100,000 bytes of ballast. This should be large enough
@@ t/t0203-gettext-setlocale-sanity.sh: test_expect_success 'git show a ISO-8859-1
git show >out 2>err &&
test_must_be_empty err &&
- grep -q "iso-c-commit" out
-+ test_grep "iso-c-commit" out
++ test_grep -q "iso-c-commit" out
'
test_expect_success GETTEXT_LOCALE 'git show a ISO-8859-1 commit under a UTF-8 locale' '
@@ t/t0203-gettext-setlocale-sanity.sh: test_expect_success GETTEXT_LOCALE 'git sho
LANGUAGE=is LC_ALL="$is_IS_locale" git show >out 2>err &&
test_must_be_empty err &&
- grep -q "iso-utf8-commit" out
-+ test_grep "iso-utf8-commit" out
++ test_grep -q "iso-utf8-commit" out
'
test_done
@@ t/t0410-partial-clone.sh: test_expect_success TTY 'promisor.quiet=unconfigured s
+ test_grep "Receiving objects" err
'
- . "$TEST_DIRECTORY"/lib-httpd.sh
+ test_expect_success 'promisor.quiet from submodule repo is honored' '
@@ t/t0410-partial-clone.sh: test_expect_success 'fetching of missing objects from an HTTP server' '
test_line_count = 1 promisorlist &&
IDX=$(sed "s/promisor$/idx/" promisorlist) &&
@@ t/t1011-read-tree-sparse-checkout.sh: test_expect_success 'read-tree will not th
read_tree_u_must_fail -m -u HEAD^ &&
test_path_is_file init.t &&
- grep -q dirty init.t
-+ test_grep dirty init.t
++ test_grep -q dirty init.t
'
test_expect_success 'read-tree will not throw away dirty changes, sparse' '
@@ t/t1011-read-tree-sparse-checkout.sh: test_expect_success 'read-tree will not th
read_tree_u_must_fail -m -u HEAD^ &&
test_path_is_file init.t &&
- grep -q dirty init.t
-+ test_grep dirty init.t
++ test_grep -q dirty init.t
'
test_expect_success 'read-tree updates worktree, dirty case' '
@@ t/t1011-read-tree-sparse-checkout.sh: test_expect_success 'read-tree updates wor
echo dirty >init.t &&
read_tree_u_must_fail -m -u HEAD^ &&
- grep -q dirty init.t &&
-+ test_grep dirty init.t &&
++ test_grep -q dirty init.t &&
rm init.t
'
@@ t/t1011-read-tree-sparse-checkout.sh: test_expect_success 'read-tree removes wor
echo dirty >added &&
read_tree_u_must_succeed -m -u HEAD^ &&
- grep -q dirty added
-+ test_grep dirty added
++ test_grep -q dirty added
'
test_expect_success 'read-tree adds to worktree, absent case' '
@@ t/t1011-read-tree-sparse-checkout.sh: test_expect_success 'read-tree adds to wor
echo dirty >sub/added &&
read_tree_u_must_succeed -u -m HEAD^ &&
- grep -q dirty sub/added
-+ test_grep dirty sub/added
++ test_grep -q dirty sub/added
'
test_expect_success 'index removal and worktree narrowing at the same time' '
@@ t/t3310-notes-merge-manual-resolve.sh: EOF
git log -1 --format=%B refs/notes/m > merge_commit_msg &&
- grep -q refs/notes/m merge_commit_msg &&
- grep -q refs/notes/z merge_commit_msg &&
-+ test_grep refs/notes/m merge_commit_msg &&
-+ test_grep refs/notes/z merge_commit_msg &&
++ test_grep -q refs/notes/m merge_commit_msg &&
++ test_grep -q refs/notes/z merge_commit_msg &&
# Merge commit mentions conflicting notes
- grep -q "Conflicts" merge_commit_msg &&
-+ test_grep "Conflicts" merge_commit_msg &&
++ test_grep -q "Conflicts" merge_commit_msg &&
( for sha1 in $(cat expect_conflicts); do
- grep -q "$sha1" merge_commit_msg ||
-+ test_grep "$sha1" merge_commit_msg ||
++ test_grep -q "$sha1" merge_commit_msg ||
exit 1
done ) &&
# Verify contents of merge result
@@ t/t3310-notes-merge-manual-resolve.sh: EOF
git log -1 --format=%B refs/notes/m > merge_commit_msg &&
- grep -q refs/notes/m merge_commit_msg &&
- grep -q refs/notes/z merge_commit_msg &&
-+ test_grep refs/notes/m merge_commit_msg &&
-+ test_grep refs/notes/z merge_commit_msg &&
++ test_grep -q refs/notes/m merge_commit_msg &&
++ test_grep -q refs/notes/z merge_commit_msg &&
# Merge commit mentions conflicting notes
- grep -q "Conflicts" merge_commit_msg &&
-+ test_grep "Conflicts" merge_commit_msg &&
++ test_grep -q "Conflicts" merge_commit_msg &&
( for sha1 in $(cat expect_conflicts); do
- grep -q "$sha1" merge_commit_msg ||
-+ test_grep "$sha1" merge_commit_msg ||
++ test_grep -q "$sha1" merge_commit_msg ||
exit 1
done ) &&
# Verify contents of merge result
@@ t/t3404-rebase-interactive.sh: test_expect_success 'rebase -i --root retain root
) &&
git cat-file commit HEAD >output &&
- grep -q "^author Twerp Snog" output &&
-+ test_grep "^author Twerp Snog" output &&
++ test_grep -q "^author Twerp Snog" output &&
git cat-file commit HEAD >actual &&
- grep -q "^different author$" actual
-+ test_grep "^different author$" actual
++ test_grep -q "^different author$" actual
'
test_expect_success 'rebase -i --root temporary sentinel commit' '
@@ t/t3420-rebase-autostash.sh: testrebase () {
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/t3501-revert-cherry-pick.sh: test_expect_success 'cherry-pick works with dirty
git cherry-pick refs/heads/unrelated &&
test $(git rev-parse :0:renamed) = $(git rev-parse HEAD~2:to-rename.t) &&
- grep -q "^modified$" renamed
-+ test_grep "^modified$" renamed
++ test_grep -q "^modified$" renamed
'
test_expect_success 'advice from failed revert' '
@@ t/t3800-mktag.sh: test_expect_success 'invalid header entry config & fsck' '
cat >tag.sig <<EOF
## t/t3901-i18n-patch.sh ##
-@@ t/t3901-i18n-patch.sh: check_encoding () {
- git cat-file commit HEAD~$j |
- case "$header" in
- 8859)
-- grep "^encoding ISO8859-1" ;;
-+ grep "^encoding ISO8859-1" ;; # lint-ok: piped stdin via case
- *)
-- ret=0; grep "^encoding ISO8859-1" || ret=$?
-+ ret=0; grep "^encoding ISO8859-1" || ret=$? # lint-ok: piped stdin via case
- test "$ret" != 0 ;;
- esac || return 1
- j=$i
@@ t/t3901-i18n-patch.sh: test_expect_success 'format-patch output (ISO-8859-1)' '
git format-patch --stdout main..HEAD^ >out-l1 &&
@@ t/t4000-diff-format.sh: test_expect_success 'git diff-files --no-patch --patch s
test_expect_success 'git diff-files --no-patch --patch-with-raw shows the patch and raw data' '
git diff-files --no-patch --patch-with-raw >actual &&
- grep -q "^:100644 100755 .* $ZERO_OID M path0\$" actual &&
-+ test_grep "^:100644 100755 .* $ZERO_OID M path0\$" actual &&
++ test_grep -q "^:100644 100755 .* $ZERO_OID M path0\$" actual &&
tail -n +4 actual >actual-patch &&
compare_diff_patch expected actual-patch
'
@@ t/t5300-pack-object.sh: test_expect_success !PTHREADS,!FAIL_PREREQS \
'
test_expect_success 'pack-objects in too-many-packs mode' '
-@@ t/t5300-pack-object.sh: test_expect_success '--path-walk pack everything' '
+@@ t/t5300-pack-object.sh: test_expect_success '--name-hash-version=2 and --write-bitmap-index are incompat
+
+ # --stdout option silently removes --write-bitmap-index
+ git pack-objects --stdout --all --name-hash-version=2 --write-bitmap-index >out 2>err &&
+- ! test_grep "currently, --write-bitmap-index requires --name-hash-version=1" err
++ test_grep ! "currently, --write-bitmap-index requires --name-hash-version=1" err
+ '
+
+ test_expect_success '--path-walk pack everything' '
git -C server rev-parse HEAD >in &&
GIT_PROGRESS_DELAY=0 git -C server pack-objects \
--stdout --revs --path-walk --progress <in >out.pack 2>err &&
@@ t/t5319-multi-pack-index.sh: test_expect_success 'git-fsck incorrect offset' '
'
test_expect_success 'corrupt MIDX is not reused' '
-@@ t/t5319-multi-pack-index.sh: test_expect_success 'usage shown without sub-command' '
+@@ t/t5319-multi-pack-index.sh: test_expect_success 'load reverse index when missing .idx, .pack' '
+
+ test_expect_success 'usage shown without sub-command' '
+ test_expect_code 129 git multi-pack-index 2>err &&
+- ! test_grep "unrecognized subcommand" err
++ test_grep ! "unrecognized subcommand" err
+ '
test_expect_success 'complains when run outside of a repository' '
nongit test_must_fail git multi-pack-index write 2>err &&
@@ t/t5326-multi-pack-bitmaps.sh: test_midx_bitmap_cases () {
(
- grep -vf before commits.raw &&
-+ test_grep -vf before commits.raw &&
++ grep -vf before commits.raw && # lint-ok: data filter
# mark missing commits as preferred
sed "s/^/+/" before
) >snapshot &&
@@ t/t5409-colorize-remote-messages.sh: test_expect_success 'setup' '
test_expect_success 'disallow (color) control sequences in sideband' '
- ## t/t5411/test-0013-bad-protocol.sh ##
-@@ t/t5411/test-0013-bad-protocol.sh: test_expect_success "proc-receive: bad protocol (hook --die-read-version, $PROTO
- ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook)
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-read-version option" out-$test_count &&
-- grep "remote: error: fail to negotiate version with proc-receive hook" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-read-version option" out-$test_count &&
-+ test_grep "remote: error: fail to negotiate version with proc-receive hook" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-\EOF
- <COMMIT-A> refs/heads/main
-@@ t/t5411/test-0013-bad-protocol.sh: test_expect_success "proc-receive: bad protocol (hook --die-write-version, $PROT
- ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook)
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-write-version option" out-$test_count &&
-- grep "remote: error: fail to negotiate version with proc-receive hook" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-write-version option" out-$test_count &&
-+ test_grep "remote: error: fail to negotiate version with proc-receive hook" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-@@ t/t5411/test-0013-bad-protocol.sh: test_expect_success "proc-receive: bad protocol (hook --die-read-commands, $PROT
- ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook)
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-read-commands option" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-read-commands option" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-@@ t/t5411/test-0013-bad-protocol.sh: test_expect_success "proc-receive: bad protocol (hook --die-read-push-options, $
- ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook)
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-read-push-options option" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-read-push-options option" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-@@ t/t5411/test-0013-bad-protocol.sh: test_expect_success "proc-receive: bad protocol (hook --die-write-report, $PROTO
- ! [remote rejected] HEAD -> refs/for/main/topic (fail to run proc-receive hook)
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-write-report option" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-write-report option" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-
- ## t/t5411/test-0014-bad-protocol--porcelain.sh ##
-@@ t/t5411/test-0014-bad-protocol--porcelain.sh: test_expect_success "proc-receive: bad protocol (hook --die-read-version, $PROTO
- Done
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-read-version option" out-$test_count &&
-- grep "remote: error: fail to negotiate version with proc-receive hook" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-read-version option" out-$test_count &&
-+ test_grep "remote: error: fail to negotiate version with proc-receive hook" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-@@ t/t5411/test-0014-bad-protocol--porcelain.sh: test_expect_success "proc-receive: bad protocol (hook --die-write-version, $PROT
- Done
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-write-version option" out-$test_count &&
-- grep "remote: error: fail to negotiate version with proc-receive hook" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-write-version option" out-$test_count &&
-+ test_grep "remote: error: fail to negotiate version with proc-receive hook" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-@@ t/t5411/test-0014-bad-protocol--porcelain.sh: test_expect_success "proc-receive: bad protocol (hook --die-read-commands, $PROT
- Done
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-read-commands option" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-read-commands option" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-@@ t/t5411/test-0014-bad-protocol--porcelain.sh: test_expect_success "proc-receive: bad protocol (hook --die-read-push-options, $
- Done
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-read-push-options option" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-read-push-options option" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-@@ t/t5411/test-0014-bad-protocol--porcelain.sh: test_expect_success "proc-receive: bad protocol (hook --die-write-report, $PROTO
- Done
- EOF
- test_cmp expect actual &&
-- grep "remote: fatal: die with the --die-write-report option" out-$test_count &&
-+ test_grep "remote: fatal: die with the --die-write-report option" out-$test_count &&
-
- test_cmp_refs -C "$upstream" <<-EOF
- <COMMIT-A> refs/heads/main
-
## t/t5500-fetch-pack.sh ##
@@ t/t5500-fetch-pack.sh: test_expect_success 'single given branch clone' '
GIT_TRACE2_EVENT="$(pwd)/branch-a/trace2_event" \
@@ t/t5516-fetch-push.sh: test_expect_success 'push --porcelain' '
mk_empty testrepo &&
test_must_fail git push >.git/bar --porcelain asdfasdfasd refs/heads/main:refs/remotes/origin/main &&
- ! grep -q Done .git/bar
-+ test_grep ! Done .git/bar
++ test_grep ! -q Done .git/bar
'
test_expect_success 'push --porcelain rejected' '
@@ t/t5520-pull.sh: test_expect_success '--rebase with rebase.autostash succeeds on
git -C dst pull --rebase >actual 2>&1 &&
- grep -q "Fast-forward" actual &&
- grep -q "Applied autostash." actual
-+ test_grep "Fast-forward" actual &&
-+ test_grep "Applied autostash." actual
++ test_grep -q "Fast-forward" actual &&
++ test_grep -q "Applied autostash." actual
'
test_expect_success '--rebase with conflicts shows advice' '
@@ 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 &&
-+ test_grep ! $d shallow-client/.git/shallow &&
++ ! grep $d shallow-client/.git/shallow && # lint-ok: file may not exist after repack
git -C shallow-server branch branch-orig $d &&
git -C shallow-client fetch --prune --depth=2 \
@@ t/t5702-protocol-v2.sh: test_expect_success 'when server does not send "ready",
test_grep "expected no other sections to be sent after no .ready." err
'
+@@ t/t5702-protocol-v2.sh: test_expect_success 'part of packfile response provided as URI' '
+ do
+ git verify-pack --object-format=$(test_oid algo) --verbose $idx >out &&
+ {
+- grep -E "^[0-9a-f]{16,} " out || :
++ grep -E "^[0-9a-f]{16,} " out || : # lint-ok: data filter
+ } >out.objectlist &&
+ if test_line_count = 1 out.objectlist
+ then
@@ t/t5702-protocol-v2.sh: test_expect_success 'packfile-uri path redacted in trace' '
-c fetch.uriprotocols=http,https \
clone "$HTTPD_URL/smart/http_parent" http_child &&
@@ t/t6040-tracking-info.sh: test_expect_success '--set-upstream-to does not change
git config branch.from-main.merge > actual &&
git rev-parse from-topic_2 >actual2 &&
- grep -q "^refs/heads/main$" actual &&
-+ test_grep "^refs/heads/main$" actual &&
++ test_grep -q "^refs/heads/main$" actual &&
cmp expect2 actual2
'
@@ t/t6423-merge-rename-directories.sh: test_expect_success '5c: Transitive rename
test_path_is_missing x/d &&
test_path_is_file y/d &&
- grep -q "<<<<" y/d # conflict markers should be present
-+ test_grep "<<<<" y/d # conflict markers should be present
++ test_grep -q "<<<<" y/d # conflict markers should be present
)
'
@@ t/t6423-merge-rename-directories.sh: test_expect_success '9e: N-to-1 whammo' '
- grep -q dir2/yo error_line &&
- grep -q dir3/yo error_line &&
- grep -q dirN/yo error_line &&
-+ test_grep dir1/yo error_line &&
-+ test_grep dir2/yo error_line &&
-+ test_grep dir3/yo error_line &&
-+ test_grep dirN/yo error_line &&
++ test_grep -q dir1/yo error_line &&
++ test_grep -q dir2/yo error_line &&
++ test_grep -q dir3/yo error_line &&
++ test_grep -q dirN/yo error_line &&
git ls-files -s >out &&
test_line_count = 16 out &&
@@ t/t6423-merge-rename-directories.sh: test_expect_success '11b: Avoid losing dirt
test_grep "error: Your local changes to the following files would be overwritten by merge" err &&
- grep -q stuff z/c &&
-+ test_grep stuff z/c &&
++ test_grep -q stuff z/c &&
test_seq 1 10 >expected &&
echo stuff >>expected &&
test_cmp expected z/c
@@ t/t6423-merge-rename-directories.sh: test_expect_success '11c: Avoid losing not-
test_grep "error: Your local changes to the following files would be overwritten by merge" err &&
- grep -q stuff y/c &&
-+ test_grep stuff y/c &&
++ test_grep -q stuff y/c &&
test_seq 1 10 >expected &&
echo stuff >>expected &&
test_cmp expected y/c &&
@@ t/t6423-merge-rename-directories.sh: test_expect_success '11d: Avoid losing not-
test_grep "error: Your local changes to the following files would be overwritten by merge" err &&
- grep -q stuff z/c &&
-+ test_grep stuff z/c &&
++ test_grep -q stuff z/c &&
test_seq 1 10 >expected &&
echo stuff >>expected &&
test_cmp expected z/c
@@ t/t6437-submodule-merge.sh: test_expect_success 'merging should fail for changes
)
'
-@@ t/t6437-submodule-merge.sh: test_expect_success 'file/submodule conflict' '
- # directory, though, so just grep for its content in all
- # files, and ignore "grep: path: Is a directory" message
- echo Checking if contents from B:path showed up anywhere &&
-- grep -q content * 2>/dev/null
-+ grep -q content * 2>/dev/null # lint-ok: glob arg breaks test_grep
- )
- '
-
@@ t/t6437-submodule-merge.sh: test_expect_success 'merging should fail with no merge base' '
git commit -m "b" &&
test_must_fail git merge a 2>actual &&
@@ t/t7012-skip-worktree-writing.sh: test_expect_success 'read-tree updates worktre
echo dirty >> init.t &&
test_must_fail git read-tree -m -u HEAD^ &&
- grep -q dirty init.t &&
-+ test_grep dirty init.t &&
++ test_grep -q dirty init.t &&
test "$(git ls-files -t init.t)" = "S init.t" &&
git update-index --no-skip-worktree init.t
'
@@ t/t7012-skip-worktree-writing.sh: test_expect_success 'read-tree removes worktre
echo dirty >> added &&
test_must_fail git read-tree -m -u HEAD^ &&
- grep -q dirty added &&
-+ test_grep dirty added &&
++ test_grep -q dirty added &&
test "$(git ls-files -t added)" = "S added" &&
git update-index --no-skip-worktree added
'
@@ 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
++ ! grep gitdir squatting-clone/d/a/git~2 # lint-ok: file may not exist after failed clone
fi
'
@@ t/t7501-commit-basic-functionality.sh: test_expect_success 'editor not invoked i
EDITOR=./editor git commit -a -F msg &&
git show -s --pretty=format:%s >subject &&
- grep -q good subject &&
-+ test_grep good subject &&
++ test_grep -q good subject &&
echo quack >file &&
echo Another good message. |
EDITOR=./editor git commit -a -F - &&
git show -s --pretty=format:%s >subject &&
- grep -q good subject
-+ test_grep good subject
++ test_grep -q good subject
'
test_expect_success 'partial commit that involves removal (1)' '
@@ t/t7501-commit-basic-functionality.sh: test_expect_success 'amend does not add s
test_expect_success 'commit complains about completely bogus dates' '
@@ t/t7501-commit-basic-functionality.sh: test_expect_success 'git commit <file> with dirty index' '
+ git add chz &&
+ git commit elif -m "tacocat is a palindrome" &&
git show --stat >stat &&
- grep elif stat &&
+- grep elif stat &&
++ test_grep elif stat &&
git diff --cached >diff &&
- grep chz diff
+ test_grep chz diff
@@ t/t7510-signed-commit.sh: test_expect_success GPG 'verify and show signatures' '
- grep -q -F -e "No public key" -e "public key not found" actual
+ test_grep ! "Good signature from" actual &&
+ test_grep ! "BAD signature from" actual &&
-+ test_grep -F -e "No public key" -e "public key not found" actual
++ test_grep -q -F -e "No public key" -e "public key not found" actual
'
test_expect_success GPG 'verify-commit exits success on untrusted signature' '
@@ t/t7519-status-fsmonitor.sh: test_expect_success 'setup' '
cat >expect <<EOF &&
## t/t7527-builtin-fsmonitor.sh ##
-@@ t/t7527-builtin-fsmonitor.sh: verify_fsmonitor_works () {
- --token 0 >/dev/null 2>&1
- maybe_timeout 5 \
- git -C test_fsmonitor_smoke fsmonitor--daemon stop 2>/dev/null
-- ! grep -q "cookie_wait timed out" "$PWD/smoke.trace" 2>/dev/null
-+ ! grep -q "cookie_wait timed out" "$PWD/smoke.trace" 2>/dev/null # lint-ok: $? capture
- ret=$?
- rm -rf test_fsmonitor_smoke smoke.trace
- return $ret
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success 'implicit daemon start' '
GIT_TRACE2_EVENT="$PWD/.git/trace" \
test-tool -C test_implicit fsmonitor-client query --token 0 >actual &&
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success CASE_INSENSITIVE_FS 'fsmonitor
- grep -q "AAA.*pos 0" "$PWD/subdir_case_wrong.log1" &&
- grep -q "zzz.*pos 6" "$PWD/subdir_case_wrong.log1" &&
-+ test_grep "AAA.*pos 0" "$PWD/subdir_case_wrong.log1" &&
-+ test_grep "zzz.*pos 6" "$PWD/subdir_case_wrong.log1" &&
++ test_grep -q "AAA.*pos 0" "$PWD/subdir_case_wrong.log1" &&
++ test_grep -q "zzz.*pos 6" "$PWD/subdir_case_wrong.log1" &&
- grep -q "dir1/DIR2/dir3/file3.*pos -3" "$PWD/subdir_case_wrong.log1" &&
-+ test_grep "dir1/DIR2/dir3/file3.*pos -3" "$PWD/subdir_case_wrong.log1" &&
++ test_grep -q "dir1/DIR2/dir3/file3.*pos -3" "$PWD/subdir_case_wrong.log1" &&
# Verify that we get a mapping event to correct the case.
- grep -q "MAP:.*dir1/DIR2/dir3/file3.*dir1/dir2/dir3/file3" \
-+ test_grep "MAP:.*dir1/DIR2/dir3/file3.*dir1/dir2/dir3/file3" \
++ test_grep -q "MAP:.*dir1/DIR2/dir3/file3.*dir1/dir2/dir3/file3" \
"$PWD/subdir_case_wrong.log1" &&
# The refresh-callbacks should have caused "git status" to clear
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success CASE_INSENSITIVE_FS 'fsmonitor
- grep -q " M AAA" "$PWD/subdir_case_wrong.out" &&
- grep -q " M zzz" "$PWD/subdir_case_wrong.out" &&
- grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
-+ test_grep " M AAA" "$PWD/subdir_case_wrong.out" &&
-+ test_grep " M zzz" "$PWD/subdir_case_wrong.out" &&
-+ test_grep " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
++ test_grep -q " M AAA" "$PWD/subdir_case_wrong.out" &&
++ test_grep -q " M zzz" "$PWD/subdir_case_wrong.out" &&
++ test_grep -q " M dir1/dir2/dir3/file3" "$PWD/subdir_case_wrong.out"
'
test_expect_success CASE_INSENSITIVE_FS 'fsmonitor file case wrong on disk' '
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success CASE_INSENSITIVE_FS 'fsmonitor
- grep -q "fsmonitor_refresh_callback.*file-3-a.*pos 4" "$PWD/file_case_wrong-try1.log" &&
- grep -q "fsmonitor_refresh_callback.*FILE-4-A.*pos 6" "$PWD/file_case_wrong-try1.log" &&
- grep -q "fsmonitor_refresh_callback.*file-4-a.*pos -9" "$PWD/file_case_wrong-try1.log" &&
-+ test_grep "fsmonitor_refresh_callback.*FILE-3-A.*pos -3" "$PWD/file_case_wrong-try1.log" &&
-+ test_grep "fsmonitor_refresh_callback.*file-3-a.*pos 4" "$PWD/file_case_wrong-try1.log" &&
-+ test_grep "fsmonitor_refresh_callback.*FILE-4-A.*pos 6" "$PWD/file_case_wrong-try1.log" &&
-+ test_grep "fsmonitor_refresh_callback.*file-4-a.*pos -9" "$PWD/file_case_wrong-try1.log" &&
++ test_grep -q "fsmonitor_refresh_callback.*FILE-3-A.*pos -3" "$PWD/file_case_wrong-try1.log" &&
++ test_grep -q "fsmonitor_refresh_callback.*file-3-a.*pos 4" "$PWD/file_case_wrong-try1.log" &&
++ test_grep -q "fsmonitor_refresh_callback.*FILE-4-A.*pos 6" "$PWD/file_case_wrong-try1.log" &&
++ test_grep -q "fsmonitor_refresh_callback.*file-4-a.*pos -9" "$PWD/file_case_wrong-try1.log" &&
# FSM refresh will have invalidated the FSM bit and cause a regular
# (real) scan of these tracked files, so they should have "H" status.
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success CASE_INSENSITIVE_FS 'fsmonitor
git -C file_case_wrong ls-files -f >"$PWD/file_case_wrong-lsf1.out" &&
- grep -q "H dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-lsf1.out" &&
- grep -q "H dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-lsf1.out" &&
-+ test_grep "H dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-lsf1.out" &&
-+ test_grep "H dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-lsf1.out" &&
++ test_grep -q "H dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-lsf1.out" &&
++ test_grep -q "H dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-lsf1.out" &&
# Try the status again. We assume that the above status command
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success CASE_INSENSITIVE_FS 'fsmonitor
- ! grep -q "fsmonitor_refresh_callback.*file-3-a.*pos" "$PWD/file_case_wrong-try2.log" &&
- ! grep -q "fsmonitor_refresh_callback.*FILE-4-A.*pos" "$PWD/file_case_wrong-try2.log" &&
- ! grep -q "fsmonitor_refresh_callback.*file-4-a.*pos" "$PWD/file_case_wrong-try2.log" &&
-+ test_grep ! "fsmonitor_refresh_callback.*FILE-3-A.*pos" "$PWD/file_case_wrong-try2.log" &&
-+ test_grep ! "fsmonitor_refresh_callback.*file-3-a.*pos" "$PWD/file_case_wrong-try2.log" &&
-+ test_grep ! "fsmonitor_refresh_callback.*FILE-4-A.*pos" "$PWD/file_case_wrong-try2.log" &&
-+ test_grep ! "fsmonitor_refresh_callback.*file-4-a.*pos" "$PWD/file_case_wrong-try2.log" &&
++ test_grep ! -q "fsmonitor_refresh_callback.*FILE-3-A.*pos" "$PWD/file_case_wrong-try2.log" &&
++ test_grep ! -q "fsmonitor_refresh_callback.*file-3-a.*pos" "$PWD/file_case_wrong-try2.log" &&
++ test_grep ! -q "fsmonitor_refresh_callback.*FILE-4-A.*pos" "$PWD/file_case_wrong-try2.log" &&
++ test_grep ! -q "fsmonitor_refresh_callback.*file-4-a.*pos" "$PWD/file_case_wrong-try2.log" &&
# FSM refresh saw nothing, so it will mark all files as valid,
# so they should now have "h" status.
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success CASE_INSENSITIVE_FS 'fsmonitor
git -C file_case_wrong ls-files -f >"$PWD/file_case_wrong-lsf2.out" &&
- grep -q "h dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-lsf2.out" &&
- grep -q "h dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-lsf2.out" &&
-+ test_grep "h dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-lsf2.out" &&
-+ test_grep "h dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-lsf2.out" &&
++ test_grep -q "h dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-lsf2.out" &&
++ test_grep -q "h dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-lsf2.out" &&
# We now have files with clean content, but with case-incorrect
@@ t/t7527-builtin-fsmonitor.sh: test_expect_success CASE_INSENSITIVE_FS 'fsmonitor
# Verify that we get a mapping event to correct the case.
- grep -q "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir3/FILE-3-A.*dir1/dir2/dir3/file-3-a" \
-+ test_grep "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir3/FILE-3-A.*dir1/dir2/dir3/file-3-a" \
++ test_grep -q "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir3/FILE-3-A.*dir1/dir2/dir3/file-3-a" \
"$PWD/file_case_wrong-try3.log" &&
- grep -q "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir4/file-4-a.*dir1/dir2/dir4/FILE-4-A" \
-+ test_grep "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir4/file-4-a.*dir1/dir2/dir4/FILE-4-A" \
++ test_grep -q "fsmonitor_refresh_callback MAP:.*dir1/dir2/dir4/file-4-a.*dir1/dir2/dir4/FILE-4-A" \
"$PWD/file_case_wrong-try3.log" &&
# FSEvents are in observed case.
- grep -q "fsmonitor_refresh_callback.*FILE-3-A.*pos -3" "$PWD/file_case_wrong-try3.log" &&
- grep -q "fsmonitor_refresh_callback.*file-4-a.*pos -9" "$PWD/file_case_wrong-try3.log" &&
-+ test_grep "fsmonitor_refresh_callback.*FILE-3-A.*pos -3" "$PWD/file_case_wrong-try3.log" &&
-+ test_grep "fsmonitor_refresh_callback.*file-4-a.*pos -9" "$PWD/file_case_wrong-try3.log" &&
++ test_grep -q "fsmonitor_refresh_callback.*FILE-3-A.*pos -3" "$PWD/file_case_wrong-try3.log" &&
++ test_grep -q "fsmonitor_refresh_callback.*file-4-a.*pos -9" "$PWD/file_case_wrong-try3.log" &&
# The refresh-callbacks should have caused "git status" to clear
# the CE_FSMONITOR_VALID bit on each of those files and caused
# the worktree scan to visit them and mark them as modified.
- grep -q " M dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-try3.out" &&
- grep -q " M dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-try3.out"
-+ test_grep " M dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-try3.out" &&
-+ test_grep " M dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-try3.out"
++ test_grep -q " M dir1/dir2/dir3/file-3-a" "$PWD/file_case_wrong-try3.out" &&
++ test_grep -q " M dir1/dir2/dir4/FILE-4-A" "$PWD/file_case_wrong-try3.out"
'
test_done
@@ t/t7700-repack.sh: test_expect_success 'repack --keep-pack' '
ls .git/objects/pack/*.pack >new-counts &&
- grep -q $P1 new-counts &&
- grep -q $P4 new-counts &&
-+ test_grep $P1 new-counts &&
-+ test_grep $P4 new-counts &&
++ test_grep -q $P1 new-counts &&
++ test_grep -q $P4 new-counts &&
test_line_count = 3 new-counts &&
git fsck &&
@@ t/t7703-repack-geometric.sh: test_expect_success '--geometric with small-pack ro
test_line_count = 3 after &&
comm -3 small before | tr -d "\t" >large &&
- grep -qFf large after
-+ test_grep -Ff large after
++ test_grep -qFf large after
)
'
@@ t/t9117-git-svn-init-clone.sh: test_expect_success 'clone to target directory wi
test ! -d trunk &&
git svn init "$svnrepo"/project/trunk trunk 2>warning &&
- ! grep -q prefix warning &&
-+ test_grep ! prefix warning &&
++ test_grep ! -q prefix warning &&
rm -rf trunk &&
rm -f warning
'
@@ t/t9117-git-svn-init-clone.sh: test_expect_success 'init without -s/-T/-b/-t doe
test ! -d trunk &&
git svn clone "$svnrepo"/project/trunk 2>warning &&
- ! grep -q prefix warning &&
-+ test_grep ! prefix warning &&
++ test_grep ! -q prefix warning &&
rm -rf trunk &&
rm -f warning
'
@@ t/t9117-git-svn-init-clone.sh: EOF
test ! -d project &&
git svn init -s "$svnrepo"/project project 2>warning &&
- ! grep -q prefix warning &&
-+ test_grep ! prefix warning &&
++ test_grep ! -q prefix warning &&
test_svn_configured_prefix "origin/" &&
rm -rf project &&
rm -f warning
@@ t/t9117-git-svn-init-clone.sh: test_expect_success 'init with -s/-T/-b/-t assume
test ! -d project &&
git svn clone -s "$svnrepo"/project 2>warning &&
- ! grep -q prefix warning &&
-+ test_grep ! prefix warning &&
++ test_grep ! -q prefix warning &&
test_svn_configured_prefix "origin/" &&
rm -rf project &&
rm -f warning
@@ t/t9117-git-svn-init-clone.sh: test_expect_success 'clone with -s/-T/-b/-t assum
test ! -d project &&
git svn init -s "$svnrepo"/project project --prefix "" 2>warning &&
- ! grep -q prefix warning &&
-+ test_grep ! prefix warning &&
++ test_grep ! -q prefix warning &&
test_svn_configured_prefix "" &&
rm -rf project &&
rm -f warning
@@ t/t9117-git-svn-init-clone.sh: test_expect_success 'init with -s/-T/-b/-t and --
test ! -d project &&
git svn clone -s "$svnrepo"/project --prefix "" 2>warning &&
- ! grep -q prefix warning &&
-+ test_grep ! prefix warning &&
++ test_grep ! -q prefix warning &&
test_svn_configured_prefix "" &&
rm -rf project &&
rm -f warning
@@ t/t9502-gitweb-standalone-parse-output.sh: test_expect_success 'forks: setup' '
- grep -q ">foo_baz\\.git<" gitweb.body &&
- grep -q ">foo/foo-forked\\.git<" gitweb.body &&
- grep -q ">fork of .*<" gitweb.body
-+ test_grep ">\\.git<" gitweb.body &&
-+ test_grep ">foo\\.git<" gitweb.body &&
-+ test_grep ">foo_baz\\.git<" gitweb.body &&
-+ test_grep ">foo\\.bar\\.git<" gitweb.body &&
-+ test_grep ">foo_baz\\.git<" gitweb.body &&
-+ test_grep ">foo/foo-forked\\.git<" gitweb.body &&
-+ test_grep ">fork of .*<" gitweb.body
++ test_grep -q ">\\.git<" gitweb.body &&
++ test_grep -q ">foo\\.git<" gitweb.body &&
++ test_grep -q ">foo_baz\\.git<" gitweb.body &&
++ test_grep -q ">foo\\.bar\\.git<" gitweb.body &&
++ test_grep -q ">foo_baz\\.git<" gitweb.body &&
++ test_grep -q ">foo/foo-forked\\.git<" gitweb.body &&
++ test_grep -q ">fork of .*<" gitweb.body
'
test_expect_success 'enable forks feature' '
@@ t/t9502-gitweb-standalone-parse-output.sh: test_expect_success 'enable forks fea
- grep -q ">foo_baz\\.git<" gitweb.body &&
- grep -v ">foo/foo-forked\\.git<" gitweb.body &&
- grep -v ">fork of .*<" gitweb.body
-+ test_grep ">\\.git<" gitweb.body &&
-+ test_grep ">foo\\.git<" gitweb.body &&
-+ test_grep ">foo_baz\\.git<" gitweb.body &&
-+ test_grep ">foo\\.bar\\.git<" gitweb.body &&
-+ test_grep ">foo_baz\\.git<" gitweb.body &&
++ test_grep -q ">\\.git<" gitweb.body &&
++ test_grep -q ">foo\\.git<" gitweb.body &&
++ test_grep -q ">foo_baz\\.git<" gitweb.body &&
++ test_grep -q ">foo\\.bar\\.git<" gitweb.body &&
++ test_grep -q ">foo_baz\\.git<" gitweb.body &&
+ test_grep -v ">foo/foo-forked\\.git<" gitweb.body &&
+ test_grep -v ">fork of .*<" gitweb.body
'
@@ t/t9502-gitweb-standalone-parse-output.sh: test_expect_success 'enable forks fea
gitweb_run "p=foo.git;a=forks" &&
- grep -q ">foo/foo-forked\\.git<" gitweb.body &&
- grep -q ">fork of foo<" gitweb.body
-+ test_grep ">foo/foo-forked\\.git<" gitweb.body &&
-+ test_grep ">fork of foo<" gitweb.body
++ test_grep -q ">foo/foo-forked\\.git<" gitweb.body &&
++ test_grep -q ">fork of foo<" gitweb.body
'
test_expect_success 'forks: can access forked repository' '
gitweb_run "p=foo/foo-forked.git;a=summary" &&
- grep -q "200 OK" gitweb.headers &&
- grep -q ">fork of foo<" gitweb.body
-+ test_grep "200 OK" gitweb.headers &&
-+ test_grep ">fork of foo<" gitweb.body
++ test_grep -q "200 OK" gitweb.headers &&
++ test_grep -q ">fork of foo<" gitweb.body
'
test_expect_success 'forks: project_index lists all projects (incl. forks)' '
@@ t/t9807-git-p4-submit.sh: test_expect_success 'submit --update-shelve' '
change=$(last_shelve) &&
p4 unshelve -c $change -s $change &&
- grep -q updated-line shelf.t &&
-+ test_grep updated-line shelf.t &&
++ test_grep -q updated-line shelf.t &&
p4 describe -S $change | grep added-file.t &&
test_path_is_missing shelved-change-1.t &&
p4 revert ...
@@ t/t9832-unshelve.sh: EOF
git p4 unshelve $change &&
git show refs/remotes/p4-unshelved/$change >actual &&
- grep -q "Further description" actual &&
-+ test_grep "Further description" actual &&
++ test_grep -q "Further description" actual &&
git cherry-pick refs/remotes/p4-unshelved/$change &&
test_path_is_file file2 &&
test_cmp file1 "$cli"/file1 &&
@@ t/t9832-unshelve.sh: test_expect_success 'create shelved changelist based on p4
change=$(last_shelved_change) &&
p4 describe -S $change >out.txt &&
- grep -q "Change to be unshelved" out.txt
-+ test_grep "Change to be unshelved" out.txt
++ test_grep -q "Change to be unshelved" out.txt
)
'
@@ t/t9832-unshelve.sh: test_expect_success 'try to unshelve the change' '
cd "$git" &&
git p4 unshelve $change >out.txt &&
- grep -q "unshelved changelist $change" out.txt
-+ test_grep "unshelved changelist $change" out.txt
++ test_grep -q "unshelved changelist $change" out.txt
)
'
@@ t/t9833-errors.sh: test_expect_success 'error handling' '
export P4PASSWD &&
test_must_fail git p4 clone //depot/foo 2>errmsg &&
- grep -q "failure accessing depot.*P4PASSWD" errmsg
-+ test_grep "failure accessing depot.*P4PASSWD" errmsg
++ test_grep -q "failure accessing depot.*P4PASSWD" errmsg
)
'
@@ t/t9833-errors.sh: test_expect_success 'ticket logged out' '
p4 logout &&
test_must_fail git p4 submit 2>errmsg &&
- grep -q "failure accessing depot" errmsg
-+ test_grep "failure accessing depot" errmsg
++ test_grep -q "failure accessing depot" errmsg
)
'
@@ t/t9902-completion.sh: test_expect_success '__git_pretty_aliases' '
run_completion "git " &&
# built-in
- grep -q "^add \$" out &&
-+ test_grep "^add \$" out &&
++ test_grep -q "^add \$" out &&
# script
- grep -q "^rebase \$" out &&
-+ test_grep "^rebase \$" out &&
++ test_grep -q "^rebase \$" out &&
# plumbing
- ! grep -q "^ls-files \$" out &&
-+ test_grep ! "^ls-files \$" out &&
++ test_grep ! -q "^ls-files \$" out &&
run_completion "git r" &&
- ! grep -q -v "^r" out
-+ test_grep ! -v "^r" out
++ test_grep ! -q -v "^r" out
'
test_expect_success 'double dash "git" itself' '
4: c1b90101ef ! 6: e5ecb37401 t: add lint-style.pl with test_grep negation rule
@@ Metadata
Author: Michael Montalbo <mmontalbo@gmail.com>
## Commit message ##
- t: add lint-style.pl with test_grep negation rule
+ t: add greplint to detect bare grep assertions
- Add a mechanical lint checker for test scripts, similar in spirit to
- check-non-portable-shell.pl but focused on test conventions rather
- than portability.
+ Without a lint guard, bare grep assertions will creep back into
+ tests over time, defeating the previous commit's conversion.
- The tool defines LintParser, a subclass of ScriptParser (from the
- shared lib-shell-parser.pl module). ScriptParser's
- parse_cmd() finds test_expect_success blocks and calls check_test()
- for each body; LintParser overrides check_test() to run lint rules
- on the parsed commands. A "# lint-ok" comment suppresses all
- checks for intentional style violations.
+ 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).
- The first rule detects '! test_grep' and replaces it with
- 'test_grep !'. Shell-level negation suppresses the diagnostic
- output that test_grep prints on failure; the built-in negation
- preserves it.
+ 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.
- Three violations inside test bodies are converted via --fix. One
- additional violation in a helper function outside test_expect_success
- (t7900's test_geometric_repack_needed) is converted manually, since
- the parser only processes test bodies.
+ 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>
@@ t/.gitattributes
@@
t[0-9][0-9][0-9][0-9]/* -whitespace
/chainlint/*.expect eol=lf -whitespace
-+/lint-style/*.expect eol=lf -whitespace
-+/lint-style/*.test eol=lf -whitespace
++/greplint/*.expect eol=lf -whitespace
++/greplint/*.test eol=lf -whitespace
/t0110/url-* binary
/t3206/* eol=lf
/t3900/*.txt eol=lf
## t/Makefile ##
-@@ t/Makefile: TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
+@@ t/Makefile: TEST_LINT ?= test-lint
+ ifdef TEST_OUTPUT_DIRECTORY
+ TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results
+ CHAINLINTTMP = $(TEST_OUTPUT_DIRECTORY)/chainlinttmp
++GREPLINTTMP = $(TEST_OUTPUT_DIRECTORY)/greplinttmp
+ else
+ TEST_RESULTS_DIRECTORY = test-results
+ CHAINLINTTMP = chainlinttmp
++GREPLINTTMP = greplinttmp
+ endif
+
+ # Shell quote;
+@@ t/Makefile: TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
+ PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
+ TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
+ CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
++GREPLINTTMP_SQ = $(subst ','\'',$(GREPLINTTMP))
+
+ T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
+ THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
+@@ t/Makefile: TLIBS = $(sort $(wildcard lib-*.sh)) annotate-tests.sh
+ TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))
CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
++GREPLINTTESTS = $(sort $(patsubst greplint/%.test,%,$(wildcard greplint/*.test)))
CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
-+LINT_STYLE_TESTS = $(sort $(wildcard lint-style/*.test))
UNIT_TEST_SOURCES = $(wildcard unit-tests/t-*.c)
UNIT_TEST_PROGRAMS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
- UNIT_TEST_PROGRAMS += unit-tests/bin/unit-tests$(X)
+@@ t/Makefile: test: pre-clean check-meson $(TEST_LINT)
+ $(CHAINLINTSUPPRESS) $(MAKE) aggregate-results-and-cleanup
+
+ ifneq ($(PERL_PATH),)
+-test: check-chainlint
+-prove: check-chainlint
++test: check-chainlint check-greplint
++prove: check-chainlint check-greplint
+ endif
+
+ failed:
+@@ t/Makefile: unit-tests-test-tool:
+ pre-clean:
+ $(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
+
+-clean-except-prove-cache: clean-chainlint
++clean-except-prove-cache: clean-chainlint clean-greplint
+ $(RM) -r 'trash directory'.*
+ $(RM) -r valgrind/bin
+
+@@ t/Makefile: check-chainlint:
+ { $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests >'$(CHAINLINTTMP_SQ)'/actual || true; } && \
+ diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
+
++clean-greplint:
++ $(RM) -r '$(GREPLINTTMP_SQ)'
++
++check-greplint:
++ @mkdir -p '$(GREPLINTTMP_SQ)' && \
++ '$(PERL_PATH_SQ)' greplint-cat.pl '$(GREPLINTTMP_SQ)' $(GREPLINTTESTS) && \
++ { '$(PERL_PATH_SQ)' greplint.pl \
++ $(patsubst %,greplint/%.test,$(GREPLINTTESTS)) \
++ >'$(GREPLINTTMP_SQ)'/actual 2>&1 || true; } && \
++ diff -u '$(GREPLINTTMP_SQ)'/expect '$(GREPLINTTMP_SQ)'/actual
++
+ check-meson:
+ @# awk acts up when trying to match single quotes, so we use \047 instead.
+ @mkdir -p mesontmp && \
@@ t/Makefile: check-meson:
test-lint: test-lint-duplicates test-lint-executable \
test-lint-filenames
ifneq ($(PERL_PATH),)
--test-lint: test-lint-shell-syntax check-shell-parser
-+test-lint: test-lint-shell-syntax test-lint-style check-lint-style check-shell-parser
+-test-lint: test-lint-shell-syntax
++test-lint: test-lint-shell-syntax test-greplint
else
GIT_TEST_CHAIN_LINT = 0
endif
-@@ t/Makefile: test-lint-shell-syntax:
+@@ t/Makefile: test-lint-executable:
+ test-lint-shell-syntax:
+ @'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
- check-shell-parser:
- @'$(PERL_PATH_SQ)' check-shell-parser.pl
-+
-+test-lint-style:
-+ @'$(PERL_PATH_SQ)' lint-style.pl $(T) $(THELPERS) $(TPERF)
-+
-+check-lint-style:
-+ @rc=0; for t in $(LINT_STYLE_TESTS); do \
-+ base=$${t%.test}; \
-+ case $$base in \
-+ *-fix) \
-+ cp "$$t" "$$t.tmp" && \
-+ '$(PERL_PATH_SQ)' lint-style.pl --fix "$$t.tmp" >/dev/null 2>&1; \
-+ fix_rc=$$?; \
-+ if test $$fix_rc != 0; then \
-+ echo "FAIL: $$t (--fix exit code $$fix_rc)"; rc=1; \
-+ elif ! diff -u "$$base.expect" "$$t.tmp"; then \
-+ echo "FAIL: $$t (--fix output)"; rc=1; \
-+ fi; \
-+ rm -f "$$t.tmp" ;; \
-+ *) \
-+ if ! '$(PERL_PATH_SQ)' lint-style.pl "$$t" 2>&1 | \
-+ diff -u "$$base.expect" -; then \
-+ echo "FAIL: $$t"; rc=1; \
-+ fi ;; \
-+ esac; \
-+ done; test $$rc = 0
++test-greplint:
++ @'$(PERL_PATH_SQ)' greplint.pl $(T) $(THELPERS) $(TPERF)
+
test-lint-filenames:
@# We do *not* pass a glob to ls-files but use grep instead, to catch
@# non-ASCII characters (which are quoted within double-quotes)
@@ t/Makefile: perf:
+ $(MAKE) -C perf/ all
.PHONY: pre-clean $(T) aggregate-results clean valgrind perf \
- check-chainlint clean-chainlint test-chainlint \
-- check-shell-parser $(UNIT_TESTS)
-+ check-shell-parser \
-+ check-lint-style test-lint-style $(UNIT_TESTS)
+- check-chainlint clean-chainlint test-chainlint $(UNIT_TESTS)
++ check-chainlint clean-chainlint test-chainlint \
++ check-greplint clean-greplint test-greplint $(UNIT_TESTS)
.PHONY: libgit-sys-test libgit-rs-test
libgit-sys-test:
- ## t/lint-style.pl (new) ##
+ ## t/greplint-cat.pl (new) ##
@@
-+#!/usr/bin/perl
-+
-+# Check test scripts for style violations that can be detected
-+# mechanically, such as using bare 'grep' where test_grep should
-+# be used. Use --fix to automatically apply suggested replacements.
-+#
-+# Detection uses parsed tokens from the shared shell parser for
-+# correct handling of heredocs, $(...), pipes, and quoting.
-+# Fixes modify the original file text to preserve formatting.
++#!/usr/bin/env perl
+
+use strict;
+use warnings;
-+use File::Basename;
-+# Force LF output so check-lint-style's diff against the
-+# pre-committed .expect files works on Windows.
-+binmode(STDOUT, ':unix');
-+binmode(STDERR, ':unix');
+
-+my $fix_mode = 0;
-+if (@ARGV && $ARGV[0] eq '--fix') {
-+ $fix_mode = 1;
-+ shift @ARGV;
++# Assemble expected output for check-greplint target.
++# Usage: greplint-cat.pl <outdir> <test-name> ...
++#
++# For each <test-name>, reads greplint/<test-name>.expect and
++# prepends "greplint/<test-name>.test:" to every non-empty line,
++# matching the output format of greplint.pl. Writes combined
++# expected output to <outdir>/expect.
++
++my $outdir = shift;
++open(my $expect, '>', "$outdir/expect")
++ or die "unable to open $outdir/expect: $!";
++
++for my $name (@ARGV) {
++ open(my $fh, '<', "greplint/$name.expect")
++ or die "unable to open greplint/$name.expect: $!";
++ while (<$fh>) {
++ print $expect "greplint/$name.test:$_";
++ }
++ close $fh;
+}
+
-+# Load the shared shell parser (Lexer, ShellParser, ScriptParser).
-+my $_lib = dirname($0) . "/lib-shell-parser.pl";
-+$_lib = "./$_lib" unless $_lib =~ m{^/};
-+do $_lib or die "$0: failed to load $_lib: $@$!\n";
++close $expect;
+
+ ## t/greplint.pl (new) ##
+@@
++#!/usr/bin/env perl
+
-+# LintParser is a subclass of ScriptParser which runs lint rules
-+# on each test body. Per-file state (file name, raw lines, dirty
-+# flag) is stored on the instance before calling parse().
++# Detect bare 'grep' used as a test assertion where 'test_grep'
++# should be used, and '! test_grep' where 'test_grep !' should
++# be used.
+#
-+# Subroutines defined below (parse_commands, check_test_grep_negation,
-+# etc.) are in package main and called with the main:: prefix.
-+# File-scoped lexicals ($fix_mode, $has_fixable, etc.) are visible
-+# across packages since 'package' does not introduce a new scope.
-+package LintParser;
-+our @ISA = ('ScriptParser');
++# The shared shell parser tokenizes test bodies so that 'grep'
++# inside heredocs, command substitutions like $(grep ...), and
++# quoted strings is collapsed into a single token and never seen
++# by our check. A line-oriented approach would need to track
++# heredoc delimiters, nested $() depth, and cross-line pipe
++# state to avoid false positives on patterns like:
++#
++# write_script foo.sh <<-\EOF
++# grep pattern file # data, not an assertion
++# EOF
++#
++# The Lexer already handles these.
+
-+package main;
++use warnings;
++use strict;
++use File::Basename;
++do(dirname($0) . "/lib-shell-parser.pl")
++ or die "$0: failed to load lib-shell-parser.pl: $@$!\n";
+
+my $exit_code = 0;
-+my $has_fixable = 0;
+
-+sub err {
-+ my ($file, $lineno, $line, $msg, %opts) = @_;
-+ $line =~ s/^\s+//;
-+ $line =~ s/\s+$//;
-+ $line =~ s/\s+/ /g;
-+ my $prefix = ($fix_mode && $opts{fixable}) ? 'fixed' : 'error';
-+ print "$file:$lineno: $prefix: $msg: $line\n";
-+ $exit_code = 1 unless $fix_mode && $opts{fixable};
-+}
++# GrepLintParser inherits ScriptParser's ability to find
++# test_expect_success/failure blocks and call check_test()
++# on each body. We override check_test() to walk the token
++# stream looking for bare grep assertions.
++package GrepLintParser;
+
-+# Report a lint violation found by a rule. In --fix mode, apply
-+# the regex substitution on the raw line and report success.
-+# Otherwise just report. Returns 1 if the line was modified.
-+sub report_violation {
-+ my ($file, $cmd, $line_ref, $match, $fix, $from) = @_;
-+ my $lineno = $cmd->{lineno};
-+ my $display = join(' ', @{$cmd->{tokens}});
-+ $has_fixable++; # count for the "--fix" hint
-+ if ($fix_mode) {
-+ if ($$line_ref =~ s/$match/$fix/) {
-+ err $file, $lineno, $display,
-+ "replace '$from' with '$fix'",
-+ fixable => 1;
-+ return 1;
-+ }
-+ err $file, $lineno, $display,
-+ "replace '$from' with '$fix' (could not auto-fix)";
-+ } else {
-+ err $file, $lineno, $display,
-+ "replace '$from' with '$fix'";
++our @ISA = ('ScriptParser');
++
++# After these tokens, the next token is a command word.
++# For example, in 'echo foo && grep bar file', the 'grep'
++# after '&&' is at command position and should be flagged.
++my %cmd_start = map { $_ => 1 } qw(&& || ; ;; do then else elif), "\n", '{', '(';
++
++# Tokens indicating grep's output is piped or redirected.
++my %filter_op = map { $_ => 1 } qw(| > >> <);
++
++# A token is at "command word" position if the shell would
++# interpret it as a program name rather than an argument.
++# Only 'grep' at command position is an assertion we should
++# flag; 'grep' as an argument ('test_must_fail grep') or
++# value ('for cmd in grep sed') is not.
++sub is_command_word {
++ my ($tokens, $pos) = @_;
++ return 1 if $pos == 0;
++ for (my $j = $pos - 1; $j >= 0; $j--) {
++ my $t = $tokens->[$j]->[0];
++ # After a separator or pipe, a new command starts.
++ return 1 if $cmd_start{$t} || $t eq '|';
++ # After '}' or ')', what follows is a separator or
++ # redirect on the compound command, not a new command.
++ return 0 if $t eq '}' || $t eq ')';
++ # '!' is a prefix that does not consume command
++ # position; keep scanning to find what precedes it.
++ next if $t eq '!';
++ # Any other word means we are past the command word.
++ return 0;
+ }
-+ return 0;
++ return 1;
+}
+
-+# Split a token stream into commands at &&, ||, ;;, and \n.
-+sub parse_commands {
-+ my ($content) = @_;
-+ my $parser = ShellParser->new(\$content);
-+ my @all_tokens = $parser->parse();
++# Some bare greps are intentional (e.g. file may not exist,
++# data filter). A '# lint-ok' annotation on the source line
++# suppresses the warning.
++sub lint_ok {
++ my ($raw_lines, $ln) = @_;
++ if ($ln < 1 || $ln > @$raw_lines) {
++ warn "lint_ok: line number $ln out of range (1.." .
++ scalar(@$raw_lines) . ")\n";
++ return 0;
++ }
++ return $raw_lines->[$ln - 1] =~ /lint-ok/;
++}
+
-+ my @commands;
-+ my @current;
-+ my $lineno = 1;
++# Grep is a filter (not an assertion) if it receives piped
++# input or sends its output to a pipe or redirect. Check
++# both directions from grep's position in the token stream.
++sub is_filter {
++ my ($tokens, $pos) = @_;
++ # Backward: is grep receiving piped input?
++ # Newlines don't break pipes ('cmd |\n grep' is one
++ # pipeline), so skip past them.
++ for (my $j = $pos - 1; $j >= 0; $j--) {
++ my $t = $tokens->[$j]->[0];
++ return 1 if $t eq '|';
++ next if $t eq "\n";
++ last if $cmd_start{$t} || $t eq '}' || $t eq ')';
++ }
++ # Forward: is grep piping or redirecting output?
++ # Unlike the backward scan, we do not skip newlines here:
++ # a bare newline is a command boundary, and redirects or
++ # pipes must appear on the same line as grep (or after a
++ # line continuation, which the Lexer consumes).
++ for (my $j = $pos + 1; $j < @$tokens; $j++) {
++ my $t = $tokens->[$j]->[0];
++ return 0 if $cmd_start{$t};
++ return 1 if $filter_op{$t};
++ }
++ return 0;
++}
+
-+ for (my $ti = 0; $ti < @all_tokens; $ti++) {
-+ my $text = $all_tokens[$ti]->[0];
-+ if ($text =~ /^(?:&&|\|\||;;|\n)$/) {
-+ if (@current) {
-+ push @commands, {
-+ tokens => [@current],
-+ lineno => $lineno,
-+ };
-+ @current = ();
-+ }
++# Map a body-relative line number to a file line number.
++# For double-quoted bodies, backslash-continuation lines
++# (\<newline>) are consumed by the Lexer without appearing
++# in the body text, so the inner parser sees fewer lines
++# than the source file has. We walk the source lines to
++# count continuations and adjust accordingly.
++sub body_to_file_line {
++ my ($body_lineno, $body_token, $raw_lines, $body_start) = @_;
++ my $body_text = $body_token->[0];
++ my $body_end_line = $body_token->[4];
++ unless ($body_start && $body_start >= 1) {
++ warn "body_start is not a positive integer\n";
++ return $body_lineno;
++ }
++ my $file_lineno = $body_lineno + $body_start - 1;
++ # Only double-quoted bodies have line splices.
++ return $file_lineno unless $body_text =~ /^"/;
++ my $adj = 0;
++ my $lines_seen = 0;
++ unless ($body_end_line && $body_end_line >= $body_start) {
++ warn "body_end_line is not set for double-quoted body\n";
++ return $file_lineno;
++ }
++ my $end = $body_end_line;
++ if ($end > @$raw_lines) {
++ warn "body_end_line ($end) exceeds file length (" .
++ scalar(@$raw_lines) . ")\n";
++ return $file_lineno;
++ }
++ my $src_ln = $body_start;
++ while ($src_ln <= $end && $lines_seen < $body_lineno) {
++ my $line = $raw_lines->[$src_ln - 1];
++ # Odd trailing backslashes = continuation (\<nl>).
++ # Even = escaped backslashes (\\), not a continuation.
++ if ($line =~ /(\\*)$/ && length($1) % 2 == 1) {
++ $adj++;
+ } else {
-+ $lineno = $all_tokens[$ti]->[3]
-+ if !@current && defined $all_tokens[$ti]->[3];
-+ push @current, $text;
++ $lines_seen++;
+ }
++ $src_ln++;
+ }
-+ if (@current) {
-+ push @commands, {
-+ tokens => [@current],
-+ lineno => $lineno,
-+ };
++ if ($lines_seen < $body_lineno) {
++ warn "body_lineno ($body_lineno) not found within body range " .
++ "($body_start..$end)\n";
+ }
-+ return @commands;
++ return $file_lineno + $adj;
+}
+
-+# --- Rule: '! test_grep' should be 'test_grep !' ---
-+# Shell-level negation suppresses test_grep's diagnostic output
-+# on failure. Built-in negation preserves it.
-+sub check_test_grep_negation {
-+ my ($cmd, $file, $line_ref) = @_;
-+ my @tokens = @{$cmd->{tokens}};
-+ return unless @tokens >= 2 && $tokens[0] eq '!' && $tokens[1] eq 'test_grep';
-+
-+ return report_violation($file, $cmd, $line_ref,
-+ qr/!\s*test_grep/, 'test_grep !', '! test_grep');
-+}
-+
-+# Map parsed commands back to raw file lines for --fix.
-+# Detection uses parsed tokens (correct handling of quoting,
-+# heredocs, pipes) but fixes must modify the original text
-+# to preserve formatting.
-+package LintParser;
-+
++# ScriptParser calls this for each test body found in the script.
+sub check_test {
-+ # Called by ScriptParser::parse_cmd for each test_expect_success
-+ # or test_expect_failure block.
+ my $self = shift @_;
+ my $title = ScriptParser::unwrap(shift @_);
-+
-+ # Two test body formats:
-+ # Quoted: test_expect_success 'title' '..body..'
-+ # Heredoc: test_expect_success 'title' - <<\EOF
-+ # ..body..
-+ # EOF
-+ # For quoted, the body token is the quoted string.
-+ # For heredoc, the body token is '-' and the actual
-+ # code arrives as the next argument from the Lexer.
+ my $body_token = shift @_;
-+ my $lineno_base = $body_token->[3] || 1;
++ my $body_start = $body_token->[3];
+ my $body = ScriptParser::unwrap($body_token);
-+
++ # Handle heredoc-style test bodies:
++ # test_expect_success 'title' - <<\EOF
++ # grep pattern file
++ # EOF
++ # The '-' signals that the body follows as a heredoc.
+ if ($body eq '-') {
+ my $herebody = shift @_;
+ if ($herebody) {
+ $body = $herebody->{content};
-+ $lineno_base = $herebody->{start_line} || 1;
++ $body_start = $herebody->{start_line};
+ }
+ }
+ return unless $body;
+
-+ # Map each command back to its file line number.
-+ # $lineno_base is where the body starts in the file;
-+ # $cmd->{lineno} is relative to the body (starting at 1).
+ my $raw_lines = $self->{raw_lines};
-+ for my $cmd (main::parse_commands($body)) {
-+ my $ln = ($cmd->{lineno} || 0) + $lineno_base - 1;
-+ $cmd->{lineno} = $ln;
-+ next unless $ln >= 1 && $ln <= @$raw_lines;
-+ next if $raw_lines->[$ln - 1] =~ /#.*lint-ok/;
+
-+ if (main::check_test_grep_negation($cmd, $self->{file}, \$raw_lines->[$ln - 1])) {
-+ $self->{dirty} = 1;
++ # The outer parser gives us the body as an opaque string.
++ # Parse it to get individual tokens with command boundaries.
++ my $parser = ShellParser->new(\$body);
++ my @tokens = $parser->parse();
++
++ my $file = $self->{file};
++
++ for (my $i = 0; $i < @tokens; $i++) {
++ my $text = $tokens[$i]->[0];
++ next unless is_command_word(\@tokens, $i);
++
++ my $token_lineno = $tokens[$i]->[3];
++ unless (defined($token_lineno) && $token_lineno >= 1) {
++ warn "token has no line number\n";
++ next;
++ }
++ my $file_lineno = body_to_file_line(
++ $token_lineno,
++ $body_token, $raw_lines, $body_start);
++
++ # '!' negates the exit code without consuming command
++ # position. '! test_grep' is an anti-pattern because
++ # test_grep only prints diagnostics on grep failure,
++ # and '!' inverts after that decision is already made.
++ if ($text eq '!') {
++ if ($i + 1 < @tokens &&
++ $tokens[$i + 1]->[0] eq 'test_grep' &&
++ !lint_ok($raw_lines, $file_lineno)) {
++ print "$file:$file_lineno: error: ",
++ 'use "test_grep !" instead of ',
++ '"! test_grep"', "\n";
++ $exit_code = 1;
++ }
++ next;
++ }
++
++ # Bare grep as a command (not a filter) is a test
++ # assertion that should use test_grep for better
++ # failure diagnostics.
++ if ($text eq 'grep' &&
++ !is_filter(\@tokens, $i) &&
++ !lint_ok($raw_lines, $file_lineno)) {
++ print "$file:$file_lineno: error: ",
++ "bare grep outside pipeline ",
++ "(use test_grep)\n";
++ $exit_code = 1;
+ }
+ }
+}
@@ t/lint-style.pl (new)
+package main;
+
+for my $file (@ARGV) {
-+ # :unix:crlf strips \r on Windows (same as chainlint.pl)
+ open(my $fh, '<:unix:crlf', $file) or die "$0: $file: $!\n";
+ my @raw_lines = <$fh>;
+ close $fh;
-+
-+ my $parser = LintParser->new(\join('', @raw_lines));
++ my $s = join('', @raw_lines);
++ my $parser = GrepLintParser->new(\$s);
+ $parser->{file} = $file;
+ $parser->{raw_lines} = \@raw_lines;
-+ $parser->{dirty} = 0;
+ $parser->parse();
-+
-+ if ($fix_mode && $parser->{dirty}) {
-+ open(my $out, '>', $file) or die "$0: $file: $!\n";
-+ print $out @{$parser->{raw_lines}};
-+ close $out;
-+ }
-+}
-+
-+if ($has_fixable && !$fix_mode) {
-+ print "hint: run with --fix to apply the suggested replacements.\n";
+}
+exit $exit_code;
- ## t/lint-style/heredoc.expect (new) ##
+ ## t/greplint/bare-grep-after-and.expect (new) ##
@@
-+lint-style/heredoc.test:8: error: replace '! test_grep' with 'test_grep !': ! test_grep "after-heredoc-is-caught" actual
-+lint-style/heredoc.test:13: error: replace '! test_grep' with 'test_grep !': ! test_grep "not-inside-sed-heredoc" actual
-+hint: run with --fix to apply the suggested replacements.
++3: error: bare grep outside pipeline (use test_grep)
- ## t/lint-style/heredoc.test (new) ##
+ ## t/greplint/bare-grep-after-and.test (new) ##
@@
-+test_expect_success 'greps inside heredocs are skipped' '
-+ cat <<-EOF &&
-+ grep "inside-strip-tabs" file
-+ EOF
-+ cat <<-\EOF &&
-+ grep "inside-no-expand" file
-+ EOF
-+ ! test_grep "after-heredoc-is-caught" actual
++test_expect_success 'grep after && is flagged' '
++ cmd &&
++ grep pattern file
+'
-+
-+test_expect_success 'sed with << does not start a heredoc' '
-+ sed "s/<< foo/bar/" file &&
-+ ! test_grep "not-inside-sed-heredoc" actual
+
+ ## t/greplint/bare-grep-after-semicolon.expect (new) ##
+@@
++3: error: bare grep outside pipeline (use test_grep)
+
+ ## t/greplint/bare-grep-after-semicolon.test (new) ##
+@@
++test_expect_success 'grep after semicolon is flagged' '
++ echo hello;
++ grep pattern file
+'
- ## t/lint-style/test-grep-negation-fix.expect (new) ##
+ ## t/greplint/bare-grep-compound-body.expect (new) ##
@@
-+test_expect_success 'negated test_grep' '
-+ test_grep ! "pattern" actual &&
-+ test_grep ! -i "insensitive" actual
++4: error: bare grep outside pipeline (use test_grep)
++8: error: bare grep outside pipeline (use test_grep)
++15: error: bare grep outside pipeline (use test_grep)
+
+ ## t/greplint/bare-grep-compound-body.test (new) ##
+@@
++test_expect_success 'grep after then/do/else is flagged' '
++ if true
++ then
++ grep pattern file
++ fi &&
++ while true
++ do
++ grep pattern file &&
++ break
++ done &&
++ if true
++ then
++ echo yes
++ else
++ grep pattern file
++ fi
+'
- ## t/lint-style/test-grep-negation-fix.test (new) ##
+ ## t/greplint/bare-grep-count-mode.expect (new) ##
+@@
++2: error: bare grep outside pipeline (use test_grep)
+
+ ## t/greplint/bare-grep-count-mode.test (new) ##
@@
-+test_expect_success 'negated test_grep' '
-+ ! test_grep "pattern" actual &&
-+ ! test_grep -i "insensitive" actual
++test_expect_success 'grep -c is flagged (not special-cased)' '
++ grep -c pattern file
+'
- ## t/lint-style/test-grep-negation.expect (new) ##
+ ## t/greplint/bare-grep-explicit-pattern.expect (new) ##
@@
-+lint-style/test-grep-negation.test:2: error: replace '! test_grep' with 'test_grep !': ! test_grep "pattern" actual
-+lint-style/test-grep-negation.test:3: error: replace '! test_grep' with 'test_grep !': ! test_grep -i "insensitive" actual
-+hint: run with --fix to apply the suggested replacements.
++2: error: bare grep outside pipeline (use test_grep)
- ## t/lint-style/test-grep-negation.test (new) ##
+ ## t/greplint/bare-grep-explicit-pattern.test (new) ##
@@
-+test_expect_success 'negated test_grep' '
-+ ! test_grep "pattern" actual &&
-+ ! test_grep -i "insensitive" actual
++test_expect_success 'grep -e is flagged' '
++ grep -e pattern file
+'
- ## t/t0031-lockfile-pid.sh ##
-@@ t/t0031-lockfile-pid.sh: test_expect_success 'PID info not shown by default' '
- test_must_fail git add . 2>err &&
- # Should not crash, just show normal error without PID
- test_grep "Unable to create" err &&
-- ! test_grep "is held by process" err
-+ test_grep ! "is held by process" err
- )
- '
-
+ ## t/greplint/bare-grep-flags.expect (new) ##
+@@
++2: error: bare grep outside pipeline (use test_grep)
- ## t/t5300-pack-object.sh ##
-@@ t/t5300-pack-object.sh: test_expect_success '--name-hash-version=2 and --write-bitmap-index are incompat
-
- # --stdout option silently removes --write-bitmap-index
- git pack-objects --stdout --all --name-hash-version=2 --write-bitmap-index >out 2>err &&
-- ! test_grep "currently, --write-bitmap-index requires --name-hash-version=1" err
-+ test_grep ! "currently, --write-bitmap-index requires --name-hash-version=1" err
- '
-
- test_expect_success '--path-walk pack everything' '
+ ## t/greplint/bare-grep-flags.test (new) ##
+@@
++test_expect_success 'grep -E is flagged' '
++ grep -E "pat+ern" file
++'
- ## t/t5319-multi-pack-index.sh ##
-@@ t/t5319-multi-pack-index.sh: test_expect_success 'load reverse index when missing .idx, .pack' '
-
- test_expect_success 'usage shown without sub-command' '
- test_expect_code 129 git multi-pack-index 2>err &&
-- ! test_grep "unrecognized subcommand" err
-+ test_grep ! "unrecognized subcommand" err
- '
-
- test_expect_success 'complains when run outside of a repository' '
-
- ## t/t7900-maintenance.sh ##
-@@ t/t7900-maintenance.sh: test_geometric_repack_needed () {
- true)
- test_grep "\[\"git\",\"repack\"," trace2.txt;;
- false)
-- ! test_grep "\[\"git\",\"repack\"," trace2.txt;;
-+ test_grep ! "\[\"git\",\"repack\"," trace2.txt;;
- *)
- BUG "invalid parameter: $NEEDED";;
- esac
+ ## t/greplint/bare-grep-lint-ok.expect (new) ##
+
+ ## t/greplint/bare-grep-lint-ok.test (new) ##
+@@
++test_expect_success 'grep with lint-ok annotation is not flagged' '
++ grep pattern file && # lint-ok
++ echo done
++'
+
+ ## t/greplint/bare-grep-negated.expect (new) ##
+@@
++2: error: bare grep outside pipeline (use test_grep)
+
+ ## t/greplint/bare-grep-negated.test (new) ##
+@@
++test_expect_success 'negated grep is flagged' '
++ ! grep pattern file
++'
+
+ ## t/greplint/bare-grep-pattern-file.expect (new) ##
+@@
++2: error: bare grep outside pipeline (use test_grep)
+
+ ## t/greplint/bare-grep-pattern-file.test (new) ##
+@@
++test_expect_success 'grep -f is flagged' '
++ grep -f patterns.txt file
++'
+
+ ## t/greplint/bare-grep-simple.expect (new) ##
+@@
++2: error: bare grep outside pipeline (use test_grep)
+
+ ## t/greplint/bare-grep-simple.test (new) ##
+@@
++test_expect_success 'bare grep is flagged' '
++ grep pattern file
++'
+
+ ## t/greplint/bare-grep-subshell.expect (new) ##
+@@
++3: error: bare grep outside pipeline (use test_grep)
+
+ ## t/greplint/bare-grep-subshell.test (new) ##
+@@
++test_expect_success 'grep in subshell is flagged' '
++ (
++ grep pattern file
++ )
++'
+
+ ## t/greplint/dqstring-continuation-offset.expect (new) ##
+@@
++10: error: bare grep outside pipeline (use test_grep)
+
+ ## t/greplint/dqstring-continuation-offset.test (new) ##
+@@
++# Double-quoted test bodies with backslash-continuation lines:
++# the splice adjustment in check_test compensates for \<newline>
++# lines that the lexer consumes without emitting into the body
++# text, so the reported line number matches the source.
++test_expect_success 'dqstring continuation offset' "
++ x=\$(echo \
++ hello) &&
++ y=\$(echo \
++ world) &&
++ grep pattern file
++"
+
+ ## t/greplint/filter-command-substitution.expect (new) ##
+
+ ## t/greplint/filter-command-substitution.test (new) ##
+@@
++test_expect_success 'grep in command substitution is not flagged' '
++ x=$(grep pattern file)
++'
+
+ ## t/greplint/filter-pipe-input.expect (new) ##
+
+ ## t/greplint/filter-pipe-input.test (new) ##
+@@
++test_expect_success 'grep receiving pipe input is not flagged' '
++ cmd | grep pattern
++'
+
+ ## t/greplint/filter-pipe-output.expect (new) ##
+
+ ## t/greplint/filter-pipe-output.test (new) ##
+@@
++test_expect_success 'grep piping to another command is not flagged' '
++ grep pattern file | wc -l
++'
+
+ ## t/greplint/filter-redirect-output.expect (new) ##
+
+ ## t/greplint/filter-redirect-output.test (new) ##
+@@
++test_expect_success 'grep with output redirect is not flagged' '
++ grep pattern file >output
++'
+
+ ## t/greplint/filter-stdin-redirect.expect (new) ##
+
+ ## t/greplint/filter-stdin-redirect.test (new) ##
+@@
++test_expect_success 'grep reading from stdin redirect is not flagged' '
++ grep pattern <input
++'
+
+ ## t/greplint/grep-as-argument.expect (new) ##
+
+ ## t/greplint/grep-as-argument.test (new) ##
+@@
++test_expect_success 'grep as argument to another command is not flagged' '
++ test_must_fail grep pattern file
++'
+
+ ## t/greplint/grep-as-value.expect (new) ##
+
+ ## t/greplint/grep-as-value.test (new) ##
+@@
++test_expect_success 'grep as value in for-loop is not flagged' '
++ for cmd in grep sed awk
++ do
++ echo $cmd
++ done
++'
+
+ ## t/greplint/wrong-negation.expect (new) ##
+@@
++2: error: use "test_grep !" instead of "! test_grep"
+
+ ## t/greplint/wrong-negation.test (new) ##
+@@
++test_expect_success 'wrong negation of test_grep is flagged' '
++ ! test_grep pattern file
++'
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 1/6] t/README: document test_grep helper
From: Michael Montalbo via GitGitGadget @ 2026-06-13 4:06 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Eric Sunshine, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>
From: Michael Montalbo <mmontalbo@gmail.com>
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>
---
t/README | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/t/README b/t/README
index adbbd9acf4..c12a1c317a 100644
--- a/t/README
+++ b/t/README
@@ -1039,6 +1039,27 @@ see test-lib-functions.sh for the full list and their options.
Check whether a file has the length it is expected to.
+ - test_grep [!] [<grep-options>] <pattern> <file>
+
+ Check whether <file> contains a line matching <pattern>, or
+ with '!' that no line matches. Use this instead of bare
+ 'grep <pattern> <file>' in test assertions. On failure,
+ test_grep prints the contents of <file> for easier debugging,
+ whereas a bare 'grep' would fail silently.
+
+ For negation, pass '!' as the first argument:
+
+ test_grep ! "^diff --git" actual
+
+ Do not negate by writing '! test_grep', as that suppresses the
+ diagnostic output.
+
+ test_grep should only be used as a test assertion. When grep
+ is used as a data filter (e.g. 'grep -v "^index" actual >filtered')
+ or inside a command substitution (e.g. '$(grep -c ...)'), plain
+ 'grep' is the right choice because the exit code is not the
+ assertion itself.
+
- test_path_is_file <path>
test_path_is_dir <path>
test_path_is_missing <path>
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/6] t: fix grep assertions missing file arguments
From: Michael Montalbo via GitGitGadget @ 2026-06-13 4:06 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Eric Sunshine, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>
From: 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>
---
t/t2402-worktree-list.sh | 2 +-
t/t7507-commit-verbose.sh | 2 +-
t/t7700-repack.sh | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index e0c6abd2f5..93f92e854a 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -144,7 +144,7 @@ test_expect_success '"list" all worktrees with prunable annotation' '
rm -rf prunable &&
git worktree list >out &&
grep "/prunable *[0-9a-f].* prunable$" out &&
- ! grep "/unprunable *[0-9a-f].* prunable$"
+ ! grep "/unprunable *[0-9a-f].* prunable$" out
'
test_expect_success '"list" all worktrees --porcelain with prunable' '
diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh
index b53d71c086..acdb6b1455 100755
--- a/t/t7507-commit-verbose.sh
+++ b/t/t7507-commit-verbose.sh
@@ -163,7 +163,7 @@ done
test_expect_success "status ignores commit.verbose=true" '
git -c commit.verbose=true status >actual &&
- ! grep "^diff --git actual"
+ ! grep "^diff --git" actual
'
test_done
diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index 63ef63fc50..c6ff3aed30 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -194,7 +194,7 @@ test_expect_success 'local packed unreachable obs that exist in alternate ODB ar
rm -f .git/objects/pack/* &&
mv pack-* .git/objects/pack/ &&
git verify-pack -v -- .git/objects/pack/*.idx >packlist &&
- ! grep "^$coid " &&
+ ! grep "^$coid " packlist &&
echo >.git/objects/info/alternates &&
test_must_fail git show $coid
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 3/6] t: extract chainlint's parser into shared module
From: Michael Montalbo via GitGitGadget @ 2026-06-13 4:06 UTC (permalink / raw)
To: git; +Cc: D. Ben Knoble, Eric Sunshine, Michael Montalbo, Michael Montalbo
In-Reply-To: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>
From: 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>
---
t/chainlint.pl | 529 +----------------------------------------
t/lib-shell-parser.pl | 531 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 543 insertions(+), 517 deletions(-)
create mode 100644 t/lib-shell-parser.pl
diff --git a/t/chainlint.pl b/t/chainlint.pl
index f0598e3934..cededf15ee 100755
--- a/t/chainlint.pl
+++ b/t/chainlint.pl
@@ -23,458 +23,9 @@ my $jobs = -1;
my $show_stats;
my $emit_all;
-# Lexer tokenizes POSIX shell scripts. It is roughly modeled after section 2.3
-# "Token Recognition" of POSIX chapter 2 "Shell Command Language". Although
-# similar to lexical analyzers for other languages, this one differs in a few
-# substantial ways due to quirks of the shell command language.
-#
-# For instance, in many languages, newline is just whitespace like space or
-# TAB, but in shell a newline is a command separator, thus a distinct lexical
-# token. A newline is significant and returned as a distinct token even at the
-# end of a shell comment.
-#
-# 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.
-# 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
-# just a plain character in the longer token "foo!uucp". In many other
-# languages, `"string"/foo:'string'` might be scanned as five tokens ("string",
-# `/`, `foo`, `:`, and 'string'), but in shell, it is just a single token.
-#
-# The lexical analyzer for the shell command language is also somewhat unusual
-# in that it recursively invokes the parser to handle the body of `$(...)`
-# expressions which can contain arbitrary shell code. Such expressions may be
-# encountered both inside and outside of double-quoted strings.
-#
-# The lexical analyzer is responsible for consuming shell here-doc bodies which
-# extend from the line following a `<<TAG` operator until a line consisting
-# solely of `TAG`. Here-doc consumption begins when a newline is encountered.
-# It is legal for multiple here-doc `<<TAG` operators to be present on a single
-# line, in which case their bodies must be present one following the next, and
-# are consumed in the (left-to-right) order the `<<TAG` operators appear on the
-# line. A special complication is that the bodies of all here-docs must be
-# consumed when the newline is encountered even if the parse context depth has
-# changed. For instance, in `cat <<A && x=$(cat <<B &&\n`, bodies of here-docs
-# "A" and "B" must be consumed even though "A" was introduced outside the
-# recursive parse context in which "B" was introduced and in which the newline
-# is encountered.
-package Lexer;
-
-sub new {
- my ($class, $parser, $s) = @_;
- bless {
- parser => $parser,
- buff => $s,
- lineno => 1,
- heretags => []
- } => $class;
-}
-
-sub scan_heredoc_tag {
- my $self = shift @_;
- ${$self->{buff}} =~ /\G(-?)/gc;
- my $indented = $1;
- my $token = $self->scan_token();
- return "<<$indented" unless $token;
- my $tag = $token->[0];
- $tag =~ s/['"\\]//g;
- $$token[0] = $indented ? "\t$tag" : "$tag";
- push(@{$self->{heretags}}, $token);
- return "<<$indented$tag";
-}
-
-sub scan_op {
- my ($self, $c) = @_;
- my $b = $self->{buff};
- return $c unless $$b =~ /\G(.)/sgc;
- my $cc = $c . $1;
- return scan_heredoc_tag($self) if $cc eq '<<';
- return $cc if $cc =~ /^(?:&&|\|\||>>|;;|<&|>&|<>|>\|)$/;
- pos($$b)--;
- return $c;
-}
-
-sub scan_sqstring {
- my $self = shift @_;
- ${$self->{buff}} =~ /\G([^']*'|.*\z)/sgc;
- my $s = $1;
- $self->{lineno} += () = $s =~ /\n/sg;
- return "'" . $s;
-}
-
-sub scan_dqstring {
- my $self = shift @_;
- my $b = $self->{buff};
- my $s = '"';
- while (1) {
- # slurp up non-special characters
- $s .= $1 if $$b =~ /\G([^"\$\\]+)/gc;
- # handle special characters
- last unless $$b =~ /\G(.)/sgc;
- my $c = $1;
- $s .= '"', last if $c eq '"';
- $s .= '$' . $self->scan_dollar(), next if $c eq '$';
- if ($c eq '\\') {
- $s .= '\\', last unless $$b =~ /\G(.)/sgc;
- $c = $1;
- $self->{lineno}++, next if $c eq "\n"; # line splice
- # backslash escapes only $, `, ", \ in dq-string
- $s .= '\\' unless $c =~ /^[\$`"\\]$/;
- $s .= $c;
- next;
- }
- die("internal error scanning dq-string '$c'\n");
- }
- $self->{lineno} += () = $s =~ /\n/sg;
- return $s;
-}
-
-sub scan_balanced {
- my ($self, $c1, $c2) = @_;
- my $b = $self->{buff};
- my $depth = 1;
- my $s = $c1;
- while ($$b =~ /\G([^\Q$c1$c2\E]*(?:[\Q$c1$c2\E]|\z))/gc) {
- $s .= $1;
- $depth++, next if $s =~ /\Q$c1\E$/;
- $depth--;
- last if $depth == 0;
- }
- $self->{lineno} += () = $s =~ /\n/sg;
- return $s;
-}
-
-sub scan_subst {
- my $self = shift @_;
- my @tokens = $self->{parser}->parse(qr/^\)$/);
- $self->{parser}->next_token(); # closing ")"
- return @tokens;
-}
-
-sub scan_dollar {
- my $self = shift @_;
- my $b = $self->{buff};
- return $self->scan_balanced('(', ')') if $$b =~ /\G\((?=\()/gc; # $((...))
- return '(' . join(' ', map {$_->[0]} $self->scan_subst()) . ')' if $$b =~ /\G\(/gc; # $(...)
- return $self->scan_balanced('{', '}') if $$b =~ /\G\{/gc; # ${...}
- return $1 if $$b =~ /\G(\w+)/gc; # $var
- return $1 if $$b =~ /\G([@*#?$!0-9-])/gc; # $*, $1, $$, etc.
- return '';
-}
-
-sub swallow_heredocs {
- my $self = shift @_;
- my $b = $self->{buff};
- my $tags = $self->{heretags};
- while (my $tag = shift @$tags) {
- my $start = pos($$b);
- my $indent = $$tag[0] =~ s/^\t// ? '\\s*' : '';
- $$b =~ /(?:\G|\n)$indent\Q$$tag[0]\E(?:\n|\z)/gc;
- if (pos($$b) > $start) {
- my $body = substr($$b, $start, pos($$b) - $start);
- $self->{parser}->{heredocs}->{$$tag[0]} = {
- content => substr($body, 0, length($body) - length($&)),
- start_line => $self->{lineno},
- };
- $self->{lineno} += () = $body =~ /\n/sg;
- next;
- }
- push(@{$self->{parser}->{problems}}, ['HEREDOC', $tag]);
- $$b =~ /(?:\G|\n).*\z/gc; # consume rest of input
- my $body = substr($$b, $start, pos($$b) - $start);
- $self->{lineno} += () = $body =~ /\n/sg;
- last;
- }
-}
-
-sub scan_token {
- my $self = shift @_;
- my $b = $self->{buff};
- my $token = '';
- my ($start, $startln);
-RESTART:
- $startln = $self->{lineno};
- $$b =~ /\G[ \t]+/gc; # skip whitespace (but not newline)
- $start = pos($$b) || 0;
- $self->{lineno}++, return ["\n", $start, pos($$b), $startln, $startln] if $$b =~ /\G#[^\n]*(?:\n|\z)/gc; # comment
- while (1) {
- # slurp up non-special characters
- $token .= $1 if $$b =~ /\G([^\\;&|<>(){}'"\$\s]+)/gc;
- # handle special characters
- last unless $$b =~ /\G(.)/sgc;
- my $c = $1;
- pos($$b)--, last if $c =~ /^[ \t]$/; # whitespace ends token
- pos($$b)--, last if length($token) && $c =~ /^[;&|<>(){}\n]$/;
- $token .= $self->scan_sqstring(), next if $c eq "'";
- $token .= $self->scan_dqstring(), next if $c eq '"';
- $token .= $c . $self->scan_dollar(), next if $c eq '$';
- $self->{lineno}++, $self->swallow_heredocs(), $token = $c, last if $c eq "\n";
- $token = $self->scan_op($c), last if $c =~ /^[;&|<>]$/;
- $token = $c, last if $c =~ /^[(){}]$/;
- if ($c eq '\\') {
- $token .= '\\', last unless $$b =~ /\G(.)/sgc;
- $c = $1;
- $self->{lineno}++, next if $c eq "\n" && length($token); # line splice
- $self->{lineno}++, goto RESTART if $c eq "\n"; # line splice
- $token .= '\\' . $c;
- next;
- }
- die("internal error scanning character '$c'\n");
- }
- return length($token) ? [$token, $start, pos($$b), $startln, $self->{lineno}] : undef;
-}
-
-# ShellParser parses POSIX shell scripts (with minor extensions for Bash). It
-# is a recursive descent parser very roughly modeled after section 2.10 "Shell
-# Grammar" of POSIX chapter 2 "Shell Command Language".
-package ShellParser;
-
-sub new {
- my ($class, $s) = @_;
- my $self = bless {
- buff => [],
- stop => [],
- output => [],
- heredocs => {},
- insubshell => 0,
- } => $class;
- $self->{lexer} = Lexer->new($self, $s);
- return $self;
-}
-
-sub next_token {
- my $self = shift @_;
- return pop(@{$self->{buff}}) if @{$self->{buff}};
- return $self->{lexer}->scan_token();
-}
-
-sub untoken {
- my $self = shift @_;
- push(@{$self->{buff}}, @_);
-}
-
-sub peek {
- my $self = shift @_;
- my $token = $self->next_token();
- return undef unless defined($token);
- $self->untoken($token);
- return $token;
-}
-
-sub stop_at {
- my ($self, $token) = @_;
- return 1 unless defined($token);
- my $stop = ${$self->{stop}}[-1] if @{$self->{stop}};
- return defined($stop) && $token->[0] =~ $stop;
-}
-
-sub expect {
- my ($self, $expect) = @_;
- my $token = $self->next_token();
- return $token if defined($token) && $token->[0] eq $expect;
- push(@{$self->{output}}, "?!ERR?! expected '$expect' but found '" . (defined($token) ? $token->[0] : "<end-of-input>") . "'\n");
- $self->untoken($token) if defined($token);
- return ();
-}
-
-sub optional_newlines {
- my $self = shift @_;
- my @tokens;
- while (my $token = $self->peek()) {
- last unless $token->[0] eq "\n";
- push(@tokens, $self->next_token());
- }
- return @tokens;
-}
-
-sub parse_group {
- my $self = shift @_;
- return ($self->parse(qr/^}$/),
- $self->expect('}'));
-}
-
-sub parse_subshell {
- my $self = shift @_;
- $self->{insubshell}++;
- my @tokens = ($self->parse(qr/^\)$/),
- $self->expect(')'));
- $self->{insubshell}--;
- return @tokens;
-}
-
-sub parse_case_pattern {
- my $self = shift @_;
- my @tokens;
- while (defined(my $token = $self->next_token())) {
- push(@tokens, $token);
- last if $token->[0] eq ')';
- }
- return @tokens;
-}
-
-sub parse_case {
- my $self = shift @_;
- my @tokens;
- push(@tokens,
- $self->next_token(), # subject
- $self->optional_newlines(),
- $self->expect('in'),
- $self->optional_newlines());
- while (1) {
- my $token = $self->peek();
- last unless defined($token) && $token->[0] ne 'esac';
- push(@tokens,
- $self->parse_case_pattern(),
- $self->optional_newlines(),
- $self->parse(qr/^(?:;;|esac)$/)); # item body
- $token = $self->peek();
- last unless defined($token) && $token->[0] ne 'esac';
- push(@tokens,
- $self->expect(';;'),
- $self->optional_newlines());
- }
- push(@tokens, $self->expect('esac'));
- return @tokens;
-}
-
-sub parse_for {
- my $self = shift @_;
- my @tokens;
- push(@tokens,
- $self->next_token(), # variable
- $self->optional_newlines());
- my $token = $self->peek();
- if (defined($token) && $token->[0] eq 'in') {
- push(@tokens,
- $self->expect('in'),
- $self->optional_newlines());
- }
- push(@tokens,
- $self->parse(qr/^do$/), # items
- $self->expect('do'),
- $self->optional_newlines(),
- $self->parse_loop_body(),
- $self->expect('done'));
- return @tokens;
-}
-
-sub parse_if {
- my $self = shift @_;
- my @tokens;
- while (1) {
- push(@tokens,
- $self->parse(qr/^then$/), # if/elif condition
- $self->expect('then'),
- $self->optional_newlines(),
- $self->parse(qr/^(?:elif|else|fi)$/)); # if/elif body
- my $token = $self->peek();
- last unless defined($token) && $token->[0] eq 'elif';
- push(@tokens, $self->expect('elif'));
- }
- my $token = $self->peek();
- if (defined($token) && $token->[0] eq 'else') {
- push(@tokens,
- $self->expect('else'),
- $self->optional_newlines(),
- $self->parse(qr/^fi$/)); # else body
- }
- push(@tokens, $self->expect('fi'));
- return @tokens;
-}
-
-sub parse_loop_body {
- my $self = shift @_;
- return $self->parse(qr/^done$/);
-}
-
-sub parse_loop {
- my $self = shift @_;
- return ($self->parse(qr/^do$/), # condition
- $self->expect('do'),
- $self->optional_newlines(),
- $self->parse_loop_body(),
- $self->expect('done'));
-}
-
-sub parse_func {
- my $self = shift @_;
- return ($self->expect('('),
- $self->expect(')'),
- $self->optional_newlines(),
- $self->parse_cmd()); # body
-}
-
-sub parse_bash_array_assignment {
- my $self = shift @_;
- my @tokens = $self->expect('(');
- while (defined(my $token = $self->next_token())) {
- push(@tokens, $token);
- last if $token->[0] eq ')';
- }
- return @tokens;
-}
-
-my %compound = (
- '{' => \&parse_group,
- '(' => \&parse_subshell,
- 'case' => \&parse_case,
- 'for' => \&parse_for,
- 'if' => \&parse_if,
- 'until' => \&parse_loop,
- 'while' => \&parse_loop);
-
-sub parse_cmd {
- my $self = shift @_;
- my $cmd = $self->next_token();
- return () unless defined($cmd);
- return $cmd if $cmd->[0] eq "\n";
-
- my $token;
- my @tokens = $cmd;
- if ($cmd->[0] eq '!') {
- push(@tokens, $self->parse_cmd());
- return @tokens;
- } elsif (my $f = $compound{$cmd->[0]}) {
- push(@tokens, $self->$f());
- } elsif (defined($token = $self->peek()) && $token->[0] eq '(') {
- if ($cmd->[0] !~ /\w=$/) {
- push(@tokens, $self->parse_func());
- return @tokens;
- }
- my @array = $self->parse_bash_array_assignment();
- $tokens[-1]->[0] .= join(' ', map {$_->[0]} @array);
- $tokens[-1]->[2] = $array[$#array][2] if @array;
- }
-
- while (defined(my $token = $self->next_token())) {
- $self->untoken($token), last if $self->stop_at($token);
- push(@tokens, $token);
- last if $token->[0] =~ /^(?:[;&\n|]|&&|\|\|)$/;
- }
- push(@tokens, $self->next_token()) if $tokens[-1]->[0] ne "\n" && defined($token = $self->peek()) && $token->[0] eq "\n";
- return @tokens;
-}
-
-sub accumulate {
- my ($self, $tokens, $cmd) = @_;
- push(@$tokens, @$cmd);
-}
-
-sub parse {
- my ($self, $stop) = @_;
- push(@{$self->{stop}}, $stop);
- goto DONE if $self->stop_at($self->peek());
- my @tokens;
- while (my @cmd = $self->parse_cmd()) {
- $self->accumulate(\@tokens, \@cmd);
- last if $self->stop_at($self->peek());
- }
-DONE:
- pop(@{$self->{stop}});
- return @tokens;
-}
+use File::Basename;
+do(dirname($0) . "/lib-shell-parser.pl")
+ or die "$0: failed to load lib-shell-parser.pl: $@$!\n";
# TestParser is a subclass of ShellParser which, beyond parsing shell script
# code, is also imbued with semantic knowledge of test construction, and checks
@@ -482,9 +33,10 @@ DONE:
# the tests themselves or in behaviors being exercised by the tests. As such,
# TestParser is only called upon to parse test bodies, not the top-level
# scripts in which the tests are defined.
+
package TestParser;
-use base 'ShellParser';
+our @ISA = ('ShellParser');
sub new {
my $class = shift @_;
@@ -578,51 +130,10 @@ DONE:
$self->SUPER::accumulate($tokens, $cmd);
}
-# ScriptParser is a subclass of ShellParser which identifies individual test
-# definitions within test scripts, and passes each test body through TestParser
-# to identify possible problems. ShellParser detects test definitions not only
-# at the top-level of test scripts but also within compound commands such as
-# loops and function definitions.
-package ScriptParser;
-
-use base 'ShellParser';
-
-sub new {
- my $class = shift @_;
- my $self = $class->SUPER::new(@_);
- $self->{ntests} = 0;
- $self->{nerrs} = 0;
- return $self;
-}
+# ChainlintParser extends ScriptParser with &&-chain checking
+package ChainlintParser;
-# extract the raw content of a token, which may be a single string or a
-# composition of multiple strings and non-string character runs; for instance,
-# `"test body"` unwraps to `test body`; `word"a b"42'c d'` to `worda b42c d`
-sub unwrap {
- my $token = (@_ ? shift @_ : $_)->[0];
- # simple case: 'sqstring' or "dqstring"
- return $token if $token =~ s/^'([^']*)'$/$1/;
- return $token if $token =~ s/^"([^"]*)"$/$1/;
-
- # composite case
- my ($s, $q, $escaped);
- while (1) {
- # slurp up non-special characters
- $s .= $1 if $token =~ /\G([^\\'"]*)/gc;
- # handle special characters
- last unless $token =~ /\G(.)/sgc;
- my $c = $1;
- $q = undef, next if defined($q) && $c eq $q;
- $q = $c, next if !defined($q) && $c =~ /^['"]$/;
- if ($c eq '\\') {
- last unless $token =~ /\G(.)/sgc;
- $c = $1;
- $s .= '\\' if $c eq "\n"; # preserve line splice
- }
- $s .= $c;
- }
- return $s
-}
+our @ISA = ('ScriptParser');
sub format_problem {
local $_ = shift;
@@ -635,10 +146,10 @@ sub format_problem {
sub check_test {
my $self = shift @_;
- my $title = unwrap(shift @_);
+ my $title = ScriptParser::unwrap(shift @_);
my $body = shift @_;
my $lineno = $body->[3];
- $body = unwrap($body);
+ $body = ScriptParser::unwrap($body);
if ($body eq '-') {
my $herebody = shift @_;
$body = $herebody->{content};
@@ -673,24 +184,8 @@ sub check_test {
push(@{$self->{output}}, "$c->{blue}# chainlint: $title$c->{reset}\n$checked");
}
-sub parse_cmd {
- my $self = shift @_;
- my @tokens = $self->SUPER::parse_cmd();
- return @tokens unless @tokens && $tokens[0]->[0] =~ /^test_expect_(?:success|failure)$/;
- my $n = $#tokens;
- $n-- while $n >= 0 && $tokens[$n]->[0] =~ /^(?:[;&\n|]|&&|\|\|)$/;
- my $herebody;
- if ($n >= 2 && $tokens[$n-1]->[0] eq '-' && $tokens[$n]->[0] =~ /^<<-?(.+)$/) {
- $herebody = $self->{heredocs}->{$1};
- $n--;
- }
- $self->check_test($tokens[1], $tokens[2], $herebody) if $n == 2; # title body
- $self->check_test($tokens[2], $tokens[3], $herebody) if $n > 2; # prereq title body
- return @tokens;
-}
-
# main contains high-level functionality for processing command-line switches,
-# feeding input test scripts to ScriptParser, and reporting results.
+# feeding input test scripts to ChainlintParser, and reporting results.
package main;
my $getnow = sub { return time(); };
@@ -803,7 +298,7 @@ sub check_script {
}
my $s = do { local $/; <$fh> };
close($fh);
- my $parser = ScriptParser->new(\$s);
+ my $parser = ChainlintParser->new(\$s);
1 while $parser->parse_cmd();
if (@{$parser->{output}}) {
my $c = fd_colors(1);
diff --git a/t/lib-shell-parser.pl b/t/lib-shell-parser.pl
new file mode 100644
index 0000000000..edbfee0839
--- /dev/null
+++ b/t/lib-shell-parser.pl
@@ -0,0 +1,531 @@
+# Copyright (c) 2021-2022 Eric Sunshine <sunshine@sunshineco.com>
+#
+# Shared shell script parser for test lint tools. Provides Lexer,
+# ShellParser, and ScriptParser. Subclass ScriptParser and override
+# check_test() to implement lint checks.
+
+use strict;
+use warnings;
+
+# Lexer tokenizes POSIX shell scripts. It is roughly modeled after section 2.3
+# "Token Recognition" of POSIX chapter 2 "Shell Command Language". Although
+# similar to lexical analyzers for other languages, this one differs in a few
+# substantial ways due to quirks of the shell command language.
+#
+# For instance, in many languages, newline is just whitespace like space or
+# TAB, but in shell a newline is a command separator, thus a distinct lexical
+# token. A newline is significant and returned as a distinct token even at the
+# end of a shell comment.
+#
+# 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.
+# 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
+# just a plain character in the longer token "foo!uucp". In many other
+# languages, `"string"/foo:'string'` might be scanned as five tokens ("string",
+# `/`, `foo`, `:`, and 'string'), but in shell, it is just a single token.
+#
+# The lexical analyzer for the shell command language is also somewhat unusual
+# in that it recursively invokes the parser to handle the body of `$(...)`
+# expressions which can contain arbitrary shell code. Such expressions may be
+# encountered both inside and outside of double-quoted strings.
+#
+# The lexical analyzer is responsible for consuming shell here-doc bodies which
+# extend from the line following a `<<TAG` operator until a line consisting
+# solely of `TAG`. Here-doc consumption begins when a newline is encountered.
+# It is legal for multiple here-doc `<<TAG` operators to be present on a single
+# line, in which case their bodies must be present one following the next, and
+# are consumed in the (left-to-right) order the `<<TAG` operators appear on the
+# line. A special complication is that the bodies of all here-docs must be
+# consumed when the newline is encountered even if the parse context depth has
+# changed. For instance, in `cat <<A && x=$(cat <<B &&\n`, bodies of here-docs
+# "A" and "B" must be consumed even though "A" was introduced outside the
+# recursive parse context in which "B" was introduced and in which the newline
+# is encountered.
+package Lexer;
+
+sub new {
+ my ($class, $parser, $s) = @_;
+ bless {
+ parser => $parser,
+ buff => $s,
+ lineno => 1,
+ heretags => []
+ } => $class;
+}
+
+sub scan_heredoc_tag {
+ my $self = shift @_;
+ ${$self->{buff}} =~ /\G(-?)/gc;
+ my $indented = $1;
+ my $token = $self->scan_token();
+ return "<<$indented" unless $token;
+ my $tag = $token->[0];
+ $tag =~ s/['"\\]//g;
+ $$token[0] = $indented ? "\t$tag" : "$tag";
+ push(@{$self->{heretags}}, $token);
+ return "<<$indented$tag";
+}
+
+sub scan_op {
+ my ($self, $c) = @_;
+ my $b = $self->{buff};
+ return $c unless $$b =~ /\G(.)/sgc;
+ my $cc = $c . $1;
+ return scan_heredoc_tag($self) if $cc eq '<<';
+ return $cc if $cc =~ /^(?:&&|\|\||>>|;;|<&|>&|<>|>\|)$/;
+ pos($$b)--;
+ return $c;
+}
+
+sub scan_sqstring {
+ my $self = shift @_;
+ ${$self->{buff}} =~ /\G([^']*'|.*\z)/sgc;
+ my $s = $1;
+ $self->{lineno} += () = $s =~ /\n/sg;
+ return "'" . $s;
+}
+
+sub scan_dqstring {
+ my $self = shift @_;
+ my $b = $self->{buff};
+ my $s = '"';
+ while (1) {
+ # slurp up non-special characters
+ $s .= $1 if $$b =~ /\G([^"\$\\]+)/gc;
+ # handle special characters
+ last unless $$b =~ /\G(.)/sgc;
+ my $c = $1;
+ $s .= '"', last if $c eq '"';
+ $s .= '$' . $self->scan_dollar(), next if $c eq '$';
+ if ($c eq '\\') {
+ $s .= '\\', last unless $$b =~ /\G(.)/sgc;
+ $c = $1;
+ $self->{lineno}++, next if $c eq "\n"; # line splice
+ # backslash escapes only $, `, ", \ in dq-string
+ $s .= '\\' unless $c =~ /^[\$`"\\]$/;
+ $s .= $c;
+ next;
+ }
+ die("internal error scanning dq-string '$c'\n");
+ }
+ $self->{lineno} += () = $s =~ /\n/sg;
+ return $s;
+}
+
+sub scan_balanced {
+ my ($self, $c1, $c2) = @_;
+ my $b = $self->{buff};
+ my $depth = 1;
+ my $s = $c1;
+ while ($$b =~ /\G([^\Q$c1$c2\E]*(?:[\Q$c1$c2\E]|\z))/gc) {
+ $s .= $1;
+ $depth++, next if $s =~ /\Q$c1\E$/;
+ $depth--;
+ last if $depth == 0;
+ }
+ $self->{lineno} += () = $s =~ /\n/sg;
+ return $s;
+}
+
+sub scan_subst {
+ my $self = shift @_;
+ my @tokens = $self->{parser}->parse(qr/^\)$/);
+ $self->{parser}->next_token(); # closing ")"
+ return @tokens;
+}
+
+sub scan_dollar {
+ my $self = shift @_;
+ my $b = $self->{buff};
+ return $self->scan_balanced('(', ')') if $$b =~ /\G\((?=\()/gc; # $((...))
+ return '(' . join(' ', map {$_->[0]} $self->scan_subst()) . ')' if $$b =~ /\G\(/gc; # $(...)
+ return $self->scan_balanced('{', '}') if $$b =~ /\G\{/gc; # ${...}
+ return $1 if $$b =~ /\G(\w+)/gc; # $var
+ return $1 if $$b =~ /\G([@*#?$!0-9-])/gc; # $*, $1, $$, etc.
+ return '';
+}
+
+sub swallow_heredocs {
+ my $self = shift @_;
+ my $b = $self->{buff};
+ my $tags = $self->{heretags};
+ while (my $tag = shift @$tags) {
+ my $start = pos($$b);
+ my $indent = $$tag[0] =~ s/^\t// ? '\\s*' : '';
+ $$b =~ /(?:\G|\n)$indent\Q$$tag[0]\E(?:\n|\z)/gc;
+ if (pos($$b) > $start) {
+ my $body = substr($$b, $start, pos($$b) - $start);
+ $self->{parser}->{heredocs}->{$$tag[0]} = {
+ content => substr($body, 0, length($body) - length($&)),
+ start_line => $self->{lineno},
+ };
+ $self->{lineno} += () = $body =~ /\n/sg;
+ next;
+ }
+ push(@{$self->{parser}->{problems}}, ['HEREDOC', $tag]);
+ $$b =~ /(?:\G|\n).*\z/gc; # consume rest of input
+ my $body = substr($$b, $start, pos($$b) - $start);
+ $self->{lineno} += () = $body =~ /\n/sg;
+ last;
+ }
+}
+
+sub scan_token {
+ my $self = shift @_;
+ my $b = $self->{buff};
+ my $token = '';
+ my ($start, $startln);
+RESTART:
+ $startln = $self->{lineno};
+ $$b =~ /\G[ \t]+/gc; # skip whitespace (but not newline)
+ $start = pos($$b) || 0;
+ $self->{lineno}++, return ["\n", $start, pos($$b), $startln, $startln] if $$b =~ /\G#[^\n]*(?:\n|\z)/gc; # comment
+ while (1) {
+ # slurp up non-special characters
+ $token .= $1 if $$b =~ /\G([^\\;&|<>(){}'"\$\s]+)/gc;
+ # handle special characters
+ last unless $$b =~ /\G(.)/sgc;
+ my $c = $1;
+ pos($$b)--, last if $c =~ /^[ \t]$/; # whitespace ends token
+ pos($$b)--, last if length($token) && $c =~ /^[;&|<>(){}\n]$/;
+ $token .= $self->scan_sqstring(), next if $c eq "'";
+ $token .= $self->scan_dqstring(), next if $c eq '"';
+ $token .= $c . $self->scan_dollar(), next if $c eq '$';
+ $self->{lineno}++, $self->swallow_heredocs(), $token = $c, last if $c eq "\n";
+ $token = $self->scan_op($c), last if $c =~ /^[;&|<>]$/;
+ $token = $c, last if $c =~ /^[(){}]$/;
+ if ($c eq '\\') {
+ $token .= '\\', last unless $$b =~ /\G(.)/sgc;
+ $c = $1;
+ $self->{lineno}++, next if $c eq "\n" && length($token); # line splice
+ $self->{lineno}++, goto RESTART if $c eq "\n"; # line splice
+ $token .= '\\' . $c;
+ next;
+ }
+ die("internal error scanning character '$c'\n");
+ }
+ return length($token) ? [$token, $start, pos($$b), $startln, $self->{lineno}] : undef;
+}
+
+# ShellParser parses POSIX shell scripts (with minor extensions for Bash). It
+# is a recursive descent parser very roughly modeled after section 2.10 "Shell
+# Grammar" of POSIX chapter 2 "Shell Command Language".
+
+package ShellParser;
+
+sub new {
+ my ($class, $s) = @_;
+ my $self = bless {
+ buff => [],
+ stop => [],
+ output => [],
+ heredocs => {},
+ insubshell => 0,
+ } => $class;
+ $self->{lexer} = Lexer->new($self, $s);
+ return $self;
+}
+
+sub next_token {
+ my $self = shift @_;
+ return pop(@{$self->{buff}}) if @{$self->{buff}};
+ return $self->{lexer}->scan_token();
+}
+
+sub untoken {
+ my $self = shift @_;
+ push(@{$self->{buff}}, @_);
+}
+
+sub peek {
+ my $self = shift @_;
+ my $token = $self->next_token();
+ return undef unless defined($token);
+ $self->untoken($token);
+ return $token;
+}
+
+sub stop_at {
+ my ($self, $token) = @_;
+ return 1 unless defined($token);
+ my $stop = ${$self->{stop}}[-1] if @{$self->{stop}};
+ return defined($stop) && $token->[0] =~ $stop;
+}
+
+sub expect {
+ my ($self, $expect) = @_;
+ my $token = $self->next_token();
+ return $token if defined($token) && $token->[0] eq $expect;
+ push(@{$self->{output}}, "?!ERR?! expected '$expect' but found '" . (defined($token) ? $token->[0] : "<end-of-input>") . "'\n");
+ $self->untoken($token) if defined($token);
+ return ();
+}
+
+sub optional_newlines {
+ my $self = shift @_;
+ my @tokens;
+ while (my $token = $self->peek()) {
+ last unless $token->[0] eq "\n";
+ push(@tokens, $self->next_token());
+ }
+ return @tokens;
+}
+
+sub parse_group {
+ my $self = shift @_;
+ return ($self->parse(qr/^}$/),
+ $self->expect('}'));
+}
+
+sub parse_subshell {
+ my $self = shift @_;
+ $self->{insubshell}++;
+ my @tokens = ($self->parse(qr/^\)$/),
+ $self->expect(')'));
+ $self->{insubshell}--;
+ return @tokens;
+}
+
+sub parse_case_pattern {
+ my $self = shift @_;
+ my @tokens;
+ while (defined(my $token = $self->next_token())) {
+ push(@tokens, $token);
+ last if $token->[0] eq ')';
+ }
+ return @tokens;
+}
+
+sub parse_case {
+ my $self = shift @_;
+ my @tokens;
+ push(@tokens,
+ $self->next_token(), # subject
+ $self->optional_newlines(),
+ $self->expect('in'),
+ $self->optional_newlines());
+ while (1) {
+ my $token = $self->peek();
+ last unless defined($token) && $token->[0] ne 'esac';
+ push(@tokens,
+ $self->parse_case_pattern(),
+ $self->optional_newlines(),
+ $self->parse(qr/^(?:;;|esac)$/)); # item body
+ $token = $self->peek();
+ last unless defined($token) && $token->[0] ne 'esac';
+ push(@tokens,
+ $self->expect(';;'),
+ $self->optional_newlines());
+ }
+ push(@tokens, $self->expect('esac'));
+ return @tokens;
+}
+
+sub parse_for {
+ my $self = shift @_;
+ my @tokens;
+ push(@tokens,
+ $self->next_token(), # variable
+ $self->optional_newlines());
+ my $token = $self->peek();
+ if (defined($token) && $token->[0] eq 'in') {
+ push(@tokens,
+ $self->expect('in'),
+ $self->optional_newlines());
+ }
+ push(@tokens,
+ $self->parse(qr/^do$/), # items
+ $self->expect('do'),
+ $self->optional_newlines(),
+ $self->parse_loop_body(),
+ $self->expect('done'));
+ return @tokens;
+}
+
+sub parse_if {
+ my $self = shift @_;
+ my @tokens;
+ while (1) {
+ push(@tokens,
+ $self->parse(qr/^then$/), # if/elif condition
+ $self->expect('then'),
+ $self->optional_newlines(),
+ $self->parse(qr/^(?:elif|else|fi)$/)); # if/elif body
+ my $token = $self->peek();
+ last unless defined($token) && $token->[0] eq 'elif';
+ push(@tokens, $self->expect('elif'));
+ }
+ my $token = $self->peek();
+ if (defined($token) && $token->[0] eq 'else') {
+ push(@tokens,
+ $self->expect('else'),
+ $self->optional_newlines(),
+ $self->parse(qr/^fi$/)); # else body
+ }
+ push(@tokens, $self->expect('fi'));
+ return @tokens;
+}
+
+sub parse_loop_body {
+ my $self = shift @_;
+ return $self->parse(qr/^done$/);
+}
+
+sub parse_loop {
+ my $self = shift @_;
+ return ($self->parse(qr/^do$/), # condition
+ $self->expect('do'),
+ $self->optional_newlines(),
+ $self->parse_loop_body(),
+ $self->expect('done'));
+}
+
+sub parse_func {
+ my $self = shift @_;
+ return ($self->expect('('),
+ $self->expect(')'),
+ $self->optional_newlines(),
+ $self->parse_cmd()); # body
+}
+
+sub parse_bash_array_assignment {
+ my $self = shift @_;
+ my @tokens = $self->expect('(');
+ while (defined(my $token = $self->next_token())) {
+ push(@tokens, $token);
+ last if $token->[0] eq ')';
+ }
+ return @tokens;
+}
+
+my %compound = (
+ '{' => \&parse_group,
+ '(' => \&parse_subshell,
+ 'case' => \&parse_case,
+ 'for' => \&parse_for,
+ 'if' => \&parse_if,
+ 'until' => \&parse_loop,
+ 'while' => \&parse_loop);
+
+sub parse_cmd {
+ my $self = shift @_;
+ my $cmd = $self->next_token();
+ return () unless defined($cmd);
+ return $cmd if $cmd->[0] eq "\n";
+
+ my $token;
+ my @tokens = $cmd;
+ if ($cmd->[0] eq '!') {
+ push(@tokens, $self->parse_cmd());
+ return @tokens;
+ } elsif (my $f = $compound{$cmd->[0]}) {
+ push(@tokens, $self->$f());
+ } elsif (defined($token = $self->peek()) && $token->[0] eq '(') {
+ if ($cmd->[0] !~ /\w=$/) {
+ push(@tokens, $self->parse_func());
+ return @tokens;
+ }
+ my @array = $self->parse_bash_array_assignment();
+ $tokens[-1]->[0] .= join(' ', map {$_->[0]} @array);
+ $tokens[-1]->[2] = $array[$#array][2] if @array;
+ }
+
+ while (defined(my $token = $self->next_token())) {
+ $self->untoken($token), last if $self->stop_at($token);
+ push(@tokens, $token);
+ last if $token->[0] =~ /^(?:[;&\n|]|&&|\|\|)$/;
+ }
+ push(@tokens, $self->next_token()) if $tokens[-1]->[0] ne "\n" && defined($token = $self->peek()) && $token->[0] eq "\n";
+ return @tokens;
+}
+
+sub accumulate {
+ my ($self, $tokens, $cmd) = @_;
+ push(@$tokens, @$cmd);
+}
+
+sub parse {
+ my ($self, $stop) = @_;
+ push(@{$self->{stop}}, $stop);
+ goto DONE if $self->stop_at($self->peek());
+ my @tokens;
+ while (my @cmd = $self->parse_cmd()) {
+ $self->accumulate(\@tokens, \@cmd);
+ last if $self->stop_at($self->peek());
+ }
+DONE:
+ pop(@{$self->{stop}});
+ return @tokens;
+}
+
+# ScriptParser is a subclass of ShellParser which identifies individual test
+# definitions within test scripts and passes each test body to check_test().
+# ScriptParser detects test definitions not only at the top-level of test
+# scripts but also within compound commands such as loops and function
+# definitions.
+
+package ScriptParser;
+
+our @ISA = ('ShellParser');
+
+sub new {
+ my $class = shift @_;
+ my $self = $class->SUPER::new(@_);
+ $self->{ntests} = 0;
+ $self->{nerrs} = 0;
+ return $self;
+}
+
+# extract the raw content of a token, which may be a single string or a
+# composition of multiple strings and non-string character runs; for instance,
+# `"test body"` unwraps to `test body`; `word"a b"42'c d'` to `worda b42c d`
+sub unwrap {
+ my $token = (@_ ? shift @_ : $_)->[0];
+ # simple case: 'sqstring' or "dqstring"
+ return $token if $token =~ s/^'([^']*)'$/$1/;
+ return $token if $token =~ s/^"([^"]*)"$/$1/;
+
+ # composite case
+ my ($s, $q, $escaped);
+ while (1) {
+ # slurp up non-special characters
+ $s .= $1 if $token =~ /\G([^\\'"]*)/gc;
+ # handle special characters
+ last unless $token =~ /\G(.)/sgc;
+ my $c = $1;
+ $q = undef, next if defined($q) && $c eq $q;
+ $q = $c, next if !defined($q) && $c =~ /^['"]$/;
+ if ($c eq '\\') {
+ last unless $token =~ /\G(.)/sgc;
+ $c = $1;
+ $s .= '\\' if $c eq "\n"; # preserve line splice
+ }
+ $s .= $c;
+ }
+ return $s
+}
+
+sub check_test {
+ # no-op; subclass and override to implement lint checks
+}
+
+sub parse_cmd {
+ my $self = shift @_;
+ my @tokens = $self->SUPER::parse_cmd();
+ return @tokens unless @tokens && $tokens[0]->[0] =~ /^test_expect_(?:success|failure)$/;
+ my $n = $#tokens;
+ $n-- while $n >= 0 && $tokens[$n]->[0] =~ /^(?:[;&\n|]|&&|\|\|)$/;
+ my $herebody;
+ if ($n >= 2 && $tokens[$n-1]->[0] eq '-' && $tokens[$n]->[0] =~ /^<<-?(.+)$/) {
+ $herebody = $self->{heredocs}->{$1};
+ $n--;
+ }
+ $self->check_test($tokens[1], $tokens[2], $herebody) if $n == 2; # title body
+ $self->check_test($tokens[2], $tokens[3], $herebody) if $n > 2; # prereq title body
+ return @tokens;
+}
+
+1;
--
gitgitgadget
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox