* [PATCH v5 07/13] path-walk: support blob size limit filter
From: Derrick Stolee via GitGitGadget @ 2026-05-22 18:24 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2101.v5.git.1779474277.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Extend the path-walk API to handle the 'blob:limit=<size>' object
filter natively. This filter omits blobs whose size is equal to or
greater than the given limit, matching the semantics used by the
list-objects-filter machinery.
When revs->filter.choice is LOFC_BLOB_LIMIT, the prepare_filters()
method stores the limit value in info->blob_limit and clears the filter
from revs. If the limit is zero, this degenerates to blob:none (all
blobs excluded), so info->blobs is set to 0 instead.
During walk_path(), blob batches are filtered before being delivered to
the callback: each blob's size is checked via odb_read_object_info(),
and only blobs strictly smaller than the limit are included. Blobs whose
size cannot be determined (e.g. missing in a partial clone) are
conservatively included, matching the existing filter behavior. Empty
batches after filtering are skipped entirely.
The check for inclusion in the path batch looks a little strange at
first glance. We use odb_read_object_info() to read the object's size.
Based on all of the assumptions to this point, this _should_ return
OBJ_BLOB. Since we are focused on the size filter, we use a
short-circuited OR (||) to skip the size check if that method returns a
different object type.
Notice that this inspection of object sizes requires the content to be
present in the repository. The odb_read_object_info() call will download
a missing blob on-demand. This means that the use of the path-walk API
within 'git backfill' would not operate nicely with this filter type.
The intention of that command is to download missing blobs in batches.
Downloading objects one-by-one would go against the point. Update the
validation in 'git backfill' to add its own compatibility check on top
of path_walk_filter_compatible().
Add tests for blob:limit=0 (equivalent to blob:none) and blob:limit=3
(which exercises partial filtering within a batch where some blobs are
kept and others are excluded).
Co-authored-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/git-pack-objects.adoc | 2 +-
builtin/backfill.c | 2 +
path-walk.c | 41 +++++++++++++--
path-walk.h | 7 +++
t/t5620-backfill.sh | 2 +-
t/t6601-path-walk.sh | 82 +++++++++++++++++++++++++++++
6 files changed, 130 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index 2994faf988..85ae48b699 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -404,7 +404,7 @@ will be automatically changed to version `1`.
+
Incompatible with `--delta-islands`. The `--use-bitmap-index` option is
ignored in the presence of `--path-walk`. The `--path-walk` option
-supports the `--filter=<spec>` form `blob:none`.
+supports the `--filter=<spec>` forms `blob:none` and `blob:limit=<n>`.
DELTA ISLANDS
diff --git a/builtin/backfill.c b/builtin/backfill.c
index b80f9ebe69..5254a42711 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -98,6 +98,8 @@ static void reject_unsupported_rev_list_options(struct rev_info *revs)
"--diff-merges");
if (!path_walk_filter_compatible(&revs->filter))
die(_("cannot backfill with these filter options"));
+ if (revs->filter.blob_limit_value)
+ die(_("cannot backfill with blob size limits"));
}
static int do_backfill(struct backfill_context *ctx)
diff --git a/path-walk.c b/path-walk.c
index bd81508163..04b924d4de 100644
--- a/path-walk.c
+++ b/path-walk.c
@@ -10,6 +10,7 @@
#include "hex.h"
#include "list-objects.h"
#include "list-objects-filter-options.h"
+#include "odb.h"
#include "object.h"
#include "oid-array.h"
#include "path.h"
@@ -327,13 +328,35 @@ static int walk_path(struct path_walk_context *ctx,
/*
* Evaluate function pointer on this data, if requested.
* Ignore object type filters for tagged objects (path starts
- * with `/`).
+ * with `/`), first for blobs and then other types.
*/
- if ((list->type == OBJ_TREE && (ctx->info->trees || path_is_for_direct_objects(path))) ||
- (list->type == OBJ_BLOB && (ctx->info->blobs || path_is_for_direct_objects(path))) ||
- (list->type == OBJ_TAG && ctx->info->tags))
+ if (list->type == OBJ_BLOB &&
+ ctx->info->blob_limit &&
+ !path_is_for_direct_objects(path)) {
+ struct oid_array filtered = OID_ARRAY_INIT;
+
+ for (size_t i = 0; i < list->oids.nr; i++) {
+ unsigned long size;
+
+ if (odb_read_object_info(ctx->repo->objects,
+ &list->oids.oid[i],
+ &size) != OBJ_BLOB ||
+ size < ctx->info->blob_limit)
+ oid_array_append(&filtered,
+ &list->oids.oid[i]);
+ }
+
+ if (filtered.nr)
+ ret = ctx->info->path_fn(path, &filtered, list->type,
+ ctx->info->path_fn_data);
+ oid_array_clear(&filtered);
+ } else if (path_is_for_direct_objects(path) ||
+ (list->type == OBJ_TREE && ctx->info->trees) ||
+ (list->type == OBJ_BLOB && ctx->info->blobs) ||
+ (list->type == OBJ_TAG && ctx->info->tags)) {
ret = ctx->info->path_fn(path, &list->oids, list->type,
ctx->info->path_fn_data);
+ }
/* Expand data for children. */
if (list->type == OBJ_TREE) {
@@ -510,6 +533,16 @@ static int prepare_filters(struct path_walk_info *info,
}
return 1;
+ case LOFC_BLOB_LIMIT:
+ if (info) {
+ if (!options->blob_limit_value)
+ info->blobs = 0;
+ else
+ info->blob_limit = options->blob_limit_value;
+ list_objects_filter_release(options);
+ }
+ return 1;
+
default:
error(_("object filter '%s' not supported by the path-walk API"),
list_objects_filter_spec(options));
diff --git a/path-walk.h b/path-walk.h
index a1736ecb2b..60ceb65433 100644
--- a/path-walk.h
+++ b/path-walk.h
@@ -47,6 +47,13 @@ struct path_walk_info {
int blobs;
int tags;
+ /**
+ * If non-zero, specifies a maximum blob size. Blobs with a
+ * size equal to or greater than this limit will not be
+ * emitted unless included in 'pending'.
+ */
+ unsigned long blob_limit;
+
/**
* When 'prune_all_uninteresting' is set and a path has all objects
* marked as UNINTERESTING, then the path-walk will not visit those
diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh
index ede89f8c33..d2ea68e065 100755
--- a/t/t5620-backfill.sh
+++ b/t/t5620-backfill.sh
@@ -20,7 +20,7 @@ test_expect_success 'backfill rejects incompatible filter options' '
test_grep "cannot backfill with these filter options" err &&
test_must_fail git backfill --objects --filter=blob:limit=10m 2>err &&
- test_grep "cannot backfill with these filter options" err
+ test_grep "cannot backfill with blob size limits" err
'
# We create objects in the 'src' repo.
diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh
index b0ee31ee2d..45f366d738 100755
--- a/t/t6601-path-walk.sh
+++ b/t/t6601-path-walk.sh
@@ -477,4 +477,86 @@ test_expect_success 'topic only, blob:none filter' '
test_cmp_sorted expect out
'
+test_expect_success 'all, blob:limit=0 filter' '
+ test-tool path-walk --filter=blob:limit=0 -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:commit::$(git rev-parse topic)
+ 0:commit::$(git rev-parse base)
+ 0:commit::$(git rev-parse base~1)
+ 0:commit::$(git rev-parse base~2)
+ 1:tag:/tags:$(git rev-parse refs/tags/first)
+ 1:tag:/tags:$(git rev-parse refs/tags/second.1)
+ 1:tag:/tags:$(git rev-parse refs/tags/second.2)
+ 1:tag:/tags:$(git rev-parse refs/tags/third)
+ 1:tag:/tags:$(git rev-parse refs/tags/fourth)
+ 1:tag:/tags:$(git rev-parse refs/tags/tree-tag)
+ 1:tag:/tags:$(git rev-parse refs/tags/blob-tag)
+ 2:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag^{})
+ 2:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag2^{})
+ 3:tree::$(git rev-parse topic^{tree})
+ 3:tree::$(git rev-parse base^{tree})
+ 3:tree::$(git rev-parse base~1^{tree})
+ 3:tree::$(git rev-parse base~2^{tree})
+ 3:tree::$(git rev-parse refs/tags/tree-tag^{})
+ 3:tree::$(git rev-parse refs/tags/tree-tag2^{})
+ 4:tree:a/:$(git rev-parse base:a)
+ 5:tree:child/:$(git rev-parse refs/tags/tree-tag:child)
+ 6:tree:left/:$(git rev-parse base:left)
+ 6:tree:left/:$(git rev-parse base~2:left)
+ 7:tree:right/:$(git rev-parse topic:right)
+ 7:tree:right/:$(git rev-parse base~1:right)
+ 7:tree:right/:$(git rev-parse base~2:right)
+ blobs:2
+ commits:4
+ tags:7
+ trees:13
+ EOF
+
+ test_cmp_sorted expect out
+'
+
+test_expect_success 'all, blob:limit=3 filter' '
+ test-tool path-walk --filter=blob:limit=3 -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:commit::$(git rev-parse topic)
+ 0:commit::$(git rev-parse base)
+ 0:commit::$(git rev-parse base~1)
+ 0:commit::$(git rev-parse base~2)
+ 1:tag:/tags:$(git rev-parse refs/tags/first)
+ 1:tag:/tags:$(git rev-parse refs/tags/second.1)
+ 1:tag:/tags:$(git rev-parse refs/tags/second.2)
+ 1:tag:/tags:$(git rev-parse refs/tags/third)
+ 1:tag:/tags:$(git rev-parse refs/tags/fourth)
+ 1:tag:/tags:$(git rev-parse refs/tags/tree-tag)
+ 1:tag:/tags:$(git rev-parse refs/tags/blob-tag)
+ 2:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag^{})
+ 2:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag2^{})
+ 3:tree::$(git rev-parse topic^{tree})
+ 3:tree::$(git rev-parse base^{tree})
+ 3:tree::$(git rev-parse base~1^{tree})
+ 3:tree::$(git rev-parse base~2^{tree})
+ 3:tree::$(git rev-parse refs/tags/tree-tag^{})
+ 3:tree::$(git rev-parse refs/tags/tree-tag2^{})
+ 4:blob:a:$(git rev-parse base~2:a)
+ 5:tree:a/:$(git rev-parse base:a)
+ 6:tree:child/:$(git rev-parse refs/tags/tree-tag:child)
+ 7:tree:left/:$(git rev-parse base:left)
+ 7:tree:left/:$(git rev-parse base~2:left)
+ 8:blob:left/b:$(git rev-parse base~2:left/b)
+ 9:tree:right/:$(git rev-parse topic:right)
+ 9:tree:right/:$(git rev-parse base~1:right)
+ 9:tree:right/:$(git rev-parse base~2:right)
+ 10:blob:right/c:$(git rev-parse base~2:right/c)
+ 11:blob:right/d:$(git rev-parse base~1:right/d)
+ blobs:6
+ commits:4
+ tags:7
+ trees:13
+ EOF
+
+ test_cmp_sorted expect out
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 06/13] backfill: die on incompatible filter options
From: Derrick Stolee via GitGitGadget @ 2026-05-22 18:24 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2101.v5.git.1779474277.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The 'git backfill' command uses the path-walk API in a critical way: it
uses the objects output from the command to find the batches of missing
objects that should be requested from the server. Unlike 'git
pack-objects', we cannot fall back to another mechanism.
The previous change added the path_walk_filter_compatible() method that
we can reuse here. Use it during argument validation in cmd_backfill().
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
builtin/backfill.c | 5 ++---
t/t5620-backfill.sh | 8 ++++++++
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/builtin/backfill.c b/builtin/backfill.c
index 7ffab2ea74..b80f9ebe69 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -96,9 +96,8 @@ static void reject_unsupported_rev_list_options(struct rev_info *revs)
if (revs->explicit_diff_merges)
die(_("'%s' cannot be used with 'git backfill'"),
"--diff-merges");
- if (revs->filter.choice)
- die(_("'%s' cannot be used with 'git backfill'"),
- "--filter");
+ if (!path_walk_filter_compatible(&revs->filter))
+ die(_("cannot backfill with these filter options"));
}
static int do_backfill(struct backfill_context *ctx)
diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh
index 94f35ce190..ede89f8c33 100755
--- a/t/t5620-backfill.sh
+++ b/t/t5620-backfill.sh
@@ -15,6 +15,14 @@ test_expect_success 'backfill rejects unexpected arguments' '
test_grep "unrecognized argument: --unexpected-arg" err
'
+test_expect_success 'backfill rejects incompatible filter options' '
+ test_must_fail git backfill --objects --filter=tree:1 2>err &&
+ test_grep "cannot backfill with these filter options" err &&
+
+ test_must_fail git backfill --objects --filter=blob:limit=10m 2>err &&
+ test_grep "cannot backfill with these filter options" err
+'
+
# We create objects in the 'src' repo.
test_expect_success 'setup repo for object creation' '
echo "{print \$1}" >print_1.awk &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 05/13] path-walk: support blobless filter
From: Derrick Stolee via GitGitGadget @ 2026-05-22 18:24 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2101.v5.git.1779474277.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The 'git pack-objects' command can opt-in to using the path-walk API for
scanning the objects. Currently, this option is dynamically disabled if
combined with '--filter=<X>', even when using a simple filter such as
'blob:none' to signal a blobless packfile. This is a common scenario for
repos at scale, so is worth integrating.
Also, users can opt-in to the '--path-walk' option by default through
the pack.usePathWalk=true config option. When using that in a blobless
partial clone, the following warning can appear even though the user did
not specify either option directly:
warning: cannot use --filter with --path-walk
Teach the path-walk API to handle the 'blob:none' object filter
natively. When revs->filter.choice is LOFC_BLOB_NONE, the path-walk
sets info->blobs to 0 (skipping all blob objects) and clears the
filter from revs so that prepare_revision_walk() does not reject the
configuration.
This check is implemented in the static prepare_filters() method, which
will simultaneously check if the input filters are compatible and will
make the appropriate mutations to the path_walk_info and filters if the
path_walk_info is non-NULL. This allows us to use this logic both in the
API method path_walk_filter_compatible() for use in
builtin/pack-objects.c and as a prep step in walk_objects_by_path().
Update the test helper (test-path-walk) to accept --filter=<spec>
as a test-tool option (before '--'), applying it to revs after
setup_revisions() to avoid the --objects requirement check. We can also
revert recent GIT_TEST_PACK_PATH_WALK overrides in t5620.
Also switch test-path-walk from REV_INFO_INIT with manual repo
assignment to repo_init_revisions(), which properly initializes
the filter_spec strbuf needed for filter parsing.
Add tests for blob:none with --all and with a single branch.
The performance test p5315 shows the impact of this change when using
blobless filters:
Test HEAD~1 HEAD
---------------------------------------------------------------------
5315.6: repack (blob:none) 13.53 13.87 +2.5%
5315.7: repack size (blob:none) 137.7M 137.8M +0.1%
5315.8: repack (blob:none, --path-walk) 13.51 23.43 +73.4%
5315.9: repack size (blob:none, --path-walk) 137.7M 115.2M -16.3%
These performance tests were run on the Git repository. The --path-walk
feature shows meaningful space savings (16% smaller for blobless packs)
at the cost of increased computation time due to the two compression
passes. This data demonstrates that the feature is engaged and provides
real compression benefits when --no-reuse-delta forces fresh deltas.
Co-Authored-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/git-pack-objects.adoc | 6 +--
builtin/pack-objects.c | 2 +-
path-walk.c | 30 ++++++++++++++
path-walk.h | 7 ++++
t/helper/test-path-walk.c | 11 ++++-
t/t5620-backfill.sh | 9 -----
t/t6601-path-walk.sh | 62 +++++++++++++++++++++++++++++
7 files changed, 113 insertions(+), 14 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index b78175fbe1..2994faf988 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -402,9 +402,9 @@ will be automatically changed to version `1`.
of filenames that cause collisions in Git's default name-hash
algorithm.
+
-Incompatible with `--delta-islands`, `--shallow`, or `--filter`. The
-`--use-bitmap-index` option will be ignored in the presence of
-`--path-walk.`
+Incompatible with `--delta-islands`. The `--use-bitmap-index` option is
+ignored in the presence of `--path-walk`. The `--path-walk` option
+supports the `--filter=<spec>` form `blob:none`.
DELTA ISLANDS
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4338962904..bc9fb5b457 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -5177,7 +5177,7 @@ int cmd_pack_objects(int argc,
if (path_walk) {
const char *option = NULL;
- if (filter_options.choice)
+ if (!path_walk_filter_compatible(&filter_options))
option = "--filter";
else if (use_delta_islands)
option = "--delta-islands";
diff --git a/path-walk.c b/path-walk.c
index 05bfc1c114..bd81508163 100644
--- a/path-walk.c
+++ b/path-walk.c
@@ -9,6 +9,7 @@
#include "hashmap.h"
#include "hex.h"
#include "list-objects.h"
+#include "list-objects-filter-options.h"
#include "object.h"
#include "oid-array.h"
#include "path.h"
@@ -495,6 +496,32 @@ static int setup_pending_objects(struct path_walk_info *info,
return 0;
}
+static int prepare_filters(struct path_walk_info *info,
+ struct list_objects_filter_options *options)
+{
+ switch (options->choice) {
+ case LOFC_DISABLED:
+ return 1;
+
+ case LOFC_BLOB_NONE:
+ if (info) {
+ info->blobs = 0;
+ list_objects_filter_release(options);
+ }
+ return 1;
+
+ default:
+ error(_("object filter '%s' not supported by the path-walk API"),
+ list_objects_filter_spec(options));
+ return 0;
+ }
+}
+
+int path_walk_filter_compatible(struct list_objects_filter_options *options)
+{
+ return prepare_filters(NULL, options);
+}
+
/**
* Given the configuration of 'info', walk the commits based on 'info->revs' and
* call 'info->path_fn' on each discovered path.
@@ -522,6 +549,9 @@ int walk_objects_by_path(struct path_walk_info *info)
trace2_region_enter("path-walk", "commit-walk", info->revs->repo);
+ if (!prepare_filters(info, &info->revs->filter))
+ return -1;
+
CALLOC_ARRAY(commit_list, 1);
commit_list->type = OBJ_COMMIT;
diff --git a/path-walk.h b/path-walk.h
index 657eeda8ec..a1736ecb2b 100644
--- a/path-walk.h
+++ b/path-walk.h
@@ -90,3 +90,10 @@ void path_walk_info_clear(struct path_walk_info *info);
* Returns nonzero on an error.
*/
int walk_objects_by_path(struct path_walk_info *info);
+
+struct list_objects_filter_options;
+/**
+ * Given a set of options for filtering objects, return 1 if the options
+ * are compatible with the path-walk API and 0 otherwise.
+ */
+int path_walk_filter_compatible(struct list_objects_filter_options *options);
diff --git a/t/helper/test-path-walk.c b/t/helper/test-path-walk.c
index fe63002c2b..88f86ae0dc 100644
--- a/t/helper/test-path-walk.c
+++ b/t/helper/test-path-walk.c
@@ -4,6 +4,7 @@
#include "dir.h"
#include "environment.h"
#include "hex.h"
+#include "list-objects-filter-options.h"
#include "object-name.h"
#include "object.h"
#include "pretty.h"
@@ -71,6 +72,8 @@ int cmd__path_walk(int argc, const char **argv)
struct rev_info revs = REV_INFO_INIT;
struct path_walk_info info = PATH_WALK_INFO_INIT;
struct path_walk_test_data data = { 0 };
+ struct list_objects_filter_options filter_options =
+ LIST_OBJECTS_FILTER_INIT;
struct option options[] = {
OPT_BOOL(0, "blobs", &info.blobs,
N_("toggle inclusion of blob objects")),
@@ -86,11 +89,12 @@ int cmd__path_walk(int argc, const char **argv)
N_("toggle aggressive edge walk")),
OPT_BOOL(0, "stdin-pl", &stdin_pl,
N_("read a pattern list over stdin")),
+ OPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),
OPT_END(),
};
setup_git_directory();
- revs.repo = the_repository;
+ repo_init_revisions(the_repository, &revs, NULL);
argc = parse_options(argc, argv, NULL,
options, path_walk_usage,
@@ -101,6 +105,10 @@ int cmd__path_walk(int argc, const char **argv)
else
usage(path_walk_usage[0]);
+ /* Apply the filter after setup_revisions to avoid the --objects check. */
+ if (filter_options.choice)
+ list_objects_filter_copy(&revs.filter, &filter_options);
+
info.revs = &revs;
info.path_fn = emit_block;
info.path_fn_data = &data;
@@ -129,6 +137,7 @@ int cmd__path_walk(int argc, const char **argv)
free(info.pl);
}
+ list_objects_filter_release(&filter_options);
release_revisions(&revs);
return res;
}
diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh
index e174290787..94f35ce190 100755
--- a/t/t5620-backfill.sh
+++ b/t/t5620-backfill.sh
@@ -298,9 +298,6 @@ test_expect_success 'backfill with prefix pathspec' '
git -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&
test_line_count = 48 missing &&
- # If we enable --path-walk here, we will get a warning overs stderr
- # due to incompatibilities with --filter.
- GIT_TEST_PACK_PATH_WALK=0 \
git -C backfill-path backfill HEAD -- d/f 2>err &&
test_must_be_empty err &&
@@ -318,9 +315,6 @@ test_expect_success 'backfill with multiple pathspecs' '
git -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&
test_line_count = 48 missing &&
- # If we enable --path-walk here, we will get a warning overs stderr
- # due to incompatibilities with --filter.
- GIT_TEST_PACK_PATH_WALK=0 \
git -C backfill-path backfill HEAD -- d/f a 2>err &&
test_must_be_empty err &&
@@ -338,9 +332,6 @@ test_expect_success 'backfill with wildcard pathspec' '
git -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&
test_line_count = 48 missing &&
- # If we enable --path-walk here, we will get a warning overs stderr
- # due to incompatibilities with --filter.
- GIT_TEST_PACK_PATH_WALK=0 \
git -C backfill-path backfill HEAD -- "d/file.*.txt" 2>err &&
test_must_be_empty err &&
diff --git a/t/t6601-path-walk.sh b/t/t6601-path-walk.sh
index 56bd1e3c5b..b0ee31ee2d 100755
--- a/t/t6601-path-walk.sh
+++ b/t/t6601-path-walk.sh
@@ -415,4 +415,66 @@ test_expect_success 'trees are reported exactly once' '
test_line_count = 1 out-filtered
'
+test_expect_success 'all, blob:none filter' '
+ test-tool path-walk --filter=blob:none -- --all >out &&
+
+ cat >expect <<-EOF &&
+ 0:commit::$(git rev-parse topic)
+ 0:commit::$(git rev-parse base)
+ 0:commit::$(git rev-parse base~1)
+ 0:commit::$(git rev-parse base~2)
+ 1:tag:/tags:$(git rev-parse refs/tags/first)
+ 1:tag:/tags:$(git rev-parse refs/tags/second.1)
+ 1:tag:/tags:$(git rev-parse refs/tags/second.2)
+ 1:tag:/tags:$(git rev-parse refs/tags/third)
+ 1:tag:/tags:$(git rev-parse refs/tags/fourth)
+ 1:tag:/tags:$(git rev-parse refs/tags/tree-tag)
+ 1:tag:/tags:$(git rev-parse refs/tags/blob-tag)
+ 2:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag^{})
+ 2:blob:/tagged-blobs:$(git rev-parse refs/tags/blob-tag2^{})
+ 3:tree::$(git rev-parse topic^{tree})
+ 3:tree::$(git rev-parse base^{tree})
+ 3:tree::$(git rev-parse base~1^{tree})
+ 3:tree::$(git rev-parse base~2^{tree})
+ 3:tree::$(git rev-parse refs/tags/tree-tag^{})
+ 3:tree::$(git rev-parse refs/tags/tree-tag2^{})
+ 4:tree:a/:$(git rev-parse base:a)
+ 5:tree:child/:$(git rev-parse refs/tags/tree-tag:child)
+ 6:tree:left/:$(git rev-parse base:left)
+ 6:tree:left/:$(git rev-parse base~2:left)
+ 7:tree:right/:$(git rev-parse topic:right)
+ 7:tree:right/:$(git rev-parse base~1:right)
+ 7:tree:right/:$(git rev-parse base~2:right)
+ blobs:2
+ commits:4
+ tags:7
+ trees:13
+ EOF
+
+ test_cmp_sorted expect out
+'
+
+test_expect_success 'topic only, blob:none filter' '
+ test-tool path-walk --filter=blob:none -- topic >out &&
+
+ cat >expect <<-EOF &&
+ 0:commit::$(git rev-parse topic)
+ 0:commit::$(git rev-parse base~1)
+ 0:commit::$(git rev-parse base~2)
+ 1:tree::$(git rev-parse topic^{tree})
+ 1:tree::$(git rev-parse base~1^{tree})
+ 1:tree::$(git rev-parse base~2^{tree})
+ 2:tree:left/:$(git rev-parse base~2:left)
+ 3:tree:right/:$(git rev-parse topic:right)
+ 3:tree:right/:$(git rev-parse base~1:right)
+ 3:tree:right/:$(git rev-parse base~2:right)
+ blobs:0
+ commits:3
+ tags:0
+ trees:7
+ EOF
+
+ test_cmp_sorted expect out
+'
+
test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 04/13] path-walk: always emit directly-requested objects
From: Derrick Stolee via GitGitGadget @ 2026-05-22 18:24 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2101.v5.git.1779474277.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
We are preparing to integrate the path-walk API with some --filter options
in 'git pack-objects', but there is a subtle issue that is revealed when
those are put together and the test suite is run with
GIT_TEST_PACK_PATH_WALK=1.
When a filter reduces the set of requested objects, this results in
filtering out directly-requested objects, such as in the download of needed
blobs in a blobless partial clone.
The root cause is that the scan of pending objects in the path-walk API
respects the filters set in the path_walk_info instead of overriding them
for pending objects.
We can tell that a path is part of the directly-referenced objects if its
path name starts with '/' (other paths, including root trees never have this
starting character). Create a path_is_for_direct_objects() to make this
meaning clear, especially as we add more references in the future as we
integrate the path-walk API with partial clone filter options.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
Documentation/technical/api-path-walk.adoc | 7 ++++
path-walk.c | 42 ++++++++++++++--------
path-walk.h | 5 +++
3 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/Documentation/technical/api-path-walk.adoc b/Documentation/technical/api-path-walk.adoc
index a67de1b143..6e17b13d61 100644
--- a/Documentation/technical/api-path-walk.adoc
+++ b/Documentation/technical/api-path-walk.adoc
@@ -48,6 +48,13 @@ commits.
applications could disable some options to make it simpler to walk
the objects or to have fewer calls to `path_fn`.
+
+Note that objects directly requested as pending objects (such as targets
+of lightweight tags or other ref tips) are always emitted to `path_fn`,
+even when the corresponding type flag is disabled. Only objects
+discovered during the tree walk are subject to these type filters. This
+ensures that objects specifically requested through the revision input
+are never silently dropped.
++
While it is possible to walk only commits in this way, consumers would be
better off using the revision walk API instead.
diff --git a/path-walk.c b/path-walk.c
index 6e426af433..05bfc1c114 100644
--- a/path-walk.c
+++ b/path-walk.c
@@ -248,6 +248,17 @@ static int add_tree_entries(struct path_walk_context *ctx,
return 0;
}
+/*
+ * Paths starting with '/' (e.g., "/tags", "/tagged-blobs") hold objects that
+ * were directly requested by 'pending' objects rather than discovered during
+ * tree traversal.
+ */
+static int path_is_for_direct_objects(const char *path)
+{
+ ASSERT(path);
+ return path[0] == '/';
+}
+
/*
* For each path in paths_to_explore, walk the trees another level
* and add any found blobs to the batch (but only if they exist and
@@ -306,14 +317,19 @@ static int walk_path(struct path_walk_context *ctx,
if (list->type == OBJ_BLOB &&
ctx->revs->prune_data.nr &&
+ !path_is_for_direct_objects(path) &&
!match_pathspec(ctx->repo->index, &ctx->revs->prune_data,
path, strlen(path), 0,
NULL, 0))
return 0;
- /* Evaluate function pointer on this data, if requested. */
- if ((list->type == OBJ_TREE && ctx->info->trees) ||
- (list->type == OBJ_BLOB && ctx->info->blobs) ||
+ /*
+ * Evaluate function pointer on this data, if requested.
+ * Ignore object type filters for tagged objects (path starts
+ * with `/`).
+ */
+ if ((list->type == OBJ_TREE && (ctx->info->trees || path_is_for_direct_objects(path))) ||
+ (list->type == OBJ_BLOB && (ctx->info->blobs || path_is_for_direct_objects(path))) ||
(list->type == OBJ_TAG && ctx->info->tags))
ret = ctx->info->path_fn(path, &list->oids, list->type,
ctx->info->path_fn_data);
@@ -374,10 +390,8 @@ static int setup_pending_objects(struct path_walk_info *info,
if (info->tags)
CALLOC_ARRAY(tags, 1);
- if (info->blobs)
- CALLOC_ARRAY(tagged_blobs, 1);
- if (info->trees)
- root_tree_list = strmap_get(&ctx->paths_to_lists, root_path);
+ CALLOC_ARRAY(tagged_blobs, 1);
+ root_tree_list = strmap_get(&ctx->paths_to_lists, root_path);
/*
* Pending objects include:
@@ -421,8 +435,6 @@ static int setup_pending_objects(struct path_walk_info *info,
switch (obj->type) {
case OBJ_TREE:
- if (!info->trees)
- continue;
if (pending->path) {
char *path = *pending->path ? xstrfmt("%s/", pending->path)
: xstrdup("");
@@ -435,8 +447,6 @@ static int setup_pending_objects(struct path_walk_info *info,
break;
case OBJ_BLOB:
- if (!info->blobs)
- continue;
if (pending->path)
add_path_to_list(ctx, pending->path, OBJ_BLOB, &obj->oid, 1);
else
@@ -532,15 +542,17 @@ int walk_objects_by_path(struct path_walk_info *info)
push_to_stack(&ctx, root_path);
/*
- * Set these values before preparing the walk to catch
- * lightweight tags pointing to non-commits and indexed objects.
+ * Ensure that prepare_revision_walk() keeps all pending objects
+ * even through an object type filter.
*/
- info->revs->blob_objects = info->blobs;
- info->revs->tree_objects = info->trees;
+ info->revs->blob_objects = info->revs->tree_objects = 1;
if (prepare_revision_walk(info->revs))
die(_("failed to setup revision walk"));
+ info->revs->blob_objects = info->blobs;
+ info->revs->tree_objects = info->trees;
+
/*
* Walk trees to mark them as UNINTERESTING.
* This is particularly important when 'edge_aggressive' is set.
diff --git a/path-walk.h b/path-walk.h
index 5ef5a8440e..657eeda8ec 100644
--- a/path-walk.h
+++ b/path-walk.h
@@ -36,6 +36,11 @@ struct path_walk_info {
/**
* Initialize which object types the path_fn should be called on. This
* could also limit the walk to skip blobs if not set.
+ *
+ * Note: even when 'blobs' or 'trees' is disabled, objects that are
+ * directly requested as pending objects will still be emitted to
+ * path_fn. Only objects discovered during the tree walk are filtered by
+ * these flags.
*/
int commits;
int trees;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 03/13] t/perf: add pack-objects filter and path-walk benchmark
From: Derrick Stolee via GitGitGadget @ 2026-05-22 18:24 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2101.v5.git.1779474277.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
Add p5315-pack-objects-filter.sh to measure the performance of
'git pack-objects --revs --all' under different filter and traversal
combinations:
* no filter (baseline)
* --filter=blob:none (blobless)
* --filter=sparse:oid=<oid> (cone-mode sparse)
Each filter scenario is tested both with and without --path-walk,
producing paired measurements that show the impact of the path-walk
traversal for each filter type as we integrate the --path-walk feature
with different --filter options. It currently has no integration so
falls back to the standard revision walk. Thus, there are no significant
differences in the current results other than a full repack (and even
then, the --path-walk feature is not incredibly different for the
default Git repository):
Test HEAD
-----------------------------------------------------
5315.2: repack (no filter) 27.91
5315.3: repack size (no filter) 250.7M
5315.4: repack (no filter, --path-walk) 34.92
5315.5: repack size (no filter, --path-walk) 220.0M
5315.6: repack (blob:none) 13.63
5315.7: repack size (blob:none) 137.6M
5315.8: repack (blob:none, --path-walk) 13.48
5315.9: repack size (blob:none, --path-walk) 137.7M
5315.10: repack (sparse:oid) 72.67
5315.11: repack size (sparse:oid) 187.4M
5315.12: repack (sparse:oid, --path-walk) 72.47
5315.13: repack size (sparse:oid, --path-walk) 187.4M
The sparse filter definition is built automatically by sampling
depth-2 directories from the test repository, making the test work
on any repo passed via GIT_PERF_LARGE_REPO. For repos that lack
depth-2 directories, a single top-level directory is used; for flat
repos, the sparse tests are skipped via prerequisite.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
t/perf/p5315-pack-objects-filter.sh | 129 ++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
create mode 100755 t/perf/p5315-pack-objects-filter.sh
diff --git a/t/perf/p5315-pack-objects-filter.sh b/t/perf/p5315-pack-objects-filter.sh
new file mode 100755
index 0000000000..445ff25be2
--- /dev/null
+++ b/t/perf/p5315-pack-objects-filter.sh
@@ -0,0 +1,129 @@
+#!/bin/sh
+
+test_description='Tests pack-objects performance with filters and --path-walk'
+. ./perf-lib.sh
+
+test_perf_large_repo
+
+test_expect_success 'setup filter inputs' '
+ # Sample a few depth-2 directories from the test repo to build
+ # a cone-mode sparse-checkout definition. The sampling picks
+ # directories at evenly-spaced positions so the choice is stable
+ # and scales to repos of any shape.
+
+ git ls-tree -d HEAD >top-entries &&
+ grep "^040000" top-entries |
+ awk "{print \$4;}" >top-dirs &&
+ top_nr=$(wc -l <top-dirs) &&
+
+ while read tdir
+ do
+ git ls-tree -d --format="$tdir/%(path)" "HEAD:$tdir" || return 1
+ done <top-dirs >depth2-dirs &&
+
+ d2_nr=$(wc -l <depth2-dirs) &&
+
+ if test "$d2_nr" -ge 2
+ then
+ # Pick two directories from evenly-spaced positions.
+ first=$(sed -n "1p" depth2-dirs) &&
+ mid=$(sed -n "$((d2_nr / 2 + 1))p" depth2-dirs) &&
+
+ p1=$(dirname "$first") &&
+ p2=$(dirname "$mid") &&
+
+ # Build cone-mode sparse-checkout patterns.
+ {
+ echo "/*" &&
+ echo "!/*/" &&
+ echo "/$p1/" &&
+ echo "!/$p1/*/" &&
+ if test "$p1" != "$p2"
+ then
+ echo "/$p2/" &&
+ echo "!/$p2/*/"
+ fi &&
+ echo "/$first/" &&
+ if test "$first" != "$mid"
+ then
+ echo "/$mid/"
+ fi
+ } >sparse-patterns &&
+
+ git hash-object -w sparse-patterns >sparse-oid &&
+ echo "Sparse cone: $first $mid" &&
+ cat sparse-patterns &&
+ test_set_prereq SPARSE_OID
+ elif test "$top_nr" -ge 1
+ then
+ # Fallback: use a single top-level directory.
+ first=$(sed -n "1p" top-dirs) &&
+ {
+ echo "/*" &&
+ echo "!/*/" &&
+ echo "/$first/"
+ } >sparse-patterns &&
+
+ git hash-object -w sparse-patterns >sparse-oid &&
+ echo "Sparse cone: $first" &&
+ cat sparse-patterns &&
+ test_set_prereq SPARSE_OID
+ fi
+'
+
+test_perf 'repack (no filter)' '
+ git pack-objects --stdout --no-reuse-delta --revs --all </dev/null >pk
+'
+
+test_size 'repack size (no filter)' '
+ test_file_size pk
+'
+
+test_perf 'repack (no filter, --path-walk)' '
+ git pack-objects --stdout --no-reuse-delta --revs --all --path-walk </dev/null >pk
+'
+
+test_size 'repack size (no filter, --path-walk)' '
+ test_file_size pk
+'
+
+test_perf 'repack (blob:none)' '
+ git pack-objects --stdout --no-reuse-delta --revs --all --filter=blob:none </dev/null >pk
+'
+
+test_size 'repack size (blob:none)' '
+ test_file_size pk
+'
+
+test_perf 'repack (blob:none, --path-walk)' '
+ git pack-objects --stdout --no-reuse-delta --revs --all --path-walk \
+ --filter=blob:none </dev/null >pk
+'
+
+test_size 'repack size (blob:none, --path-walk)' '
+ test_file_size pk
+'
+
+test_perf 'repack (sparse:oid)' \
+ --prereq SPARSE_OID '
+ git pack-objects --stdout --no-reuse-delta --revs --all \
+ --filter=sparse:oid=$(cat sparse-oid) </dev/null >pk
+'
+
+test_size 'repack size (sparse:oid)' \
+ --prereq SPARSE_OID '
+ test_file_size pk
+'
+
+test_perf 'repack (sparse:oid, --path-walk)' \
+ --prereq SPARSE_OID '
+ git pack-objects --stdout --no-reuse-delta --revs --all --path-walk \
+ --filter=sparse:oid=$(cat sparse-oid) </dev/null >pk
+'
+
+test_size 'repack size (sparse:oid, --path-walk)' \
+ --prereq SPARSE_OID '
+ test_file_size pk
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 02/13] pack-objects: pass --objects with --path-walk
From: Derrick Stolee via GitGitGadget @ 2026-05-22 18:24 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2101.v5.git.1779474277.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
When 'git pack-objects' has the --path-walk option enabled, it uses a
different set of revision walk parameters than normal. For one,
--objects was previously assumed by the path-walk API and could be
omitted. We also needed --boundary to allow discovering UNINTERESTING
objects to use as delta bases.
We will be updating the path-walk API soon to work with some filter
options. However, the revision machinery will trigger a fatal error:
fatal: object filtering requires --objects
The fix is easy: add the --objects option as an argument. This has no
effect on the path-walk API but does simplify the revision option
parsing for the objects filter.
We can remove the comment about "removing" the options because they were
never removed and instead not added. We still need to disable using
bitmaps.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
builtin/pack-objects.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index dd2480a73d..4338962904 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -5190,10 +5190,7 @@ int cmd_pack_objects(int argc,
}
if (path_walk) {
strvec_push(&rp, "--boundary");
- /*
- * We must disable the bitmaps because we are removing
- * the --objects / --objects-edge[-aggressive] options.
- */
+ strvec_push(&rp, "--objects");
use_bitmap_index = 0;
} else if (thin) {
use_internal_rev_list = 1;
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 01/13] t5620: make test work with path-walk var
From: Derrick Stolee via GitGitGadget @ 2026-05-22 18:24 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee, Derrick Stolee
In-Reply-To: <pull.2101.v5.git.1779474277.gitgitgadget@gmail.com>
From: Derrick Stolee <stolee@gmail.com>
The GIT_TEST_PACK_PATH_WALK test variable allows enabling the
--path-walk option to 'git pack-objects' by default. This sometimes
engages the warning that --path-walk is incompatible with the --filter
option. These tests in t5620 fail due to this warning over stderr in
this case. Disable this variable for this moment until these options
work together.
Signed-off-by: Derrick Stolee <stolee@gmail.com>
---
t/t5620-backfill.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh
index 94f35ce190..e174290787 100755
--- a/t/t5620-backfill.sh
+++ b/t/t5620-backfill.sh
@@ -298,6 +298,9 @@ test_expect_success 'backfill with prefix pathspec' '
git -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&
test_line_count = 48 missing &&
+ # If we enable --path-walk here, we will get a warning overs stderr
+ # due to incompatibilities with --filter.
+ GIT_TEST_PACK_PATH_WALK=0 \
git -C backfill-path backfill HEAD -- d/f 2>err &&
test_must_be_empty err &&
@@ -315,6 +318,9 @@ test_expect_success 'backfill with multiple pathspecs' '
git -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&
test_line_count = 48 missing &&
+ # If we enable --path-walk here, we will get a warning overs stderr
+ # due to incompatibilities with --filter.
+ GIT_TEST_PACK_PATH_WALK=0 \
git -C backfill-path backfill HEAD -- d/f a 2>err &&
test_must_be_empty err &&
@@ -332,6 +338,9 @@ test_expect_success 'backfill with wildcard pathspec' '
git -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&
test_line_count = 48 missing &&
+ # If we enable --path-walk here, we will get a warning overs stderr
+ # due to incompatibilities with --filter.
+ GIT_TEST_PACK_PATH_WALK=0 \
git -C backfill-path backfill HEAD -- "d/file.*.txt" 2>err &&
test_must_be_empty err &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v5 00/13] pack-objects: integrate --path-walk and some --filter options
From: Derrick Stolee via GitGitGadget @ 2026-05-22 18:24 UTC (permalink / raw)
To: git
Cc: christian.couder, gitster, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps,
Taylor Blau, Derrick Stolee
In-Reply-To: <pull.2101.v4.git.1778707135.gitgitgadget@gmail.com>
NOTE: This series is based on en/backfill-fixes-and-edges.
The 'git pack-objects' command has a '--path-walk' option that uses the
path-walk API instead of a typical revision walk to group objects into
chunks by path name instead of relying solely on name-hashes to group
similar files together. (It also does a second compression pass looking for
better deltas after the first pass that is focused within chunks per path.)
The '--path-walk' feature was not previously integrated with the '--filter'
feature, so a warning would appear and disable the path-walk API when a
filter is given. This patch series integrates these together in the
following ways:
* --filter=blob:none updates the path-walk API options to skip blobs.
* --filter=blob:limit=<size> adds a scan to a list of blob objects to
remove objects that are too large.
* --filter=sparse:<oid> adds a scan to the chunks to validate that the
paths match the sparse-checkout patterns.
In particular, this last check is significantly faster than the previous
algorithm because it can check all objects at a given path simultaneously
instead of checking all sparse-checkout patterns for each discovered blob
object.
A subtlety must be added here, in that we must change how we mark an object
as "seen" during the path-walk. We may need to add an object to multiple
paths and only mark it as "seen" if it indeed matched the sparse-checkout
patterns as the path is accepted for emitting to the callback. This adds a
new filter that the "seen" objects must also be removed from later chunks to
avoid sending the same object as grouped to multiple chunks.
There's also a subtle detail here in that the path-walk API also prunes tree
paths based on cone-mode sparse-checkouts, to enable 'git backfill --sparse'
operating quickly for small sparse-checkout scopes. But the
--filter=sparse:<oid> feature doesn't prune trees!
As a future step, I do plan to recommend that we add a treesparse:<oid>
setting that does allow us to trim the tree set by cone-mode sparse
patterns. At the time that partial clone filters were being created, cone
mode sparse-checkout didn't exist and neither did the sparse index. Those
features together make a smaller tree set possible, assuming the user never
needs to change their scope. This would be a significant change so it is not
implemented here, though the git pack-objects integration would be quick
after this series completes.
Neither the sparse:<oid> or hypothetical treesparse:<oid> options are or
should necessarily be supported by Git servers. It's too expensive to
compute dynamically and it doesn't work well with reachability bitmaps. What
becomes possible with this change is that it becomes reasonably fast to
construct bundles with these filters that can bootstrap a working
environment with the full history of all files within a given
sparse-checkout scope.
Performance Results
===================
Since the '--path-walk' option is ignored in today's Git version when a
'--filter' is added, the performance matches the behavior without
'--path-walk'. For the tables below, you can compare the rows against each
other (time and then packfile size) for the mode without and then with
'--path-walk' as a representation of "before" and "after". (These tables are
repeated in the commit messages as new implementations improve specific
rows.)
I chose a number of open source repositories of various sizes and shapes:
git/git
=======
Test HEAD
-------------------------------------------------------------------
5315.2: repack (no filter) 27.73
5315.3: repack size (no filter) 250.6M
5315.4: repack (no filter, --path-walk) 35.19
5315.5: repack size (no filter, --path-walk) 220.1M
5315.6: repack (blob:none) 13.42
5315.7: repack size (blob:none) 137.6M
5315.8: repack (blob:none, --path-walk) 20.98
5315.9: repack size (blob:none, --path-walk) 115.2M
5315.10: repack (sparse:oid) 72.53
5315.11: repack size (sparse:oid) 187.5M
5315.12: repack (sparse:oid, --path-walk) 29.00
5315.13: repack size (sparse:oid, --path-walk) 161.0M
nodejs/node
===========
Test HEAD
--------------------------------------------------------------------
5315.2: repack (no filter) 75.53
5315.3: repack size (no filter) 0.9G
5315.4: repack (no filter, --path-walk) 80.54
5315.5: repack size (no filter, --path-walk) 885.7M
5315.6: repack (blob:none) 12.65
5315.7: repack size (blob:none) 148.6M
5315.8: repack (blob:none, --path-walk) 17.60
5315.9: repack size (blob:none, --path-walk) 134.6M
5315.10: repack (sparse:oid) 518.84
5315.11: repack size (sparse:oid) 153.4M
5315.12: repack (sparse:oid, --path-walk) 27.99
5315.13: repack size (sparse:oid, --path-walk) 139.4M
microsoft/fluentui
==================
Test HEAD
--------------------------------------------------------------------
5315.2: repack (no filter) 146.77
5315.3: repack size (no filter) 562.1M
5315.4: repack (no filter, --path-walk) 72.82
5315.5: repack size (no filter, --path-walk) 172.6M
5315.6: repack (blob:none) 4.84
5315.7: repack size (blob:none) 62.7M
5315.8: repack (blob:none, --path-walk) 5.19
5315.9: repack size (blob:none, --path-walk) 59.9M
5315.10: repack (sparse:oid) 59.95
5315.11: repack size (sparse:oid) 85.6M
5315.12: repack (sparse:oid, --path-walk) 15.16
5315.13: repack size (sparse:oid, --path-walk) 72.5M
microsoftdocs/azure-devops-docs
===============================
Test HEAD
---------------------------------------------------------------------
5315.2: repack (no filter) 4.41
5315.3: repack size (no filter) 1.6G
5315.4: repack (no filter, --path-walk) 6.00
5315.5: repack size (no filter, --path-walk) 1.6G
5315.6: repack (blob:none) 1.35
5315.7: repack size (blob:none) 60.0M
5315.8: repack (blob:none, --path-walk) 1.23
5315.9: repack size (blob:none, --path-walk) 60.0M
5315.10: repack (sparse:oid) 138.24
5315.11: repack size (sparse:oid) 84.4M
5315.12: repack (sparse:oid, --path-walk) 1.86
5315.13: repack size (sparse:oid, --path-walk) 84.4M
Performance conclusions
=======================
As seen in earlier series around the '--path-walk' feature, the space
savings can be valuable but is not always guaranteed. When the space savings
doesn't happen, then the time spent is generally slower because of the
two-pass mechanism. The microsoftdocs/azure-devops-docs repo demonstrates
this case quite clearly.
However, even in these cases the 'sparse:<oid>' filters are much faster
because of the ability to check an entire set of objects against the
sparse-checkout patterns only once.
Thanks, -Stolee
UPDATES IN V2
=============
* Rebased onto en/backfill-fixes-and-edges to properly integrate with the
incompatible rev-list options logic in that series.
* Updated documentation as behavior changes. Credit to Taylor Blau for
presenting these suggestions in his RFC [2].
* Added three patches of Taylor's to extend more filter options.
UPDATES IN V3
=============
Upon realizing that the tests were not passing with
GIT_TEST_PACK_PATH_WALK=1, I spent a lot of time reworking each patch to
pass all tests with that variable enabled. This led to a lot of meaningful
changes:
* A new patch updates t5620-backfill.sh because they are currently failing
due to a check for quiet stderr checks. These changes are reverted later
when the filters are integrated so the warning stops being written.
* I move the logic for the path-walk API emitting "directly requested"
objects (non-commits in the 'pending' list). This is substantial enough
to be its own patch.
* The filtering logic is pulled entirely within the path-walk API instead
of needing integration within builtin/pack-objects.c.
* The tree:0 filter had a lot to be desired when fetching missing objects,
so is substantially updated.
* The object-type filter requires a change to the typical direct-request
behavior, including a new 'strict_types' member that prevents ever
allowing objects against type.
* The combine filter needed better logic around multiple blob size limits,
to take the smaller of the two.
* The t6601-path-walk.sh script has many test updates to better reflect the
new behavior, as required by the other partial clone tests under
GIT_TEST_PACK_PATH_WALK=1.
* Doc updates for 'form' to 'forms' when multiple forms are supported.
I've also updated Taylor's bitmap-related patches into three commits on top
of this series (see [3]).
UPDATES IN V4
=============
Thanks, Taylor for the careful review.
* Several typos are fixed.
* The performance test is corrected for issues around piping Git commands
and made more robust to the existence of submodules.
* BIG: The tree:0 patch is significantly updated in this version. Taylor
correctly smelled a problem with the new logic to emit the /tagged-trees
object set, and that signaled that those trees were previously never
emitted. I update the test to demonstrate that changing the data shape
(including tagged trees that are otherwise-unreachable) doesn't change
the test behavior, signaling a bug. The behavior change details all the
complexities of visiting only directly-requested trees under a tree:0
filter and recursing on all trees in other cases.
UPDATES IN V5
=============
Small adjustments to the performance test script.
P.S. I've CC'd the folks who were on the original path-walk feature thread
[1]
[1]
https://lore.kernel.org/git/pull.1819.git.1741571455.gitgitgadget@gmail.com/
[2] https://lore.kernel.org/git/cover.1777853408.git.me@ttaylorr.com/
[3]
https://github.com/derrickstolee/git/compare/path-walk-filters...derrickstolee:git:path-walk-bitmaps
Derrick Stolee (10):
t5620: make test work with path-walk var
pack-objects: pass --objects with --path-walk
t/perf: add pack-objects filter and path-walk benchmark
path-walk: always emit directly-requested objects
path-walk: support blobless filter
backfill: die on incompatible filter options
path-walk: support blob size limit filter
path-walk: add pl_sparse_trees to control tree pruning
pack-objects: support sparse:oid filter with path-walk
t6601: tag otherwise-unreachable trees
Taylor Blau (3):
path-walk: support `tree:0` filter
path-walk: support `object:type` filter
path-walk: support `combine` filter
Documentation/git-backfill.adoc | 4 +
Documentation/git-pack-objects.adoc | 8 +-
Documentation/technical/api-path-walk.adoc | 7 +
builtin/backfill.c | 8 +-
builtin/pack-objects.c | 23 +-
path-walk.c | 270 ++++++++--
path-walk.h | 31 ++
t/helper/test-path-walk.c | 17 +-
t/perf/p5315-pack-objects-filter.sh | 129 +++++
t/t5317-pack-objects-filter-objects.sh | 125 +++++
t/t5620-backfill.sh | 8 +
t/t6601-path-walk.sh | 572 +++++++++++++++++++--
12 files changed, 1124 insertions(+), 78 deletions(-)
create mode 100755 t/perf/p5315-pack-objects-filter.sh
base-commit: a1ad4a0fca14cdeb55ab9fb065551b15cafa8a4f
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2101%2Fderrickstolee%2Fpath-walk-filters-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2101/derrickstolee/path-walk-filters-v5
Pull-Request: https://github.com/gitgitgadget/git/pull/2101
Range-diff vs v4:
1: 0840110116 = 1: 0840110116 t5620: make test work with path-walk var
2: d7c87545f3 = 2: d7c87545f3 pack-objects: pass --objects with --path-walk
3: fb8a0f9c43 ! 3: 697ef716d2 t/perf: add pack-objects filter and path-walk benchmark
@@ t/perf/p5315-pack-objects-filter.sh (new)
+ awk "{print \$4;}" >top-dirs &&
+ top_nr=$(wc -l <top-dirs) &&
+
-+ >depth2-dirs &&
+ while read tdir
+ do
-+ git ls-tree -d --name-only "HEAD:$tdir" 2>/dev/null || return 1
-+ done <top-dirs >depth2-dirs.raw &&
-+ sed "s|^|$tdir/|" <depth2-dirs.raw >depth2-dirs &&
++ git ls-tree -d --format="$tdir/%(path)" "HEAD:$tdir" || return 1
++ done <top-dirs >depth2-dirs &&
+
+ d2_nr=$(wc -l <depth2-dirs) &&
+
4: e77c8a6bbc = 4: 91845bcef0 path-walk: always emit directly-requested objects
5: f4904f81e0 = 5: fdb9361198 path-walk: support blobless filter
6: f37467e46f = 6: 89726faf7e backfill: die on incompatible filter options
7: 133c1b156c = 7: 3884d4737f path-walk: support blob size limit filter
8: 0f517be8e3 = 8: 31b4ef0fa1 path-walk: add pl_sparse_trees to control tree pruning
9: b4dc09ab69 = 9: 7d8f0aa036 pack-objects: support sparse:oid filter with path-walk
10: 0b1eed0790 = 10: a68676d0de t6601: tag otherwise-unreachable trees
11: b23244c4c2 = 11: b0db73c6cc path-walk: support `tree:0` filter
12: 7e1e503361 = 12: 6845988f50 path-walk: support `object:type` filter
13: a615b1a707 = 13: d33d899251 path-walk: support `combine` filter
--
gitgitgadget
^ permalink raw reply
* Re: [PATCH v4 00/13] pack-objects: integrate --path-walk and some --filter options
From: Derrick Stolee @ 2026-05-22 18:24 UTC (permalink / raw)
To: Taylor Blau
Cc: Derrick Stolee via GitGitGadget, git, christian.couder, gitster,
johannes.schindelin, johncai86, karthik.188, kristofferhaugsbakk,
newren, peff, ps
In-Reply-To: <ahCGL+AsIaR+63Pr@nand.local>
On 5/22/2026 12:37 PM, Taylor Blau wrote:
> On Thu, May 21, 2026 at 07:01:33PM -0400, Derrick Stolee wrote:
>>> I'm curious what your thoughts are there. I think barring that things
>>> are near-complete here, though I did note one issue with the t/perf
>>> changes (that is my fault for having a bad suggestion on the earlier
>>> round).
>>
>> I like the suggested change to t/perf but I don't share your concerns
>> around the '/' character in the path (I go deeper into why in the
>> thread).
>
> Sounds good. I think a minor re-roll for that would be good, but I
> better understand your viewpoint around the '/' leading character now,
> so I think other than that we're good to go from my perspective.
Thanks! v5 is on its way with the update to the perf script.
Thanks,
-Stolee
^ permalink raw reply
* Re: I discovered a minor issue with `git fetch`.
From: Ben Knoble @ 2026-05-22 17:46 UTC (permalink / raw)
To: SURA; +Cc: git
In-Reply-To: <CAD6AYr9YmcnkdW=Nx=HUKcuaNbv1ukrAbXRnKyGibCQDy8N3hQ@mail.gmail.com>
> Le 22 mai 2026 à 03:48, SURA <surak8806@gmail.com> a écrit :
>
> Hello everyone
>
> The child processes spawned by `git fetch` can become zombie processes.
> In most scenarios, these zombie processes are reaped by Process 1, so
> this typically doesn't cause any problems.
>
> However, within a Docker container, the application service itself is
> sometimes designated as Process 1 (for instance, a service written in
> Go). Since these application services lack the capability to reap
> zombie processes, the zombies will gradually exhaust the available PID
> resources.
See also lore.kernel.org/git/202602231615147.3294516-1-cshung@gmail.com and subsequent discussion for related material.
>
> Here are the simple steps to reproduce this issue:
> 1. `git clone https://github.com/SURA907/pid-1.git`
> 2. `cd pid-1`
> 3. `docker build -t pid-1 .`
> 4. `docker run -d --name pid-1 pid-1:latest`
> 5. `docker exec -it pid-1 /bin/bash`
> 6. `mkdir repo && cd repo && git init --bare`
> 7. `ps -ef`
> ------
> UID PID PPID C STIME TTY TIME CMD
> root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
> root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
> root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
> root 29 7 0 07:17 pts/0 00:00:00 ps -ef
> ------
>
> 8. `git fetch https://github.com/git/git.git`
> 9. `ps -ef` (Run this command from a separate terminal session
> connected to the container)
> ------
> UID PID PPID C STIME TTY TIME CMD
> root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
> root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
> root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
> root 30 13 1 07:17 pts/1 00:00:00 git fetch https://github.com/git/git.git
> root 31 30 0 07:17 pts/1 00:00:00 /usr/local/libexec/git-core/git
> remote-https https://github.com/git/git.git
> https://github.com/git/git.git
> root 32 31 2 07:17 pts/1 00:00:00
> /usr/local/libexec/git-core/git-remote-https
> https://github.com/git/git.git https://github.com/git/git.git
> root 36 30 30 07:17 pts/1 00:00:00 /usr/local/libexec/git-core/git
> index-pack --stdin -v --fix-thin --keep=fetch-pack 30 on sura-pc
> --pack_header=2,399455
> root 38 7 0 07:17 pts/0 00:00:00 ps -ef
> ------
>
> 10. ps -ef (after fetch ends)
> ------
> UID PID PPID C STIME TTY TIME CMD
> root 1 0 0 07:16 ? 00:00:00 tail -f /dev/null
> root 7 0 0 07:16 pts/0 00:00:00 /bin/bash
> root 13 0 0 07:16 pts/1 00:00:00 /bin/bash
> root 52 1 0 07:19 ? 00:00:00 [git] <defunct>
> root 53 7 0 07:19 pts/0 00:00:00 ps -ef
> ------
>
> A zombie process has appeared. It appears to originate from a `fetch`
> subprocess that terminates very quickly; despite several attempts, I
> have been unable to successfully capture it.
>
> This issue was discovered within a legacy service. A few days after
> upgrading to Git 2.53.0, the system's PID resources were exhausted by
> zombie processes. This is likely the result of recent changes, as this
> problem did not exist in earlier versions (2.4x).
>
> To be honest, this is not an urgent matter; I have already deployed
> `tini` as the init process (PID 1) to prevent the service from
> becoming unavailable.
>
^ permalink raw reply
* Re: [PATCH 0/5] [RFC] diff: add diff.<driver>.process for external hunk providers
From: Michael Montalbo @ 2026-05-22 17:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Montalbo via GitGitGadget, git
In-Reply-To: <xmqq8q9cui5c.fsf@gitster.g>
On Thu, May 21, 2026 at 10:29 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > This series adds diff.<driver>.process, a long-running subprocess protocol
> > that lets external tools provide hunks to git's diff and blame pipelines.
> >
> > Over the past 18 years, git's diff pipeline accumulated many features that
> > operate on hunks: word diff, function context, color-moved, indent
> > heuristic, blame. External tools can replace the pipeline entirely
> > (diff.<driver>.command) or select among builtin algorithms
> > (diff.<driver>.algorithm), but there is no way for a tool to provide
> > line-change information into the pipeline. Tools that understand code
> > structure (tree-sitter parsers, format-aware analyzers, tools like
> > Difftastic and Mergiraf) must bypass git's pipeline and lose access to
> > everything downstream.
> >
> > The protocol follows filter.<driver>.process: pkt-line over stdin/stdout,
> > capability negotiation, one tool invocation per git command. The tool
> > receives file pairs and returns hunk descriptors that git feeds into the
> > standard xdiff pipeline. All output features work normally.
> >
> > Zero hunks with status=success means the tool considers the files
> > equivalent. git diff shows no output for the file, and git blame skips the
> > commit, attributing lines to earlier commits.
> >
> > On error or tool crash, git falls back silently to the builtin diff
> > algorithm. The feature is opt-in via diff.<driver>.process and
> > .gitattributes; unconfigured files are unaffected.
> >
> > The series includes git diff-process-normalize, a built-in tool that
> > compares files line by line ignoring whitespace (same logic as "git diff -w"
> > via xdiff_compare_lines):
>
> Interesting.
>
> If the goal is purely to normalize content before comparison
> (e.g. stripping comments or canonicalizing formatting), we already
> have the `textconv` mechanism. While `textconv` is a "one-shot"
> per-file process, it is significantly simpler.
>
> I suspect, however, that the primary focus here is to allow external
> tools to provide structural alignment (e.g. for AST- aware diffs
> like Difftastic or Mergiraf) without losing the original content in
> the display. Unlike `textconv`, which transforms the text the user
> sees, this protocol lets the display remain identical to the source
> while using a custom engine for the line-matching logic.
>
> If that is the intent, it should be stated more explicitly in the
> documentation and commit messages. The "whitespace-normalize"
> demonstration in [PATCH 5/5] is misleading because it's exactly the
> case where `textconv` would be sufficient.
>
Thank you for looking at this.
Yes, you have correctly identified the primary focus. My intention with the
whitespace normalization example was to provide a kind of "hello world"
diff process that would showcase how such a tool could interact with
the pipeline further down (i.e., blame vs diff output). However, I do agree
that it is a confusing example because it seems to clash with something
`textconv` already provides. I will update messaging across the series to
make the true intention of these changes more clear.
> I am afraid that the use of a long-running subprocess for every
> diff/blame invocation adds significant complexity and overhead. In
> particular, wouldn't the `blame` implementation performs a
> round-trip to the subprocess for every commit in the history? Even
> with a persistent process, the overhead of serializing and
> deserializing the entire file content twice (old and new) for every
> commit could be prohibitive for large files or deep histories.
>
> So, I dunno.
I hear you on this point. Anecdotally, my measurement of running blame
on diff.c:
Performance:
without process: 0.57s
with process: 0.67s
Blame attribution:
without process: 726 unique commits
with process: 721 unique commits (5 whitespace-only skipped)
Skipped commits:
0ea7d5b6f8 diff.c: fix some recent whitespace style violations
2775d92c53 diff.c: stylefix
4b25d091ba Fix a bunch of pointer declarations (codestyle)
a6080a0a44 War on whitespace
eeefa7c90e Style fixes, add a space after if/for/while.
I was imagining we could potentially optimize performance by extending the
protocol to enable passing OIDs as a capability so tools could read objects
directly without needing any serialization/deserialization.
^ permalink raw reply
* Re: Why do we need to wait 1s between a git add and commit
From: Fabrice SALVAIRE @ 2026-05-22 17:13 UTC (permalink / raw)
To: Kristoffer Haugsbakk, git
In-Reply-To: <b403477d-5587-4afc-bd02-dbd207c22e67@app.fastmail.com>
Hi,
Yes it throws 3000+ sequential subprocess calls at the speed of Python...
And the sleep time seems to be critical, 1s is ok but 100ms is not so ok.
I also have the feeling this is due to a git upgrade. But I didn't test
to downgrade.
That is a major issue if we have to be slow while using Git...
It is not unusual to write a shell script with a sequence of add/commit.
Le 22/05/2026 à 17:12, Kristoffer Haugsbakk a écrit :
> Hi
>
> On Fri, May 22, 2026, at 14:28, Fabrice SALVAIRE wrote:
>> I wrote a Python tool to dump a wiki to a git repository, that does
>> basically a succession of subprocess calls to git add and commit.
>>
>> Recently, I discovered this tool doesn't work any longer and that git
>> commit (2.54 on Fedora 42 / 43) crashes randomly.
>>
>> I cannot explain this behavior since my code is trivial.
>>
>> I had the intuition to add a sleep time of 1s just after a git call, and
>> it solves the issue.
>>
>> I noticed for some cases that another call to git commit were
>> successful. For most cases, git fsck and gitk report issues.
>>
>> It looks like the state of the git repository was not yet completed
>> before the end of the git subprocess.
> This might be caused by git-maintenance(1) being run in the background
> without locking? That’s a new issue in Git 2.54.0. See:
>
> https://lore.kernel.org/git/20260509175249.GA2336928@coredump.intra.peff.net/
>
> The following script reproduces the issue on Git 2.53.0. I am guessing
> that your script does something similar? It depends on how many commits
> it creates in a short timeframe.
>
> https://lore.kernel.org/git/20260508180341.GB737125@coredump.intra.peff.net/
^ permalink raw reply
* Re: [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
From: Mark Levedahl @ 2026-05-22 16:57 UTC (permalink / raw)
To: Shroom Moo; +Cc: git, Johannes Sixt, Aina Boot
In-Reply-To: <tencent_9FA5CF10A1D7CAC770366E84AD5CA9A2D107@qq.com>
On 5/22/26 11:09 AM, Shroom Moo wrote:
> On 5/22/26 1:45 AM, Mark Levedahl wrote:
>>> Additionally, [file exists {gitdir}] checks for the gitdir file in
>>> the current working directory. Since the function has not yet
>>> switched to $_gitdir when this check runs, it is almost impossible
>>> to find the file. Consequently, this logic never triggers, preventing
>>> linked worktrees from being recognized.
>>>
>>> Maybe the identification of linked worktree should not directly look
>>> for the gitdir file, but should check whether there is a.git file and
>>> its content points to... /.git/worktrees/... ? Anyways, using the
>>> literal {gitdir} to search in the current directory lead to risks.
>>>
>>> Shroom
>>>
>> We cannot get to this code if not inside the gitdir, and if the user set GIT_DIR and/or
>> GIT_WORK_TREE to do something clever, that either worked or the code already threw an
>> error. git, without GIT_WORK_TREE set, uses the current directory as the worktree, or the
>> parent directory containing .git. So, we must be inside the gitdir if this code path gets hit.
>>
>> Mark
> The relative path issue with {gitdir} is indeed difficult to trigger
> in practice. To trigger this problem, the following conditions must
> be met simultaneously:
> - git rev-parse --absolute-git-dir succeeds (the repository is valid).
> - git rev-parse --show-toplevel fails (the working tree is
> undetectable).
> - The current directory is not $_gitdir.
> - A gitdir file exists under $_gitdir (i.e., the gitdir of a linked
> worktree).
>
> Unluckily, this occurs in:
>
> ```MINGW64 Shell
> # Firstly, enter a test folder
> mkdir test-main
> cd test-main
>
> git init
>
> echo main > file.txt
> git add file.txt
> git commit -m "initial"
>
> # In feature branch
> git branch feature
>
> # Add worktree
> git worktree add ../test-feature feature
>
> cd ../
> cd test-feature
> cat .git
>
> cd ../
>
> mkdir outside
> cd outside
>
> export GIT_DIR="/(PREVIOUSPATH)/test-main/.git/worktrees/feature"
> unset GIT_WORK_TREE
> git gui blame HEAD file.txt --trace
> ```
>
> ```Wish
> puts $_gitworktree
> ```
>
> The current directory is "outside", not the expected linked worktree.
> [file exists {gitdir}] in find_worktree_from_gitdir searched in the
> current directory but failed, resulting in the inability to identify
> linked worktree. The blame mode can still work without worktree.
>
> Shroom
>
You exported GIT_DIR pointing to a valid gitdir, and as documented git takes your current
directory ("outside") to be the worktree. This is working as expected. Had finding a
worktree failed, git-gui would issue an error message and stop because you defined GIT_DIR.
Mark
^ permalink raw reply
* Re: [PATCH v4 00/13] pack-objects: integrate --path-walk and some --filter options
From: Taylor Blau @ 2026-05-22 16:37 UTC (permalink / raw)
To: Derrick Stolee
Cc: Derrick Stolee via GitGitGadget, git, christian.couder, gitster,
johannes.schindelin, johncai86, karthik.188, kristofferhaugsbakk,
newren, peff, ps
In-Reply-To: <4d7a75e9-8260-4e33-a786-72e0aa3026ae@gmail.com>
On Thu, May 21, 2026 at 07:01:33PM -0400, Derrick Stolee wrote:
> > I'm curious what your thoughts are there. I think barring that things
> > are near-complete here, though I did note one issue with the t/perf
> > changes (that is my fault for having a bad suggestion on the earlier
> > round).
>
> I like the suggested change to t/perf but I don't share your concerns
> around the '/' character in the path (I go deeper into why in the
> thread).
Sounds good. I think a minor re-roll for that would be good, but I
better understand your viewpoint around the '/' leading character now,
so I think other than that we're good to go from my perspective.
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH v4 04/13] path-walk: always emit directly-requested objects
From: Taylor Blau @ 2026-05-22 16:36 UTC (permalink / raw)
To: Derrick Stolee
Cc: Derrick Stolee via GitGitGadget, git, christian.couder, gitster,
johannes.schindelin, johncai86, karthik.188, kristofferhaugsbakk,
newren, peff, ps
In-Reply-To: <53d46614-7d9d-49ab-ace7-71367f6d2f40@gmail.com>
On Thu, May 21, 2026 at 07:00:39PM -0400, Derrick Stolee wrote:
> > Could we instead store this explicitly on the type_and_oid_list, e.g. a
> > "direct" flag? I'm not sure whether that type has the right scope for
> > this information. If not, I wonder if there is another way to store this
> > information, since I worry that future callers may not know about this
> > convention and end up changing the result of the path-walk depending on
> > how they name their paths.
>
> I don't find this as fragile as you do, because these "direct" paths
> _need_ to start with '/' to avoid collisions with other paths that may
> exist _and_ this meaning is internal to the data within the API. Callers
> can't change this data, though they will see the paths themselves in the
> callback function.
>
> And as I mentioned before, this is a memory-efficient storage of this
> indicator bit because it only consumes memory when it is "on" and the
> vast majority of cases where it is "off" it doesn't take any extra
> storage.
Ah, I didn't realize that we added '/' at the beginning to avoid
collisions. This approach makes sense to me!
Thanks,
Taylor
^ permalink raw reply
* Re: [PATCH] receive-pack: fix updateInstead with core.worktree
From: Kristoffer Haugsbakk @ 2026-05-22 16:21 UTC (permalink / raw)
To: Alyssa Ross, git; +Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano
In-Reply-To: <20260522154418.5883-1-hi@alyssa.is>
On Fri, May 22, 2026, at 17:44, Alyssa Ross wrote:
> This used to work, but when push_to_checkout() started being called
> before push_to_deploy(), push_to_checkout()'s side effect of adding
> GIT_WORK_TREE to the same environment that would be used by
> push_to_deploy() wasn't taken into account. Fix by only mutating the
> environment for push_to_commit(), rather than the shared environment.
>
> Fixes: a8cc594333 ("hooks: fix an obscure TOCTOU "did we just run a hook?" race")
This project doesn’t use `Fixes` trailers.[1] Mentions of commits go in
the commit message body (outside the trailers) using `git log -1
--format-reference <cmt>`.
The Linux project has uses for this structured information since there
is a lot of backporting of bugfixes. But I haven’t heard of a need for
that in this project.
🔗 1: https://lore.kernel.org/git/72839071-153f-4306-a705-3be0dc203109@app.fastmail.com/
> Signed-off-by: Alyssa Ross <hi@alyssa.is>
> ---
>[snip]
^ permalink raw reply
* [PATCH v2 4/4] notes: support an external command to display notes
From: Siddh Raman Pant @ 2026-05-22 16:09 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Kristoffer Haugsbakk,
Junio C Hamano, brian m. carlson, Jeff King, Johannes Sixt,
Oswald Buddenhagen
In-Reply-To: <cover.1779464886.git.siddh.raman.pant@oracle.com>
git notes is a very very helpful feature to show user-supplied
information about a commit alongside its message transparently.
For distributed teams working on large git repos (huge number of
branches/refs, files, etc.) and using the notes feature to mark
information on git commits, the problem is often not that two users
update the same note object at the same time. It is that the local
notes state used while reading history can be stale.
In kernel work, the same logical upstream fix can appear as different
commit objects across many downstream branches, such as the stable
branches and vendor-specific branches (based on which the released
kernel is actually built). Different developers may be working on those
branches in parallel, and a review decision recorded for one backport
is useful context for the others.
Today, seeing that decision in ordinary history output requires first
synchronizing the local notes ref, and then interpreting those notes
for the branch being inspected. The latter step is workflow-specific
and can be cheap, but keeping the local notes state fresh enough can be
expensive in a large kernel repository with a large shared notes
history (and if we are to extrapolate, a slow git server conn/ops can
be a factor too).
This TOCTOU problem exacerbates on scale (rapid updates, more devs,
larger repos, more git server traffic, etc).
One solution to this is to move the freshness policy out of git so that
it is someone else's problem. We can have a realtime fetch or faster
updation via external helper means. But unfortunately we lose the
coherence in the display of information, and so the user would end up
reinventing git log in his quest to have same workflow.
Let's add support for notes.externalCommand, a protected-configuration
command that git runs as a long-lived helper when displaying notes. git
sends commit IDs to the helper and displays any returned text through
the existing notes formatting path. This keeps presentation in git
while letting the helper decide how fresh note text is obtained.
We also add configuration for the displayed notes header name, timeout
enforcement for the helper so that git doesn't hang waiting on it,
optional --grep participation, and command-line controls to enable or
disable external notes. The new help text should make the usage clear.
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
Documentation/config/notes.adoc | 61 +++
Documentation/git-format-patch.adoc | 11 +-
Documentation/git-range-diff.adoc | 6 +
Documentation/pretty-options.adoc | 9 +
Makefile | 2 +
builtin/log.c | 17 +-
builtin/name-rev.c | 9 +-
builtin/range-diff.c | 2 +
contrib/completion/git-completion.bash | 4 +-
log-tree.c | 7 +-
meson.build | 1 +
notes-external.c | 414 +++++++++++++++++++
notes-external.h | 53 +++
notes.c | 264 +++++++++---
notes.h | 33 +-
revision.c | 36 +-
t/helper/meson.build | 1 +
t/helper/test-external-notes | 64 +++
t/helper/test-notes-external-config-reset.c | 24 ++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/lib-notes.sh | 19 +
t/t3206-range-diff.sh | 68 ++++
t/t3301-notes.sh | 424 ++++++++++++++++++++
t/t6120-describe.sh | 17 +
25 files changed, 1485 insertions(+), 63 deletions(-)
create mode 100644 notes-external.c
create mode 100644 notes-external.h
create mode 100755 t/helper/test-external-notes
create mode 100644 t/helper/test-notes-external-config-reset.c
create mode 100644 t/lib-notes.sh
diff --git a/Documentation/config/notes.adoc b/Documentation/config/notes.adoc
index b7e536496f51..3b58bf684524 100644
--- a/Documentation/config/notes.adoc
+++ b/Documentation/config/notes.adoc
@@ -34,6 +34,67 @@ The effective value of `core.notesRef` (possibly overridden by
`GIT_NOTES_REF`) is also implicitly added to the list of refs to be
displayed.
+`notes.externalCommand`::
+ Command to invoke as a long-lived helper when showing commit messages
+ with the `git log` family of commands. Git sends one commit object ID
+ per request on the command's standard input:
++
+------------
+<hex-commit-id>
+------------
++
+For each request, the helper must respond on its standard output with either
+`<hex-commit-id> missing` followed by a newline, or `<hex-commit-id> ok <n>`
+followed by a newline and exactly `<n>` bytes of UTF-8 note text followed by a
+newline. The helper must respond to each request as it is received; Git does
+not send all commit object IDs before reading responses. Empty note text is not
+displayed. External notes are only used while formatting output by default; see
+`notes.externalCommandForGrep` to include them when matching commits.
++
+If Git cannot start or communicate with the helper, or the helper sends an
+invalid response, Git warns once and disables it for the rest of the Git run.
+When stopping the helper process, Git sends SIGTERM and waits for 0.1s on POSIX
+systems; if the helper does not exit, Git force-terminates it. On Windows, Git
+terminates the process directly.
++
+This setting is only respected in protected configuration (see
+linkgit:git-config[1]). This prevents untrusted repositories from running
+arbitrary commands when notes are displayed.
++
+This setting does not take effect when:
++
+--
+* the value is empty;
+* `--no-notes` is given;
+* `--no-external-notes` is given; or
+* `--notes=<ref>` is given by itself without `--external-notes` or `--notes`.
+--
+
+`notes.externalCommandName`::
+ Name to use in the `Notes (<name>):` header for notes returned by
+ `notes.externalCommand`. Defaults to `external`. This setting is only
+ respected in protected configuration.
+
+`notes.externalCommandTimeoutMs`::
+ Number of milliseconds to wait when reading each response from
+ `notes.externalCommand`. Defaults to `100`. If the command does not
+ produce the expected response in time, Git warns once and disables it
+ for the rest of the command. A value of `0` disables timeout handling,
+ so reads can block until the command writes output or exits. This
+ setting is only respected in protected configuration.
+
+`notes.externalCommandForGrep`::
+ Boolean indicating whether notes returned by `notes.externalCommand`
+ are included when matching commits with `--grep`, wherever notes would
+ normally participate in grep matching. Defaults to false. This does
+ not make hidden notes searchable in formats such as `--oneline` or
+ `--pretty=%s`; use `--notes` or `--external-notes` if those formats
+ should search notes too. When enabled, revision traversal may invoke
+ the external command for many commits that are not ultimately
+ displayed, which can be expensive for slow commands. The note output
+ can also change which commits match. This setting is only respected in
+ protected configuration.
+
`notes.rewrite.<command>`::
When rewriting commits with _<command>_ (currently `amend` or
`rebase`), if this variable is `false`, git will not copy
diff --git a/Documentation/git-format-patch.adoc b/Documentation/git-format-patch.adoc
index 566238245028..472b37e5237a 100644
--- a/Documentation/git-format-patch.adoc
+++ b/Documentation/git-format-patch.adoc
@@ -26,7 +26,7 @@ SYNOPSIS
[--[no-]cover-letter] [--quiet]
[--commit-list-format=<format-spec>]
[--[no-]encode-email-headers]
- [--no-notes | --notes[=<ref>]]
+ [--no-notes | --notes[=<ref>]] [--[no-]external-notes]
[--interdiff=<previous>]
[--range-diff=<previous> [--creation-factor=<percent>]]
[--filename-max-length=<n>]
@@ -395,6 +395,15 @@ configuration options in linkgit:git-notes[1] to use this workflow).
The default is `--no-notes`, unless the `format.notes` configuration is
set.
+--external-notes::
+--no-external-notes::
+ Invoke or do not invoke `notes.externalCommand` to obtain external
+ notes. Like `--notes=<ref>`, `--external-notes` names an explicit
+ note source and by itself does not include the default notes refs.
+ Use `--external-notes --notes` to include the default notes refs
+ too, or combine `--external-notes` with `--notes=<ref>` to include
+ external notes with specific notes refs.
+
--signature=<signature>::
--no-signature::
Add a signature to each message produced. Per RFC 3676 the signature
diff --git a/Documentation/git-range-diff.adoc b/Documentation/git-range-diff.adoc
index 5cc5e2ed5673..1de23f300517 100644
--- a/Documentation/git-range-diff.adoc
+++ b/Documentation/git-range-diff.adoc
@@ -12,6 +12,7 @@ git range-diff [--color=[<when>]] [--no-color] [<diff-options>]
[--no-dual-color] [--creation-factor=<factor>]
[--left-only | --right-only] [--diff-merges=<format>]
[--remerge-diff] [--no-notes | --notes[=<ref>]]
+ [--[no-]external-notes]
( <range1> <range2> | <rev1>...<rev2> | <base> <rev1> <rev2> )
[[--] <path>...]
@@ -101,6 +102,11 @@ diff.
This flag is passed to the `git log` program
(see linkgit:git-log[1]) that generates the patches.
+`--external-notes`::
+`--no-external-notes`::
+ This flag is passed to the `git log` program
+ (see linkgit:git-log[1]) that generates the patches.
+
`<range1> <range2>`::
Compare the commits specified by the two ranges, where
_<range1>_ is considered an older version of _<range2>_.
diff --git a/Documentation/pretty-options.adoc b/Documentation/pretty-options.adoc
index 658e462b2533..aad851c92cfd 100644
--- a/Documentation/pretty-options.adoc
+++ b/Documentation/pretty-options.adoc
@@ -93,6 +93,15 @@ being displayed. Examples: "`--notes=foo`" will show only notes from
"`--notes --notes=foo --no-notes --notes=bar`" will only show notes
from `refs/notes/bar`.
+`--external-notes`::
+`--no-external-notes`::
+ Invoke or do not invoke `notes.externalCommand` to obtain external
+ notes. Like `--notes=<ref>`, `--external-notes` names an explicit
+ note source and by itself does not include the default notes refs.
+ Use `--external-notes --notes` to include the default notes refs
+ too, or combine `--external-notes` with `--notes=<ref>` to include
+ external notes with specific notes refs.
+
`--show-notes-by-default`::
Show the default notes unless options for displaying specific
notes are given.
diff --git a/Makefile b/Makefile
index fb50c57e4f25..898da8936e84 100644
--- a/Makefile
+++ b/Makefile
@@ -834,6 +834,7 @@ TEST_BUILTINS_OBJS += test-match-trees.o
TEST_BUILTINS_OBJS += test-mergesort.o
TEST_BUILTINS_OBJS += test-mktemp.o
TEST_BUILTINS_OBJS += test-name-hash.o
+TEST_BUILTINS_OBJS += test-notes-external-config-reset.o
TEST_BUILTINS_OBJS += test-online-cpus.o
TEST_BUILTINS_OBJS += test-pack-deltas.o
TEST_BUILTINS_OBJS += test-pack-mtimes.o
@@ -1206,6 +1207,7 @@ LIB_OBJS += negotiator/default.o
LIB_OBJS += negotiator/noop.o
LIB_OBJS += negotiator/skipping.o
LIB_OBJS += notes-cache.o
+LIB_OBJS += notes-external.o
LIB_OBJS += notes-merge.o
LIB_OBJS += notes-utils.o
LIB_OBJS += notes.o
diff --git a/builtin/log.c b/builtin/log.c
index 8c0939dd42ad..bed4c1576f2d 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1337,9 +1337,24 @@ static void get_notes_args(struct strvec *arg, struct rev_info *rev)
(rev->notes_opt.use_default_notes == -1 &&
!rev->notes_opt.extra_notes_refs.nr)) {
strvec_push(arg, "--notes");
- } else {
+ } else if (rev->notes_opt.extra_notes_refs.nr) {
for_each_string_list(&rev->notes_opt.extra_notes_refs, get_notes_refs, arg);
+ } else if (rev->notes_opt.use_external_notes <= 0) {
+ /*
+ * rev->show_notes can stay set after
+ * --external-notes --no-external-notes.
+ *
+ * Since range-diff's child log starts with
+ * --show-notes-by-default, explicitly suppress
+ * notes when no notes source remains.
+ */
+ strvec_push(arg, "--no-notes");
}
+
+ if (rev->notes_opt.use_external_notes > 0)
+ strvec_push(arg, "--external-notes");
+ else if (rev->notes_opt.use_external_notes == 0)
+ strvec_push(arg, "--no-external-notes");
}
static void generate_shortlog_cover_letter(struct shortlog *log,
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 60cbbfb4b7d1..5a0e7daac803 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -277,6 +277,7 @@ struct name_ref_data {
struct pretty_format {
struct pretty_print_context ctx;
struct userformat_want want;
+ struct external_notes_state *external_notes_state;
};
enum command_type {
@@ -525,9 +526,9 @@ static const char *get_format_rev(const struct commit *c,
if (format_ctx->want.notes) {
struct strbuf notebuf = STRBUF_INIT;
- format_display_notes(&c->object.oid, ¬ebuf,
- get_log_output_encoding(),
- format_ctx->ctx.fmt == CMIT_FMT_USERFORMAT);
+ format_display_notes(c, ¬ebuf, get_log_output_encoding(),
+ format_ctx->ctx.fmt == CMIT_FMT_USERFORMAT,
+ format_ctx->external_notes_state);
format_ctx->ctx.notes_message = strbuf_detach(¬ebuf, NULL);
}
@@ -879,6 +880,8 @@ int cmd_format_rev(int argc,
&ignore_show_notes,
n->string);
load_display_notes(&format_notes_opt);
+ format_pp.external_notes_state =
+ format_notes_opt.external_notes_state;
}
init_format_rev_command(&cmd, &format_pp);
diff --git a/builtin/range-diff.c b/builtin/range-diff.c
index e54c0f7fe156..41c27250404a 100644
--- a/builtin/range-diff.c
+++ b/builtin/range-diff.c
@@ -56,6 +56,8 @@ int cmd_range_diff(int argc,
OPT_PASSTHRU_ARGV(0, "notes", &log_arg,
N_("notes"), N_("passed to 'git log'"),
PARSE_OPT_OPTARG),
+ OPT_PASSTHRU_ARGV(0, "external-notes", &log_arg, NULL,
+ N_("passed to 'git log'"), PARSE_OPT_NOARG),
OPT_PASSTHRU_ARGV(0, "diff-merges", &diff_merges_arg,
N_("style"), N_("passed to 'git log'"), 0),
OPT_CALLBACK(0, "max-memory", &range_diff_opts.max_memory,
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a8e7c6ddbfb2..146444e65860 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2023,7 +2023,7 @@ _git_fetch ()
__git_format_patch_extra_options="
--full-index --not --all --no-prefix --src-prefix=
- --dst-prefix= --notes
+ --dst-prefix= --notes --external-notes --no-external-notes
"
_git_format_patch ()
@@ -2215,7 +2215,7 @@ __git_log_common_options="
__git_log_gitk_options="
--dense --sparse --full-history
--simplify-merges --simplify-by-decoration
- --left-right --notes --no-notes
+ --left-right --notes --no-notes --external-notes --no-external-notes
"
# Options that go well for log and shortlog (not gitk)
__git_log_shortlog_options="
diff --git a/log-tree.c b/log-tree.c
index 4503a42dde6b..f37c8b14e9a1 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -856,9 +856,12 @@ void show_log(struct rev_info *opt)
if (opt->show_notes) {
struct strbuf notebuf = STRBUF_INIT;
bool raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
+ const struct display_notes_opt *notes_opt = &opt->notes_opt;
+
+ format_display_notes(commit, ¬ebuf,
+ get_log_output_encoding(), raw,
+ notes_opt->external_notes_state);
- format_display_notes(&commit->object.oid, ¬ebuf,
- get_log_output_encoding(), raw);
ctx.notes_message = strbuf_detach(¬ebuf, NULL);
}
diff --git a/meson.build b/meson.build
index 052c81f2887b..83845f84fed0 100644
--- a/meson.build
+++ b/meson.build
@@ -397,6 +397,7 @@ libgit_sources = [
'notes-merge.c',
'notes-utils.c',
'notes.c',
+ 'notes-external.c',
'object-file-convert.c',
'object-file.c',
'object-name.c',
diff --git a/notes-external.c b/notes-external.c
new file mode 100644
index 000000000000..09da102a7901
--- /dev/null
+++ b/notes-external.c
@@ -0,0 +1,414 @@
+#include "git-compat-util.h"
+#include "gettext.h"
+#include "hex.h"
+#include "notes-external.h"
+#include "run-command.h"
+#include "sigchain.h"
+#include "strbuf.h"
+#include "trace.h"
+
+#define convert_ms_to_ns(ms) (uint64_t)(ms) * 1000000ULL
+#define convert_ns_to_ms(ns) (uint64_t)(ns) / 1000000ULL
+#define EXTERNAL_NOTES_DEFAULT_TIMEOUT_MS 100
+#define EXTERNAL_NOTES_READ_CHUNK_SIZE 16384 /* (16 * 1024) bytes */
+
+/* Configuration helpers. */
+
+static void init_external_notes_config(struct external_notes_config *config)
+{
+ if (!config)
+ return;
+
+ memset(config, 0, sizeof(*config));
+ config->read_timeout_ns =
+ convert_ms_to_ns(EXTERNAL_NOTES_DEFAULT_TIMEOUT_MS);
+}
+
+static void release_external_notes_config(struct external_notes_config *config)
+{
+ if (!config)
+ return;
+
+ FREE_AND_NULL(config->command);
+ FREE_AND_NULL(config->command_name_value);
+}
+
+struct external_notes_state *external_notes_new(void)
+{
+ struct external_notes_state *state = xcalloc(1, sizeof(*state));
+
+ init_external_notes_config(&state->config);
+ child_process_init(&state->process.process);
+ state->process.out_fd = -1;
+
+ return state;
+}
+
+void set_external_notes_command(struct external_notes_state *state,
+ const char *command)
+{
+ struct external_notes_config *config;
+
+ if (!state)
+ return;
+
+ config = &state->config;
+ FREE_AND_NULL(config->command);
+
+ if (command && *command)
+ config->command = xstrdup(command);
+}
+
+bool external_notes_command_configured(const struct external_notes_state *state)
+{
+ return state && state->config.command && !state->process.failed;
+}
+
+void external_notes_reset(struct external_notes_state *state)
+{
+ if (!state)
+ return;
+
+ if (state->process.started)
+ BUG("cannot reset external notes config while cmd is running");
+
+ release_external_notes_config(&state->config);
+ init_external_notes_config(&state->config);
+ state->process.failed = false;
+}
+
+void set_external_notes_command_name(struct external_notes_state *state,
+ const char *name)
+{
+ struct external_notes_config *config;
+
+ if (!state)
+ return;
+
+ config = &state->config;
+ FREE_AND_NULL(config->command_name_value);
+
+ if (name && *name)
+ config->command_name_value = xstrdup(name);
+}
+
+const char *external_notes_command_name(const struct external_notes_state *state)
+{
+ if (state && state->config.command_name_value)
+ return state->config.command_name_value;
+
+ return "external";
+}
+
+void set_external_notes_command_timeout_ms(struct external_notes_state *state,
+ int timeout_ms)
+{
+ if (!state)
+ return;
+
+ if (timeout_ms < 0)
+ BUG("negative notes.externalCommandTimeoutMs");
+
+ state->config.read_timeout_ns = convert_ms_to_ns(timeout_ms);
+}
+
+int external_notes_command_timeout_ms(const struct external_notes_state *state)
+{
+ if (!state)
+ return -1;
+
+ return (int)convert_ns_to_ms(state->config.read_timeout_ns);
+}
+
+void set_external_notes_for_grep(struct external_notes_state *state,
+ int enabled)
+{
+ if (!state)
+ return;
+
+ state->config.for_grep = (bool)enabled;
+}
+
+bool external_notes_for_grep_enabled(const struct external_notes_state *state)
+{
+ return state && state->config.for_grep;
+}
+
+/* Process management helpers. */
+
+static void mute_routine(const char *msg UNUSED, va_list params UNUSED)
+{
+ /* do nothing */
+}
+
+static void close_external_notes_pipes(struct external_notes_process *state)
+{
+ struct child_process *process;
+
+ if (!state)
+ return;
+
+ process = &state->process;
+
+ sigchain_push(SIGPIPE, SIG_IGN);
+
+ if (state->in) {
+ fclose(state->in);
+ state->in = NULL;
+ } else {
+ close(process->in);
+ }
+
+ if (state->out_fd >= 0) {
+ close(state->out_fd);
+ state->out_fd = -1;
+ } else {
+ close(process->out);
+ }
+
+ sigchain_pop(SIGPIPE);
+}
+
+/* We set this as callback later, so can't have void argument. */
+static void cleanup_external_notes_process(struct child_process *process)
+{
+ report_fn old_error = NULL;
+ struct external_notes_process *state;
+
+ if (!process)
+ return;
+
+ state = container_of(process, struct external_notes_process, process);
+
+ kill(process->pid, SIGTERM);
+ old_error = get_error_routine();
+ set_error_routine(mute_routine);
+
+ close_external_notes_pipes(state);
+ finish_command(process);
+
+ if (old_error)
+ set_error_routine(old_error);
+
+ state->started = false;
+}
+
+static void stop_external_notes_process(struct external_notes_process *state)
+{
+ if (!state)
+ return;
+
+ if (!state->started)
+ return;
+
+ state->process.clean_on_exit = 0;
+ cleanup_external_notes_process(&state->process);
+ child_process_init(&state->process);
+ state->out_fd = -1;
+}
+
+static int fail_external_notes_command(struct external_notes_state *state)
+{
+ const struct external_notes_config *config;
+ struct external_notes_process *process;
+
+ if (!state)
+ return -1;
+
+ config = &state->config;
+ process = &state->process;
+ if (!process->failed)
+ warning(_("notes.externalCommand failed: %s"),
+ config->command);
+
+ process->failed = true;
+ stop_external_notes_process(process);
+ return -1;
+}
+
+static int start_external_notes_command(struct external_notes_state *state)
+{
+ const struct external_notes_config *config;
+ struct external_notes_process *process;
+ struct child_process *cmd;
+
+ if (!state)
+ return -1;
+
+ config = &state->config;
+ process = &state->process;
+ cmd = &process->process;
+
+ if (process->started)
+ return 0;
+
+ if (!config->command || process->failed)
+ return -1;
+
+ child_process_init(cmd);
+ strvec_push(&cmd->args, config->command);
+ cmd->use_shell = 1;
+ cmd->in = -1;
+ cmd->out = -1;
+ cmd->clean_on_exit = 1;
+ cmd->clean_on_exit_handler = cleanup_external_notes_process;
+ cmd->trace2_child_class = "notes-external";
+
+ if (start_command(cmd))
+ return fail_external_notes_command(state);
+
+ process->in = xfdopen(cmd->in, "wb");
+ process->out_fd = cmd->out;
+ process->started = true;
+
+ return 0;
+}
+
+void external_notes_free(struct external_notes_state *state)
+{
+ if (!state)
+ return;
+
+ stop_external_notes_process(&state->process);
+ release_external_notes_config(&state->config);
+ free(state);
+}
+
+/* Command parser. Essentially the main() function of this file. */
+int format_external_note(struct external_notes_state *state,
+ const struct object_id *object_oid,
+ struct strbuf *note_buf)
+{
+ struct strbuf status = STRBUF_INIT;
+ char commit_id_hex_str[GIT_MAX_HEXSZ + 1];
+ const char *arg;
+ char *end;
+ char ch;
+ unsigned long len;
+ uint64_t deadline_ns;
+ bool input_fail;
+ int ret = 0;
+ const struct external_notes_config *config;
+ struct external_notes_process *process;
+
+ if (!state)
+ return -1;
+
+ /* Exit early if starting the command fails. */
+ if (start_external_notes_command(state) != 0)
+ return -1;
+
+ config = &state->config;
+ process = &state->process;
+
+ /* Fetch the commit ID hex. */
+ oid_to_hex_r(commit_id_hex_str, object_oid);
+
+ /* Pass the input to the external command. */
+ sigchain_push(SIGPIPE, SIG_IGN);
+ input_fail = fprintf(process->in, "%s\n", commit_id_hex_str) < 0
+ || fflush(process->in) != 0;
+ sigchain_pop(SIGPIPE);
+
+ if (input_fail)
+ goto out_fail;
+
+ if (config->read_timeout_ns == 0)
+ deadline_ns = 0;
+ else
+ deadline_ns = getnanotime() + config->read_timeout_ns;
+
+ /**
+ * The output for each commit is either of the two:
+ * "{commit id} missing\n"
+ * "{commit id} ok {num_bytes}\n{str_of_num_bytes}\n"
+ *
+ * We can have "\r\n" instead of "\n" due to Windows.
+ */
+
+ /* Read the first line with its delimiter. */
+ if (strbuf_getwholeline_fd_deadline(&status, process->out_fd, '\n',
+ deadline_ns) == EOF)
+ goto out_fail;
+
+ /* Reject EOF-terminated partial lines. */
+ if (!status.len || status.buf[status.len - 1] != '\n')
+ goto out_fail;
+
+ /**
+ * Strip LF and then optional CR so both LF and CRLF protocol lines
+ * are accepted.
+ */
+ strbuf_setlen(&status, status.len - 1);
+ strbuf_strip_suffix(&status, "\r");
+
+ /* Check if line starts with the commit ID. */
+ if (!skip_prefix(status.buf, commit_id_hex_str, &arg))
+ goto out_fail;
+
+ if (*arg++ != ' ') /* After commit ID there should be a space. */
+ goto out_fail;
+
+ if (strcmp(arg, "missing") == 0) /* No note available. */
+ goto out_success; /* Ending newline is already ensured. */
+
+ if (!skip_prefix(arg, "ok ", &arg)) /* Neither missing nor ok. */
+ goto out_fail;
+
+ /* We are in "ok" case. */
+
+ /* The next thing is length of the note. It must be unsigned digits. */
+ if (!isdigit(*arg))
+ goto out_fail;
+
+ /* Get the length of note. */
+ errno = 0;
+ len = strtoul(arg, &end, 10);
+ if (errno != 0 || *end != '\0' || end == arg)
+ goto out_fail;
+
+ /* Ending newline is already ensured. */
+
+ /* Read the trailing note in bounded-chunks. */
+ while (note_buf->len < len) {
+ ssize_t got;
+ size_t remaining = len - note_buf->len;
+ size_t want = remaining < EXTERNAL_NOTES_READ_CHUNK_SIZE ?
+ remaining : EXTERNAL_NOTES_READ_CHUNK_SIZE;
+
+ strbuf_grow(note_buf, want);
+
+ got = read_in_full_deadline(process->out_fd,
+ note_buf->buf + note_buf->len,
+ want, deadline_ns);
+ if (got < 0 || (size_t)got != want)
+ goto out_fail;
+
+ strbuf_setlen(note_buf, note_buf->len + (size_t)got);
+ }
+
+ /* Ensure the ending newline (LF/CRLF) after the note. */
+ if (xread_deadline(process->out_fd, &ch, 1, deadline_ns) != 1)
+ goto out_fail;
+
+ if (ch != '\n') { /* Not a LF. */
+ if (ch != '\r') /* Not a CRLF. */
+ goto out_fail;
+
+ /* We have '\r', let's read the next char. */
+ if (xread_deadline(process->out_fd, &ch, 1,
+ deadline_ns) != 1)
+ goto out_fail;
+
+ if (ch != '\n') /* Not a CRLF. */
+ goto out_fail;
+ }
+
+ goto out_success;
+
+out_fail:
+ ret = fail_external_notes_command(state);
+out_success:
+ strbuf_release(&status);
+ return ret;
+}
diff --git a/notes-external.h b/notes-external.h
new file mode 100644
index 000000000000..1b5c2d3919a2
--- /dev/null
+++ b/notes-external.h
@@ -0,0 +1,53 @@
+#ifndef NOTES_EXTERNAL_H
+#define NOTES_EXTERNAL_H
+
+#include "run-command.h"
+
+struct object_id;
+struct strbuf;
+
+struct external_notes_config {
+ char *command;
+ char *command_name_value;
+ uint64_t read_timeout_ns;
+ bool for_grep;
+};
+
+struct external_notes_process {
+ struct child_process process;
+ FILE *in;
+ int out_fd;
+ bool started;
+ bool failed;
+};
+
+struct external_notes_state {
+ struct external_notes_config config;
+ struct external_notes_process process;
+};
+
+struct external_notes_state *external_notes_new(void);
+void external_notes_free(struct external_notes_state *state);
+void external_notes_reset(struct external_notes_state *state);
+
+void set_external_notes_command(struct external_notes_state *state,
+ const char *command);
+bool external_notes_command_configured(const struct external_notes_state *state);
+
+void set_external_notes_command_name(struct external_notes_state *state,
+ const char *name);
+const char *external_notes_command_name(const struct external_notes_state *state);
+
+void set_external_notes_command_timeout_ms(struct external_notes_state *state,
+ int timeout_ms);
+int external_notes_command_timeout_ms(const struct external_notes_state *state);
+
+void set_external_notes_for_grep(struct external_notes_state *state,
+ int enabled);
+bool external_notes_for_grep_enabled(const struct external_notes_state *state);
+
+int format_external_note(struct external_notes_state *state,
+ const struct object_id *object_oid,
+ struct strbuf *out);
+
+#endif /* NOTES_EXTERNAL_H */
diff --git a/notes.c b/notes.c
index 201f1df3dc29..624d4aba223d 100644
--- a/notes.c
+++ b/notes.c
@@ -3,9 +3,12 @@
#include "git-compat-util.h"
#include "config.h"
+#include "commit.h"
#include "environment.h"
+#include "gettext.h"
#include "hex.h"
#include "notes.h"
+#include "notes-external.h"
#include "object-file.h"
#include "object-name.h"
#include "odb.h"
@@ -983,18 +986,59 @@ void string_list_add_refs_from_colon_sep(struct string_list *list,
free(globs_copy);
}
+struct notes_display_config_data {
+ int load_refs;
+ int load_command;
+ struct external_notes_state *external_notes_state;
+};
+
static int notes_display_config(const char *k, const char *v,
- const struct config_context *ctx UNUSED,
+ const struct config_context *ctx,
void *cb)
{
- int *load_refs = cb;
+ struct notes_display_config_data *data = cb;
- if (*load_refs && !strcmp(k, "notes.displayref")) {
+ if (data->load_refs && !strcmp(k, "notes.displayref")) {
if (!v)
return config_error_nonbool(k);
string_list_add_refs_by_glob(&display_notes_refs, v);
}
+ if (data->load_command && !strcmp(k, "notes.externalcommand")) {
+ if (!v)
+ return config_error_nonbool(k);
+
+ set_external_notes_command(data->external_notes_state, v);
+ }
+
+ if (data->load_command && !strcmp(k, "notes.externalcommandname")) {
+ if (!v)
+ return config_error_nonbool(k);
+
+ if (strchr(v, '\n') || strchr(v, '\r'))
+ return error(_("notes.externalCommandName must not contain a newline"));
+
+ set_external_notes_command_name(data->external_notes_state, v);
+ }
+
+ if (data->load_command && !strcmp(k, "notes.externalcommandtimeoutms")) {
+ int timeout_ms;
+
+ if (!v)
+ return config_error_nonbool(k);
+
+ timeout_ms = git_config_int(k, v, ctx->kvi);
+ if (timeout_ms < 0)
+ return error(_("notes.externalCommandTimeoutMs must be non-negative"));
+
+ set_external_notes_command_timeout_ms(data->external_notes_state,
+ timeout_ms);
+ }
+
+ if (data->load_command && !strcmp(k, "notes.externalcommandforgrep"))
+ set_external_notes_for_grep(data->external_notes_state,
+ git_config_bool(k, v));
+
return 0;
}
@@ -1075,17 +1119,21 @@ void init_display_notes(struct display_notes_opt *opt)
{
memset(opt, 0, sizeof(*opt));
opt->use_default_notes = -1;
+ opt->use_external_notes = -1;
string_list_init_dup(&opt->extra_notes_refs);
}
void release_display_notes(struct display_notes_opt *opt)
{
string_list_clear(&opt->extra_notes_refs, 0);
+ external_notes_free(opt->external_notes_state);
+ opt->external_notes_state = NULL;
}
void enable_default_display_notes(struct display_notes_opt *opt, int *show_notes)
{
opt->use_default_notes = 1;
+ opt->default_notes_suppressed_by_external = 0;
*show_notes = 1;
}
@@ -1102,31 +1150,96 @@ void enable_ref_display_notes(struct display_notes_opt *opt, int *show_notes,
void disable_display_notes(struct display_notes_opt *opt, int *show_notes)
{
opt->use_default_notes = -1;
+ opt->use_external_notes = -1;
+ opt->default_notes_suppressed_by_external = 0;
string_list_clear(&opt->extra_notes_refs, 0);
*show_notes = 0;
}
+/*
+ * Resolve the default-notes tri-state in one place. Callers must not test
+ * use_default_notes directly unless they specifically need the unresolved
+ * command-line state.
+ */
+static bool display_notes_use_default(const struct display_notes_opt *opt)
+{
+ /* Options aren't specified, default to true. */
+ if (!opt)
+ return true;
+
+ /* Explicitly enabled. */
+ if (opt->use_default_notes > 0)
+ return true;
+
+ /* Undefined and no explicit notes-ref specified, default to true. */
+ if (opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)
+ return true;
+
+ return false;
+}
+
+/*
+ * Resolve the external-notes tri-state. The unset value follows the resolved
+ * default-notes decision, which means "git log" runs the helper by default
+ * but "git log --notes=<ref>" does not.
+ */
+static bool display_notes_use_external(const struct display_notes_opt *opt)
+{
+ /* Options aren't specified, default to false. */
+ if (!opt)
+ return false;
+
+ /* Explicitly enabled. */
+ if (opt->use_external_notes > 0)
+ return true;
+
+ /* Undefined and to use default notes set, default to true. */
+ if (opt->use_external_notes < 0 && display_notes_use_default(opt))
+ return true;
+
+ return false;
+}
+
void load_display_notes(struct display_notes_opt *opt)
{
char *display_ref_env;
- int load_config_refs = 0;
+ struct notes_display_config_data config = { 0, 0 };
+ struct notes_display_config_data protected_config = { 0, 0 };
+ bool use_default_notes = display_notes_use_default(opt);
+ bool use_external_notes = display_notes_use_external(opt);
+
display_notes_refs.strdup_strings = 1;
+ if (use_external_notes && opt->external_notes_state) {
+ external_notes_reset(opt->external_notes_state);
+ } else if (opt) {
+ external_notes_free(opt->external_notes_state);
+ opt->external_notes_state = NULL;
+ }
+
assert(!display_notes_trees);
- if (!opt || opt->use_default_notes > 0 ||
- (opt->use_default_notes == -1 && !opt->extra_notes_refs.nr)) {
+ if (use_default_notes) {
string_list_append_nodup(&display_notes_refs, default_notes_ref(the_repository));
display_ref_env = getenv(GIT_NOTES_DISPLAY_REF_ENVIRONMENT);
if (display_ref_env) {
string_list_add_refs_from_colon_sep(&display_notes_refs,
display_ref_env);
- load_config_refs = 0;
+ config.load_refs = 0;
} else
- load_config_refs = 1;
+ config.load_refs = 1;
}
- repo_config(the_repository, notes_display_config, &load_config_refs);
+ if (use_external_notes) {
+ if (!opt->external_notes_state)
+ opt->external_notes_state = external_notes_new();
+
+ protected_config.load_command = 1;
+ protected_config.external_notes_state = opt->external_notes_state;
+ }
+
+ repo_config(the_repository, notes_display_config, &config);
+ git_protected_config(notes_display_config, &protected_config);
if (opt) {
struct string_list_item *item;
@@ -1266,47 +1379,31 @@ void free_notes(struct notes_tree *t)
}
/*
- * Fill the given strbuf with the notes associated with the given object.
+ * Append one already-loaded note message to the given strbuf.
*
- * If the given notes_tree structure is not initialized, it will be auto-
- * initialized to the default value (see documentation for init_notes() above).
- * If the given notes_tree is NULL, the internal/default notes_tree will be
- * used instead.
+ * Notes read from refs and notes obtained from notes.externalCommand both use
+ * this helper so they share the same encoding, header, and indentation rules.
*
* (raw == true) gives the %N userformat; otherwise, the note message is given
* for human consumption.
*/
-static void format_note(struct notes_tree *t, const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, bool raw)
+static void format_note_data(const char *ref, const char *msg, size_t msglen,
+ struct strbuf *sb, const char *output_encoding,
+ bool raw, bool literal_ref)
{
static const char utf8[] = "utf-8";
- const struct object_id *oid;
- char *msg, *msg_p;
- unsigned long linelen, msglen;
- enum object_type type;
-
- if (!t)
- t = &default_notes_tree;
- if (!t->initialized)
- init_notes(t, NULL, NULL, 0);
-
- oid = get_note(t, object_oid);
- if (!oid)
- return;
-
- if (!(msg = odb_read_object(the_repository->objects, oid, &type, &msglen)) ||
- type != OBJ_BLOB) {
- free(msg);
- return;
- }
+ char *reencoded = NULL;
+ const char *msg_p, *msg_end;
+ /* Convert the note text from UTF-8 to the requested output encoding. */
if (output_encoding && *output_encoding &&
!is_encoding_utf8(output_encoding)) {
- char *reencoded = reencode_string(msg, output_encoding, utf8);
+ size_t reencoded_len;
+ reencoded = reencode_string_len(msg, msglen, output_encoding,
+ utf8, &reencoded_len);
if (reencoded) {
- free(msg);
msg = reencoded;
- msglen = strlen(msg);
+ msglen = reencoded_len;
}
}
@@ -1314,37 +1411,106 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
if (msglen && msg[msglen - 1] == '\n')
msglen--;
+ /* Raw mode is the %N userformat, so it omits the "Notes" header. */
if (!raw) {
- const char *ref = t->ref;
- if (!ref || !strcmp(ref, GIT_NOTES_DEFAULT_REF)) {
+ if (!ref)
strbuf_addstr(sb, "\nNotes:\n");
- } else {
- skip_prefix(ref, "refs/", &ref);
- skip_prefix(ref, "notes/", &ref);
+ else if (!literal_ref && !strcmp(ref, GIT_NOTES_DEFAULT_REF))
+ strbuf_addstr(sb, "\nNotes:\n");
+ else {
+ if (!literal_ref) {
+ skip_prefix(ref, "refs/", &ref);
+ skip_prefix(ref, "notes/", &ref);
+ }
strbuf_addf(sb, "\nNotes (%s):\n", ref);
}
}
- for (msg_p = msg; msg_p < msg + msglen; msg_p += linelen + 1) {
- linelen = strchrnul(msg_p, '\n') - msg_p;
+ msg_end = msg + msglen;
+ for (msg_p = msg; msg_p < msg_end; ) {
+ const char *eol = memchr(msg_p, '\n', msg_end - msg_p);
+ size_t linelen = eol ? eol - msg_p : msg_end - msg_p;
+ /* Human output indents note body lines under the header. */
if (!raw)
strbuf_addstr(sb, " ");
+
strbuf_add(sb, msg_p, linelen);
strbuf_addch(sb, '\n');
+
+ msg_p += linelen;
+ if (msg_p < msg_end)
+ msg_p++;
+ }
+
+ free(reencoded);
+}
+
+/*
+ * Fill the given strbuf with the notes associated with the given object.
+ *
+ * If the given notes_tree structure is not initialized, it will be auto-
+ * initialized to the default value (see documentation for init_notes() above).
+ * If the given notes_tree is NULL, the internal/default notes_tree will be
+ * used instead.
+ */
+static void format_note_from_tree(struct notes_tree *t,
+ const struct object_id *object_oid,
+ struct strbuf *sb,
+ const char *output_encoding, bool raw)
+{
+ const struct object_id *oid;
+ char *msg;
+ unsigned long msglen;
+ enum object_type type;
+
+ if (!t)
+ t = &default_notes_tree;
+ if (!t->initialized)
+ init_notes(t, NULL, NULL, 0);
+
+ oid = get_note(t, object_oid);
+ if (!oid)
+ return;
+
+ if (!(msg = odb_read_object(the_repository->objects, oid, &type, &msglen)) ||
+ type != OBJ_BLOB) {
+ free(msg);
+ return;
}
+ format_note_data(t->ref, msg, msglen, sb, output_encoding, raw, false);
+
free(msg);
}
-void format_display_notes(const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, bool raw)
+void format_display_notes(const struct commit *commit,
+ struct strbuf *sb, const char *output_encoding,
+ bool raw,
+ struct external_notes_state *external_state)
{
int i;
+ const struct object_id *commit_oid = &commit->object.oid;
+
assert(display_notes_trees);
for (i = 0; display_notes_trees[i]; i++)
- format_note(display_notes_trees[i], object_oid, sb,
- output_encoding, raw);
+ format_note_from_tree(display_notes_trees[i], commit_oid, sb,
+ output_encoding, raw);
+
+ if (external_notes_command_configured(external_state)) {
+ struct strbuf out = STRBUF_INIT;
+
+ if (format_external_note(external_state, commit_oid, &out) == 0
+ && out.len) {
+ const char *label =
+ external_notes_command_name(external_state);
+
+ format_note_data(label, out.buf, out.len, sb,
+ output_encoding, raw, true);
+ }
+
+ strbuf_release(&out);
+ }
}
int copy_note(struct notes_tree *t,
diff --git a/notes.h b/notes.h
index f6410b31e1c9..5ac6a7e01dfb 100644
--- a/notes.h
+++ b/notes.h
@@ -6,6 +6,8 @@
struct object_id;
struct repository;
struct strbuf;
+struct commit;
+struct external_notes_state;
/*
* Function type for combining two notes annotating the same object.
@@ -264,11 +266,31 @@ struct display_notes_opt {
*/
int use_default_notes;
+ /*
+ * Less than `0` is "unset", which means external notes are shown iff
+ * the default notes are shown. Otherwise, treat it like a boolean.
+ */
+ int use_external_notes;
+
+ /*
+ * Tracks the synthetic "default notes off" state introduced by
+ * `--external-notes`, so a later deprecated `--show-notes=<ref>`
+ * can still preserve its historical additive behavior without
+ * overriding an explicit `--no-standard-notes`.
+ */
+ int default_notes_suppressed_by_external;
+
/*
* A list of globs (in the same style as notes.displayRef) where
* notes should be loaded from.
*/
struct string_list extra_notes_refs;
+
+ /*
+ * State for notes.externalCommand. This is initialized lazily by
+ * load_display_notes() when external notes may be used.
+ */
+ struct external_notes_state *external_notes_state;
};
/*
@@ -304,16 +326,21 @@ void disable_display_notes(struct display_notes_opt *opt, int *show_notes);
void load_display_notes(struct display_notes_opt *opt);
/*
- * Append notes for the given 'object_sha1' from all trees set up by
+ * Append notes for the given commit from all trees set up by
* load_display_notes() to 'sb'.
*
* If 'raw' is false the note will be indented by 4 places and
* a 'Notes (refname):' header added.
*
+ * If 'external_state' is not NULL, notes.externalCommand will be used to
+ * append the note from an external source.
+ *
* You *must* call load_display_notes() before using this function.
*/
-void format_display_notes(const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, bool raw);
+void format_display_notes(const struct commit *commit,
+ struct strbuf *sb, const char *output_encoding,
+ bool raw,
+ struct external_notes_state *external_state);
/*
* Load the notes tree from each ref listed in 'refs'. The output is
diff --git a/revision.c b/revision.c
index cd9fcefa0a88..f9581fa82f95 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@
#include "environment.h"
#include "gettext.h"
#include "hex.h"
+#include "notes-external.h"
#include "object-name.h"
#include "object-file.h"
#include "odb.h"
@@ -2583,18 +2584,40 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
} else if (skip_prefix(arg, "--show-notes=", &optarg) ||
skip_prefix(arg, "--notes=", &optarg)) {
if (starts_with(arg, "--show-notes=") &&
- revs->notes_opt.use_default_notes < 0)
+ (revs->notes_opt.use_default_notes < 0 ||
+ revs->notes_opt.default_notes_suppressed_by_external)) {
revs->notes_opt.use_default_notes = 1;
+ revs->notes_opt.default_notes_suppressed_by_external = 0;
+ }
enable_ref_display_notes(&revs->notes_opt, &revs->show_notes, optarg);
revs->show_notes_given = 1;
} else if (!strcmp(arg, "--no-notes")) {
disable_display_notes(&revs->notes_opt, &revs->show_notes);
revs->show_notes_given = 1;
+ } else if (!strcmp(arg, "--external-notes")) {
+ revs->notes_opt.use_external_notes = 1;
+ revs->show_notes = 1;
+ revs->show_notes_given = 1;
+ /*
+ * `--external-notes` names a note source on its own. If the
+ * default notes ref is still undecided, settle it to "off" so
+ * this option does not also trigger the "no explicit notes
+ * refs" fallback. A later use of `--notes` or the deprecated
+ * `--show-notes=<ref>` can still turn the default ref on.
+ */
+ if (revs->notes_opt.use_default_notes < 0) {
+ revs->notes_opt.use_default_notes = 0;
+ revs->notes_opt.default_notes_suppressed_by_external = 1;
+ }
+ } else if (!strcmp(arg, "--no-external-notes")) {
+ revs->notes_opt.use_external_notes = 0;
} else if (!strcmp(arg, "--standard-notes")) {
revs->show_notes_given = 1;
revs->notes_opt.use_default_notes = 1;
+ revs->notes_opt.default_notes_suppressed_by_external = 0;
} else if (!strcmp(arg, "--no-standard-notes")) {
revs->notes_opt.use_default_notes = 0;
+ revs->notes_opt.default_notes_suppressed_by_external = 0;
} else if (!strcmp(arg, "--oneline")) {
revs->verbose_header = 1;
get_commit_format("oneline", revs);
@@ -4105,9 +4128,18 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
/* Append "fake" message parts as needed */
if (opt->show_notes) {
+ const struct display_notes_opt *notes_opt = &opt->notes_opt;
+ struct external_notes_state *external_notes_state =
+ notes_opt->external_notes_state;
+
if (!buf.len)
strbuf_addstr(&buf, message);
- format_display_notes(&commit->object.oid, &buf, encoding, true);
+
+ if (!external_notes_for_grep_enabled(external_notes_state))
+ external_notes_state = NULL;
+
+ format_display_notes(commit, &buf, encoding, true,
+ external_notes_state);
}
/*
diff --git a/t/helper/meson.build b/t/helper/meson.build
index 3235f10ab8aa..15b6198c19fe 100644
--- a/t/helper/meson.build
+++ b/t/helper/meson.build
@@ -35,6 +35,7 @@ test_tool_sources = [
'test-mergesort.c',
'test-mktemp.c',
'test-name-hash.c',
+ 'test-notes-external-config-reset.c',
'test-online-cpus.c',
'test-pack-deltas.c',
'test-pack-mtimes.c',
diff --git a/t/helper/test-external-notes b/t/helper/test-external-notes
new file mode 100755
index 000000000000..5e9dde3977ab
--- /dev/null
+++ b/t/helper/test-external-notes
@@ -0,0 +1,64 @@
+#!/bin/sh
+
+prefix=${TEST_EXTERNAL_NOTES_PREFIX:-external-notes}
+response=${TEST_EXTERNAL_NOTES_RESPONSE:-ok}
+line_ending=${TEST_EXTERNAL_NOTES_LINE_ENDING:-lf}
+exit_after_response=${TEST_EXTERNAL_NOTES_EXIT_AFTER_RESPONSE:-}
+exit_delay=${TEST_EXTERNAL_NOTES_EXIT_DELAY:-}
+delay=${TEST_EXTERNAL_NOTES_DELAY:-}
+char_delay=${TEST_EXTERNAL_NOTES_CHAR_DELAY:-}
+ignore_term=${TEST_EXTERNAL_NOTES_IGNORE_TERM:-}
+
+newline='\n'
+case "$line_ending" in
+crlf)
+ newline='\r\n'
+ ;;
+none)
+ newline=
+ ;;
+esac
+
+echo start >>"$prefix-starts"
+
+test "$ignore_term" = true && trap '' TERM
+
+emit_output() {
+ if test -n "$char_delay"
+ then
+ LC_ALL=C
+ payload=$(printf "$@"; printf .)
+ payload=${payload%.}
+
+ while test -n "$payload"
+ do
+ char=${payload%"${payload#?}"}
+ printf '%s' "$char" || return 1
+ payload=${payload#?}
+ sleep "$char_delay" || return 1
+ done
+ else
+ printf "$@"
+ fi
+}
+
+while IFS= read -r commit; do
+ if test "${TEST_EXTERNAL_NOTES_BODY+x}" = x
+ then
+ note=$TEST_EXTERNAL_NOTES_BODY
+ else
+ note=$commit
+ fi
+ printf "%s\n" "$commit" >>"$prefix-requests"
+ test -z "$delay" || sleep "$delay"
+ if test "$response" = missing
+ then
+ emit_output "%s missing%b" "$commit" "$newline"
+ else
+ emit_output "%s ok %d%b%s%b" \
+ "$commit" "${#note}" "$newline" "$note" "$newline"
+ fi
+ test "$exit_after_response" = true && break
+done
+
+test -z "$exit_delay" || sleep "$exit_delay"
diff --git a/t/helper/test-notes-external-config-reset.c b/t/helper/test-notes-external-config-reset.c
new file mode 100644
index 000000000000..a64d03346fb9
--- /dev/null
+++ b/t/helper/test-notes-external-config-reset.c
@@ -0,0 +1,24 @@
+#include "test-tool.h"
+#include "notes-external.h"
+
+int cmd__notes_external_config_reset(int argc, const char **argv UNUSED)
+{
+ struct external_notes_state *state;
+
+ if (argc != 1)
+ die("usage: test-tool notes-external-config-reset");
+
+ state = external_notes_new();
+ set_external_notes_command(state, "helper");
+ set_external_notes_command_name(state, "label");
+ set_external_notes_command_timeout_ms(state, 250);
+ set_external_notes_for_grep(state, 1);
+ external_notes_reset(state);
+
+ printf("configured=%d\n", external_notes_command_configured(state));
+ printf("name=%s\n", external_notes_command_name(state));
+ printf("timeout_ms=%d\n", external_notes_command_timeout_ms(state));
+ printf("grep=%d\n", external_notes_for_grep_enabled(state));
+ external_notes_free(state);
+ return 0;
+}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index b71a22b43bbc..b4de5a2f5c06 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -45,6 +45,7 @@ static struct test_cmd cmds[] = {
{ "mergesort", cmd__mergesort },
{ "mktemp", cmd__mktemp },
{ "name-hash", cmd__name_hash },
+ { "notes-external-config-reset", cmd__notes_external_config_reset },
{ "online-cpus", cmd__online_cpus },
{ "pack-deltas", cmd__pack_deltas },
{ "pack-mtimes", cmd__pack_mtimes },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index f2885b33d58a..e74d4d934b14 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -38,6 +38,7 @@ int cmd__match_trees(int argc, const char **argv);
int cmd__mergesort(int argc, const char **argv);
int cmd__mktemp(int argc, const char **argv);
int cmd__name_hash(int argc, const char **argv);
+int cmd__notes_external_config_reset(int argc, const char **argv);
int cmd__online_cpus(int argc, const char **argv);
int cmd__pack_deltas(int argc, const char **argv);
int cmd__pack_mtimes(int argc, const char **argv);
diff --git a/t/lib-notes.sh b/t/lib-notes.sh
new file mode 100644
index 000000000000..07422540d58f
--- /dev/null
+++ b/t/lib-notes.sh
@@ -0,0 +1,19 @@
+# Helpers for scripts testing notes behavior.
+
+# notes.externalCommand is run through a shell, so quote the path.
+external_notes_command=$(
+ printf "%s\n" "$TEST_DIRECTORY/helper/test-external-notes" |
+ sed "s/'/'\\\\''/g; s/^/'/; s/$/'/"
+)
+
+# The helper above is a shell script. Few Windows CI tests (3 out of 10
+# in matrix) are spending more than the production default timeout just
+# starting the shell and exchanging the first response, so tests that
+# are not about timeout behavior fail. So let us opt into a wider 1s
+# deadline for Windows instead of 100ms.
+external_notes_command_timeout_config=
+if test_have_prereq MINGW
+then
+ _timeout_config="notes.externalCommandTimeoutMs=1000"
+ external_notes_command_timeout_config="-c $_timeout_config"
+fi
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 1e812df806bb..96adeb9bc4fe 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-notes.sh
# Note that because of the range-diff's heuristics, test_commit does more
# harm than good. We need some real history.
@@ -690,6 +691,37 @@ test_expect_success 'range-diff with --notes=custom does not show default notes'
grep "## Notes (custom) ##" actual
'
+test_expect_success 'range-diff with --external-notes' '
+ topic_oid=$(git rev-parse topic) &&
+ unmodified_oid=$(git rev-parse unmodified) &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ range-diff --no-color --external-notes \
+ main..topic main..unmodified >actual &&
+ test_grep "## Notes (external) ##" actual &&
+ test_grep "^ - $topic_oid$" actual &&
+ test_grep "^ + $unmodified_oid$" actual &&
+ ! grep "## Notes ##" actual
+'
+
+test_expect_success 'range-diff with disabled external notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
+ git notes add -m "topic note" topic &&
+ git notes add -m "unmodified note" unmodified &&
+ TEST_EXTERNAL_NOTES_PREFIX=range-diff-external-notes \
+ git -c notes.externalCommand="$external_notes_command" \
+ range-diff --no-color --external-notes --no-external-notes \
+ main..topic main..unmodified >actual &&
+ cat >expect <<-EOF &&
+ 1: $(test_oid t1) = 1: $(test_oid u1) s/5/A/
+ 2: $(test_oid t2) = 2: $(test_oid u2) s/4/A/
+ 3: $(test_oid t3) = 3: $(test_oid u3) s/11/B/
+ 4: $(test_oid t4) = 4: $(test_oid u4) s/12/B/
+ EOF
+ test_cmp expect actual &&
+ test_path_is_missing range-diff-external-notes-starts
+'
+
test_expect_success 'format-patch --range-diff does not compare notes by default' '
test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
@@ -780,6 +812,42 @@ test_expect_success 'format-patch --range-diff with --notes' '
test_cmp expect actual
'
+test_expect_success 'format-patch --range-diff with --external-notes' '
+ topic_oid=$(git rev-parse topic) &&
+ unmodified_oid=$(git rev-parse unmodified) &&
+ test_when_finished "rm -f 000?-*" &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ format-patch --external-notes --cover-letter --range-diff=$prev \
+ main..unmodified >actual &&
+ test_line_count = 5 actual &&
+ test_grep "^Range-diff:$" 0000-* &&
+ test_grep "## Notes (external) ##" 0000-* &&
+ test_grep "^ - $topic_oid$" 0000-* &&
+ test_grep "^ + $unmodified_oid$" 0000-* &&
+ ! grep "## Notes ##" 0000-*
+'
+
+test_expect_success 'format-patch --range-diff with disabled external notes' '
+ test_when_finished "git notes remove topic unmodified || :" &&
+ git notes add -m "topic note" topic &&
+ git notes add -m "unmodified note" unmodified &&
+ test_when_finished "rm -f 000?-*" &&
+ TEST_EXTERNAL_NOTES_PREFIX=range-diff-external-notes \
+ git -c notes.externalCommand="$external_notes_command" \
+ format-patch --external-notes --no-external-notes \
+ --cover-letter --range-diff=$prev main..unmodified >actual &&
+ test_line_count = 5 actual &&
+ test_grep "^Range-diff:$" 0000-* &&
+ grep "= 1: .* s/5/A" 0000-* &&
+ grep "= 2: .* s/4/A" 0000-* &&
+ grep "= 3: .* s/11/B" 0000-* &&
+ grep "= 4: .* s/12/B" 0000-* &&
+ ! grep "Notes" 0000-* &&
+ ! grep "note" 0000-* &&
+ test_path_is_missing range-diff-external-notes-starts
+'
+
test_expect_success 'format-patch --range-diff with format.notes config' '
test_when_finished "git notes remove topic unmodified || :" &&
git notes add -m "topic note" topic &&
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 27439010dfbc..7fd82767c1f1 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -6,6 +6,7 @@
test_description='Test commit notes'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-notes.sh
write_script fake_editor <<\EOF
echo "$MSG" >"$1"
@@ -16,6 +17,11 @@ export GIT_EDITOR
indent=" "
+run_with_limited_time () (
+ { set +x; } 2>/dev/null
+ "$PERL_PATH" -e 'alarm shift; exec @ARGV' -- "$@"
+)
+
test_expect_success 'cannot annotate non-existing HEAD' '
test_must_fail env MSG=3 git notes add
'
@@ -909,6 +915,424 @@ test_expect_success 'displayed notes are used for grep matching' '
test_must_be_empty actual
'
+test_expect_success 'notes.externalCommand shows external notes from protected config' '
+ commit=$(git rev-parse HEAD) &&
+ parent=$(git rev-parse HEAD^) &&
+ rm -f external-notes-starts external-notes-requests &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log -2 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ {
+ printf "%s\n" "$commit" &&
+ printf "%s\n" "$parent"
+ } >expect-requests &&
+ test_cmp expect-requests external-notes-requests &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ test_grep "^ $parent$" actual
+'
+
+test_expect_success PERL,EXECKEEPSPID 'notes.externalCommand terminates helper during exit cleanup' '
+ commit=$(git rev-parse HEAD) &&
+ test_env TEST_EXTERNAL_NOTES_EXIT_DELAY=10 \
+ run_with_limited_time 2 \
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommandName labels external notes' '
+ commit=$(git rev-parse HEAD) &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ -c notes.externalCommandName=commit-id log -1 >actual &&
+ test_grep "Notes (commit-id):" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommandName is rendered literally' '
+ commit=$(git rev-parse HEAD) &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ -c notes.externalCommandName=refs/notes/commits \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (refs/notes/commits):$" actual &&
+ ! grep "^Notes:$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommandTimeoutMs rejects negative values' '
+ test_must_fail git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=-1 log -1 2>err &&
+ test_grep "notes.externalCommandTimeoutMs must be non-negative" err
+'
+
+test_expect_success 'notes.externalCommandTimeoutMs times out delayed response' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_DELAY=1 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=1 \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success 'notes.externalCommandTimeoutMs applies to whole response' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_BODY=x \
+ TEST_EXTERNAL_NOTES_CHAR_DELAY=0.02 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=50 \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success PERL,EXECKEEPSPID 'notes.externalCommandTimeoutMs terminates timed-out helper' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_DELAY=10 \
+ run_with_limited_time 2 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=1 \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success 'notes.externalCommandTimeoutMs=0 disables timeout' '
+ commit=$(git rev-parse HEAD) &&
+ test_env TEST_EXTERNAL_NOTES_DELAY=1 \
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandTimeoutMs=0 \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommand handles CRLF note bodies' '
+ body=$(printf "A\r\nB") &&
+ test_env TEST_EXTERNAL_NOTES_BODY="$body" \
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ B$" actual
+'
+
+test_expect_success 'notes.externalCommand accepts CRLF missing response' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_RESPONSE=missing \
+ TEST_EXTERNAL_NOTES_LINE_ENDING=crlf \
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log -1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'notes.externalCommand rejects unterminated missing response' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_RESPONSE=missing \
+ TEST_EXTERNAL_NOTES_LINE_ENDING=none \
+ TEST_EXTERNAL_NOTES_EXIT_AFTER_RESPONSE=true \
+ git -c notes.externalCommand="$external_notes_command" \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success PERL,EXECKEEPSPID 'notes.externalCommand rejects unterminated live response without deadlock' '
+ git log -1 >expect &&
+ test_env TEST_EXTERNAL_NOTES_RESPONSE=missing \
+ TEST_EXTERNAL_NOTES_LINE_ENDING=none \
+ run_with_limited_time 2 \
+ git -c notes.externalCommand="$external_notes_command" \
+ log -1 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
+test_expect_success 'notes.externalCommand accepts CRLF protocol lines' '
+ commit=$(git rev-parse HEAD) &&
+ test_env TEST_EXTERNAL_NOTES_LINE_ENDING=crlf \
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes -1 >actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommand missing response shows no external notes' '
+ write_script external-notes-missing <<-\EOF &&
+ while IFS= read -r commit
+ do
+ printf "%s missing\n" "$commit"
+ done
+ EOF
+ git log -1 >expect &&
+ git -c notes.externalCommand=./external-notes-missing log -1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'notes.externalCommand empty note shows no external notes' '
+ write_script external-notes-empty <<-\EOF &&
+ while IFS= read -r commit
+ do
+ printf "%s ok 0\n\n" "$commit"
+ done
+ EOF
+ git log -1 >expect &&
+ git -c notes.externalCommand=./external-notes-empty log -1 >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'notes.externalCommand rejects invalid note lengths' '
+ write_script external-notes-invalid-length <<-\EOF &&
+ while IFS= read -r commit
+ do
+ printf "%s ok %s\n" "$commit" "$1"
+ done
+ EOF
+ git log -2 >expect &&
+ for bad_length in -1 +1 1x x
+ do
+ git -c notes.externalCommand="./external-notes-invalid-length $bad_length" \
+ log -2 >actual 2>err &&
+ test_cmp expect actual &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err || return 1
+ done
+'
+
+test_expect_success 'notes.externalCommand is suppressed by --no-notes' '
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" log --no-notes -1 >actual &&
+ test_path_is_missing external-notes-starts &&
+ ! grep "Notes (external):" actual
+'
+
+test_expect_success 'notes.externalCommand is suppressed by --no-external-notes' '
+ rm -f external-notes-starts &&
+ git log -1 >expect &&
+ git -c notes.externalCommand="$external_notes_command" \
+ log --no-external-notes -1 >actual &&
+ test_cmp expect actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommand combines with explicit notes ref' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --notes=other --external-notes -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (other):" actual &&
+ test_grep "^ other note$" actual &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ order test$" actual
+'
+
+test_expect_success '--show-notes=ref remains additive after --external-notes' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes --show-notes=other -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "^Notes:$" actual &&
+ test_grep "^ order test$" actual &&
+ test_grep "^Notes (other):$" actual &&
+ test_grep "^ other note$" actual &&
+ test_grep "^Notes (external):$" actual &&
+ test_grep "^ $commit$" actual
+'
+
+test_expect_success 'notes.externalCommand can be enabled without default notes refs' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ order test$" actual &&
+ ! grep "^ other note$" actual
+'
+
+test_expect_success 'notes.externalCommand combines with default notes refs' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --external-notes --notes -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes:" actual &&
+ test_grep "^ order test$" actual &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ other note$" actual
+'
+
+test_expect_success 'notes.externalCommand obeys last --external-notes option' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git log --no-notes -1 >expect &&
+ git -c notes.externalCommand="$external_notes_command" \
+ log --external-notes --no-external-notes -1 >actual &&
+ test_cmp expect actual &&
+ test_path_is_missing external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log --notes=other --no-external-notes --external-notes -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (other):" actual &&
+ test_grep "^ other note$" actual &&
+ test_grep "Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ order test$" actual
+'
+
+test_expect_success 'notes.externalCommand honors raw notes formatting' '
+ commit=$(git rev-parse HEAD) &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ show -s --format=%N >actual &&
+ test_grep "^$commit$" actual &&
+ ! grep "Notes (external):" actual
+'
+
+test_expect_success 'format-patch --external-notes includes external notes only' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ format-patch --external-notes -1 --stdout >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "^Notes (external):" actual &&
+ test_grep "^ $commit$" actual &&
+ ! grep "^ order test$" actual
+'
+
+test_expect_success 'notes.externalCommand is not used for grep matching' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ log --grep="$commit" >actual &&
+ test_must_be_empty actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommandForGrep includes external notes in grep matching' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ -c notes.externalCommandForGrep=true \
+ log --grep="$commit" -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (external):" actual
+'
+
+test_expect_success 'notes.externalCommandForGrep does not search hidden notes' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandForGrep=true \
+ log --oneline --grep="$commit" -1 >actual &&
+ test_must_be_empty actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommandForGrep honors --no-external-notes' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ -c notes.externalCommandForGrep=true \
+ log --no-external-notes --grep="$commit" -1 >actual &&
+ test_must_be_empty actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommandForGrep combines with explicit notes ref' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ -c notes.externalCommandForGrep=true \
+ log --notes=other --external-notes --grep="$commit" -1 >actual &&
+ test_line_count = 1 external-notes-starts &&
+ test_grep "Notes (external):" actual &&
+ test_grep "Notes (other):" actual &&
+ ! grep "^ order test$" actual
+'
+
+test_expect_success 'notes.externalCommandForGrep is ignored from local config' '
+ commit=$(git rev-parse HEAD) &&
+ rm -f external-notes-starts &&
+ test_config notes.externalCommandForGrep true &&
+ git -c notes.externalCommand="$external_notes_command" \
+ log --grep="$commit" >actual &&
+ test_must_be_empty actual &&
+ test_path_is_missing external-notes-starts
+'
+
+test_expect_success 'notes.externalCommand is not used with explicit notes ref' '
+ rm -f external-notes-starts &&
+ git -c notes.externalCommand="$external_notes_command" log --notes=other -1 >actual &&
+ test_path_is_missing external-notes-starts &&
+ ! grep "Notes (external):" actual
+'
+
+test_expect_success 'notes.externalCommand is ignored from local config' '
+ rm -f external-notes-starts &&
+ test_config notes.externalCommand "$external_notes_command" &&
+ git log -1 >actual &&
+ test_path_is_missing external-notes-starts &&
+ ! grep "Notes (external):" actual
+'
+
+test_expect_success 'notes.externalCommandName is ignored from local config' '
+ test_config notes.externalCommandName local &&
+ git -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ log -1 >actual &&
+ test_grep "Notes (external):" actual &&
+ ! grep "Notes (local):" actual
+'
+
+test_expect_success 'external_notes_reset clears cached helper config' '
+ test-tool notes-external-config-reset >actual &&
+ cat >expect <<-\EOF &&
+ configured=0
+ name=external
+ timeout_ms=100
+ grep=0
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_success 'notes.externalCommand warning is shown once' '
+ write_script external-notes-fail <<-\EOF &&
+ while IFS= read -r commit
+ do
+ printf "%s-mismatch missing\n" "$commit"
+ done
+ EOF
+ git -c notes.externalCommand=./external-notes-fail log -2 >actual 2>err &&
+ test_grep "notes.externalCommand failed" err &&
+ test_line_count = 1 err
+'
+
test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
test_config core.notesRef refs/notes/other &&
echo "Note on a tree" >expect &&
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 8ee3d2c37d02..abbdb42dc9f7 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -15,6 +15,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-notes.sh
check_describe () {
indir= &&
@@ -867,6 +868,22 @@ test_expect_success 'format-rev with %N (note)' '
test_cmp expect actual
'
+test_expect_success 'format-rev with %N uses external notes' '
+ commit=$(git -C repo-format rev-parse HEAD) &&
+ rm -f repo-format/format-rev-external-notes-starts \
+ repo-format/format-rev-external-notes-requests &&
+ printf "%s\n" "$commit" >input &&
+ printf "%s\n\n" "$commit" >expect &&
+ TEST_EXTERNAL_NOTES_PREFIX=format-rev-external-notes \
+ git -C repo-format -c notes.externalCommand="$external_notes_command" \
+ $external_notes_command_timeout_config \
+ format-rev --stdin-mode=text --format="tformat:%N" \
+ <input >actual &&
+ test_line_count = 1 repo-format/format-rev-external-notes-starts &&
+ test_cmp input repo-format/format-rev-external-notes-requests &&
+ test_cmp expect actual
+'
+
test_expect_success 'format-rev --notes<ref> (custom notes ref)' '
# One custom notes ref
test_when_finished "git -C repo-format notes remove" &&
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/4] t3301: cover generic displayed notes behavior
From: Siddh Raman Pant @ 2026-05-22 16:09 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Kristoffer Haugsbakk,
Junio C Hamano, brian m. carlson, Jeff King, Johannes Sixt,
Oswald Buddenhagen
In-Reply-To: <cover.1779464886.git.siddh.raman.pant@oracle.com>
Displayed notes already participate in common log behavior.
Add explicit coverage for raw notes formatting, --no-notes
suppression, explicit notes refs, and --grep matching before
teaching external notes to feed the same display path.
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
t/t3301-notes.sh | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index d6c50460d086..27439010dfbc 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh
@@ -885,6 +885,30 @@ test_expect_success '--show-notes=ref accumulates' '
test_cmp expect-both-reversed actual
'
+test_expect_success 'displayed notes honor raw notes formatting' '
+ git show -s --format=%N >actual &&
+ test_grep "^order test$" actual &&
+ ! grep "Notes" actual
+'
+
+test_expect_success 'displayed notes are suppressed by --no-notes' '
+ git log --no-notes -1 >actual &&
+ test_cmp expect-not-other actual
+'
+
+test_expect_success 'explicit notes ref replaces default displayed notes' '
+ git log --notes=other -1 >actual &&
+ test_cmp expect-other actual
+'
+
+test_expect_success 'displayed notes are used for grep matching' '
+ commit=$(git rev-parse HEAD) &&
+ git log --grep="order test" -1 >actual &&
+ test_grep "^commit $commit$" actual &&
+ git log --no-notes --grep="order test" -1 >actual &&
+ test_must_be_empty actual
+'
+
test_expect_success 'Allow notes on non-commits (trees, blobs, tags)' '
test_config core.notesRef refs/notes/other &&
echo "Note on a tree" >expect &&
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/4] wrapper: add support for timeout and deadline in read helpers
From: Siddh Raman Pant @ 2026-05-22 16:09 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Kristoffer Haugsbakk,
Junio C Hamano, brian m. carlson, Jeff King, Johannes Sixt,
Oswald Buddenhagen
In-Reply-To: <cover.1779464886.git.siddh.raman.pant@oracle.com>
Add read helpers which allow a caller to enforce a timeout per read,
and a deadline for the read in case multiple reads have to be done
under a common timeout.
Assisted-by: Codex:gpt-5.5-xhigh-fast
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
strbuf.c | 26 +++++++++-
strbuf.h | 4 ++
wrapper.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++++++----
wrapper.h | 23 +++++++++
4 files changed, 182 insertions(+), 10 deletions(-)
diff --git a/strbuf.c b/strbuf.c
index 3e04addc22fe..b3fc7c624aa2 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -749,13 +749,15 @@ int strbuf_getline_nul(struct strbuf *sb, FILE *fp)
return strbuf_getdelim(sb, fp, '\0');
}
-int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)
+static int strbuf_getwholeline_fd_with(struct strbuf *sb, int fd, int term,
+ xread_cb_t xread_cb,
+ void *cb_data)
{
strbuf_reset(sb);
while (1) {
char ch;
- ssize_t len = xread(fd, &ch, 1);
+ ssize_t len = xread_cb(fd, &ch, 1, cb_data);
if (len <= 0)
return EOF;
strbuf_addch(sb, ch);
@@ -765,6 +767,26 @@ int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)
return 0;
}
+int strbuf_getwholeline_fd_deadline(struct strbuf *sb, int fd, int term,
+ uint64_t deadline_ns)
+{
+ return strbuf_getwholeline_fd_with(sb, fd, term, xread_deadline_fn,
+ &deadline_ns);
+}
+
+int strbuf_getwholeline_fd_timeout(struct strbuf *sb, int fd, int term,
+ int timeout_ms)
+{
+ return strbuf_getwholeline_fd_with(sb, fd, term, xread_timeout_fn,
+ &timeout_ms);
+}
+
+/* Non-timeout version for compatibility. */
+int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)
+{
+ return strbuf_getwholeline_fd_timeout(sb, fd, term, 0);
+}
+
ssize_t strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
{
int fd;
diff --git a/strbuf.h b/strbuf.h
index 06e284f9cca4..f896da1277a6 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -535,6 +535,10 @@ int strbuf_appendwholeline(struct strbuf *sb, FILE *file, int term);
* descriptor.
*/
int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term);
+int strbuf_getwholeline_fd_timeout(struct strbuf *sb, int fd, int term,
+ int timeout_ms);
+int strbuf_getwholeline_fd_deadline(struct strbuf *sb, int fd, int term,
+ uint64_t deadline_ns);
/**
* Set the buffer to the path of the current working directory.
diff --git a/wrapper.c b/wrapper.c
index 16f5a63fbb61..1f42845e031e 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -9,6 +9,7 @@
#include "parse.h"
#include "gettext.h"
#include "strbuf.h"
+#include "trace.h"
#include "trace2.h"
#ifdef HAVE_RTLGENRANDOM
@@ -220,28 +221,129 @@ static int handle_nonblock(int fd, short poll_events, int err)
return 1;
}
-/*
- * xread() is the same a read(), but it automatically restarts read()
- * operations with a recoverable error (EAGAIN and EINTR). xread()
+static int wait_for_fd(int fd, short poll_events, int timeout_ms)
+{
+ struct pollfd pfd;
+
+ if (timeout_ms < 0) {
+ /* Negative timeout makes no sense. */
+ errno = EINVAL;
+ return -1;
+ }
+
+ pfd.fd = fd;
+ pfd.events = poll_events;
+
+ while(1) {
+ int ret = poll(&pfd, 1, timeout_ms);
+
+ if (ret <= 0) {
+ /* Retry if interrupted. */
+ if (ret < 0 && errno == EINTR)
+ continue;
+
+ /* Set errno if timeout happened. */
+ if (ret == 0)
+ errno = ETIMEDOUT;
+
+ return -1;
+ }
+
+ /* Invalid FD passed. */
+ if (pfd.revents & POLLNVAL) {
+ errno = EBADF;
+ return -1;
+ }
+
+ /* Some error happened. */
+ if (pfd.revents & POLLERR) {
+ errno = EIO;
+ return -1;
+ }
+
+ /* HangUp => We are ready to consume output till EOF. */
+ if (pfd.revents & (poll_events | POLLHUP))
+ return 0;
+ }
+}
+
+/**
+ * xread_timeout() is the same as read(), but it automatically restarts read()
+ * operations with a recoverable error (EAGAIN and EINTR). xread_timeout()
* DOES NOT GUARANTEE that "len" bytes is read even if the data is available.
+ *
+ * Fails with ETIMEDOUT when no bytes become available within timeout_ms
+ * milliseconds. A zero timeout disables timeout handling, so reads can
+ * block until the file descriptor is readable. Negative timeouts are invalid.
*/
-ssize_t xread(int fd, void *buf, size_t len)
+ssize_t xread_timeout(int fd, void *buf, size_t len, int timeout_ms)
{
ssize_t nr;
+
if (len > MAX_IO_SIZE)
len = MAX_IO_SIZE;
+
while (1) {
+ if (timeout_ms && wait_for_fd(fd, POLLIN, timeout_ms))
+ return -1;
+
nr = read(fd, buf, len);
+
if (nr < 0) {
if (errno == EINTR)
continue;
- if (handle_nonblock(fd, POLLIN, errno))
- continue;
+
+ if (timeout_ms) {
+ if (errno == EAGAIN || errno == EWOULDBLOCK)
+ continue;
+ } else {
+ if (handle_nonblock(fd, POLLIN, errno))
+ continue;
+ }
}
+
return nr;
}
}
+/* Non-timeout version for compatibility. */
+ssize_t xread(int fd, void *buf, size_t len)
+{
+ return xread_timeout(fd, buf, len, 0);
+}
+
+static int remaining_timeout_ms(uint64_t deadline_ns)
+{
+ uint64_t now, remaining_ns;
+
+ if (!deadline_ns)
+ return 0;
+
+ now = getnanotime();
+ if (now >= deadline_ns) {
+ errno = ETIMEDOUT;
+ return -1;
+ }
+
+ remaining_ns = deadline_ns - now;
+ return (int)((remaining_ns + 999999ULL) / 1000000ULL);
+}
+
+/* (deadline_ns = 0) disables the deadline and short-circuits to xread(). */
+ssize_t xread_deadline(int fd, void *buf, size_t len, uint64_t deadline_ns)
+{
+ int timeout_ms;
+
+ if (deadline_ns == 0)
+ return xread(fd, buf, len);
+
+ timeout_ms = remaining_timeout_ms(deadline_ns);
+ if (timeout_ms < 0)
+ return -1;
+
+ return xread_timeout(fd, buf, len, timeout_ms);
+}
+
/*
* xwrite() is the same a write(), but it automatically restarts write()
* operations with a recoverable error (EAGAIN and EINTR). xwrite() DOES NOT
@@ -283,13 +385,15 @@ ssize_t xpread(int fd, void *buf, size_t len, off_t offset)
}
}
-ssize_t read_in_full(int fd, void *buf, size_t count)
+static ssize_t read_in_full_with(int fd, void *buf, size_t count,
+ xread_cb_t xread_cb,
+ void *cb_data)
{
char *p = buf;
ssize_t total = 0;
while (count > 0) {
- ssize_t loaded = xread(fd, p, count);
+ ssize_t loaded = xread_cb(fd, p, count, cb_data);
if (loaded < 0)
return -1;
if (loaded == 0)
@@ -302,6 +406,25 @@ ssize_t read_in_full(int fd, void *buf, size_t count)
return total;
}
+ssize_t read_in_full_deadline(int fd, void *buf, size_t count,
+ uint64_t deadline_ns)
+{
+ return read_in_full_with(fd, buf, count, xread_deadline_fn,
+ &deadline_ns);
+}
+
+ssize_t read_in_full_timeout(int fd, void *buf, size_t count, int timeout_ms)
+{
+ return read_in_full_with(fd, buf, count, xread_timeout_fn,
+ &timeout_ms);
+}
+
+/* Non-timeout version for compatibility. */
+ssize_t read_in_full(int fd, void *buf, size_t count)
+{
+ return read_in_full_timeout(fd, buf, count, 0);
+}
+
ssize_t write_in_full(int fd, const void *buf, size_t count)
{
const char *p = buf;
diff --git a/wrapper.h b/wrapper.h
index 15ac3bab6e97..10d85c467b86 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -15,6 +15,8 @@ const char *mmap_os_err(void);
void *xmmap_gently(void *start, size_t length, int prot, int flags, int fd, off_t offset);
int xopen(const char *path, int flags, ...);
ssize_t xread(int fd, void *buf, size_t len);
+ssize_t xread_timeout(int fd, void *buf, size_t len, int timeout_ms);
+ssize_t xread_deadline(int fd, void *buf, size_t len, uint64_t deadline_ns);
ssize_t xwrite(int fd, const void *buf, size_t len);
ssize_t xpread(int fd, void *buf, size_t len, off_t offset);
int xdup(int fd);
@@ -44,9 +46,30 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode);
int git_mkstemp_mode(char *pattern, int mode);
ssize_t read_in_full(int fd, void *buf, size_t count);
+ssize_t read_in_full_timeout(int fd, void *buf, size_t count, int timeout_ms);
+ssize_t read_in_full_deadline(int fd, void *buf, size_t count,
+ uint64_t deadline_ns);
ssize_t write_in_full(int fd, const void *buf, size_t count);
ssize_t pread_in_full(int fd, void *buf, size_t count, off_t offset);
+typedef ssize_t xread_cb_t(int fd, void *buf, size_t len, const void *cb_data);
+
+static inline ssize_t xread_timeout_fn(int fd, void *buf, size_t len,
+ const void *cb_data)
+{
+ const int *timeout_ms = cb_data;
+
+ return xread_timeout(fd, buf, len, *timeout_ms);
+}
+
+static inline ssize_t xread_deadline_fn(int fd, void *buf, size_t len,
+ const void *cb_data)
+{
+ const uint64_t *deadline_ns = cb_data;
+
+ return xread_deadline(fd, buf, len, *deadline_ns);
+}
+
static inline ssize_t write_str_in_full(int fd, const char *str)
{
return write_in_full(fd, str, strlen(str));
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/4] notes: convert raw arg in format_display_notes() to bool
From: Siddh Raman Pant @ 2026-05-22 16:09 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Kristoffer Haugsbakk,
Junio C Hamano, brian m. carlson, Jeff King, Johannes Sixt,
Oswald Buddenhagen
In-Reply-To: <cover.1779464886.git.siddh.raman.pant@oracle.com>
It's used as a boolean flag, let's not use an int.
Signed-off-by: Siddh Raman Pant <siddh.raman.pant@oracle.com>
---
log-tree.c | 3 +--
notes.c | 6 +++---
notes.h | 2 +-
revision.c | 2 +-
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index 7e048701d0c5..4503a42dde6b 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -854,10 +854,9 @@ void show_log(struct rev_info *opt)
}
if (opt->show_notes) {
- int raw;
struct strbuf notebuf = STRBUF_INIT;
+ bool raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
- raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
format_display_notes(&commit->object.oid, ¬ebuf,
get_log_output_encoding(), raw);
ctx.notes_message = strbuf_detach(¬ebuf, NULL);
diff --git a/notes.c b/notes.c
index 8f315e2a00d2..201f1df3dc29 100644
--- a/notes.c
+++ b/notes.c
@@ -1273,11 +1273,11 @@ void free_notes(struct notes_tree *t)
* If the given notes_tree is NULL, the internal/default notes_tree will be
* used instead.
*
- * (raw != 0) gives the %N userformat; otherwise, the note message is given
+ * (raw == true) gives the %N userformat; otherwise, the note message is given
* for human consumption.
*/
static void format_note(struct notes_tree *t, const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, int raw)
+ struct strbuf *sb, const char *output_encoding, bool raw)
{
static const char utf8[] = "utf-8";
const struct object_id *oid;
@@ -1338,7 +1338,7 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
}
void format_display_notes(const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, int raw)
+ struct strbuf *sb, const char *output_encoding, bool raw)
{
int i;
assert(display_notes_trees);
diff --git a/notes.h b/notes.h
index 6dc6d7b26548..f6410b31e1c9 100644
--- a/notes.h
+++ b/notes.h
@@ -313,7 +313,7 @@ void load_display_notes(struct display_notes_opt *opt);
* You *must* call load_display_notes() before using this function.
*/
void format_display_notes(const struct object_id *object_oid,
- struct strbuf *sb, const char *output_encoding, int raw);
+ struct strbuf *sb, const char *output_encoding, bool raw);
/*
* Load the notes tree from each ref listed in 'refs'. The output is
diff --git a/revision.c b/revision.c
index 599b3a66c369..cd9fcefa0a88 100644
--- a/revision.c
+++ b/revision.c
@@ -4107,7 +4107,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
if (opt->show_notes) {
if (!buf.len)
strbuf_addstr(&buf, message);
- format_display_notes(&commit->object.oid, &buf, encoding, 1);
+ format_display_notes(&commit->object.oid, &buf, encoding, true);
}
/*
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/4] Add support for an external command for fetching notes
From: Siddh Raman Pant @ 2026-05-22 16:09 UTC (permalink / raw)
To: git
Cc: Elijah Newren, Patrick Steinhardt, Kristoffer Haugsbakk,
Junio C Hamano, brian m. carlson, Jeff King, Johannes Sixt,
Oswald Buddenhagen
v1: https://lore.kernel.org/git/cover.1779207350.git.siddh.raman.pant@oracle.com/
<...insert text from v1 cover here...>
Changes since v1:
- Removed Documentation commit and sent as a standalone patch.
- Removed finish_command_with_timeout addition (and thus sleep_nanosec).
- Squashed the external notes command code, doc, and test commits.
- Removed horizontal separators from note-external.c.
- Removed global variables from translation unit and instead store config in
a dedicated new struct member in struct display_notes_opt.
- Reworded the main commit to have better explanation of the motivation.
Testing: https://github.com/siddhpant/git/actions/runs/26295998442
Please review and let me know if any changes are needed or errors are there,
hopefully this gets accepted.
Thanks,
Siddh
Siddh Raman Pant (4):
notes: convert raw arg in format_display_notes() to bool
wrapper: add support for timeout and deadline in read helpers
t3301: cover generic displayed notes behavior
notes: support an external command to display notes
Documentation/config/notes.adoc | 61 +++
Documentation/git-format-patch.adoc | 11 +-
Documentation/git-range-diff.adoc | 6 +
Documentation/pretty-options.adoc | 9 +
Makefile | 2 +
builtin/log.c | 17 +-
builtin/name-rev.c | 9 +-
builtin/range-diff.c | 2 +
contrib/completion/git-completion.bash | 4 +-
log-tree.c | 10 +-
meson.build | 1 +
notes-external.c | 414 ++++++++++++++++++
notes-external.h | 53 +++
notes.c | 266 +++++++++---
notes.h | 33 +-
revision.c | 36 +-
strbuf.c | 26 +-
strbuf.h | 4 +
t/helper/meson.build | 1 +
t/helper/test-external-notes | 64 +++
t/helper/test-notes-external-config-reset.c | 24 ++
t/helper/test-tool.c | 1 +
t/helper/test-tool.h | 1 +
t/lib-notes.sh | 19 +
t/t3206-range-diff.sh | 68 +++
t/t3301-notes.sh | 448 ++++++++++++++++++++
t/t6120-describe.sh | 17 +
wrapper.c | 139 +++++-
wrapper.h | 23 +
29 files changed, 1693 insertions(+), 76 deletions(-)
create mode 100644 notes-external.c
create mode 100644 notes-external.h
create mode 100755 t/helper/test-external-notes
create mode 100644 t/helper/test-notes-external-config-reset.c
create mode 100644 t/lib-notes.sh
--
2.53.0
^ permalink raw reply
* [PATCH] receive-pack: fix updateInstead with core.worktree
From: Alyssa Ross @ 2026-05-22 15:44 UTC (permalink / raw)
To: git; +Cc: Ævar Arnfjörð Bjarmason, Junio C Hamano
This used to work, but when push_to_checkout() started being called
before push_to_deploy(), push_to_checkout()'s side effect of adding
GIT_WORK_TREE to the same environment that would be used by
push_to_deploy() wasn't taken into account. Fix by only mutating the
environment for push_to_commit(), rather than the shared environment.
Fixes: a8cc594333 ("hooks: fix an obscure TOCTOU "did we just run a hook?" race")
Signed-off-by: Alyssa Ross <hi@alyssa.is>
---
builtin/receive-pack.c | 2 +-
t/t5516-fetch-push.sh | 11 +++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c7b2818f20..7ee157532d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1460,8 +1460,8 @@ static const char *push_to_checkout(unsigned char *hash,
opt.invoked_hook = invoked_hook;
- strvec_pushf(env, "GIT_WORK_TREE=%s", absolute_path(work_tree));
strvec_pushv(&opt.env, env->v);
+ strvec_pushf(&opt.env, "GIT_WORK_TREE=%s", absolute_path(work_tree));
strvec_push(&opt.args, hash_to_hex(hash));
if (run_hooks_opt(the_repository, push_to_checkout_hook, &opt))
return "push-to-checkout hook declined";
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 117cfa051f..f51fb11a6d 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1791,6 +1791,17 @@ test_expect_success 'updateInstead with push-to-checkout hook' '
)
'
+test_expect_success 'denyCurrentBranch and core.worktree' '
+ test_when_finished "rm -fr cloned cloned.git" &&
+ git clone --separate-git-dir cloned.git . cloned &&
+ git --git-dir cloned.git config receive.denyCurrentBranch updateInstead &&
+ git --git-dir cloned.git config core.worktree "$PWD/cloned" &&
+ test_commit raspberry &&
+ git push cloned.git HEAD:main &&
+ test_path_exists cloned/raspberry.t &&
+ test_must_fail git push --delete cloned.git main
+'
+
test_expect_success 'denyCurrentBranch and worktrees' '
test_when_finished "rm -fr cloned && git worktree remove --force new-wt" &&
git worktree add new-wt &&
base-commit: aec3f587505a472db67e9462d0702e7d463a449d
--
2.53.0
^ permalink raw reply related
* Re: Why do we need to wait 1s between a git add and commit
From: Kristoffer Haugsbakk @ 2026-05-22 15:12 UTC (permalink / raw)
To: Fabrice SALVAIRE, git
In-Reply-To: <36eec9c1-1d2b-4cd1-a64c-a02936d8a2ae@orange.fr>
Hi
On Fri, May 22, 2026, at 14:28, Fabrice SALVAIRE wrote:
> I wrote a Python tool to dump a wiki to a git repository, that does
> basically a succession of subprocess calls to git add and commit.
>
> Recently, I discovered this tool doesn't work any longer and that git
> commit (2.54 on Fedora 42 / 43) crashes randomly.
>
> I cannot explain this behavior since my code is trivial.
>
> I had the intuition to add a sleep time of 1s just after a git call, and
> it solves the issue.
>
> I noticed for some cases that another call to git commit were
> successful. For most cases, git fsck and gitk report issues.
>
> It looks like the state of the git repository was not yet completed
> before the end of the git subprocess.
This might be caused by git-maintenance(1) being run in the background
without locking? That’s a new issue in Git 2.54.0. See:
https://lore.kernel.org/git/20260509175249.GA2336928@coredump.intra.peff.net/
The following script reproduces the issue on Git 2.53.0. I am guessing
that your script does something similar? It depends on how many commits
it creates in a short timeframe.
https://lore.kernel.org/git/20260508180341.GB737125@coredump.intra.peff.net/
^ permalink raw reply
* Re: [PATCH v2 07/11] git-gui: try harder to find worktree from gitdir
From: Shroom Moo @ 2026-05-22 15:09 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git, Johannes Sixt, Aina Boot
In-Reply-To: <780a52d9-7119-4870-990a-16280005503d@gmail.com>
On 5/22/26 1:45 AM, Mark Levedahl wrote:
> >
> > Additionally, [file exists {gitdir}] checks for the gitdir file in
> > the current working directory. Since the function has not yet
> > switched to $_gitdir when this check runs, it is almost impossible
> > to find the file. Consequently, this logic never triggers, preventing
> > linked worktrees from being recognized.
> >
> > Maybe the identification of linked worktree should not directly look
> > for the gitdir file, but should check whether there is a.git file and
> > its content points to... /.git/worktrees/... ? Anyways, using the
> > literal {gitdir} to search in the current directory lead to risks.
> >
> > Shroom
> >
> We cannot get to this code if not inside the gitdir, and if the user set GIT_DIR and/or
> GIT_WORK_TREE to do something clever, that either worked or the code already threw an
> error. git, without GIT_WORK_TREE set, uses the current directory as the worktree, or the
> parent directory containing .git. So, we must be inside the gitdir if this code path gets hit.
>
> Mark
The relative path issue with {gitdir} is indeed difficult to trigger
in practice. To trigger this problem, the following conditions must
be met simultaneously:
- git rev-parse --absolute-git-dir succeeds (the repository is valid).
- git rev-parse --show-toplevel fails (the working tree is
undetectable).
- The current directory is not $_gitdir.
- A gitdir file exists under $_gitdir (i.e., the gitdir of a linked
worktree).
Unluckily, this occurs in:
```MINGW64 Shell
# Firstly, enter a test folder
mkdir test-main
cd test-main
git init
echo main > file.txt
git add file.txt
git commit -m "initial"
# In feature branch
git branch feature
# Add worktree
git worktree add ../test-feature feature
cd ../
cd test-feature
cat .git
cd ../
mkdir outside
cd outside
export GIT_DIR="/(PREVIOUSPATH)/test-main/.git/worktrees/feature"
unset GIT_WORK_TREE
git gui blame HEAD file.txt --trace
```
```Wish
puts $_gitworktree
```
The current directory is "outside", not the expected linked worktree.
[file exists {gitdir}] in find_worktree_from_gitdir searched in the
current directory but failed, resulting in the inability to identify
linked worktree. The blame mode can still work without worktree.
Shroom
^ permalink raw reply
* Re: [PATCH v2 10/11] git-gui: adapt blame/browser parsing for bare operation
From: Shroom Moo @ 2026-05-22 15:05 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git, Johannes Sixt, Aina Boot
In-Reply-To: <bb38c5d4-b388-4eca-badd-69ec7ce67b90@gmail.com>
On 5/22/26 1:35 AM, Mark Levedahl wrote:
> Asking the worktree that is on version 33 about whether frotz is a directory in version 2
> is just asking for trouble, at best the worktree is authoritative for the checked out
> version, but even then there can be uncommitted changed. In the root of git-gui, I get
>
> /git-gui.sh browser gitgui-0.9.0 Makefile
> 'Makefile' is not a directory in rev 'gitgui-0.9.0'
>
> so the types of objects are being checked.
>
> Mark
You are right. The input is subdir instead of subdir/. All the
current tests work well. Patch 10/11 does not cause damage to the
functionality of browser and blame commands.
Shroom
^ permalink raw reply
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