* Re: [PATCH] worktree: record creation time and free-form note
From: Junio C Hamano @ 2026-06-02 23:52 UTC (permalink / raw)
To: Kiesel, Norbert; +Cc: git
In-Reply-To: <CAPGaHku+RAV+FA3C0md0xHiavfdB_anoqcMM06MAiU1VyMAdLA@mail.gmail.com>
"Kiesel, Norbert" <norbert.kiesel@creditkarma.com> writes:
> From 130cd5e4a25e6672b2a97268e1100b6ef03fa552 Mon Sep 17 00:00:00 2001
> From: Norbert Kiesel <norbert.kiesel@creditkarma.com>
> Date: Mon, 1 Jun 2026 17:03:39 -0700
> Subject: [PATCH] worktree: record creation time and free-form note
>
> Add per-worktree metadata so users can answer "what is this worktree
> for, and when did I make it?" without resorting to external notes.
Although I am not personally interested in this topic all that much,
let me point out that we have $GIT_DIR/description file that may be
useful for something like this. It has been the canonical place for
the main repository to identify itself long before secondary worktrees
were invented and $GIT_COMMON_DIR/worktrees/$worktree/description would
be a natural extension of the concept, I'd presume.
^ permalink raw reply
* Re: [PATCH v2] prio-queue: use cascade-down for faster extract-min
From: Kristofer Karlsson @ 2026-06-02 22:40 UTC (permalink / raw)
To: René Scharfe; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <90270818-c52b-4611-8da2-6cee20628fc2@web.de>
On Tue, 2 Jun 2026 at 18:37, René Scharfe <l.s.r@web.de> wrote:
>
> Would you be interested in benchmarking the following patch for making
> prio_queue_replace() unnecessary by doing its optimization
> automatically? I get a 1% performance hit for the describe command
> that I can't explain. And it leaves the heap unbalanced after a
> prio_queue_get(), which complicates things, so I found it lacking.
> But I wonder how it stacks up against your cascade approach for your
> use case and if there's anything to salvage.
>
> René
Thank you for the detailed feedback and the patch! It was very
helpful to have a concrete alternative to compare against.
I spent some time benchmarking the different approaches on a
large monorepo with a wide DAG.
All measurements include the nonstale O(1) tracking from my other
series as a common base, since that dominates the merge-base path.
The approaches I compared:
1. cascade-only: the sift_up_rebalance from this patch (v2)
2. rene-lazy: your deferred sift_down_root patch
3. cascade+lazy: cascade for unfused gets, lazy fusion for
get+put pairs
Results (10 runs, 1 warmup, CPU pinned to performance):
merge-base --all master master~1000 (~4s workload):
cascade-only 4.18s (median)
rene-lazy 4.25s
cascade+lazy 4.24s
rev-list --count master~1000..master (~3.8s workload):
cascade-only 3.86s
rene-lazy 3.75s
cascade+lazy 3.74s
The lazy approaches show a small win on rev-list (~3%) where get+put
pairs are common in limit_list. On merge-base --all, everything is
within noise, the prio_queue is a small fraction of total runtime
there. Combining cascade with lazy fusion didn't produce additional
gains beyond what each gives individually.
Looking at your patch, I think the deferred sift-down logic is
essentially the same optimization as the lazy_queue wrapper you
wrote for describe.c - both defer the work from get and fuse it
with a following put. So I'd be hesitant to add a second form of
that deferral directly into prio_queue when lazy_queue already
"owns" that responsibility as a wrapper.
That said, I think it would make sense to fold lazy_queue entirely
into prio_queue. It's an optimization that never hurts as far as I can
tell, and it would simplify several callers. pop_most_recent_commit
and show-branch both independently re-implement the same
peek+replace pattern that lazy_queue formalizes. Making it automatic
in prio_queue would clean up all of them.
I have a local branch exploring that direction. Maybe it makes more
sense to do the lazy_queue fold first, and then see if the cascade
change is still worth adding on top?
Either way, I think the two directions are complementary - cascade
reduces comparisons per sift, while lazy fusion can eliminate full
rebalance cycles.
I'm on a company offsite now so I may be slow to answer, but I will
definitely resume this when I get back home.
- Kristofer
^ permalink raw reply
* [PATCH v2 4/4] pack-objects: support `--delta-islands` with `--path-walk`
From: Taylor Blau @ 2026-06-02 22:21 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1780438896.git.me@ttaylorr.com>
Since the inception of `--path-walk`, this option has had a documented
incompatibility with `--delta-islands`.
When discussing those original patches on the list, a message from
Stolee in [1] noted the following:
this could be remedied by [...] doing a separate walk to identify
islands using the normal method
In a related portion of the thread, Peff explains[2]:
The delta islands code already does its own tree walk to propagate
the bits down (it does rely on the base walk's show_commit() to
propagate through the commits).
Once each object has its island bitmaps, I think however you
choose to come up with delta candidates [...] you should be able
to use it. It's fundamentally just answering the question of "am
I allowed to delta between these two objects".
That is similar to what this patch does, and it turns out the cheaper
option is sufficient: perform the same island side effects from the
path-walk callback rather than doing a second walk.
Recall how delta-islands are computed during a normal repack:
- `show_commit()` calls `propagate_island_marks()` for each commit,
which merges the commit's island bitset onto its root tree object and
onto each of its parent commits.
- `show_object()` for a tree records the tree's depth derived from the
slash-separated pathname. Subsequent `resolve_tree_islands()` uses
that depth to walk trees in increasing-depth order, propagating each
tree's marks to its children.
- At delta-search time, `in_same_island()` enforces that a delta
target's island bitmap is a subset of its base's: every island that
reaches the target must also reach the base.
Path-walk's enumeration callback is `add_objects_by_path()`. It already
adds objects to `to_pack`, but until now did not perform the
island-related side effects. Two things are needed:
- For each commit batch, call `propagate_island_marks()` on commits,
exactly as `show_commit()` does.
We have to be careful about the order in which we call this function,
and we must see a commit before its parents in order to have
island marks to propagate.
The path-walk batch preserves that order. Path-walk appends commits
to its `OBJ_COMMIT` batch as they come back from the same
`get_revision()` loop the regular traversal uses, and
`add_objects_by_path()` iterates the batch in array order. So every
commit reaches `propagate_island_marks()` in the same sequence that
`show_commit()` would have seen it, and the descendant-first chain
that the algorithm relies on is intact.
Skip island propagation for excluded commits to match the regular
traversal, whose `show_commit()` callback is only invoked for
interesting commits. Boundary commits may still be present in
path-walk's callback so they can serve as thin-pack bases, but they
should not contribute island marks.
- For each tree batch, record the tree's depth from the path. Use the
`record_tree_depth()` helper from the previous commit so both
callbacks behave identically, including the max-depth-wins behavior
when a tree is reached via more than one path. The helper accepts
both the `show_object()` path shape ("foo", "foo/bar") and the
path-walk shape with a trailing slash ("foo/", "foo/bar/"), so depths
recorded from either traversal mode are directly comparable.
This is implicit in the implementation sketch from Peff above.
`resolve_tree_islands()` sorts trees by `oe->tree_depth` in
increasing-depth order before propagating marks down, so that a
parent tree's marks are finalized before its children inherit them.
Without recording the depth at path-walk time, every
path-walk-discovered tree would land at depth 0 in `to_pack`, the
sort would lose its ordering, and children could inherit marks from
parents whose own contributions had not yet been merged in.
With those two pieces in place, `resolve_tree_islands()` receives the
same island inputs from path-walk as it would from the regular
traversal, so the existing island checks can be reused unchanged.
Drop the documented incompatibility between `--path-walk` and
`--delta-islands`, and add t5320 coverage for path-walk island repacks
with and without bitmap writing, as well as the same-island case where a
delta remains allowed.
[1]: https://lore.kernel.org/git/9aa2471b-0850-4707-9733-d3b33609f5f2@gmail.com/
[2]: https://lore.kernel.org/git/20240911063203.GA1538586@coredump.intra.peff.net/
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/git-pack-objects.adoc | 14 +++++++-------
builtin/pack-objects.c | 22 ++++++++++++++++++----
t/t5320-delta-islands.sh | 29 +++++++++++++++++++++++++++++
3 files changed, 54 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index 0adce8961a3..65cd00c152f 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -402,13 +402,13 @@ will be automatically changed to version `1`.
of filenames that cause collisions in Git's default name-hash
algorithm.
+
-Incompatible with `--delta-islands`. When `--use-bitmap-index` is
-specified with `--path-walk`, a successful bitmap traversal is used for
-object enumeration, with path-walk remaining as the fallback traversal
-when the bitmap cannot satisfy the request. The `--path-walk` option
-supports the `--filter=<spec>` forms `blob:none`, `blob:limit=<n>`,
-`tree:0`, `object:type=<type>`, and `sparse:<oid>`. These supported filter
-types can be combined with the `combine:<spec>+<spec>` form.
+When `--use-bitmap-index` is specified with `--path-walk`, a successful
+bitmap traversal is used for object enumeration, with path-walk
+remaining as the fallback traversal when the bitmap cannot satisfy the
+request. The `--path-walk` option supports the `--filter=<spec>` forms
+`blob:none`, `blob:limit=<n>`, `tree:0`, `object:type=<type>`, and
+`sparse:<oid>`. These supported filter types can be combined with the
+`combine:<spec>+<spec>` form.
DELTA ISLANDS
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index ec02e2b21d2..f48ea7a888b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4737,13 +4737,29 @@ static int add_objects_by_path(const char *path,
add_object_entry(oid, type, path, exclude);
- if (type == OBJ_COMMIT && write_bitmap_index) {
+ if (type == OBJ_COMMIT) {
struct commit *commit;
+ if (!write_bitmap_index && !use_delta_islands)
+ continue;
+
commit = lookup_commit(the_repository, oid);
if (!commit)
die(_("could not find commit %s"), oid_to_hex(oid));
- index_commit_for_bitmap(commit);
+ if (write_bitmap_index)
+ index_commit_for_bitmap(commit);
+ /*
+ * Skip island propagation for boundary commits.
+ * The regular traversal's show_commit() is only
+ * called for interesting commits; matching that
+ * here keeps path-walk from doing extra work that
+ * would only be a no-op anyway (boundary commits
+ * are not in island_marks).
+ */
+ if (use_delta_islands && !exclude)
+ propagate_island_marks(the_repository, commit);
+ } else if (type == OBJ_TREE && use_delta_islands) {
+ record_tree_depth(oid, path);
}
}
@@ -5205,8 +5221,6 @@ int cmd_pack_objects(int argc,
const char *option = NULL;
if (!path_walk_filter_compatible(&filter_options))
option = "--filter";
- else if (use_delta_islands)
- option = "--delta-islands";
if (option) {
warning(_("cannot use %s with %s"),
diff --git a/t/t5320-delta-islands.sh b/t/t5320-delta-islands.sh
index 2c961c70963..9b28344a0a3 100755
--- a/t/t5320-delta-islands.sh
+++ b/t/t5320-delta-islands.sh
@@ -53,6 +53,35 @@ test_expect_success 'separate islands disallows delta' '
! is_delta_base $two $one
'
+test_expect_success 'path-walk island repack respects islands' '
+ GIT_TRACE2_EVENT="$(pwd)/trace.path-walk-islands" \
+ git -c "pack.island=refs/heads/(.*)" repack -adfi \
+ --path-walk 2>err &&
+ test_region pack-objects path-walk trace.path-walk-islands &&
+ test_grep ! "cannot use --delta-islands with --path-walk" err &&
+ ! is_delta_base $one $two &&
+ ! is_delta_base $two $one
+'
+
+test_expect_success 'path-walk island bitmap repack respects islands' '
+ GIT_TRACE2_EVENT="$(pwd)/trace.path-walk-island-bitmap" \
+ git -c "pack.island=refs/heads/(.*)" repack -a -d -f -i -b \
+ --path-walk 2>err &&
+ test_region pack-objects path-walk trace.path-walk-island-bitmap &&
+ test_path_is_file .git/objects/pack/*.bitmap &&
+ git rev-list --test-bitmap --use-bitmap-index one &&
+ test_grep ! "cannot use --delta-islands with --path-walk" err &&
+ ! is_delta_base $one $two &&
+ ! is_delta_base $two $one
+'
+
+test_expect_success 'path-walk same island allows delta' '
+ GIT_TRACE2_EVENT="$(pwd)/trace.path-walk-same-island" \
+ git -c "pack.island=refs/heads" repack -adfi --path-walk &&
+ test_region pack-objects path-walk trace.path-walk-same-island &&
+ is_delta_base $one $two
+'
+
test_expect_success 'same island allows delta' '
git -c "pack.island=refs/heads" repack -adfi &&
is_delta_base $one $two
--
2.54.0.23.gae57607b57f
^ permalink raw reply related
* [PATCH v2 3/4] pack-objects: extract `record_tree_depth()` helper
From: Taylor Blau @ 2026-06-02 22:21 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1780438896.git.me@ttaylorr.com>
Prepare for a subsequent change that needs to record tree depths from a
second call site by factoring the delta-islands tree-depth bookkeeping
out of `show_object()` and into a helper, `record_tree_depth()`.
The helper looks up the object in `to_pack`, returns early when the
object was not added there, computes the depth from the slash count in
the supplied name, and preserves the existing max-depth-wins behavior
when a tree is reached by more than one path.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/pack-objects.c | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index e4dcb563b7d..ec02e2b21d2 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2722,6 +2722,22 @@ static inline void oe_set_tree_depth(struct packing_data *pack,
pack->tree_depth[e - pack->objects] = tree_depth;
}
+static void record_tree_depth(const struct object_id *oid, const char *name)
+{
+ const char *p;
+ unsigned depth;
+ struct object_entry *ent;
+
+ /* the empty string is a root tree, which is depth 0 */
+ depth = *name ? 1 : 0;
+ for (p = strchr(name, '/'); p; p = strchr(p + 1, '/'))
+ depth++;
+
+ ent = packlist_find(&to_pack, oid);
+ if (ent && depth > oe_tree_depth(&to_pack, ent))
+ oe_set_tree_depth(&to_pack, ent, depth);
+}
+
/*
* Return the size of the object without doing any delta
* reconstruction (so non-deltas are true object sizes, but deltas
@@ -4375,20 +4391,8 @@ static void show_object(struct object *obj, const char *name,
add_preferred_base_object(name);
add_object_entry(&obj->oid, obj->type, name, 0);
- if (use_delta_islands) {
- const char *p;
- unsigned depth;
- struct object_entry *ent;
-
- /* the empty string is a root tree, which is depth 0 */
- depth = *name ? 1 : 0;
- for (p = strchr(name, '/'); p; p = strchr(p + 1, '/'))
- depth++;
-
- ent = packlist_find(&to_pack, &obj->oid);
- if (ent && depth > oe_tree_depth(&to_pack, ent))
- oe_set_tree_depth(&to_pack, ent, depth);
- }
+ if (use_delta_islands)
+ record_tree_depth(&obj->oid, name);
}
static void show_object__ma_allow_any(struct object *obj, const char *name, void *data)
--
2.54.0.23.gae57607b57f
^ permalink raw reply related
* [PATCH v2 2/4] pack-objects: support reachability bitmaps with `--path-walk`
From: Taylor Blau @ 2026-06-02 22:21 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1780438896.git.me@ttaylorr.com>
When 'pack-objects' is invoked with '--path-walk', it prevents us from
using reachability bitmaps.
This behavior dates back to 70664d2865c (pack-objects: add --path-walk
option, 2025-05-16), which included a comment in the relevant portion of
the command-line arguments handling that read as follows:
/*
* We must disable the bitmaps because we are removing
* the --objects / --objects-edge[-aggressive] options.
*/
In fb2c309b7d3 (pack-objects: pass --objects with --path-walk,
2026-05-02), path-walk learned to pass '--objects' again, but still
kept bitmap traversal disabled. That leaves two useful cases
unsupported:
* A path-walk repack that writes bitmaps does not give the bitmap
selector any commits, because path-walk reveals commits through
`add_objects_by_path()` rather than through `show_commit()`, where
`index_commit_for_bitmap()` is normally called.
* An invocation like "git pack-objects --use-bitmap-index --path-walk"
never tries an existing bitmap, even when one is available and could
answer the request.
Fortunately for us, neither restriction is required.
* On the writing side: teach the path-walk object callback to call
`index_commit_for_bitmap()` for commits that it adds to the pack.
That gives the bitmap selector the commit candidates it would have
seen from the regular traversal.
* For bitmap reading, keep passing '--objects' to the internal rev_list
machinery, but stop clearing `use_bitmap_index`. If an existing
bitmap can answer the request, use it; otherwise fall back to
path-walk's own enumeration.
As a result, we can see significantly reduced pack sizes from p5311
before this commit:
Test HEAD^ HEAD
----------------------------------------------------------------------------------
5311.38: server (1 days, --path-walk) 2.56(2.52+0.03) 0.01(0.01+0.00) -99.6%
5311.39: size (1 days, --path-walk) 123.9K 123.9K +0.0%
5311.40: client (1 days, --path-walk) 0.00(0.01+0.00) 0.00(0.00+0.00) =
5311.42: server (2 days, --path-walk) 2.57(2.52+0.05) 0.01(0.01+0.00) -99.6%
5311.43: size (2 days, --path-walk) 123.9K 123.9K +0.0%
5311.44: client (2 days, --path-walk) 0.00(0.00+0.00) 0.00(0.00+0.00) =
5311.46: server (4 days, --path-walk) 2.58(2.51+0.07) 0.01(0.01+0.00) -99.6%
5311.47: size (4 days, --path-walk) 123.9K 123.9K +0.0%
5311.48: client (4 days, --path-walk) 0.00(0.00+0.00) 0.00(0.00+0.00) =
5311.50: server (8 days, --path-walk) 2.58(2.53+0.04) 0.02(0.02+0.00) -99.2%
5311.51: size (8 days, --path-walk) 152.4K 152.4K +0.0%
5311.52: client (8 days, --path-walk) 0.00(0.01+0.00) 0.00(0.01+0.00) =
5311.54: server (16 days, --path-walk) 2.58(2.52+0.05) 0.03(0.02+0.00) -98.8%
5311.55: size (16 days, --path-walk) 205.3K 205.3K +0.0%
5311.56: client (16 days, --path-walk) 0.01(0.01+0.00) 0.01(0.01+0.00) +0.0%
5311.58: server (32 days, --path-walk) 2.59(2.53+0.06) 0.03(0.03+0.00) -98.8%
5311.59: size (32 days, --path-walk) 209.3K 209.3K +0.0%
5311.60: client (32 days, --path-walk) 0.01(0.02+0.00) 0.01(0.02+0.00) +0.0%
5311.62: server (64 days, --path-walk) 2.70(2.76+0.06) 0.16(0.24+0.04) -94.1%
5311.63: size (64 days, --path-walk) 4.1M 4.1M +0.0%
5311.64: client (64 days, --path-walk) 0.44(0.50+0.02) 0.44(0.51+0.02) +0.0%
5311.66: server (128 days, --path-walk) 2.88(3.20+0.05) 0.34(0.65+0.05) -88.2%
5311.67: size (128 days, --path-walk) 9.0M 9.0M -0.0%
5311.68: client (128 days, --path-walk) 0.93(1.22+0.07) 0.93(1.20+0.08) +0.0%
We get the same size of output pack, but this commit allows us to do so
in a significantly shorter amount of time. Intuitively, we're generating
the same pack (hence the unchanged 'test_size' output from run to run),
but varying how we get there. Before this commit, pack-objects prefers
'--path-walk' to '--use-bitmap-index', so we generate the output pack by
performing a normal '--path-walk' traversal. With this commit, we are
operating over a *repacked* state (that itself was done with a
'--path-walk' traversal), but are able to perform pack-reuse on that
repacked state via bitmaps.
There is one wrinkle when it comes to '--boundary', which we must not
pass into the bitmap walk in the presence of both '--path-walk' and
'--use-bitmap-index'. Path-walk needs boundary commits when it performs
its own traversal, in order to discover bases for thin packs, but the
bitmap traversal does not expect this. Work around this by setting
`revs->boundary` as late as possible within the '--path-walk' traversal,
after any bitmap attempt has either succeeded or declined to answer the
request.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Documentation/git-pack-objects.adoc | 6 +++--
builtin/pack-objects.c | 18 +++++++++++++--
t/perf/p5311-pack-bitmaps-fetch.sh | 14 +++++++----
t/t5310-pack-bitmaps.sh | 36 +++++++++++++++++++++++++++++
4 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-pack-objects.adoc b/Documentation/git-pack-objects.adoc
index 8a27aa19fd3..0adce8961a3 100644
--- a/Documentation/git-pack-objects.adoc
+++ b/Documentation/git-pack-objects.adoc
@@ -402,8 +402,10 @@ will be automatically changed to version `1`.
of filenames that cause collisions in Git's default name-hash
algorithm.
+
-Incompatible with `--delta-islands`. The `--use-bitmap-index` option is
-ignored in the presence of `--path-walk`. The `--path-walk` option
+Incompatible with `--delta-islands`. When `--use-bitmap-index` is
+specified with `--path-walk`, a successful bitmap traversal is used for
+object enumeration, with path-walk remaining as the fallback traversal
+when the bitmap cannot satisfy the request. The `--path-walk` option
supports the `--filter=<spec>` forms `blob:none`, `blob:limit=<n>`,
`tree:0`, `object:type=<type>`, and `sparse:<oid>`. These supported filter
types can be combined with the `combine:<spec>+<spec>` form.
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index b783dc62bc9..e4dcb563b7d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4732,6 +4732,15 @@ static int add_objects_by_path(const char *path,
continue;
add_object_entry(oid, type, path, exclude);
+
+ if (type == OBJ_COMMIT && write_bitmap_index) {
+ struct commit *commit;
+
+ commit = lookup_commit(the_repository, oid);
+ if (!commit)
+ die(_("could not find commit %s"), oid_to_hex(oid));
+ index_commit_for_bitmap(commit);
+ }
}
oe_end = to_pack.nr_objects;
@@ -4764,6 +4773,13 @@ static int get_object_list_path_walk(struct rev_info *revs)
info.path_fn = add_objects_by_path;
info.path_fn_data = &processed;
+ /*
+ * Path-walk needs boundary commits to discover thin-pack bases, but
+ * bitmap traversal does not understand the boundary state. Set it
+ * here so any prior bitmap attempt sees the usual non-boundary walk.
+ */
+ revs->boundary = 1;
+
/*
* Allow the --[no-]sparse option to be interesting here, if only
* for testing purposes. Paths with no interesting objects will not
@@ -5195,9 +5211,7 @@ int cmd_pack_objects(int argc,
}
}
if (path_walk) {
- strvec_push(&rp, "--boundary");
strvec_push(&rp, "--objects");
- use_bitmap_index = 0;
} else if (thin) {
use_internal_rev_list = 1;
strvec_push(&rp, shallow
diff --git a/t/perf/p5311-pack-bitmaps-fetch.sh b/t/perf/p5311-pack-bitmaps-fetch.sh
index 5bea5c64e7b..1b115d921a1 100755
--- a/t/perf/p5311-pack-bitmaps-fetch.sh
+++ b/t/perf/p5311-pack-bitmaps-fetch.sh
@@ -4,15 +4,18 @@ test_description='performance of fetches from bitmapped packs'
. ./perf-lib.sh
test_fetch_bitmaps () {
+ argv=$1
+ export argv
+
test_expect_success 'setup test directory' '
rm -fr * .git
'
test_perf_default_repo
- test_expect_success 'create bitmapped server repo' '
+ test_expect_success "create bitmapped server repo ${argv:+($argv)}" '
git config pack.writebitmaps true &&
- git repack -ad
+ git repack -ad $argv
'
# simulate a fetch from a repository that last fetched N days ago, for
@@ -20,7 +23,7 @@ test_fetch_bitmaps () {
# and assume the first entry in the chain that is N days older than the current
# HEAD is where the HEAD would have been then.
for days in 1 2 4 8 16 32 64 128; do
- title=$(printf '%10s' "($days days)")
+ title=$(printf '%10s' "($days days${argv:+, $argv})")
test_expect_success "setup revs from $days days ago" '
now=$(git log -1 --format=%ct HEAD) &&
then=$(($now - ($days * 86400))) &&
@@ -47,6 +50,9 @@ test_fetch_bitmaps () {
done
}
-test_fetch_bitmaps
+for argv in '' --path-walk
+do
+ test_fetch_bitmaps $argv || return 1
+done
test_done
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index f693cb56691..69c5da1580a 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -577,6 +577,42 @@ test_bitmap_cases
sane_unset GIT_TEST_PACK_USE_BITMAP_BOUNDARY_TRAVERSAL
+test_expect_success 'path-walk repack can write and use bitmap indexes' '
+ test_when_finished "rm -rf path-walk-bitmap" &&
+ git init path-walk-bitmap &&
+ (
+ cd path-walk-bitmap &&
+ test_commit first &&
+ test_commit second &&
+ test_commit third &&
+
+ git repack -a -d -b --path-walk &&
+ git rev-list --test-bitmap --use-bitmap-index HEAD &&
+
+ git rev-parse HEAD >in &&
+
+ git rev-list --objects --no-object-names HEAD >expect.raw &&
+ sort expect.raw >expect &&
+
+ for reuse in true false
+ do
+ : >trace.txt &&
+
+ GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
+ git -c pack.allowPackReuse=$reuse pack-objects \
+ --stdout --revs --path-walk --use-bitmap-index \
+ <in >out.pack &&
+ grep "\"category\":\"bitmap\",\"key\":\"bitmap/hits\"" trace.txt &&
+
+ git index-pack out.pack &&
+
+ list_packed_objects out.idx >actual.raw &&
+ sort actual.raw >actual &&
+ test_cmp expect actual || return 1
+ done
+ )
+'
+
test_expect_success 'incremental repack fails when bitmaps are requested' '
test_commit more-1 &&
test_must_fail git repack -d 2>err &&
--
2.54.0.23.gae57607b57f
^ permalink raw reply related
* [PATCH v2 1/4] t/perf: drop p5311's lookup-table permutation
From: Taylor Blau @ 2026-06-02 22:21 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1780438896.git.me@ttaylorr.com>
p5311 measures the cost of serving a fetch from a bitmapped pack and
indexing the resulting pack on the client. Since 761416ef91d
(bitmap-lookup-table: add performance tests for lookup table,
2022-08-14), p5311 effectively runs itself twice: once with the bitmap's
lookup table extension enabled, and again with it disabled.
This comparison has served its useful purpose, as the lookup table is
almost four years old, and the de-facto default in server-side Git
deployments.
A following commit will want to test a different combination (repacking
with and without '--path-walk' instead of the lookup table). Instead of
multiplying the current test count by two again to produce four
variations of `test_fetch_bitmaps()`, drop the lookup table option to
reduce the number of perf tests we run. Retain `test_fetch_bitmaps()`
itself, since we will use this in the future for the new
parameterization.
(As an aside, a future commit outside of this series will adjust the
default value of 'pack.writeBitmapLookupTable' to "true", matching the
de-facto norm for deployments where the existence of bitmap lookup
tables is meaningful. Punt on that to a later series and instead make
the minimal change for now.)
Suggested-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
t/perf/p5311-pack-bitmaps-fetch.sh | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/t/perf/p5311-pack-bitmaps-fetch.sh b/t/perf/p5311-pack-bitmaps-fetch.sh
index 047efb995d6..5bea5c64e7b 100755
--- a/t/perf/p5311-pack-bitmaps-fetch.sh
+++ b/t/perf/p5311-pack-bitmaps-fetch.sh
@@ -12,7 +12,6 @@ test_fetch_bitmaps () {
test_expect_success 'create bitmapped server repo' '
git config pack.writebitmaps true &&
- git config pack.writeBitmapLookupTable '"$1"' &&
git repack -ad
'
@@ -32,7 +31,7 @@ test_fetch_bitmaps () {
} >revs
'
- test_perf "server $title (lookup=$1)" '
+ test_perf "server $title" '
git pack-objects --stdout --revs \
--thin --delta-base-offset \
<revs >tmp.pack
@@ -42,13 +41,12 @@ test_fetch_bitmaps () {
test_file_size tmp.pack
'
- test_perf "client $title (lookup=$1)" '
+ test_perf "client $title" '
git index-pack --stdin --fix-thin <tmp.pack
'
done
}
-test_fetch_bitmaps true
-test_fetch_bitmaps false
+test_fetch_bitmaps
test_done
--
2.54.0.23.gae57607b57f
^ permalink raw reply related
* [PATCH v2 0/4] pack-objects: support bitmaps and delta-islands with `--path-walk`
From: Taylor Blau @ 2026-06-02 22:21 UTC (permalink / raw)
To: git; +Cc: Derrick Stolee, Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1779923907.git.me@ttaylorr.com>
Note to the maintainer:
* This series is based on 'ds/path-walk-filters' with Patrick's
'ps/clang-w-glibc-2.43-and-_Generic' merged in. The former has since
graduated. These are the three remaining patches from my earlier RFC
after Stolee's series incorporated the filter-related pieces.
Here is a very small reroll of my series to make `--path-walk` work with
reachability bitmaps and delta-islands.
Since the previous round, the only changes are:
* A new commit making some adjustments to p5311 to facilitate
performance testing bitmaps in repositories repacked with
'--path-walk'.
* Updates to (what is now) the second commit's message, including
performance results based on the aforementioned changes.
Outside of the above, the series is otherwise unchanged.
Thanks in advance for your review!
Taylor Blau (4):
t/perf: drop p5311's lookup-table permutation
pack-objects: support reachability bitmaps with `--path-walk`
pack-objects: extract `record_tree_depth()` helper
pack-objects: support `--delta-islands` with `--path-walk`
Documentation/git-pack-objects.adoc | 12 ++---
builtin/pack-objects.c | 68 +++++++++++++++++++++--------
t/perf/p5311-pack-bitmaps-fetch.sh | 20 +++++----
t/t5310-pack-bitmaps.sh | 36 +++++++++++++++
t/t5320-delta-islands.sh | 29 ++++++++++++
5 files changed, 134 insertions(+), 31 deletions(-)
Range-diff against v1:
-: ----------- > 1: 52d63e8910e t/perf: drop p5311's lookup-table permutation
1: 3fa8bfbfd59 ! 2: ffad584a43e pack-objects: support reachability bitmaps with `--path-walk`
@@ Commit message
bitmap can answer the request, use it; otherwise fall back to
path-walk's own enumeration.
+ As a result, we can see significantly reduced pack sizes from p5311
+ before this commit:
+
+ Test HEAD^ HEAD
+ ----------------------------------------------------------------------------------
+ 5311.38: server (1 days, --path-walk) 2.56(2.52+0.03) 0.01(0.01+0.00) -99.6%
+ 5311.39: size (1 days, --path-walk) 123.9K 123.9K +0.0%
+ 5311.40: client (1 days, --path-walk) 0.00(0.01+0.00) 0.00(0.00+0.00) =
+ 5311.42: server (2 days, --path-walk) 2.57(2.52+0.05) 0.01(0.01+0.00) -99.6%
+ 5311.43: size (2 days, --path-walk) 123.9K 123.9K +0.0%
+ 5311.44: client (2 days, --path-walk) 0.00(0.00+0.00) 0.00(0.00+0.00) =
+ 5311.46: server (4 days, --path-walk) 2.58(2.51+0.07) 0.01(0.01+0.00) -99.6%
+ 5311.47: size (4 days, --path-walk) 123.9K 123.9K +0.0%
+ 5311.48: client (4 days, --path-walk) 0.00(0.00+0.00) 0.00(0.00+0.00) =
+ 5311.50: server (8 days, --path-walk) 2.58(2.53+0.04) 0.02(0.02+0.00) -99.2%
+ 5311.51: size (8 days, --path-walk) 152.4K 152.4K +0.0%
+ 5311.52: client (8 days, --path-walk) 0.00(0.01+0.00) 0.00(0.01+0.00) =
+ 5311.54: server (16 days, --path-walk) 2.58(2.52+0.05) 0.03(0.02+0.00) -98.8%
+ 5311.55: size (16 days, --path-walk) 205.3K 205.3K +0.0%
+ 5311.56: client (16 days, --path-walk) 0.01(0.01+0.00) 0.01(0.01+0.00) +0.0%
+ 5311.58: server (32 days, --path-walk) 2.59(2.53+0.06) 0.03(0.03+0.00) -98.8%
+ 5311.59: size (32 days, --path-walk) 209.3K 209.3K +0.0%
+ 5311.60: client (32 days, --path-walk) 0.01(0.02+0.00) 0.01(0.02+0.00) +0.0%
+ 5311.62: server (64 days, --path-walk) 2.70(2.76+0.06) 0.16(0.24+0.04) -94.1%
+ 5311.63: size (64 days, --path-walk) 4.1M 4.1M +0.0%
+ 5311.64: client (64 days, --path-walk) 0.44(0.50+0.02) 0.44(0.51+0.02) +0.0%
+ 5311.66: server (128 days, --path-walk) 2.88(3.20+0.05) 0.34(0.65+0.05) -88.2%
+ 5311.67: size (128 days, --path-walk) 9.0M 9.0M -0.0%
+ 5311.68: client (128 days, --path-walk) 0.93(1.22+0.07) 0.93(1.20+0.08) +0.0%
+
+ We get the same size of output pack, but this commit allows us to do so
+ in a significantly shorter amount of time. Intuitively, we're generating
+ the same pack (hence the unchanged 'test_size' output from run to run),
+ but varying how we get there. Before this commit, pack-objects prefers
+ '--path-walk' to '--use-bitmap-index', so we generate the output pack by
+ performing a normal '--path-walk' traversal. With this commit, we are
+ operating over a *repacked* state (that itself was done with a
+ '--path-walk' traversal), but are able to perform pack-reuse on that
+ repacked state via bitmaps.
+
There is one wrinkle when it comes to '--boundary', which we must not
pass into the bitmap walk in the presence of both '--path-walk' and
'--use-bitmap-index'. Path-walk needs boundary commits when it performs
its own traversal, in order to discover bases for thin packs, but the
- bitmap traversal expects the usual non-boundary state. Work around this
- by setting `revs->boundary` as late as possible within
- `get_object_list_path_walk()`, after any bitmap attempt has either
- succeeded or declined to answer the request.
+ bitmap traversal does not expect this. Work around this by setting
+ `revs->boundary` as late as possible within the '--path-walk' traversal,
+ after any bitmap attempt has either succeeded or declined to answer the
+ request.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
@@ builtin/pack-objects.c: int cmd_pack_objects(int argc,
use_internal_rev_list = 1;
strvec_push(&rp, shallow
+ ## t/perf/p5311-pack-bitmaps-fetch.sh ##
+@@ t/perf/p5311-pack-bitmaps-fetch.sh: test_description='performance of fetches from bitmapped packs'
+ . ./perf-lib.sh
+
+ test_fetch_bitmaps () {
++ argv=$1
++ export argv
++
+ test_expect_success 'setup test directory' '
+ rm -fr * .git
+ '
+
+ test_perf_default_repo
+
+- test_expect_success 'create bitmapped server repo' '
++ test_expect_success "create bitmapped server repo ${argv:+($argv)}" '
+ git config pack.writebitmaps true &&
+- git repack -ad
++ git repack -ad $argv
+ '
+
+ # simulate a fetch from a repository that last fetched N days ago, for
+@@ t/perf/p5311-pack-bitmaps-fetch.sh: test_fetch_bitmaps () {
+ # and assume the first entry in the chain that is N days older than the current
+ # HEAD is where the HEAD would have been then.
+ for days in 1 2 4 8 16 32 64 128; do
+- title=$(printf '%10s' "($days days)")
++ title=$(printf '%10s' "($days days${argv:+, $argv})")
+ test_expect_success "setup revs from $days days ago" '
+ now=$(git log -1 --format=%ct HEAD) &&
+ then=$(($now - ($days * 86400))) &&
+@@ t/perf/p5311-pack-bitmaps-fetch.sh: test_fetch_bitmaps () {
+ done
+ }
+
+-test_fetch_bitmaps
++for argv in '' --path-walk
++do
++ test_fetch_bitmaps $argv || return 1
++done
+
+ test_done
+
## t/t5310-pack-bitmaps.sh ##
@@ t/t5310-pack-bitmaps.sh: test_bitmap_cases
2: bdae873eaab = 3: 069c50d3370 pack-objects: extract `record_tree_depth()` helper
3: a642305e3c9 = 4: ae57607b57f pack-objects: support `--delta-islands` with `--path-walk`
base-commit: 45a9ecee26839cc880fdd5e704339dd3cf4ffc26
--
2.54.0.23.gae57607b57f
^ permalink raw reply
* [PATCH] worktree: record creation time and free-form note
From: Kiesel, Norbert @ 2026-06-02 21:40 UTC (permalink / raw)
To: git
From 130cd5e4a25e6672b2a97268e1100b6ef03fa552 Mon Sep 17 00:00:00 2001
From: Norbert Kiesel <norbert.kiesel@creditkarma.com>
Date: Mon, 1 Jun 2026 17:03:39 -0700
Subject: [PATCH] worktree: record creation time and free-form note
Add per-worktree metadata so users can answer "what is this worktree
for, and when did I make it?" without resorting to external notes.
When `git worktree add` creates a linked worktree, it now writes a
`created` file containing the unix timestamp. A new `--note <string>`
option to `add`, and a new `git worktree annotate <worktree> [<note>]`
subcommand, store an optional free-form description in a `note` file
next to the other administrative files. Passing `annotate` without a
note clears it. The main worktree carries no metadata and cannot be
annotated.
`git worktree list` learns `--show-created` and `--show-note` for
human-readable output, and `--sort=<key>` (path or created, optionally
prefixed with `-` to reverse) for ordering linked worktrees; the main
worktree always stays first. Worktrees without a recorded timestamp
(those created before this change) display as `created: unknown` and
sort after timestamped ones. Porcelain output unconditionally emits
`created` and `note` lines when the corresponding metadata is present.
Tests cover add/annotate/list behaviour and the legacy-worktree case.
The two existing porcelain assertions in t2402 are taught to strip the
new `created` line so they continue to pass.
Signed-off-by: Norbert Kiesel <norbert.kiesel@creditkarma.com>
---
Documentation/git-worktree.adoc | 61 ++++++++++++-
builtin/worktree.c | 152 +++++++++++++++++++++++++++++++-
t/meson.build | 1 +
t/t2402-worktree-list.sh | 10 ++-
t/t2410-worktree-metadata.sh | 143 ++++++++++++++++++++++++++++++
worktree.c | 78 ++++++++++++++++
worktree.h | 23 +++++
7 files changed, 459 insertions(+), 9 deletions(-)
create mode 100755 t/t2410-worktree-metadata.sh
diff --git a/Documentation/git-worktree.adoc b/Documentation/git-worktree.adoc
index fbf8426cd9..200f3d7772 100644
--- a/Documentation/git-worktree.adoc
+++ b/Documentation/git-worktree.adoc
@@ -10,8 +10,11 @@ SYNOPSIS
--------
[synopsis]
git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]
+ [--note <string>]
[--orphan] [(-b | -B) <new-branch>] <path> [<commit-ish>]
-git worktree list [-v | --porcelain [-z]]
+git worktree annotate <worktree> [<note>]
+git worktree list [-v | --porcelain [-z]] [--show-created] [--show-note]
+ [--sort=<key>]
git worktree lock [--reason <string>] <worktree>
git worktree move <worktree> <new-path>
git worktree prune [-n] [-v] [--expire <expire>]
@@ -106,6 +109,15 @@ passed to the command. In the event the
repository has a remote and
command fails with a warning reminding the user to fetch from their remote
first (or override by using `-f`/`--force`).
+`annotate <worktree> [<note>]`::
+
+Set, replace, or clear a free-form note (description) on a linked worktree.
+Useful for recording what a worktree was created for so it can be identified
+later. With _<note>_, the worktree's note is set or replaced; without a note
+argument, the existing note is cleared. The note for a worktree may also be
+set at creation time with `git worktree add --note <note>`. The main
+worktree cannot be annotated.
+
`list`::
List details of each worktree. The main worktree is listed first,
@@ -114,6 +126,20 @@ whether the worktree is bare, the revision
currently checked out, the
branch currently checked out (or "detached HEAD" if none), "locked" if
the worktree is locked, "prunable" if the worktree can be pruned by the
`prune` command.
++
+Each worktree's creation timestamp is recorded when it is created with
+`git worktree add`. Worktrees created before this feature existed have no
+recorded creation timestamp; for them, `list` reports `created: unknown`
+in human output and omits the `created` line in `--porcelain` output. Pass
+`--show-created` to include creation timestamps in human output. Worktrees
+without a recorded timestamp sort last (or first when reversed) with
+`--sort=created`.
++
+Pass `--show-note` to include any user-provided note in human output. In
+`--porcelain` output, both `created` and `note` lines are emitted whenever
+present. Use `--sort=<key>` (where _<key>_ is `path` or `created`,
+optionally prefixed with `-` to reverse) to order the linked worktrees;
+the main worktree always remains first.
`lock`::
@@ -286,6 +312,32 @@ _<time>_.
With `lock` or with `add --lock`, an explanation why the worktree
is locked.
+`--note <string>`::
+ With `add`, attach a free-form note (description) to the new worktree.
+ The note is stored alongside the worktree's administrative files and
+ can be displayed with `git worktree list --show-note` or in
+ `--porcelain` output. It can be changed later with
+ `git worktree annotate`.
+
+`--show-created`::
+ With `list`, include each worktree's creation timestamp in the
+ human-readable output. Worktrees with no recorded creation time are
+ shown as `created: unknown`. In `--porcelain` output, the creation
+ timestamp is always included (when available) on a `created` line.
+
+`--show-note`::
+ With `list`, include each worktree's note (if set) in the
+ human-readable output. In `--porcelain` output, the note is always
+ included (when set) on a `note` line.
+
+`--sort=<key>`::
+ With `list`, sort linked worktrees by _<key>_, which is one of
+ `path` or `created`. Prefix with `-` to reverse the order, e.g.
+ `--sort=-created` lists newest first. The main worktree is always
+ listed first regardless of sort order. Worktrees with no recorded
+ creation timestamp sort after those that have one (or before, when
+ reversed).
+
_<worktree>_::
Worktrees can be identified by path, either relative or absolute.
+
@@ -462,7 +514,9 @@ are terminated with NUL rather than a newline.
Attributes are listed with a
label and value separated by a single space. Boolean attributes (like `bare`
and `detached`) are listed as a label only, and are present only
if the value is true. Some attributes (like `locked`) can be listed as a label
-only or with a value depending upon whether a reason is available. The first
+only or with a value depending upon whether a reason is available. Optional
+valued attributes (like `created` and `note`) appear only when the
+corresponding metadata has been recorded for that worktree. The first
attribute of a worktree is always `worktree`, an empty line indicates the
end of the record. For example:
@@ -474,10 +528,13 @@ bare
worktree /path/to/linked-worktree
HEAD abcd1234abcd1234abcd1234abcd1234abcd1234
branch refs/heads/master
+created 2026-06-01T12:34:56Z
+note investigating login bug
worktree /path/to/other-linked-worktree
HEAD 1234abc1234abc1234abc1234abc1234abc1234a
detached
+created 2026-05-28T08:15:00Z
worktree /path/to/linked-worktree-locked-no-reason
HEAD 5678abc5678abc5678abc5678abc5678abc5678c
diff --git a/builtin/worktree.c b/builtin/worktree.c
index d21c43fde3..ac22277d6c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -27,13 +27,16 @@
#include "utf8.h"
#include "worktree.h"
#include "quote.h"
+#include "date.h"
#define BUILTIN_WORKTREE_ADD_USAGE \
N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason
<string>]]\n" \
+ " [--note <string>]\n" \
" [--orphan] [(-b | -B) <new-branch>] <path>
[<commit-ish>]")
#define BUILTIN_WORKTREE_LIST_USAGE \
- N_("git worktree list [-v | --porcelain [-z]]")
+ N_("git worktree list [-v | --porcelain [-z]] [--show-created]
[--show-note]\n" \
+ " [--sort=<key>]")
#define BUILTIN_WORKTREE_LOCK_USAGE \
N_("git worktree lock [--reason <string>] <worktree>")
#define BUILTIN_WORKTREE_MOVE_USAGE \
@@ -46,6 +49,8 @@
N_("git worktree repair [<path>...]")
#define BUILTIN_WORKTREE_UNLOCK_USAGE \
N_("git worktree unlock <worktree>")
+#define BUILTIN_WORKTREE_ANNOTATE_USAGE \
+ N_("git worktree annotate <worktree> [<note>]")
#define WORKTREE_ADD_DWIM_ORPHAN_INFER_TEXT \
_("No possible source branch, inferring '--orphan'")
@@ -66,6 +71,7 @@
static const char * const git_worktree_usage[] = {
BUILTIN_WORKTREE_ADD_USAGE,
+ BUILTIN_WORKTREE_ANNOTATE_USAGE,
BUILTIN_WORKTREE_LIST_USAGE,
BUILTIN_WORKTREE_LOCK_USAGE,
BUILTIN_WORKTREE_MOVE_USAGE,
@@ -116,6 +122,11 @@ static const char * const git_worktree_unlock_usage[] = {
NULL
};
+static const char * const git_worktree_annotate_usage[] = {
+ BUILTIN_WORKTREE_ANNOTATE_USAGE,
+ NULL
+};
+
struct add_opts {
int force;
int detach;
@@ -124,6 +135,7 @@ struct add_opts {
int orphan;
int relative_paths;
const char *keep_locked;
+ const char *note;
};
static int show_only;
@@ -131,6 +143,8 @@ static int verbose;
static int guess_remote;
static int use_relative_paths;
static timestamp_t expire;
+static int show_created;
+static int show_note;
static int git_worktree_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
@@ -544,6 +558,16 @@ static int add_worktree(const char *path, const
char *refname,
strbuf_addf(&sb, "%s/commondir", sb_repo.buf);
write_file(sb.buf, "../..");
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%s/created", sb_repo.buf);
+ write_file(sb.buf, "%"PRItime, (timestamp_t) time(NULL));
+
+ if (opts->note && *opts->note) {
+ strbuf_reset(&sb);
+ strbuf_addf(&sb, "%s/note", sb_repo.buf);
+ write_file(sb.buf, "%s", opts->note);
+ }
+
/*
* Set up the ref store of the worktree and create the HEAD reference.
*/
@@ -815,6 +839,8 @@ static int add(int ac, const char **av, const char *prefix,
OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
OPT_STRING(0, "reason", &lock_reason, N_("string"),
N_("reason for locking")),
+ OPT_STRING(0, "note", &opts.note, N_("string"),
+ N_("attach a free-form note/description to the worktree")),
OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
OPT_PASSTHRU(0, "track", &opt_track, NULL,
N_("set up tracking mode (see git-branch(1))"),
@@ -963,6 +989,8 @@ static int add(int ac, const char **av, const char *prefix,
static void show_worktree_porcelain(struct worktree *wt, int line_terminator)
{
const char *reason;
+ const char *note;
+ timestamp_t created;
printf("worktree %s%c", wt->path, line_terminator);
if (wt->is_bare)
@@ -975,6 +1003,18 @@ static void show_worktree_porcelain(struct
worktree *wt, int line_terminator)
printf("branch %s%c", wt->head_ref, line_terminator);
}
+ created = worktree_created_at(wt);
+ if (created)
+ printf("created %s%c",
+ show_date(created, 0, DATE_MODE(ISO8601_STRICT)),
+ line_terminator);
+
+ note = worktree_note(wt);
+ if (note && *note) {
+ fputs("note ", stdout);
+ write_name_quoted(note, stdout, line_terminator);
+ }
+
reason = worktree_lock_reason(wt);
if (reason) {
fputs("locked", stdout);
@@ -1034,6 +1074,21 @@ static void show_worktree(struct worktree *wt,
struct worktree_display *display,
else if (reason)
strbuf_addstr(&sb, " prunable");
+ if (show_created || verbose) {
+ timestamp_t created = worktree_created_at(wt);
+ if (created)
+ strbuf_addf(&sb, "\n\tcreated: %s",
+ show_date(created, 0, DATE_MODE(ISO8601)));
+ else if (show_created && !is_main_worktree(wt))
+ strbuf_addstr(&sb, "\n\tcreated: unknown");
+ }
+
+ if (show_note || verbose) {
+ const char *note = worktree_note(wt);
+ if (note && *note)
+ strbuf_addf(&sb, "\n\tnote: %s", note);
+ }
+
printf("%s\n", sb.buf);
strbuf_release(&sb);
}
@@ -1068,6 +1123,27 @@ static int pathcmp(const void *a_, const void *b_)
return fspathcmp((*a)->path, (*b)->path);
}
+static int createdcmp(const void *a_, const void *b_)
+{
+ struct worktree *const *a = a_;
+ struct worktree *const *b = b_;
+ timestamp_t ta = worktree_created_at(*a);
+ timestamp_t tb = worktree_created_at(*b);
+
+ /* Worktrees without a recorded timestamp (legacy) sort after those
with one. */
+ if (!ta && !tb)
+ return fspathcmp((*a)->path, (*b)->path);
+ if (!ta)
+ return 1;
+ if (!tb)
+ return -1;
+ if (ta < tb)
+ return -1;
+ if (ta > tb)
+ return 1;
+ return 0;
+}
+
static void pathsort(struct worktree **wt)
{
int n = 0;
@@ -1078,11 +1154,43 @@ static void pathsort(struct worktree **wt)
QSORT(wt, n, pathcmp);
}
+static int sort_worktrees(struct worktree **wt, const char *key)
+{
+ int n = 0, reverse = 0;
+ struct worktree **p = wt;
+ int (*cmp)(const void *, const void *);
+
+ if (*key == '-') {
+ reverse = 1;
+ key++;
+ }
+ if (!strcmp(key, "path"))
+ cmp = pathcmp;
+ else if (!strcmp(key, "created"))
+ cmp = createdcmp;
+ else
+ return -1;
+
+ while (*p++)
+ n++;
+ QSORT(wt, n, cmp);
+ if (reverse) {
+ int i;
+ for (i = 0; i < n / 2; i++) {
+ struct worktree *tmp = wt[i];
+ wt[i] = wt[n - 1 - i];
+ wt[n - 1 - i] = tmp;
+ }
+ }
+ return 0;
+}
+
static int list(int ac, const char **av, const char *prefix,
struct repository *repo UNUSED)
{
int porcelain = 0;
int line_terminator = '\n';
+ const char *sort_key = NULL;
struct option options[] = {
OPT_BOOL(0, "porcelain", &porcelain, N_("machine-readable output")),
@@ -1091,6 +1199,12 @@ static int list(int ac, const char **av, const
char *prefix,
N_("add 'prunable' annotation to missing worktrees older than <time>")),
OPT_SET_INT('z', NULL, &line_terminator,
N_("terminate records with a NUL character"), '\0'),
+ OPT_BOOL(0, "show-created", &show_created,
+ N_("show worktree creation timestamps")),
+ OPT_BOOL(0, "show-note", &show_note,
+ N_("show worktree notes")),
+ OPT_STRING(0, "sort", &sort_key, N_("key"),
+ N_("sort worktrees by key (path, created); prefix with - to reverse")),
OPT_END()
};
@@ -1107,8 +1221,13 @@ static int list(int ac, const char **av, const
char *prefix,
int path_maxwidth = 0, abbrev = DEFAULT_ABBREV, i;
struct worktree_display *display = NULL;
- /* sort worktrees by path but keep main worktree at top */
- pathsort(worktrees + 1);
+ /* sort worktrees but keep main worktree at top */
+ if (sort_key) {
+ if (sort_worktrees(worktrees + 1, sort_key))
+ die(_("unknown sort key '%s'"), sort_key);
+ } else {
+ pathsort(worktrees + 1);
+ }
if (!porcelain)
measure_widths(worktrees, &abbrev,
@@ -1200,6 +1319,32 @@ static int unlock_worktree(int ac, const char
**av, const char *prefix,
return ret;
}
+static int annotate_worktree(int ac, const char **av, const char *prefix,
+ struct repository *repo UNUSED)
+{
+ struct option options[] = {
+ OPT_END()
+ };
+ struct worktree **worktrees, *wt;
+ int ret;
+
+ ac = parse_options(ac, av, prefix, options, git_worktree_annotate_usage, 0);
+ if (ac < 1 || ac > 2)
+ usage_with_options(git_worktree_annotate_usage, options);
+
+ worktrees = get_worktrees();
+ wt = find_worktree(worktrees, prefix, av[0]);
+ if (!wt)
+ die(_("'%s' is not a working tree"), av[0]);
+ if (is_main_worktree(wt))
+ die(_("The main working tree cannot be annotated"));
+
+ ret = set_worktree_note(wt, ac == 2 ? av[1] : NULL);
+
+ free_worktrees(worktrees);
+ return ret;
+}
+
static void validate_no_submodules(const struct worktree *wt)
{
struct index_state istate = INDEX_STATE_INIT(the_repository);
@@ -1469,6 +1614,7 @@ int cmd_worktree(int ac,
parse_opt_subcommand_fn *fn = NULL;
struct option options[] = {
OPT_SUBCOMMAND("add", &fn, add),
+ OPT_SUBCOMMAND("annotate", &fn, annotate_worktree),
OPT_SUBCOMMAND("prune", &fn, prune),
OPT_SUBCOMMAND("list", &fn, list),
OPT_SUBCOMMAND("lock", &fn, lock_worktree),
diff --git a/t/meson.build b/t/meson.build
index 2af8d01279..7b6e8435d7 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -308,6 +308,7 @@ integration_tests = [
't2405-worktree-submodule.sh',
't2406-worktree-repair.sh',
't2407-worktree-heads.sh',
+ 't2410-worktree-metadata.sh',
't2500-untracked-overwriting.sh',
't2501-cwd-empty.sh',
't3000-ls-files-others.sh',
diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
index e0c6abd2f5..8422340443 100755
--- a/t/t2402-worktree-list.sh
+++ b/t/t2402-worktree-list.sh
@@ -71,7 +71,8 @@ test_expect_success '"list" all worktrees --porcelain' '
echo "HEAD $(git rev-parse HEAD)" >>expect &&
echo "detached" >>expect &&
echo >>expect &&
- git worktree list --porcelain >actual &&
+ git worktree list --porcelain >actual.raw &&
+ grep -v "^created " actual.raw >actual &&
test_cmp expect actual
'
@@ -86,7 +87,7 @@ test_expect_success '"list" all worktrees --porcelain -z' '
"$(git -C here rev-parse --show-toplevel)" \
"$(git rev-parse HEAD)" >>expect &&
git worktree list --porcelain -z >_actual &&
- nul_to_q <_actual >actual &&
+ nul_to_q <_actual | tr Q "\n" | grep -v "^created " | tr "\n" Q >actual &&
test_cmp expect actual
'
@@ -220,7 +221,7 @@ test_expect_success '"list" all worktrees from bare main' '
'
test_expect_success '"list" all worktrees --porcelain from bare main' '
- test_when_finished "rm -rf there actual expect && git -C bare1
worktree prune" &&
+ test_when_finished "rm -rf there actual actual.raw expect && git -C
bare1 worktree prune" &&
git -C bare1 worktree add --detach ../there main &&
echo "worktree $(pwd)/bare1" >expect &&
echo "bare" >>expect &&
@@ -229,7 +230,8 @@ test_expect_success '"list" all worktrees
--porcelain from bare main' '
echo "HEAD $(git -C there rev-parse HEAD)" >>expect &&
echo "detached" >>expect &&
echo >>expect &&
- git -C bare1 worktree list --porcelain >actual &&
+ git -C bare1 worktree list --porcelain >actual.raw &&
+ grep -v "^created " actual.raw >actual &&
test_cmp expect actual
'
diff --git a/t/t2410-worktree-metadata.sh b/t/t2410-worktree-metadata.sh
new file mode 100755
index 0000000000..3f8b508593
--- /dev/null
+++ b/t/t2410-worktree-metadata.sh
@@ -0,0 +1,143 @@
+#!/bin/sh
+
+test_description='git worktree creation timestamp and note metadata'
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit init
+'
+
+test_expect_success 'add writes created file' '
+ test_when_finished "git worktree remove -f wt1 && git worktree prune" &&
+ git worktree add wt1 &&
+ test_path_is_file .git/worktrees/wt1/created &&
+ # contents should be a positive integer (unix timestamp)
+ created=$(cat .git/worktrees/wt1/created) &&
+ test "$created" -gt 0
+'
+
+test_expect_success 'add --note writes note file' '
+ test_when_finished "git worktree remove -f wt2 && git worktree prune" &&
+ git worktree add --note "investigating bug" wt2 &&
+ test_path_is_file .git/worktrees/wt2/note &&
+ echo "investigating bug" >expect &&
+ test_cmp expect .git/worktrees/wt2/note
+'
+
+test_expect_success 'add without --note does not create note file' '
+ test_when_finished "git worktree remove -f wt3 && git worktree prune" &&
+ git worktree add wt3 &&
+ test_path_is_missing .git/worktrees/wt3/note
+'
+
+test_expect_success 'annotate sets a note on an existing worktree' '
+ test_when_finished "git worktree remove -f wt4 && git worktree prune" &&
+ git worktree add wt4 &&
+ git worktree annotate wt4 "later note" &&
+ echo "later note" >expect &&
+ test_cmp expect .git/worktrees/wt4/note
+'
+
+test_expect_success 'annotate replaces an existing note' '
+ test_when_finished "git worktree remove -f wt5 && git worktree prune" &&
+ git worktree add --note "old" wt5 &&
+ git worktree annotate wt5 "new" &&
+ echo "new" >expect &&
+ test_cmp expect .git/worktrees/wt5/note
+'
+
+test_expect_success 'annotate with no text clears the note' '
+ test_when_finished "git worktree remove -f wt6 && git worktree prune" &&
+ git worktree add --note "to delete" wt6 &&
+ test_path_is_file .git/worktrees/wt6/note &&
+ git worktree annotate wt6 &&
+ test_path_is_missing .git/worktrees/wt6/note
+'
+
+test_expect_success 'annotate refuses to operate on the main worktree' '
+ test_must_fail git worktree annotate . "should fail" 2>err &&
+ grep -i "main working tree" err
+'
+
+test_expect_success 'list --show-note displays note in human output' '
+ test_when_finished "git worktree remove -f wt7 && git worktree prune" &&
+ git worktree add --note "release branch" wt7 &&
+ git worktree list --show-note >actual &&
+ grep "note: release branch" actual
+'
+
+test_expect_success 'list --show-created displays created timestamp' '
+ test_when_finished "git worktree remove -f wt8 && git worktree prune" &&
+ git worktree add wt8 &&
+ git worktree list --show-created >actual &&
+ grep "created: " actual
+'
+
+test_expect_success 'list --show-created shows unknown for legacy worktrees' '
+ test_when_finished "git worktree remove -f wt9 && git worktree prune" &&
+ git worktree add wt9 &&
+ rm .git/worktrees/wt9/created &&
+ git worktree list --show-created >actual &&
+ grep "created: unknown" actual
+'
+
+test_expect_success 'list --porcelain always includes created and note' '
+ test_when_finished "git worktree remove -f wtp && git worktree prune" &&
+ git worktree add --note "porcelain test" wtp &&
+ git worktree list --porcelain >actual &&
+ grep "^created " actual &&
+ grep "^note porcelain test" actual
+'
+
+test_expect_success 'list --sort=created orders by creation time' '
+ test_when_finished "git worktree remove -f a && git worktree remove
-f b && git worktree remove -f c && git worktree prune" &&
+ git worktree add a &&
+ git worktree add b &&
+ git worktree add c &&
+ echo 1000 >.git/worktrees/a/created &&
+ echo 2000 >.git/worktrees/b/created &&
+ echo 3000 >.git/worktrees/c/created &&
+ git worktree list --sort=created --porcelain >actual &&
+ grep "^worktree " actual | sed -n "2,4p" >linked &&
+ awk "NR==1" linked | grep -q "/a$" &&
+ awk "NR==2" linked | grep -q "/b$" &&
+ awk "NR==3" linked | grep -q "/c$"
+'
+
+test_expect_success 'list --sort=-created reverses order' '
+ test_when_finished "git worktree remove -f a && git worktree remove
-f b && git worktree remove -f c && git worktree prune" &&
+ git worktree add a &&
+ git worktree add b &&
+ git worktree add c &&
+ echo 1000 >.git/worktrees/a/created &&
+ echo 2000 >.git/worktrees/b/created &&
+ echo 3000 >.git/worktrees/c/created &&
+ git worktree list --sort=-created --porcelain >actual &&
+ grep "^worktree " actual | sed -n "2,4p" >linked &&
+ awk "NR==1" linked | grep -q "/c$" &&
+ awk "NR==2" linked | grep -q "/b$" &&
+ awk "NR==3" linked | grep -q "/a$"
+'
+
+test_expect_success 'list --sort=created places legacy worktrees last' '
+ test_when_finished "git worktree remove -f early && git worktree
remove -f legacy && git worktree prune" &&
+ git worktree add early &&
+ echo 1000 >.git/worktrees/early/created &&
+ git worktree add legacy &&
+ rm .git/worktrees/legacy/created &&
+ git worktree list --sort=created --porcelain >actual &&
+ grep "^worktree " actual | sed -n "2,3p" >linked &&
+ awk "NR==1" linked | grep -q "/early$" &&
+ awk "NR==2" linked | grep -q "/legacy$"
+'
+
+test_expect_success 'list --sort with unknown key fails' '
+ test_must_fail git worktree list --sort=bogus 2>err &&
+ grep -i "unknown sort key" err
+'
+
+test_done
diff --git a/worktree.c b/worktree.c
index 97eddc3916..7989e694b7 100644
--- a/worktree.c
+++ b/worktree.c
@@ -14,6 +14,8 @@
#include "dir.h"
#include "wt-status.h"
#include "config.h"
+#include "date.h"
+#include "wrapper.h"
void free_worktree(struct worktree *worktree)
{
@@ -24,6 +26,7 @@ void free_worktree(struct worktree *worktree)
free(worktree->head_ref);
free(worktree->lock_reason);
free(worktree->prune_reason);
+ free(worktree->note);
free(worktree);
}
@@ -324,6 +327,81 @@ const char *worktree_lock_reason(struct worktree *wt)
return wt->lock_reason;
}
+timestamp_t worktree_created_at(struct worktree *wt)
+{
+ if (is_main_worktree(wt))
+ return 0;
+
+ if (!wt->created_at_valid) {
+ struct strbuf path = STRBUF_INIT;
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addstr(&path, worktree_git_path(wt, "created"));
+ if (file_exists(path.buf) &&
+ strbuf_read_file(&buf, path.buf, 0) >= 0) {
+ char *end;
+ timestamp_t t;
+ strbuf_trim(&buf);
+ t = parse_timestamp(buf.buf, &end, 10);
+ if (end != buf.buf && *end == '\0')
+ wt->created_at = t;
+ }
+ wt->created_at_valid = 1;
+ strbuf_release(&path);
+ strbuf_release(&buf);
+ }
+
+ return wt->created_at;
+}
+
+const char *worktree_note(struct worktree *wt)
+{
+ if (is_main_worktree(wt))
+ return NULL;
+
+ if (!wt->note_valid) {
+ struct strbuf path = STRBUF_INIT;
+
+ strbuf_addstr(&path, worktree_git_path(wt, "note"));
+ if (file_exists(path.buf)) {
+ struct strbuf note = STRBUF_INIT;
+ if (strbuf_read_file(¬e, path.buf, 0) < 0)
+ die_errno(_("failed to read '%s'"), path.buf);
+ strbuf_trim_trailing_newline(¬e);
+ wt->note = strbuf_detach(¬e, NULL);
+ } else
+ wt->note = NULL;
+ wt->note_valid = 1;
+ strbuf_release(&path);
+ }
+
+ return wt->note;
+}
+
+int set_worktree_note(struct worktree *wt, const char *text)
+{
+ char *path;
+ int ret = 0;
+
+ if (is_main_worktree(wt))
+ return error(_("cannot set note on the main worktree"));
+
+ path = repo_common_path(wt->repo, "worktrees/%s/note", wt->id);
+ if (!text || !*text) {
+ if (file_exists(path) && unlink(path))
+ ret = error_errno(_("failed to remove '%s'"), path);
+ } else {
+ write_file(path, "%s", text);
+ }
+
+ /* invalidate cache so a follow-up worktree_note() re-reads */
+ FREE_AND_NULL(wt->note);
+ wt->note_valid = 0;
+
+ free(path);
+ return ret;
+}
+
const char *worktree_prune_reason(struct worktree *wt, timestamp_t expire)
{
struct strbuf reason = STRBUF_INIT;
diff --git a/worktree.h b/worktree.h
index 1075409f9a..0fcdb8bd1b 100644
--- a/worktree.h
+++ b/worktree.h
@@ -13,12 +13,16 @@ struct worktree {
char *head_ref; /* NULL if HEAD is broken or detached */
char *lock_reason; /* private - use worktree_lock_reason */
char *prune_reason; /* private - use worktree_prune_reason */
+ char *note; /* private - use worktree_note */
struct object_id head_oid;
+ timestamp_t created_at; /* private - use worktree_created_at; 0 if unknown */
int is_detached;
int is_bare;
int is_current; /* does `path` match `repo->worktree` */
int lock_reason_valid; /* private */
int prune_reason_valid; /* private */
+ int note_valid; /* private */
+ int created_at_valid; /* private */
};
/*
@@ -96,6 +100,25 @@ int is_main_worktree(const struct worktree *wt);
*/
const char *worktree_lock_reason(struct worktree *wt);
+/*
+ * Return the worktree's recorded creation timestamp, or 0 if no timestamp
+ * was recorded (e.g. a worktree created before this metadata existed, or
+ * the main worktree which never carries the file).
+ */
+timestamp_t worktree_created_at(struct worktree *wt);
+
+/*
+ * Return the user-supplied note/description for the given worktree, or NULL
+ * if none was set.
+ */
+const char *worktree_note(struct worktree *wt);
+
+/*
+ * Write or replace the worktree's note. Pass NULL or "" to delete the note.
+ * Returns 0 on success, -1 on failure. Not valid for the main worktree.
+ */
+int set_worktree_note(struct worktree *wt, const char *text);
+
/*
* Return the reason string if the given worktree should be pruned, otherwise
* NULL if it should not be pruned. `expire` defines a grace period to prune
--
^ permalink raw reply related
* Re: [PATCH v3 00/12] Improve git gui operation without a worktree
From: Johannes Sixt @ 2026-06-02 21:05 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <6f4276f9-cf0e-4840-88ce-9e0009c669a9@gmail.com>
Am 02.06.26 um 20:54 schrieb Mark Levedahl:
> You have duplicate signoffs on 02/12 in your repo.
That's intentional. The patch went from my hands to yours, then back to
mine.
-- Hannes
^ permalink raw reply
* [PATCH] transport-helper: fix TSAN race in transfer_debug()
From: Pushkar Singh @ 2026-06-02 20:13 UTC (permalink / raw)
To: git; +Cc: gitster, peff, Pushkar Singh
Currently, transfer_debug() lazily initializes a static variable based
on GIT_TRANSLOOP_DEBUG. Since the function may be called from multiple
worker threads, this initialization is racy and is therefore suppressed
in .tsan-suppressions.
Initialize the variable in bidirectional_transfer_loop() before any
worker threads or processes are created. This patch removes the race and
allows dropping the corresponding TSAN suppression.
Signed-off-by: Pushkar Singh <pushkarkumarsingh1970@gmail.com>
---
.tsan-suppressions | 1 -
transport-helper.c | 17 ++++++-----------
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/.tsan-suppressions b/.tsan-suppressions
index 5ba86d6845..d84883bd90 100644
--- a/.tsan-suppressions
+++ b/.tsan-suppressions
@@ -7,7 +7,6 @@
# A static variable is written to racily, but we always write the same value, so
# in practice it (hopefully!) doesn't matter.
race:^want_color$
-race:^transfer_debug$
# A boolean value, which tells whether the replace_map has been initialized or
# not, is read racily with an update. As this variable is written to only once,
diff --git a/transport-helper.c b/transport-helper.c
index 04d55572a9..95a7fa7d86 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -1361,24 +1361,16 @@ int transport_helper_init(struct transport *transport, const char *name)
/* This should be enough to hold debugging message. */
#define PBUFFERSIZE 8192
+static int transfer_debug_enabled = -1;
+
/* Print bidirectional transfer loop debug message. */
__attribute__((format (printf, 1, 2)))
static void transfer_debug(const char *fmt, ...)
{
- /*
- * NEEDSWORK: This function is sometimes used from multiple threads, and
- * we end up using debug_enabled racily. That "should not matter" since
- * we always write the same value, but it's still wrong. This function
- * is listed in .tsan-suppressions for the time being.
- */
-
va_list args;
char msgbuf[PBUFFERSIZE];
- static int debug_enabled = -1;
- if (debug_enabled < 0)
- debug_enabled = getenv("GIT_TRANSLOOP_DEBUG") ? 1 : 0;
- if (!debug_enabled)
+ if (!transfer_debug_enabled)
return;
va_start(args, fmt);
@@ -1648,6 +1640,9 @@ int bidirectional_transfer_loop(int input, int output)
{
struct bidirectional_transfer_state state;
+ if (transfer_debug_enabled < 0)
+ transfer_debug_enabled = getenv("GIT_TRANSLOOP_DEBUG") ? 1 : 0;
+
/* Fill the state fields. */
state.ptg.src = input;
state.ptg.dest = 1;
--
2.53.0.582.gca1db8a0f7
^ permalink raw reply related
* Re: [PATCH v3 00/12] Improve git gui operation without a worktree
From: Mark Levedahl @ 2026-06-02 18:54 UTC (permalink / raw)
To: Johannes Sixt; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <8515a482-9a08-4b0a-bd7c-385e1bda1a20@kdbg.org>
On 6/2/26 1:34 PM, Johannes Sixt wrote:
>> 2 files changed, 223 insertions(+), 175 deletions(-)
> This round looks excellent! Thank you very much!
>
> While queuing, I applied the small fixup below to 03/12.
>
> -- Hannes
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 933e72c9b255..15dd2b3a84cc 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -2064,7 +2064,6 @@ proc incr_font_size {font {amt 1}} {
>
> proc do_gitk {revs {is_submodule false}} {
> global current_diff_path file_states current_diff_side ui_index
> - global _gitworktree
>
> # -- Always start gitk through whatever we were loaded with. This
> # lets us bypass using shell process on Windows systems.
>
Oops, missed that. Thanks for catching.
You have duplicate signoffs on 02/12 in your repo.
Mark
^ permalink raw reply
* [PATCH v6 2/2] config: improve diagnostic for "set" with missing value
From: Harald Nordgren via GitGitGadget @ 2026-06-02 18:43 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2302.v6.git.git.1780425808.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
"git config set pull.rebase=false" currently fails with "wrong
number of arguments", and the implicit form "git config
pull.rebase=false" fails with "invalid key". Neither points at
the real problem: the value is missing.
Report that directly, and when the argument has the shape
"<valid-key>=<value>", also suggest the split form:
$ git config set pull.rebase=false
error: missing value to set to the variable 'pull.rebase=false'
hint: did you mean "git config set pull.rebase false"?
When the prefix before "=" is not a valid key, drop the hint:
$ git config set foo=bar
error: missing value to set to a variable with an invalid name 'foo=bar'
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
builtin/config.c | 32 ++++++++++++++++++++++++++-
t/t1300-config.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 87 insertions(+), 1 deletion(-)
diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc..8d8ec0beea 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "builtin.h"
#include "abspath.h"
+#include "advice.h"
#include "config.h"
#include "color.h"
#include "date.h"
@@ -210,6 +211,26 @@ static void check_argc(int argc, int min, int max)
exit(129);
}
+static NORETURN void die_missing_set_value(const char *arg)
+{
+ const char *last_dot = strrchr(arg, '.');
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
+ if (prefix && git_config_key_is_valid(prefix)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
+ } else if (git_config_key_is_valid(arg)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
+ arg);
+ }
+ free(prefix);
+ exit(129);
+}
+
static void show_config_origin(const struct config_display_options *opts,
const struct key_value_info *kvi,
struct strbuf *buf)
@@ -1133,6 +1154,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
+ if (argc == 1)
+ die_missing_set_value(argv[0]);
check_argc(argc, 2, 2);
if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1371,6 +1394,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
};
char *value = NULL, *comment = NULL;
int ret = 0;
+ int actions_implicit;
struct key_value_info default_kvi = KVI_INIT;
argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1409,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
exit(129);
}
- if (actions == 0)
+ actions_implicit = (actions == 0);
+ if (actions_implicit)
switch (argc) {
case 1: actions = ACTION_GET; break;
case 2: actions = ACTION_SET; break;
@@ -1394,6 +1419,11 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
error(_("no action specified"));
exit(129);
}
+ if (actions_implicit && argc == 1) {
+ const char *last_dot = strrchr(argv[0], '.');
+ if (last_dot && strchr(last_dot + 1, '='))
+ die_missing_set_value(argv[0]);
+ }
if (display_opts.omit_values &&
!(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
error(_("--name-only is only applicable to --list or --get-regexp"));
diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 11fc976f3a..87ca11a127 100755
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -469,6 +469,62 @@ test_expect_success 'invalid key' '
test_must_fail git config inval.2key blabla
'
+test_expect_success 'set with 1 arg of "key=value": valid key suggests split form' '
+ test_must_fail git config set pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": implicit form suggests split form' '
+ test_must_fail git config pull.rebase=false 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase=false." err &&
+ test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'set with 1 arg of "key=value": invalid key does not suggest split form' '
+ test_must_fail git config set foo=bar 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo=bar." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: variable name starting with digit is invalid' '
+ test_must_fail git config set foo.1bar=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.1bar=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg: digit-led section name is valid' '
+ test_must_fail git config set 1foo.bar=baz 2>err &&
+ test_grep "missing value to set to the variable .1foo\\.bar=baz." err &&
+ test_grep "did you mean .git config set 1foo\\.bar baz." err
+'
+
+test_expect_success 'set with 1 arg: subsection plus invalid variable name' '
+ test_must_fail git config set foo.some.b_r=baz 2>err &&
+ test_grep "missing value to set to a variable with an invalid name .foo\\.some\\.b_r=baz." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 1 arg of valid key reports missing value' '
+ test_must_fail git config set pull.rebase 2>err &&
+ test_grep "missing value to set to the variable .pull\\.rebase." err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
+ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid' '
+ test_when_finished "rm -f subsection.cfg" &&
+ git config set -f subsection.cfg foo.bar=baz.boo qux &&
+ echo qux >expect &&
+ git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+ test_cmp expect actual
+'
+
test_expect_success 'correct key' '
git config 123456.a123 987
'
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 1/2] config: add git_config_key_is_valid() for quiet validation
From: Harald Nordgren via GitGitGadget @ 2026-06-02 18:43 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren, Harald Nordgren
In-Reply-To: <pull.2302.v6.git.git.1780425808.gitgitgadget@gmail.com>
From: Harald Nordgren <haraldnordgren@gmail.com>
Move the body of git_config_parse_key() into a static helper
do_parse_config_key() that takes a "quiet" flag and treats
store_key as optional. git_config_parse_key() becomes a thin
wrapper.
Add git_config_key_is_valid() for callers that only need to
know whether a key is well-formed.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
config.c | 38 +++++++++++++++++++++++++++++---------
config.h | 2 ++
2 files changed, 31 insertions(+), 9 deletions(-)
diff --git a/config.c b/config.c
index a1b92fe083..45144f73c5 100644
--- a/config.c
+++ b/config.c
@@ -536,11 +536,14 @@ static inline int iskeychar(int c)
* -2 if there is no section name in the key.
*
* store_key - pointer to char* which will hold a copy of the key with
- * lowercase section and variable name
+ * lowercase section and variable name, can be NULL to skip
+ * allocation when only validation is needed
* baselen - pointer to size_t which will hold the length of the
* section + subsection part, can be NULL
+ * quiet - when non-zero, suppress error() reports on rejection
*/
-int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
+static int do_parse_config_key(const char *key, char **store_key,
+ size_t *baselen_, int quiet)
{
size_t i, baselen;
int dot;
@@ -552,12 +555,14 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
*/
if (last_dot == NULL || last_dot == key) {
- error(_("key does not contain a section: %s"), key);
+ if (!quiet)
+ error(_("key does not contain a section: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
if (!last_dot[1]) {
- error(_("key does not contain variable name: %s"), key);
+ if (!quiet)
+ error(_("key does not contain variable name: %s"), key);
return -CONFIG_NO_SECTION_OR_NAME;
}
@@ -568,7 +573,8 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
/*
* Validate the key and while at it, lower case it for matching.
*/
- *store_key = xmallocz(strlen(key));
+ if (store_key)
+ *store_key = xmallocz(strlen(key));
dot = 0;
for (i = 0; key[i]; i++) {
@@ -579,24 +585,38 @@ int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
if (!dot || i > baselen) {
if (!iskeychar(c) ||
(i == baselen + 1 && !isalpha(c))) {
- error(_("invalid key: %s"), key);
+ if (!quiet)
+ error(_("invalid key: %s"), key);
goto out_free_ret_1;
}
c = tolower(c);
} else if (c == '\n') {
- error(_("invalid key (newline): %s"), key);
+ if (!quiet)
+ error(_("invalid key (newline): %s"), key);
goto out_free_ret_1;
}
- (*store_key)[i] = c;
+ if (store_key)
+ (*store_key)[i] = c;
}
return 0;
out_free_ret_1:
- FREE_AND_NULL(*store_key);
+ if (store_key)
+ FREE_AND_NULL(*store_key);
return -CONFIG_INVALID_KEY;
}
+int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
+{
+ return do_parse_config_key(key, store_key, baselen_, 0);
+}
+
+int git_config_key_is_valid(const char *key)
+{
+ return !do_parse_config_key(key, NULL, NULL, 1);
+}
+
static int config_parse_pair(const char *key, const char *value,
struct key_value_info *kvi,
config_fn_t fn, void *data)
diff --git a/config.h b/config.h
index bf47fb3afc..31fe3e2961 100644
--- a/config.h
+++ b/config.h
@@ -343,6 +343,8 @@ void repo_config_set(struct repository *, const char *, const char *);
int git_config_parse_key(const char *, char **, size_t *);
+int git_config_key_is_valid(const char *);
+
/*
* The following macros specify flag bits that alter the behavior
* of the repo_config_set_multivar*() methods.
--
gitgitgadget
^ permalink raw reply related
* [PATCH v6 0/2] config: suggest the correct form when key contains "="
From: Harald Nordgren via GitGitGadget @ 2026-06-02 18:43 UTC (permalink / raw)
To: git; +Cc: Kristoffer Haugsbakk, Harald Nordgren
In-Reply-To: <pull.2302.v5.git.git.1780407557.gitgitgadget@gmail.com>
* The quiet parameter now lives on a static do_parse_config_key() instead
of git_config_parse_key() itself. git_config_parse_key() is back to its
three-argument signature; existing callers don't change.
* New public git_config_key_is_valid() for callers that only need a yes/no
check.
Harald Nordgren (2):
config: add git_config_key_is_valid() for quiet validation
config: improve diagnostic for "set" with missing value
builtin/config.c | 32 ++++++++++++++++++++++++++-
config.c | 38 ++++++++++++++++++++++++--------
config.h | 2 ++
t/t1300-config.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 118 insertions(+), 10 deletions(-)
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v6
Pull-Request: https://github.com/git/git/pull/2302
Range-diff vs v5:
1: d938ebf95a ! 1: 7400ca41bb config: let git_config_parse_key() validate quietly
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- config: let git_config_parse_key() validate quietly
+ config: add git_config_key_is_valid() for quiet validation
- Add a "quiet" parameter that suppresses the error() calls, and let
- store_key be NULL to skip the canonical-copy allocation. Existing
- callers pass 0 for quiet.
+ Move the body of git_config_parse_key() into a static helper
+ do_parse_config_key() that takes a "quiet" flag and treats
+ store_key as optional. git_config_parse_key() becomes a thin
+ wrapper.
- Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
+ Add git_config_key_is_valid() for callers that only need to
+ know whether a key is well-formed.
- ## builtin/config.c ##
-@@ builtin/config.c: static int get_value(const struct config_location_options *opts,
- goto free_strings;
- }
- } else {
-- if (git_config_parse_key(key_, &key, NULL)) {
-+ if (git_config_parse_key(key_, &key, NULL, 0)) {
- ret = CONFIG_INVALID_KEY;
- goto free_strings;
- }
+ Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
## config.c ##
@@ config.c: static inline int iskeychar(int c)
@@ config.c: static inline int iskeychar(int c)
+ * quiet - when non-zero, suppress error() reports on rejection
*/
-int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
-+int git_config_parse_key(const char *key, char **store_key, size_t *baselen_,
-+ int quiet)
++static int do_parse_config_key(const char *key, char **store_key,
++ size_t *baselen_, int quiet)
{
size_t i, baselen;
int dot;
@@ config.c: int git_config_parse_key(const char *key, char **store_key, size_t *ba
return -CONFIG_INVALID_KEY;
}
-@@ config.c: static int config_parse_pair(const char *key, const char *value,
-
- if (!strlen(key))
- return error(_("empty config key"));
-- if (git_config_parse_key(key, &canonical_name, NULL))
-+ if (git_config_parse_key(key, &canonical_name, NULL, 0))
- return -1;
-
- ret = (fn(canonical_name, value, &ctx, data) < 0) ? -1 : 0;
-@@ config.c: static int configset_find_element(struct config_set *set, const char *key,
- * `key` may come from the user, so normalize it before using it
- * for querying entries from the hashmap.
- */
-- ret = git_config_parse_key(key, &normalized_key, NULL);
-+ ret = git_config_parse_key(key, &normalized_key, NULL, 0);
- if (ret)
- return ret;
-
-@@ config.c: int repo_config_set_multivar_in_file_gently(struct repository *r,
- validate_comment_string(comment);
-
- /* parse-key returns negative; flip the sign to feed exit(3) */
-- ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
-+ ret = 0 - git_config_parse_key(key, &store.key, &store.baselen, 0);
- if (ret)
- goto out_free;
-
++int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
++{
++ return do_parse_config_key(key, store_key, baselen_, 0);
++}
++
++int git_config_key_is_valid(const char *key)
++{
++ return !do_parse_config_key(key, NULL, NULL, 1);
++}
++
+ static int config_parse_pair(const char *key, const char *value,
+ struct key_value_info *kvi,
+ config_fn_t fn, void *data)
## config.h ##
-@@ config.h: int repo_config_set_worktree_gently(struct repository *, const char *, const cha
- */
- void repo_config_set(struct repository *, const char *, const char *);
+@@ config.h: void repo_config_set(struct repository *, const char *, const char *);
--int git_config_parse_key(const char *, char **, size_t *);
-+int git_config_parse_key(const char *, char **, size_t *, int quiet);
+ int git_config_parse_key(const char *, char **, size_t *);
++int git_config_key_is_valid(const char *);
++
/*
* The following macros specify flag bits that alter the behavior
-
- ## submodule-config.c ##
-@@ submodule-config.c: int print_config_from_gitmodules(struct repository *repo, const char *key)
- int ret;
- char *store_key;
-
-- ret = git_config_parse_key(key, &store_key, NULL);
-+ ret = git_config_parse_key(key, &store_key, NULL, 0);
- if (ret < 0)
- return CONFIG_INVALID_KEY;
-
+ * of the repo_config_set_multivar*() methods.
2: e5a2070ee1 ! 2: a7f8a084c7 config: improve diagnostic for "set" with missing value
@@ builtin/config.c: static void check_argc(int argc, int min, int max)
+ const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
+ char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
+
-+ if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
++ if (prefix && git_config_key_is_valid(prefix)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ advise(_("did you mean \"git config set %s %s\"?"),
+ prefix, eq + 1);
-+ } else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
++ } else if (git_config_key_is_valid(arg)) {
+ error(_("missing value to set to the variable '%s'"), arg);
+ } else {
+ error(_("missing value to set to a variable with an invalid name '%s'"),
@@ t/t1300-config.sh: test_expect_success 'invalid key' '
+
+test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
+ test_must_fail git config set pull.rebase=false true 2>err &&
++ test_grep "invalid key: pull\\.rebase=false" err &&
+ test_grep ! "did you mean" err
+'
+
--
gitgitgadget
^ permalink raw reply
* Re: Suggetsions for collaboration workflows in large repos
From: Matthew Hughes @ 2026-06-02 18:35 UTC (permalink / raw)
To: Ben Knoble; +Cc: git
In-Reply-To: <82F556A1-A5C6-414E-8EFB-13F83FA30E44@gmail.com>
On Fri, May 29, 2026 at 01:56:02PM -0400, Ben Knoble wrote:
> My current advice is to enable git-maintenance on such a repo, where
> prefetches and commit graphs and so on will give you a nice perf boost. Then
> I keep the default fetch all heads config and don’t mind the noise too much.
Thanks, I do have maintenance activated (I believe `scalar` handled that for
me) and that does noticeable speed up some operations, and I have find the
performance in general for almost all operations to be much better than I
expected (having not worked in such a large repo before). My only real issue is
in fetching, since I really don't want to waste the time pulling down all the
other branches in the repo that I almost certainly will never need locally.
^ permalink raw reply
* Re: [PATCH v3 00/12] Improve git gui operation without a worktree
From: Johannes Sixt @ 2026-06-02 17:34 UTC (permalink / raw)
To: Mark Levedahl; +Cc: egg_mushroomcow, bootaina702, git
In-Reply-To: <20260531230225.126817-1-mlevedahl@gmail.com>
Am 01.06.26 um 01:02 schrieb Mark Levedahl:
> git gui has a number of inter-related problems that result in problems
> during startup from anything but a checked out worktree pointing at a
> valid git repository. Some of the symptoms are:
> - blame / browser subcommands, and launching gitk, are intended to be
> useful without a worktree, but fail to work.
> - unlike git, git-gui is supposed to use the parent directory as a
> worktree if started from the .git subdirectory in the very common
> single worktree + embedded git repository format. This does not
> work.
> - git-gui includes a repository picker allowing a user to select a
> worktree from a list and/or start a new repo+worktree: this dialog can
> appear at unexpected times, masking useful error feedback on
> configuration problems.
>
> This patch series addresses the above issues, substantially rewriting
> the initial repository/worktree process to rely upon git rev-parse so
> that git's knowledge of access rules, repository configuration, and use
> of GIT_DIR / GIT_WORK_TREE (or git --gitdir / --work-tree) is used
> throughout, replacing code largely based upon what git did in 2008. This
> also means that git gui will naturally gain any new rules implmented in
> git-core.
>
> With this, git-gui only exports GIT_WORK_TREE when non-empty.
> GIT_WORK_TREE is needed, and must be exported, if the user is overriding
> core.worktree in the git repository. But, GIT_WORK_TREE cannot be used
> to specify the lack of a worktree, so exporting an empty GIT_WORK_TREE
> is one of the problems fixed by this series.
>
> v3 of this series addresses j6t's review of v2, with some reordering of
> patches (1 from j6t added, patch #8 moved to #1), adds another rewrite
> of the browser / blame parser that eliminates the notion of path before
> rev on the command line, blame works correctly with a user modified file
> in the worktree. Clarification is added on the need for GTI_WORK_TREE,
> and the logic in finding a worktree from the gitdir is simplified.
>
> Johannes Sixt (1):
> git-gui: remove unnecessary 'cd $_gitworktree' from do_gitk
>
> Mark Levedahl (11):
> git-gui: use HEAD as current branch when detached
> git-gui: guard set/unset of GIT_DIR and GIT_WORK_TREE
> git-gui: do not change global vars in choose_repository::pick
> git-gui: use --absolute-git-dir
> git-gui: use rev-parse exclusively to find a repository
> git-gui: use git rev-parse for worktree discovery
> git-gui: simplify [is_bare] to report if a worktree is known
> git-gui: try harder to find worktree from gitdir
> git-gui: allow specifying path '.' to the browser
> git-gui: check browser/blame arguments carefully
> git-gui: add gui and pick as explicit subcommands
>
> git-gui.sh | 377 ++++++++++++++++++++++----------------
> lib/choose_repository.tcl | 21 +--
> 2 files changed, 223 insertions(+), 175 deletions(-)
This round looks excellent! Thank you very much!
While queuing, I applied the small fixup below to 03/12.
-- Hannes
diff --git a/git-gui.sh b/git-gui.sh
index 933e72c9b255..15dd2b3a84cc 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -2064,7 +2064,6 @@ proc incr_font_size {font {amt 1}} {
proc do_gitk {revs {is_submodule false}} {
global current_diff_path file_states current_diff_side ui_index
- global _gitworktree
# -- Always start gitk through whatever we were loaded with. This
# lets us bypass using shell process on Windows systems.
^ permalink raw reply related
* Re: [PATCH v9] revision.c: implement --max-count-oldest
From: Mirko Faina @ 2026-06-02 17:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble, Johannes Sixt, Chris Torek, Mirko Faina
In-Reply-To: <xmqq4ijm3p2x.fsf@gitster.g>
On Tue, Jun 02, 2026 at 06:53:10AM +0900, Junio C Hamano wrote:
> It has been a while, and we saw no further comments by other
> reviewers.
>
> Perhaps we should declare a victory and mark the topic for 'next'.
Yes, if no one objects it might be time for 'next'.
^ permalink raw reply
* [PATCH v5 8/8] environment: move "warn_on_object_refname_ambiguity" into `struct repo_config_values`
From: Olamide Caleb Bello @ 2026-06-02 17:09 UTC (permalink / raw)
To: git
Cc: phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, Olamide Caleb Bello
In-Reply-To: <20260602170921.35869-1-belkid98@gmail.com>
The `core.warnAmbiguousRefs` configuration was previously stored in a
global `int` variable, making it shared across repository instances
and risking cross‑repository state leakage.
Store it instead in `repo_config_values`, where eagerly‑parsed
repository configuration lives. This option is parsed eagerly because
ambiguity warnings influence how users interpret object references in
many commands; a lazy parse could cause these warnings to behave
inconsistently or to appear for the wrong repository, confusing users
and hindering libification. This preserves the existing behavior while
tying the value to the repository from which it was read, avoiding
cross‑repository state leakage and continuing the effort to reduce
reliance on global configuration state.
Update all references to use `repo_config_values()`.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
builtin/cat-file.c | 7 ++++---
builtin/pack-objects.c | 7 ++++---
environment.c | 2 +-
environment.h | 2 +-
object-name.c | 3 ++-
revision.c | 7 ++++---
submodule.c | 7 ++++---
7 files changed, 20 insertions(+), 15 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index d9fbad5358..cfc5430186 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -901,6 +901,7 @@ static int batch_objects(struct batch_options *opt)
struct strbuf input = STRBUF_INIT;
struct strbuf output = STRBUF_INIT;
struct expand_data data = EXPAND_DATA_INIT;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
int save_warning;
int retval = 0;
@@ -973,8 +974,8 @@ static int batch_objects(struct batch_options *opt)
* warn) ends up dwarfing the actual cost of the object lookups
* themselves. We can work around it by just turning off the warning.
*/
- save_warning = warn_on_object_refname_ambiguity;
- warn_on_object_refname_ambiguity = 0;
+ save_warning = cfg->warn_on_object_refname_ambiguity;
+ cfg->warn_on_object_refname_ambiguity = 0;
if (opt->batch_mode == BATCH_MODE_QUEUE_AND_DISPATCH) {
batch_objects_command(opt, &output, &data);
@@ -1002,7 +1003,7 @@ static int batch_objects(struct batch_options *opt)
cleanup:
strbuf_release(&input);
strbuf_release(&output);
- warn_on_object_refname_ambiguity = save_warning;
+ cfg->warn_on_object_refname_ambiguity = save_warning;
return retval;
}
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8ccbe7e178..7df75fe91e 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -4788,6 +4788,7 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
struct setup_revision_opt s_r_opt = {
.allow_exclude_promisor_objects = 1,
};
+ struct repo_config_values *cfg = repo_config_values(the_repository);
char line[1000];
int flags = 0;
int save_warning;
@@ -4798,8 +4799,8 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
/* make sure shallows are read */
is_repository_shallow(the_repository);
- save_warning = warn_on_object_refname_ambiguity;
- warn_on_object_refname_ambiguity = 0;
+ save_warning = cfg->warn_on_object_refname_ambiguity;
+ cfg->warn_on_object_refname_ambiguity = 0;
while (fgets(line, sizeof(line), stdin) != NULL) {
int len = strlen(line);
@@ -4827,7 +4828,7 @@ static void get_object_list(struct rev_info *revs, struct strvec *argv)
die(_("bad revision '%s'"), line);
}
- warn_on_object_refname_ambiguity = save_warning;
+ cfg->warn_on_object_refname_ambiguity = save_warning;
if (use_bitmap_index && !get_object_list_from_bitmap(revs))
return;
diff --git a/environment.c b/environment.c
index 57587ede56..ba2c60103f 100644
--- a/environment.c
+++ b/environment.c
@@ -47,7 +47,6 @@ int minimum_abbrev = 4, default_abbrev = -1;
int ignore_case;
int assume_unchanged;
int is_bare_repository_cfg = -1; /* unspecified */
-int warn_on_object_refname_ambiguity = 1;
char *git_commit_encoding;
char *git_log_output_encoding;
char *apply_default_whitespace;
@@ -725,4 +724,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
cfg->core_sparse_checkout_cone = 0;
cfg->sparse_expect_files_outside_of_patterns = 0;
+ cfg->warn_on_object_refname_ambiguity = 1;
}
diff --git a/environment.h b/environment.h
index 609cdaa07f..1ff0a7ba8b 100644
--- a/environment.h
+++ b/environment.h
@@ -97,6 +97,7 @@ struct repo_config_values {
int pack_compression_level;
int precomposed_unicode;
int core_sparse_checkout_cone;
+ int warn_on_object_refname_ambiguity;
/* section "sparse" config values */
int sparse_expect_files_outside_of_patterns;
@@ -174,7 +175,6 @@ extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
-extern int warn_on_object_refname_ambiguity;
extern char *apply_default_whitespace;
extern char *apply_default_ignorewhitespace;
extern unsigned long pack_size_limit_cfg;
diff --git a/object-name.c b/object-name.c
index 21dcdc4a0e..319d3db01d 100644
--- a/object-name.c
+++ b/object-name.c
@@ -684,11 +684,12 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
int refs_found = 0;
int at, reflog_len, nth_prior = 0;
int fatal = !(flags & GET_OID_QUIETLY);
+ struct repo_config_values *cfg = repo_config_values(the_repository);
if (len == r->hash_algo->hexsz && !get_oid_hex(str, oid)) {
if (!(flags & GET_OID_SKIP_AMBIGUITY_CHECK) &&
repo_settings_get_warn_ambiguous_refs(r) &&
- warn_on_object_refname_ambiguity) {
+ cfg->warn_on_object_refname_ambiguity) {
refs_found = repo_dwim_ref(r, str, len, &tmp_oid, &real_ref, 0);
if (refs_found > 0) {
warning(warn_msg, len, str);
diff --git a/revision.c b/revision.c
index 599b3a66c3..4e7faa7eb1 100644
--- a/revision.c
+++ b/revision.c
@@ -2922,9 +2922,10 @@ static void read_revisions_from_stdin(struct rev_info *revs,
int seen_end_of_options = 0;
int save_warning;
int flags = 0;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
- save_warning = warn_on_object_refname_ambiguity;
- warn_on_object_refname_ambiguity = 0;
+ save_warning = cfg->warn_on_object_refname_ambiguity;
+ cfg->warn_on_object_refname_ambiguity = 0;
strbuf_init(&sb, 1000);
while (strbuf_getline(&sb, stdin) != EOF) {
@@ -2958,7 +2959,7 @@ static void read_revisions_from_stdin(struct rev_info *revs,
read_pathspec_from_stdin(&sb, prune);
strbuf_release(&sb);
- warn_on_object_refname_ambiguity = save_warning;
+ cfg->warn_on_object_refname_ambiguity = save_warning;
}
static void NORETURN diagnose_missing_default(const char *def)
diff --git a/submodule.c b/submodule.c
index b1a0363f9d..f26235bbb7 100644
--- a/submodule.c
+++ b/submodule.c
@@ -898,12 +898,13 @@ static void collect_changed_submodules(struct repository *r,
struct setup_revision_opt s_r_opt = {
.assume_dashdash = 1,
};
+ struct repo_config_values *cfg = repo_config_values(the_repository);
- save_warning = warn_on_object_refname_ambiguity;
- warn_on_object_refname_ambiguity = 0;
+ save_warning = cfg->warn_on_object_refname_ambiguity;
+ cfg->warn_on_object_refname_ambiguity = 0;
repo_init_revisions(r, &rev, NULL);
setup_revisions_from_strvec(argv, &rev, &s_r_opt);
- warn_on_object_refname_ambiguity = save_warning;
+ cfg->warn_on_object_refname_ambiguity = save_warning;
if (prepare_revision_walk(&rev))
die(_("revision walk setup failed"));
--
2.53.0.155.g9f36b15afa
^ permalink raw reply related
* [PATCH v5 7/8] environment: move "sparse_expect_files_outside_of_patterns" into `struct repo_config_values`
From: Olamide Caleb Bello @ 2026-06-02 17:09 UTC (permalink / raw)
To: git
Cc: phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, Olamide Caleb Bello
In-Reply-To: <20260602170921.35869-1-belkid98@gmail.com>
The `core.sparseCheckoutExpectFilesOutsideOfPatterns` configuration was
previously stored in a global `int` variable, making it shared across
repository instances and risking cross‑repository state leakage.
Store it instead in `repo_config_values`, where eagerly‑parsed
repository configuration lives. This option is parsed eagerly because
it controls how sparse‑checkout paths are interpreted – a fundamental
behavior that many commands rely on; a lazy parse could cause
inconsistent sparse‑checkout handling and complicate libification.
This preserves the existing behavior while tying the value to the
repository from which it was read, avoiding cross‑repository state
leakage and continuing the effort to reduce reliance on global
configuration state.
Update all references to use `repo_config_values()`.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
environment.c | 6 ++++--
environment.h | 5 +++--
sparse-index.c | 2 +-
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/environment.c b/environment.c
index b0e873e9f5..57587ede56 100644
--- a/environment.c
+++ b/environment.c
@@ -70,7 +70,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
#endif
enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
int grafts_keep_true_parents;
-int sparse_expect_files_outside_of_patterns;
unsigned long pack_size_limit_cfg;
#ifndef PROTECT_HFS_DEFAULT
@@ -550,8 +549,10 @@ int git_default_core_config(const char *var, const char *value,
static int git_default_sparse_config(const char *var, const char *value)
{
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+
if (!strcmp(var, "sparse.expectfilesoutsideofpatterns")) {
- sparse_expect_files_outside_of_patterns = git_config_bool(var, value);
+ cfg->sparse_expect_files_outside_of_patterns = git_config_bool(var, value);
return 0;
}
@@ -723,4 +724,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
cfg->core_sparse_checkout_cone = 0;
+ cfg->sparse_expect_files_outside_of_patterns = 0;
}
diff --git a/environment.h b/environment.h
index befad9a388..609cdaa07f 100644
--- a/environment.h
+++ b/environment.h
@@ -98,6 +98,9 @@ struct repo_config_values {
int precomposed_unicode;
int core_sparse_checkout_cone;
+ /* section "sparse" config values */
+ int sparse_expect_files_outside_of_patterns;
+
/* section "branch" config values */
enum branch_track branch_track;
};
@@ -179,8 +182,6 @@ extern unsigned long pack_size_limit_cfg;
extern int protect_hfs;
extern int protect_ntfs;
-extern int sparse_expect_files_outside_of_patterns;
-
enum rebase_setup_type {
AUTOREBASE_NEVER = 0,
AUTOREBASE_LOCAL,
diff --git a/sparse-index.c b/sparse-index.c
index 53cb8d64fc..1ed769b78d 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -675,7 +675,7 @@ void clear_skip_worktree_from_present_files(struct index_state *istate)
struct repo_config_values *cfg = repo_config_values(the_repository);
if (!cfg->apply_sparse_checkout ||
- sparse_expect_files_outside_of_patterns)
+ cfg->sparse_expect_files_outside_of_patterns)
return;
if (clear_skip_worktree_from_present_files_sparse(istate)) {
--
2.53.0.155.g9f36b15afa
^ permalink raw reply related
* [PATCH v5 6/8] environment: move "core_sparse_checkout_cone" into `struct repo_config_values`
From: Olamide Caleb Bello @ 2026-06-02 17:09 UTC (permalink / raw)
To: git
Cc: phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, Olamide Caleb Bello
In-Reply-To: <20260602170921.35869-1-belkid98@gmail.com>
The `core.sparseCheckoutCone` configuration was previously stored in an
uninitialized global `int` variable, risking cross‑repository state
leakage.
Move it into `repo_config_values`, where eagerly‑parsed repository
configuration lives. `core.sparseCheckoutCone` is parsed eagerly
because it determines the fundamental sparse‑checkout mode and is
consulted very early during repository setup; a lazy parse could
leave the sparse‑checkout state undefined and complicate
libification. This preserves the existing behavior while tying the
value to the repository from which it was read, avoiding cross‑
repository state leakage and continuing the effort to reduce reliance
on global configuration state.
Update all references to use `repo_config_values()`.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
builtin/mv.c | 2 +-
builtin/sparse-checkout.c | 37 ++++++++++++++++++++++---------------
dir.c | 3 ++-
environment.c | 4 ++--
environment.h | 2 +-
sparse-index.c | 2 +-
6 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/builtin/mv.c b/builtin/mv.c
index 2215d34e31..ef3a326c90 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -574,7 +574,7 @@ int cmd_mv(int argc,
if (ignore_sparse &&
cfg->apply_sparse_checkout &&
- core_sparse_checkout_cone) {
+ cfg->core_sparse_checkout_cone) {
/*
* NEEDSWORK: we are *not* paying attention to
* "out-to-out" move (<source> is out-of-cone and
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index f4aa405da9..92d017b81f 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -73,7 +73,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix,
memset(&pl, 0, sizeof(pl));
- pl.use_cone_patterns = core_sparse_checkout_cone;
+ pl.use_cone_patterns = cfg->core_sparse_checkout_cone;
sparse_filename = get_sparse_checkout_filename();
res = add_patterns_from_file_to_list(sparse_filename, "", 0, &pl, NULL, 0);
@@ -334,6 +334,7 @@ static int write_patterns_and_update(struct repository *repo,
FILE *fp;
struct lock_file lk = LOCK_INIT;
int result;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
sparse_filename = get_sparse_checkout_filename();
@@ -353,7 +354,7 @@ static int write_patterns_and_update(struct repository *repo,
if (!fp)
die_errno(_("unable to fdopen %s"), get_lock_file_path(&lk));
- if (core_sparse_checkout_cone)
+ if (cfg->core_sparse_checkout_cone)
write_cone_to_file(fp, pl);
else
write_patterns_to_file(fp, pl);
@@ -402,15 +403,15 @@ static enum sparse_checkout_mode update_cone_mode(int *cone_mode) {
/* If not specified, use previous definition of cone mode */
if (*cone_mode == -1 && cfg->apply_sparse_checkout)
- *cone_mode = core_sparse_checkout_cone;
+ *cone_mode = cfg->core_sparse_checkout_cone;
/* Set cone/non-cone mode appropriately */
cfg->apply_sparse_checkout = 1;
if (*cone_mode == 1 || *cone_mode == -1) {
- core_sparse_checkout_cone = 1;
+ cfg->core_sparse_checkout_cone = 1;
return MODE_CONE_PATTERNS;
}
- core_sparse_checkout_cone = 0;
+ cfg->core_sparse_checkout_cone = 0;
return MODE_ALL_PATTERNS;
}
@@ -577,7 +578,9 @@ static void add_patterns_from_input(struct pattern_list *pl,
FILE *file)
{
int i;
- if (core_sparse_checkout_cone) {
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+
+ if (cfg->core_sparse_checkout_cone) {
struct strbuf line = STRBUF_INIT;
hashmap_init(&pl->recursive_hashmap, pl_hashmap_cmp, NULL, 0);
@@ -636,13 +639,14 @@ static void add_patterns_cone_mode(int argc, const char **argv,
struct pattern_entry *pe;
struct hashmap_iter iter;
struct pattern_list existing;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
char *sparse_filename = get_sparse_checkout_filename();
add_patterns_from_input(pl, argc, argv,
use_stdin ? stdin : NULL);
memset(&existing, 0, sizeof(existing));
- existing.use_cone_patterns = core_sparse_checkout_cone;
+ existing.use_cone_patterns = cfg->core_sparse_checkout_cone;
if (add_patterns_from_file_to_list(sparse_filename, "", 0,
&existing, NULL, 0))
@@ -690,7 +694,7 @@ static int modify_pattern_list(struct repository *repo,
switch (m) {
case ADD:
- if (core_sparse_checkout_cone)
+ if (cfg->core_sparse_checkout_cone)
add_patterns_cone_mode(args->nr, args->v, pl, use_stdin);
else
add_patterns_literal(args->nr, args->v, pl, use_stdin);
@@ -723,11 +727,12 @@ static void sanitize_paths(struct repository *repo,
const char *prefix, int skip_checks)
{
int i;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
if (!args->nr)
return;
- if (prefix && *prefix && core_sparse_checkout_cone) {
+ if (prefix && *prefix && cfg->core_sparse_checkout_cone) {
/*
* The args are not pathspecs, so unfortunately we
* cannot imitate how cmd_add() uses parse_pathspec().
@@ -744,10 +749,10 @@ static void sanitize_paths(struct repository *repo,
if (skip_checks)
return;
- if (prefix && *prefix && !core_sparse_checkout_cone)
+ if (prefix && *prefix && !cfg->core_sparse_checkout_cone)
die(_("please run from the toplevel directory in non-cone mode"));
- if (core_sparse_checkout_cone) {
+ if (cfg->core_sparse_checkout_cone) {
for (i = 0; i < args->nr; i++) {
if (args->v[i][0] == '/')
die(_("specify directories rather than patterns (no leading slash)"));
@@ -769,7 +774,7 @@ static void sanitize_paths(struct repository *repo,
if (S_ISSPARSEDIR(ce->ce_mode))
continue;
- if (core_sparse_checkout_cone)
+ if (cfg->core_sparse_checkout_cone)
die(_("'%s' is not a directory; to treat it as a directory anyway, rerun with --skip-checks"), args->v[i]);
else
warning(_("pass a leading slash before paths such as '%s' if you want a single file (see NON-CONE PROBLEMS in the git-sparse-checkout manual)."), args->v[i]);
@@ -836,6 +841,7 @@ static struct sparse_checkout_set_opts {
static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
struct repository *repo)
{
+ struct repo_config_values *cfg = repo_config_values(the_repository);
int default_patterns_nr = 2;
const char *default_patterns[] = {"/*", "!/*/", NULL};
@@ -873,7 +879,7 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix,
* non-cone mode, if nothing is specified, manually select just the
* top-level directory (much as 'init' would do).
*/
- if (!core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
+ if (!cfg->core_sparse_checkout_cone && !set_opts.use_stdin && argc == 0) {
for (int i = 0; i < default_patterns_nr; i++)
strvec_push(&patterns, default_patterns[i]);
} else {
@@ -977,7 +983,7 @@ static int sparse_checkout_clean(int argc, const char **argv,
setup_work_tree();
if (!cfg->apply_sparse_checkout)
die(_("must be in a sparse-checkout to clean directories"));
- if (!core_sparse_checkout_cone)
+ if (!cfg->core_sparse_checkout_cone)
die(_("must be in a cone-mode sparse-checkout to clean directories"));
argc = parse_options(argc, argv, prefix,
@@ -1141,6 +1147,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char *
FILE *fp;
int ret;
struct pattern_list pl = {0};
+ struct repo_config_values *cfg = repo_config_values(the_repository);
char *sparse_filename;
check_rules_opts.cone_mode = -1;
@@ -1152,7 +1159,7 @@ static int sparse_checkout_check_rules(int argc, const char **argv, const char *
check_rules_opts.cone_mode = 1;
update_cone_mode(&check_rules_opts.cone_mode);
- pl.use_cone_patterns = core_sparse_checkout_cone;
+ pl.use_cone_patterns = cfg->core_sparse_checkout_cone;
if (check_rules_opts.rules_file) {
fp = xfopen(check_rules_opts.rules_file, "r");
add_patterns_from_input(&pl, argc, argv, fp);
diff --git a/dir.c b/dir.c
index fcb8f6dd2a..4f493b64c6 100644
--- a/dir.c
+++ b/dir.c
@@ -3508,8 +3508,9 @@ int get_sparse_checkout_patterns(struct pattern_list *pl)
{
int res;
char *sparse_filename = get_sparse_checkout_filename();
+ struct repo_config_values *cfg = repo_config_values(the_repository);
- pl->use_cone_patterns = core_sparse_checkout_cone;
+ pl->use_cone_patterns = cfg->core_sparse_checkout_cone;
res = add_patterns_from_file_to_list(sparse_filename, "", 0, pl, NULL, 0);
free(sparse_filename);
diff --git a/environment.c b/environment.c
index 739b647ebe..b0e873e9f5 100644
--- a/environment.c
+++ b/environment.c
@@ -70,7 +70,6 @@ enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
#endif
enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
int grafts_keep_true_parents;
-int core_sparse_checkout_cone;
int sparse_expect_files_outside_of_patterns;
unsigned long pack_size_limit_cfg;
@@ -526,7 +525,7 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.sparsecheckoutcone")) {
- core_sparse_checkout_cone = git_config_bool(var, value);
+ cfg->core_sparse_checkout_cone = git_config_bool(var, value);
return 0;
}
@@ -723,4 +722,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->zlib_compression_level = Z_BEST_SPEED;
cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
+ cfg->core_sparse_checkout_cone = 0;
}
diff --git a/environment.h b/environment.h
index 508cb1afbc..befad9a388 100644
--- a/environment.h
+++ b/environment.h
@@ -96,6 +96,7 @@ struct repo_config_values {
int zlib_compression_level;
int pack_compression_level;
int precomposed_unicode;
+ int core_sparse_checkout_cone;
/* section "branch" config values */
enum branch_track branch_track;
@@ -178,7 +179,6 @@ extern unsigned long pack_size_limit_cfg;
extern int protect_hfs;
extern int protect_ntfs;
-extern int core_sparse_checkout_cone;
extern int sparse_expect_files_outside_of_patterns;
enum rebase_setup_type {
diff --git a/sparse-index.c b/sparse-index.c
index 13629c075d..53cb8d64fc 100644
--- a/sparse-index.c
+++ b/sparse-index.c
@@ -154,7 +154,7 @@ int is_sparse_index_allowed(struct index_state *istate, int flags)
{
struct repo_config_values *cfg = repo_config_values(the_repository);
- if (!cfg->apply_sparse_checkout || !core_sparse_checkout_cone)
+ if (!cfg->apply_sparse_checkout || !cfg->core_sparse_checkout_cone)
return 0;
if (!(flags & SPARSE_INDEX_MEMORY_ONLY)) {
--
2.53.0.155.g9f36b15afa
^ permalink raw reply related
* [PATCH v5 5/8] environment: move "precomposed_unicode" into `struct repo_config_values`
From: Olamide Caleb Bello @ 2026-06-02 17:09 UTC (permalink / raw)
To: git
Cc: phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, Olamide Caleb Bello
In-Reply-To: <20260602170921.35869-1-belkid98@gmail.com>
The `core.precomposeunicode` configuration is currently stored in the
global variable `precomposed_unicode`, which makes it shared across
repository instances within a single process.
Store it instead in `repo_config_values`, where eagerly‑parsed
repository configuration lives. `core.precomposeunicode` is parsed
eagerly because it controls Unicode path normalization on macOS,
a fundamental filesystem‑level behavior that many operations depend
on; a lazy parse could lead to inconsistent results and hamper
libification. This preserves the existing behavior while tying the
value to the repository from which it was read, avoiding cross‑
repository state leakage and continuing the effort to reduce reliance
on global configuration state.
Update all references to use `repo_config_values()`.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
compat/precompose_utf8.c | 20 +++++++++++++-------
environment.c | 4 ++--
environment.h | 2 +-
upload-pack.c | 3 ++-
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 43b3be0114..0e94dbd862 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -48,16 +48,18 @@ void probe_utf8_pathname_composition(void)
static const char *auml_nfc = "\xc3\xa4";
static const char *auml_nfd = "\x61\xcc\x88";
int output_fd;
- if (precomposed_unicode != -1)
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+
+ if (cfg->precomposed_unicode != -1)
return; /* We found it defined in the global config, respect it */
repo_git_path_replace(the_repository, &path, "%s", auml_nfc);
output_fd = open(path.buf, O_CREAT|O_EXCL|O_RDWR, 0600);
if (output_fd >= 0) {
close(output_fd);
repo_git_path_replace(the_repository, &path, "%s", auml_nfd);
- precomposed_unicode = access(path.buf, R_OK) ? 0 : 1;
+ cfg->precomposed_unicode = access(path.buf, R_OK) ? 0 : 1;
repo_config_set(the_repository, "core.precomposeunicode",
- precomposed_unicode ? "true" : "false");
+ cfg->precomposed_unicode ? "true" : "false");
repo_git_path_replace(the_repository, &path, "%s", auml_nfc);
if (unlink(path.buf))
die_errno(_("failed to unlink '%s'"), path.buf);
@@ -69,14 +71,16 @@ const char *precompose_string_if_needed(const char *in)
{
size_t inlen;
size_t outlen;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+
if (!in)
return NULL;
if (has_non_ascii(in, (size_t)-1, &inlen)) {
iconv_t ic_prec;
char *out;
- if (precomposed_unicode < 0)
- repo_config_get_bool(the_repository, "core.precomposeunicode", &precomposed_unicode);
- if (precomposed_unicode != 1)
+ if (cfg->precomposed_unicode < 0)
+ repo_config_get_bool(the_repository, "core.precomposeunicode", &cfg->precomposed_unicode);
+ if (cfg->precomposed_unicode != 1)
return in;
ic_prec = iconv_open(repo_encoding, path_encoding);
if (ic_prec == (iconv_t) -1)
@@ -130,7 +134,9 @@ PREC_DIR *precompose_utf8_opendir(const char *dirname)
struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir)
{
+ struct repo_config_values *cfg = repo_config_values(the_repository);
struct dirent *res;
+
res = readdir(prec_dir->dirp);
if (res) {
size_t namelenz = strlen(res->d_name) + 1; /* \0 */
@@ -149,7 +155,7 @@ struct dirent_prec_psx *precompose_utf8_readdir(PREC_DIR *prec_dir)
prec_dir->dirent_nfc->d_ino = res->d_ino;
prec_dir->dirent_nfc->d_type = res->d_type;
- if ((precomposed_unicode == 1) && has_non_ascii(res->d_name, (size_t)-1, NULL)) {
+ if ((cfg->precomposed_unicode == 1) && has_non_ascii(res->d_name, (size_t)-1, NULL)) {
if (prec_dir->ic_precompose == (iconv_t)-1) {
die("iconv_open(%s,%s) failed, but needed:\n"
" precomposed unicode is not supported.\n"
diff --git a/environment.c b/environment.c
index d0d3a4b7d2..739b647ebe 100644
--- a/environment.c
+++ b/environment.c
@@ -72,7 +72,6 @@ enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
int grafts_keep_true_parents;
int core_sparse_checkout_cone;
int sparse_expect_files_outside_of_patterns;
-int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
unsigned long pack_size_limit_cfg;
#ifndef PROTECT_HFS_DEFAULT
@@ -532,7 +531,7 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.precomposeunicode")) {
- precomposed_unicode = git_config_bool(var, value);
+ cfg->precomposed_unicode = git_config_bool(var, value);
return 0;
}
@@ -723,4 +722,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->check_stat = 1;
cfg->zlib_compression_level = Z_BEST_SPEED;
cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
+ cfg->precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
}
diff --git a/environment.h b/environment.h
index 514576b67a..508cb1afbc 100644
--- a/environment.h
+++ b/environment.h
@@ -95,6 +95,7 @@ struct repo_config_values {
int check_stat;
int zlib_compression_level;
int pack_compression_level;
+ int precomposed_unicode;
/* section "branch" config values */
enum branch_track branch_track;
@@ -174,7 +175,6 @@ extern char *apply_default_whitespace;
extern char *apply_default_ignorewhitespace;
extern unsigned long pack_size_limit_cfg;
-extern int precomposed_unicode;
extern int protect_hfs;
extern int protect_ntfs;
diff --git a/upload-pack.c b/upload-pack.c
index 9f6d6fe48c..3a52237134 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1336,6 +1336,7 @@ static int upload_pack_config(const char *var, const char *value,
void *cb_data)
{
struct upload_pack_data *data = cb_data;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
if (!strcmp("uploadpack.allowtipsha1inwant", var)) {
if (git_config_bool(var, value))
@@ -1366,7 +1367,7 @@ static int upload_pack_config(const char *var, const char *value,
if (value)
data->allow_packfile_uris = 1;
} else if (!strcmp("core.precomposeunicode", var)) {
- precomposed_unicode = git_config_bool(var, value);
+ cfg->precomposed_unicode = git_config_bool(var, value);
} else if (!strcmp("transfer.advertisesid", var)) {
data->advertise_sid = git_config_bool(var, value);
}
--
2.53.0.155.g9f36b15afa
^ permalink raw reply related
* [PATCH v5 4/8] environment: move "pack_compression_level" into `struct repo_config_values`
From: Olamide Caleb Bello @ 2026-06-02 17:09 UTC (permalink / raw)
To: git
Cc: phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, Olamide Caleb Bello
In-Reply-To: <20260602170921.35869-1-belkid98@gmail.com>
The `pack_compression_level` configuration is currently stored in the
global variable `pack_compression_level`, which makes it shared across
repository instances within a single process.
Store it instead in `repo_config_values`, where eagerly‑parsed
repository configuration lives. `pack_compression_level` is parsed
eagerly because it influences packfile compression, a core operation
where a lazy parse could cause inconsistent behavior and hamper
libification. This preserves the existing eager‑parsing behavior while
tying the value to the repository from which it was read, avoiding
cross‑repository state leakage and continuing the effort to reduce
reliance on global configuration state.
Update all references to use `repo_config_values()`.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
builtin/fast-import.c | 8 +++++---
builtin/pack-objects.c | 17 ++++++++++-------
environment.c | 8 +++++---
environment.h | 2 +-
object-file.c | 3 ++-
5 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 82bc6dcc00..070a5af3e4 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -965,6 +965,7 @@ static int store_object(
unsigned long hdrlen, deltalen;
struct git_hash_ctx c;
git_zstream s;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
hdrlen = format_object_header((char *)hdr, sizeof(hdr), type,
dat->len);
@@ -1005,7 +1006,7 @@ static int store_object(
} else
delta = NULL;
- git_deflate_init(&s, pack_compression_level);
+ git_deflate_init(&s, cfg->pack_compression_level);
if (delta) {
s.next_in = delta;
s.avail_in = deltalen;
@@ -1032,7 +1033,7 @@ static int store_object(
if (delta) {
FREE_AND_NULL(delta);
- git_deflate_init(&s, pack_compression_level);
+ git_deflate_init(&s, cfg->pack_compression_level);
s.next_in = (void *)dat->buf;
s.avail_in = dat->len;
s.avail_out = git_deflate_bound(&s, s.avail_in);
@@ -1115,6 +1116,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
struct git_hash_ctx c;
git_zstream s;
struct hashfile_checkpoint checkpoint;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
int status = Z_OK;
/* Determine if we should auto-checkpoint. */
@@ -1134,7 +1136,7 @@ static void stream_blob(uintmax_t len, struct object_id *oidout, uintmax_t mark)
crc32_begin(pack_file);
- git_deflate_init(&s, pack_compression_level);
+ git_deflate_init(&s, cfg->pack_compression_level);
hdrlen = encode_in_pack_object_header(out_buf, out_sz, OBJ_BLOB, len);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index dd2480a73d..8ccbe7e178 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -386,8 +386,9 @@ static unsigned long do_compress(void **pptr, unsigned long size)
git_zstream stream;
void *in, *out;
unsigned long maxsize;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
- git_deflate_init(&stream, pack_compression_level);
+ git_deflate_init(&stream, cfg->pack_compression_level);
maxsize = git_deflate_bound(&stream, size);
in = *pptr;
@@ -413,8 +414,9 @@ static unsigned long write_large_blob_data(struct odb_read_stream *st, struct ha
unsigned char ibuf[1024 * 16];
unsigned char obuf[1024 * 16];
unsigned long olen = 0;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
- git_deflate_init(&stream, pack_compression_level);
+ git_deflate_init(&stream, cfg->pack_compression_level);
for (;;) {
ssize_t readlen;
@@ -5003,6 +5005,7 @@ int cmd_pack_objects(int argc,
struct string_list keep_pack_list = STRING_LIST_INIT_NODUP;
struct list_objects_filter_options filter_options =
LIST_OBJECTS_FILTER_INIT;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
struct option pack_objects_options[] = {
OPT_CALLBACK_F('q', "quiet", &progress, NULL,
@@ -5084,7 +5087,7 @@ int cmd_pack_objects(int argc,
N_("ignore packs that have companion .keep file")),
OPT_STRING_LIST(0, "keep-pack", &keep_pack_list, N_("name"),
N_("ignore this pack")),
- OPT_INTEGER(0, "compression", &pack_compression_level,
+ OPT_INTEGER(0, "compression", &cfg->pack_compression_level,
N_("pack compression level")),
OPT_BOOL(0, "keep-true-parents", &grafts_keep_true_parents,
N_("do not hide commits by grafts")),
@@ -5243,10 +5246,10 @@ int cmd_pack_objects(int argc,
if (!reuse_object)
reuse_delta = 0;
- if (pack_compression_level == -1)
- pack_compression_level = Z_DEFAULT_COMPRESSION;
- else if (pack_compression_level < 0 || pack_compression_level > Z_BEST_COMPRESSION)
- die(_("bad pack compression level %d"), pack_compression_level);
+ if (cfg->pack_compression_level == -1)
+ cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
+ else if (cfg->pack_compression_level < 0 || cfg->pack_compression_level > Z_BEST_COMPRESSION)
+ die(_("bad pack compression level %d"), cfg->pack_compression_level);
if (!delta_search_threads) /* --threads=0 means autodetect */
delta_search_threads = online_cpus();
diff --git a/environment.c b/environment.c
index 5b0e88b65c..d0d3a4b7d2 100644
--- a/environment.c
+++ b/environment.c
@@ -52,7 +52,6 @@ char *git_commit_encoding;
char *git_log_output_encoding;
char *apply_default_whitespace;
char *apply_default_ignorewhitespace;
-int pack_compression_level = Z_DEFAULT_COMPRESSION;
int fsync_object_files = -1;
int use_fsync = -1;
enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
@@ -390,7 +389,7 @@ int git_default_core_config(const char *var, const char *value,
if (!zlib_compression_seen)
cfg->zlib_compression_level = level;
if (!pack_compression_seen)
- pack_compression_level = level;
+ cfg->pack_compression_level = level;
return 0;
}
@@ -662,6 +661,8 @@ static int git_default_attr_config(const char *var, const char *value)
int git_default_config(const char *var, const char *value,
const struct config_context *ctx, void *cb)
{
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+
if (starts_with(var, "core."))
return git_default_core_config(var, value, ctx, cb);
@@ -701,7 +702,7 @@ int git_default_config(const char *var, const char *value,
level = Z_DEFAULT_COMPRESSION;
else if (level < 0 || level > Z_BEST_COMPRESSION)
die(_("bad pack compression level %d"), level);
- pack_compression_level = level;
+ cfg->pack_compression_level = level;
pack_compression_seen = 1;
return 0;
}
@@ -721,4 +722,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->trust_ctime = 1;
cfg->check_stat = 1;
cfg->zlib_compression_level = Z_BEST_SPEED;
+ cfg->pack_compression_level = Z_DEFAULT_COMPRESSION;
}
diff --git a/environment.h b/environment.h
index 93201620af..514576b67a 100644
--- a/environment.h
+++ b/environment.h
@@ -94,6 +94,7 @@ struct repo_config_values {
int trust_ctime;
int check_stat;
int zlib_compression_level;
+ int pack_compression_level;
/* section "branch" config values */
enum branch_track branch_track;
@@ -171,7 +172,6 @@ extern int assume_unchanged;
extern int warn_on_object_refname_ambiguity;
extern char *apply_default_whitespace;
extern char *apply_default_ignorewhitespace;
-extern int pack_compression_level;
extern unsigned long pack_size_limit_cfg;
extern int precomposed_unicode;
diff --git a/object-file.c b/object-file.c
index 7c122ac419..37def5cc59 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1437,8 +1437,9 @@ static int stream_blob_to_pack(struct transaction_packfile *state,
int status = Z_OK;
int write_object = (flags & INDEX_WRITE_OBJECT);
off_t offset = 0;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
- git_deflate_init(&s, pack_compression_level);
+ git_deflate_init(&s, cfg->pack_compression_level);
hdrlen = encode_in_pack_object_header(obuf, sizeof(obuf), OBJ_BLOB, size);
s.next_out = obuf + hdrlen;
--
2.53.0.155.g9f36b15afa
^ permalink raw reply related
* [PATCH v5 3/8] environment: move `zlib_compression_level` into `struct repo_config_values`
From: Olamide Caleb Bello @ 2026-06-02 17:09 UTC (permalink / raw)
To: git
Cc: phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, Olamide Caleb Bello
In-Reply-To: <20260602170921.35869-1-belkid98@gmail.com>
The `zlib_compression_level` configuration is currently stored in the
global variable `zlib_compression_level`, which makes it shared across
repository instances within a single process.
Store it instead in `repo_config_values`, where eagerly‑parsed
repository configuration lives. `zlib_compression_level` is parsed
eagerly because it determines compression behaviour for objects and
packs – core operations where a lazy parse could lead to unpredictable
results and hinder libification. This preserves the existing
eager‑parsing behavior while tying the value to the repository it
was read from, avoiding cross‑repository state leakage and continuing
the effort to reduce reliance on global configuration state.
Update all references to use `repo_config_values()`.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
builtin/index-pack.c | 3 ++-
diff.c | 3 ++-
environment.c | 6 +++---
environment.h | 2 +-
http-push.c | 3 ++-
object-file.c | 3 ++-
6 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index ca7784dc2c..3942d3e0d0 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -1416,8 +1416,9 @@ static int write_compressed(struct hashfile *f, void *in, unsigned int size)
git_zstream stream;
int status;
unsigned char outbuf[4096];
+ struct repo_config_values *cfg = repo_config_values(the_repository);
- git_deflate_init(&stream, zlib_compression_level);
+ git_deflate_init(&stream, cfg->zlib_compression_level);
stream.next_in = in;
stream.avail_in = size;
diff --git a/diff.c b/diff.c
index 397e38b41c..7d17b0bf3f 100644
--- a/diff.c
+++ b/diff.c
@@ -3589,8 +3589,9 @@ static unsigned char *deflate_it(char *data,
int bound;
unsigned char *deflated;
git_zstream stream;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
- git_deflate_init(&stream, zlib_compression_level);
+ git_deflate_init(&stream, cfg->zlib_compression_level);
bound = git_deflate_bound(&stream, size);
deflated = xmalloc(bound);
stream.next_out = deflated;
diff --git a/environment.c b/environment.c
index 8542ac3141..5b0e88b65c 100644
--- a/environment.c
+++ b/environment.c
@@ -52,7 +52,6 @@ char *git_commit_encoding;
char *git_log_output_encoding;
char *apply_default_whitespace;
char *apply_default_ignorewhitespace;
-int zlib_compression_level = Z_BEST_SPEED;
int pack_compression_level = Z_DEFAULT_COMPRESSION;
int fsync_object_files = -1;
int use_fsync = -1;
@@ -377,7 +376,7 @@ int git_default_core_config(const char *var, const char *value,
level = Z_DEFAULT_COMPRESSION;
else if (level < 0 || level > Z_BEST_COMPRESSION)
die(_("bad zlib compression level %d"), level);
- zlib_compression_level = level;
+ cfg->zlib_compression_level = level;
zlib_compression_seen = 1;
return 0;
}
@@ -389,7 +388,7 @@ int git_default_core_config(const char *var, const char *value,
else if (level < 0 || level > Z_BEST_COMPRESSION)
die(_("bad zlib compression level %d"), level);
if (!zlib_compression_seen)
- zlib_compression_level = level;
+ cfg->zlib_compression_level = level;
if (!pack_compression_seen)
pack_compression_level = level;
return 0;
@@ -721,4 +720,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
cfg->check_stat = 1;
+ cfg->zlib_compression_level = Z_BEST_SPEED;
}
diff --git a/environment.h b/environment.h
index 1d3e2e4f23..93201620af 100644
--- a/environment.h
+++ b/environment.h
@@ -93,6 +93,7 @@ struct repo_config_values {
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
+ int zlib_compression_level;
/* section "branch" config values */
enum branch_track branch_track;
@@ -170,7 +171,6 @@ extern int assume_unchanged;
extern int warn_on_object_refname_ambiguity;
extern char *apply_default_whitespace;
extern char *apply_default_ignorewhitespace;
-extern int zlib_compression_level;
extern int pack_compression_level;
extern unsigned long pack_size_limit_cfg;
diff --git a/http-push.c b/http-push.c
index d143fe2845..8ac107a56e 100644
--- a/http-push.c
+++ b/http-push.c
@@ -369,13 +369,14 @@ static void start_put(struct transfer_request *request)
int hdrlen;
ssize_t size;
git_zstream stream;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
unpacked = odb_read_object(the_repository->objects, &request->obj->oid,
&type, &len);
hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
/* Set it up */
- git_deflate_init(&stream, zlib_compression_level);
+ git_deflate_init(&stream, cfg->zlib_compression_level);
size = git_deflate_bound(&stream, len + hdrlen);
strbuf_grow(&request->buffer.buf, size);
request->buffer.posn = 0;
diff --git a/object-file.c b/object-file.c
index 2acc9522df..7c122ac419 100644
--- a/object-file.c
+++ b/object-file.c
@@ -906,6 +906,7 @@ static int start_loose_object_common(struct odb_source *source,
const struct git_hash_algo *algo = source->odb->repo->hash_algo;
const struct git_hash_algo *compat = source->odb->repo->compat_hash_algo;
int fd;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
fd = create_tmpfile(source->odb->repo, tmp_file, filename);
if (fd < 0) {
@@ -921,7 +922,7 @@ static int start_loose_object_common(struct odb_source *source,
}
/* Setup zlib stream for compression */
- git_deflate_init(stream, zlib_compression_level);
+ git_deflate_init(stream, cfg->zlib_compression_level);
stream->next_out = buf;
stream->avail_out = buflen;
algo->init_fn(c);
--
2.53.0.155.g9f36b15afa
^ permalink raw reply related
* [PATCH v5 2/8] environment: move "check_stat" into `struct repo_config_values`
From: Olamide Caleb Bello @ 2026-06-02 17:09 UTC (permalink / raw)
To: git
Cc: phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, Olamide Caleb Bello
In-Reply-To: <20260602170921.35869-1-belkid98@gmail.com>
The `core.checkstat` configuration is currently stored in the global
variable `check_stat`, which makes it shared across repository
instances within a single process.
Store it instead in `repo_config_values`, where eagerly‑parsed
repository configuration lives. `core.checkstat` is parsed eagerly
because it controls how `match_stat_data()` and related functions
decide file freshness; a lazy parse could lead to unexpected
behavior or complicate libification. This preserves the existing
eager‑parsing behavior while tying the value to the repository it
was read from, avoiding cross‑repository state leakage, and
continuing the effort to reduce reliance on global configuration
state.
Update all references to use `repo_config_values()`.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
entry.c | 3 ++-
environment.c | 6 +++---
environment.h | 2 +-
statinfo.c | 10 +++++-----
4 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/entry.c b/entry.c
index 7817aee362..c55e867d8a 100644
--- a/entry.c
+++ b/entry.c
@@ -443,7 +443,8 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)
static void mark_colliding_entries(const struct checkout *state,
struct cache_entry *ce, struct stat *st)
{
- int trust_ino = check_stat;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+ int trust_ino = cfg->check_stat;
#if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)
trust_ino = 0;
diff --git a/environment.c b/environment.c
index 0a9067729e..8542ac3141 100644
--- a/environment.c
+++ b/environment.c
@@ -42,7 +42,6 @@ static int pack_compression_seen;
static int zlib_compression_seen;
int trust_executable_bit = 1;
-int check_stat = 1;
int has_symlinks = 1;
int minimum_abbrev = 4, default_abbrev = -1;
int ignore_case;
@@ -315,9 +314,9 @@ int git_default_core_config(const char *var, const char *value,
if (!value)
return config_error_nonbool(var);
if (!strcasecmp(value, "default"))
- check_stat = 1;
+ cfg->check_stat = 1;
else if (!strcasecmp(value, "minimal"))
- check_stat = 0;
+ cfg->check_stat = 0;
else
return error(_("invalid value for '%s': '%s'"),
var, value);
@@ -721,4 +720,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
+ cfg->check_stat = 1;
}
diff --git a/environment.h b/environment.h
index 64d537686e..1d3e2e4f23 100644
--- a/environment.h
+++ b/environment.h
@@ -92,6 +92,7 @@ struct repo_config_values {
char *attributes_file;
int apply_sparse_checkout;
int trust_ctime;
+ int check_stat;
/* section "branch" config values */
enum branch_track branch_track;
@@ -162,7 +163,6 @@ extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
extern int trust_executable_bit;
-extern int check_stat;
extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
diff --git a/statinfo.c b/statinfo.c
index 4fc12053f4..5e00af127d 100644
--- a/statinfo.c
+++ b/statinfo.c
@@ -68,19 +68,19 @@ int match_stat_data(const struct stat_data *sd, struct stat *st)
if (sd->sd_mtime.sec != (unsigned int)st->st_mtime)
changed |= MTIME_CHANGED;
- if (cfg->trust_ctime && check_stat &&
+ if (cfg->trust_ctime && cfg->check_stat &&
sd->sd_ctime.sec != (unsigned int)st->st_ctime)
changed |= CTIME_CHANGED;
#ifdef USE_NSEC
- if (check_stat && sd->sd_mtime.nsec != ST_MTIME_NSEC(*st))
+ if (cfg->check_stat && sd->sd_mtime.nsec != ST_MTIME_NSEC(*st))
changed |= MTIME_CHANGED;
- if (cfg->trust_ctime && check_stat &&
+ if (cfg->trust_ctime && cfg->check_stat &&
sd->sd_ctime.nsec != ST_CTIME_NSEC(*st))
changed |= CTIME_CHANGED;
#endif
- if (check_stat) {
+ if (cfg->check_stat) {
if (sd->sd_uid != (unsigned int) st->st_uid ||
sd->sd_gid != (unsigned int) st->st_gid)
changed |= OWNER_CHANGED;
@@ -94,7 +94,7 @@ int match_stat_data(const struct stat_data *sd, struct stat *st)
* clients will have different views of what "device"
* the filesystem is on
*/
- if (check_stat && sd->sd_dev != (unsigned int) st->st_dev)
+ if (cfg->check_stat && sd->sd_dev != (unsigned int) st->st_dev)
changed |= INODE_CHANGED;
#endif
--
2.53.0.155.g9f36b15afa
^ permalink raw reply related
* [PATCH v5 1/8] environment: move "trust_ctime" into `struct repo_config_values`
From: Olamide Caleb Bello @ 2026-06-02 17:09 UTC (permalink / raw)
To: git
Cc: phillip.wood123, christian.couder, usmanakinyemi202,
kaartic.sivaraam, Olamide Caleb Bello
In-Reply-To: <20260602170921.35869-1-belkid98@gmail.com>
The `core.trustctime` configuration is currently stored in the global
variable `trust_ctime`, which makes it shared across repository
instances in a single process.
Store it instead in `repo_config_values`, where eagerly‑parsed
repository configuration lives. `core.trustctime` is parsed eagerly
because it is used in low‑level stat‑matching functions
(`match_stat_data()`), where a lazy parse could cause unexpected
fatal errors, result in a performance regression and complicate
libification efforts. This preserves that behavior while tying the
value to the repository from which it was read, avoiding cross‑repository
state leakage and continuing the effort to reduce reliance on global
configuration state.
Update all references to use repo_config_values().
Mentored-by: Christian Couder <christian.couder@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
---
environment.c | 4 ++--
environment.h | 2 +-
statinfo.c | 6 ++++--
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/environment.c b/environment.c
index fc3ed8bb1c..0a9067729e 100644
--- a/environment.c
+++ b/environment.c
@@ -42,7 +42,6 @@ static int pack_compression_seen;
static int zlib_compression_seen;
int trust_executable_bit = 1;
-int trust_ctime = 1;
int check_stat = 1;
int has_symlinks = 1;
int minimum_abbrev = 4, default_abbrev = -1;
@@ -309,7 +308,7 @@ int git_default_core_config(const char *var, const char *value,
return 0;
}
if (!strcmp(var, "core.trustctime")) {
- trust_ctime = git_config_bool(var, value);
+ cfg->trust_ctime = git_config_bool(var, value);
return 0;
}
if (!strcmp(var, "core.checkstat")) {
@@ -721,4 +720,5 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->attributes_file = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
+ cfg->trust_ctime = 1;
}
diff --git a/environment.h b/environment.h
index 123a71cdc8..64d537686e 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
int apply_sparse_checkout;
+ int trust_ctime;
/* section "branch" config values */
enum branch_track branch_track;
@@ -161,7 +162,6 @@ extern char *git_work_tree_cfg;
/* Environment bits from configuration mechanism */
extern int trust_executable_bit;
-extern int trust_ctime;
extern int check_stat;
extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
diff --git a/statinfo.c b/statinfo.c
index 30a164b0e6..4fc12053f4 100644
--- a/statinfo.c
+++ b/statinfo.c
@@ -3,6 +3,7 @@
#include "git-compat-util.h"
#include "environment.h"
#include "statinfo.h"
+#include "repository.h"
/*
* Munge st_size into an unsigned int.
@@ -63,17 +64,18 @@ void fake_lstat_data(const struct stat_data *sd, struct stat *st)
int match_stat_data(const struct stat_data *sd, struct stat *st)
{
int changed = 0;
+ struct repo_config_values *cfg = repo_config_values(the_repository);
if (sd->sd_mtime.sec != (unsigned int)st->st_mtime)
changed |= MTIME_CHANGED;
- if (trust_ctime && check_stat &&
+ if (cfg->trust_ctime && check_stat &&
sd->sd_ctime.sec != (unsigned int)st->st_ctime)
changed |= CTIME_CHANGED;
#ifdef USE_NSEC
if (check_stat && sd->sd_mtime.nsec != ST_MTIME_NSEC(*st))
changed |= MTIME_CHANGED;
- if (trust_ctime && check_stat &&
+ if (cfg->trust_ctime && check_stat &&
sd->sd_ctime.nsec != ST_CTIME_NSEC(*st))
changed |= CTIME_CHANGED;
#endif
--
2.53.0.155.g9f36b15afa
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox