* Re: [PATCH v7 0/3] includeIf: add "worktree" condition for matching working tree path
From: Patrick Steinhardt @ 2026-07-09 10:09 UTC (permalink / raw)
To: me; +Cc: git, Kristoffer Haugsbakk, Junio C Hamano, Phillip Wood
In-Reply-To: <20260709-includeif-worktree-v7-0-e87e705e8df6@black-desk.cn>
On Thu, Jul 09, 2026 at 10:41:40AM +0800, Chen Linxuan via B4 Relay wrote:
> Changes in v7:
> - Preserve the symlinked spelling of the worktree path and match
> includeIf "worktree:" against it, so the condition now matches both
> the symlinked and the real path, consistent with "gitdir:"
> (Patrick Steinhardt, v6 review).
> - Split the work into a preparatory commit that stores a non-realpath
> worktree path and a follow-up that wires it into includeIf.
> - Extend symlink test coverage to subdirectories and linked worktrees.
> - Link to v6: https://lore.kernel.org/r/20260703-includeif-worktree-v6-0-a13893ad9a7f@black-desk.cn
One note: it would be nice if you could send newer versions of your
patch series in reply to the old version. I see you're using the b4
relay, so this should be configurable via `b4.send-same-thread`.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v3 4/4] notes: support an external command to display notes
From: Siddh Raman Pant @ 2026-07-09 11:10 UTC (permalink / raw)
To: j6t@kdbg.org
Cc: oswald.buddenhagen@gmx.de, gitster@pobox.com,
code@khaugsbakk.name, peff@peff.net, ps@pks.im,
git@vger.kernel.org, sandals@crustytoothpaste.net,
newren@gmail.com
In-Reply-To: <76cc093d2835a7b3ed110e20f1480dbe5fc2ecbb.camel@oracle.com>
[-- Attachment #1: Type: text/plain, Size: 2732 bytes --]
Going by no response, I assume this is NACK'd then?
Thanks,
Siddh
On Wed, Jun 24 2026 at 15:23:23 +0530, Siddh Raman Pant wrote:
> On Wed, Jun 24 2026 at 13:19:26 +0530, Johannes Sixt wrote:
> > > One solution to this is to move the freshness policy out of git so that
> > > it is someone else's problem. We can have a realtime fetch or faster
> > > updation via external helper means. But unfortunately we lose the
> > > coherence in the display of information, and so the user would end up
> > > reinventing git log in his quest to have same workflow.
> >
> > You are presenting one solution here. But a more obvious solution would
> > have been to make Git's notes implementation capable enough to keep up
> > with the volume of notes that are produced by your team.
>
> Git storage is inherently based on refs, so that would require massive
> changes IMO. The actual fundamental problem here is that only the
> latest state is useful at any given point of time, and not the past
> history.
>
> > Another solution would be to track the information outside of Git notes
> > entirely, similar to how pull requests, issues, reviews, and
> > conversations are tracked by Git hosters in databases outside of Git.
>
> This is precisely what this allows for. The information is tracked
> outside of Git, and the notes path just shows it along with the commit.
>
> A developer works on the code using Git. An external website doesn't
> allow the same level of coherence in display of information as a note.
> The commit is a fundamental unit of change. IMO it makes sense for Git
> to be able to show a note about it from a provided external medium.
>
> > > Let's add support for notes.externalCommand, a protected-configuration
> > > command that git runs as a long-lived helper when displaying notes. git
> > > sends commit IDs to the helper and displays any returned text through
> > > the existing notes formatting path. This keeps presentation in git
> > > while letting the helper decide how fresh note text is obtained.
> >
> > To my eyes, this looks like an overengineered solution that helps one
> > user of a niche feature of Git.
>
> This can also allow for other uses too. For example, searching lore I
> just found out that a colleague in Oracle Linux (Vegard) was trying to
> solve a related problem in 2022:
>
> https://lore.kernel.org/git/20220802075401.2393-1-vegard.nossum@oracle.com/
>
> I think it was for achieving something like this more generally:
> https://git.kernel.org/pub/scm/linux/kernel/git/vegard/linux.git/commit/?id=339f83612f3a569b194680768b22bf113c26a29d
>
> An external notes command can be a solution for it.
>
> Thanks,
> Siddh
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH v2 0/2] reftable: fix quadratic behavior when re-creating deleted refs
From: Kristofer Karlsson via GitGitGadget @ 2026-07-09 12:08 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson
In-Reply-To: <pull.2166.git.1783344957.gitgitgadget@gmail.com>
This series fixes quadratic behavior in the reftable backend when many
tombstones are present. Any operation that seeks into a range containing
tombstones is affected, including ref lookups and D/F conflict checks.
The root cause is the merged iterator's suppress_deletions flag, which
silently consumes tombstone records in a tight internal loop. This prevents
higher-level code from checking iteration bounds until after all tombstones
have been scanned, making both refs_verify_refnames_available() and
reftable_backend_read_ref() O(n) per call in the presence of tombstones.
The fix stops setting suppress_deletions on the stack's merged table and
instead handles deletion records at each call site in the reftable backend,
where prefix and refname bounds are available. This lets existing bounds
checks terminate iteration early when encountering tombstones past the
relevant bound.
The suppress_deletions flag and its logic are retained in the merged
iterator for downstream users of the reftable library (e.g. libgit2).
The first patch adds a perf test (p1401) exercising two tombstone scenarios
with 8000 refs. The second patch is the optimization. Both p1401 tests go
from ~13s to ~0.2s with the fix.
Note that auto-compaction typically merges tombstones before they accumulate
to this degree, so the quadratic behavior may not show up in every workflow.
But the fix ensures correct time complexity regardless of compaction state,
and the change is fairly contained.
Changes since v1:
* Keep suppress_deletions in the reftable library for downstream users;
only stop setting it in stack.c
* Broaden scope description to cover all readers, not just ref creation
* Use separate repositories in perf test to avoid cross-scenario state
* Drop correctness test (implicitly covered by t1400)
Previous discussion:
https://lore.kernel.org/git/20260701080014.GA3748390@coredump.intra.peff.net/
Kristofer Karlsson (2):
t/perf: add perf test for ref tombstone scenarios
reftable: fix quadratic behavior in the presence of tombstones
refs/reftable-backend.c | 54 ++++++++++++++++++++++------
reftable/stack.c | 1 -
t/perf/p1401-ref-store-tombstones.sh | 46 ++++++++++++++++++++++++
3 files changed, 89 insertions(+), 12 deletions(-)
create mode 100755 t/perf/p1401-ref-store-tombstones.sh
base-commit: f85a7e662054a7b0d9070e432508831afa214b47
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2166%2Fspkrka%2Freftable-tombstone-perf-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2166/spkrka/reftable-tombstone-perf-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2166
Range-diff vs v1:
1: d8ffdcb4f8 ! 1: 889d0d38bc t: add tests for ref tombstone scenarios
@@ Metadata
Author: Kristofer Karlsson <krka@spotify.com>
## Commit message ##
- t: add tests for ref tombstone scenarios
+ t/perf: add perf test for ref tombstone scenarios
- Add a performance test and a correctness test for update-ref when
- many tombstones are present in a reftable.
+ Add performance tests for update-ref when many tombstones are present
+ in a reftable.
- The performance test (p1401) exercises two scenarios:
+ The first test exercises the scenario where all refs are deleted
+ (creating tombstones) and then re-created with the same names, which
+ currently exhibits quadratic behavior.
- - All refs are deleted (creating tombstones) and then re-created
- with the same names, which currently exhibits quadratic behavior.
-
- - An asymmetric variant where refs are deleted and then new,
- differently-named refs are created. When the tombstones sort
- after the new refs, every create scans all tombstones, making
- this case even worse than re-creating the same refs.
-
- The correctness test (t0610) verifies that refs deleted and then
- re-created with the same names are visible afterwards.
+ The second test uses a separate repository with an asymmetric variant
+ where refs are deleted and then new, differently-named refs are
+ created. When the tombstones sort after the new refs, every create
+ scans all tombstones, making this case even worse than re-creating
+ the same refs.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
@@ t/perf/p1401-ref-store-tombstones.sh (new)
+'
+
+test_expect_success "setup asymmetric" '
++ git init --ref-format=reftable repo2 &&
++ blob=$(echo foo | git -C repo2 hash-object -w --stdin) &&
+ for i in $(test_seq 8000)
+ do
+ printf "create refs/tags/old-%d %s\n" "$i" "$blob" ||
+ return 1
-+ done >repo/input-old &&
-+ sed "s/old-/new-/" <repo/input-old >repo/input-new &&
-+ git -C repo update-ref --stdin <repo/input-old &&
-+ git -C repo for-each-ref --format="delete %(refname)" |
-+ git -C repo update-ref --stdin
++ done >repo2/input-old &&
++ sed "s/old-/new-/" <repo2/input-old >repo2/input-new &&
++ git -C repo2 update-ref --stdin <repo2/input-old &&
++ git -C repo2 for-each-ref --format="delete %(refname)" |
++ git -C repo2 update-ref --stdin
+'
+
+test_perf "create new refs after deleting differently-named refs" '
-+ git -C repo update-ref --stdin <repo/input-new &&
-+ git -C repo for-each-ref --format="delete %(refname)" |
-+ git -C repo update-ref --stdin
++ git -C repo2 update-ref --stdin <repo2/input-new &&
++ git -C repo2 for-each-ref --format="delete %(refname)" refs/tags/ |
++ git -C repo2 update-ref --stdin
+'
+
+test_done
-
- ## t/t0610-reftable-basics.sh ##
-@@ t/t0610-reftable-basics.sh: test_expect_success 'writes do not persist peeled value for invalid tags' '
- )
- '
-
-+test_expect_success 'delete and re-create refs with tombstones' '
-+ test_when_finished "rm -rf repo" &&
-+ git init repo &&
-+ test_commit -C repo A &&
-+ A=$(git -C repo rev-parse HEAD) &&
-+ cat >input <<-EOF &&
-+ create refs/tags/a $A
-+ create refs/tags/b $A
-+ create refs/tags/c $A
-+ EOF
-+ git -C repo update-ref --stdin <input &&
-+
-+ # delete all tags, leaving tombstones
-+ git -C repo for-each-ref --format="delete %(refname)" refs/tags/ |
-+ git -C repo update-ref --stdin &&
-+
-+ # re-create the same refs and verify they are visible
-+ git -C repo update-ref --stdin <input &&
-+ git -C repo tag -l >actual &&
-+ test_line_count = 3 actual
-+'
-+
- test_done
2: 1459371d3a ! 2: c13f15ddc2 reftable: fix quadratic behavior when re-creating deleted refs
@@ Metadata
Author: Kristofer Karlsson <krka@spotify.com>
## Commit message ##
- reftable: fix quadratic behavior when re-creating deleted refs
+ reftable: fix quadratic behavior in the presence of tombstones
- When many refs are deleted and then re-created, update-ref exhibits
- quadratic behavior. With 8000 refs deleted and re-created, the
- runtime is ~15s, quadrupling for each doubling of input size.
+ When many tombstones are present in a reftable, operations that need
+ to look up or iterate over refs exhibit quadratic behavior. With
+ 8000 refs deleted and re-created, update-ref takes ~15s, quadrupling
+ for each doubling of input size.
The root cause is the merged iterator's suppress_deletions flag.
When set, merged_iter_next_void() silently consumes tombstone records
@@ Commit message
prefix or refname comparisons) until after all tombstones have been
scanned.
- This affects two code paths during ref creation:
+ This affects any code path that seeks into a range containing
+ tombstones, including:
- refs_verify_refnames_available() seeks to "refs/tags/foo-1/" to
check for D/F conflicts and must scan through all subsequent
@@ Commit message
found", because the merged iterator skips the matching tombstone
and searches for the next live record.
- Fix this by removing suppress_deletions from the merged iterator and
- instead handling deletion records at each call site in the reftable
- backend, where prefix and refname bounds are available. Tombstones
- are now returned to callers, which skip them after their existing
- bounds checks. This allows iteration to terminate as soon as a
- tombstone past the relevant bound is encountered.
+ Fix this by no longer setting suppress_deletions on the stack's
+ merged table and instead handling deletion records at each call site
+ in the reftable backend, where prefix and refname bounds are
+ available. Tombstones are now returned to callers, which skip them
+ after their existing bounds checks. This allows iteration to
+ terminate as soon as a tombstone past the relevant bound is
+ encountered.
+
+ The suppress_deletions flag and its logic in the merged iterator are
+ retained for downstream users of the reftable library (e.g. libgit2).
This also requires adding deletion checks to the log iteration paths,
since suppress_deletions applied to both ref and log iterators.
@@ refs/reftable-backend.c: static int reftable_be_fsck(struct ref_store *ref_store
case REFTABLE_REF_VAL2: {
struct object_id oid;
- ## reftable/merged.c ##
-@@ reftable/merged.c: struct merged_iter {
- struct merged_subiter *subiters;
- struct merged_iter_pqueue pq;
- size_t subiters_len;
-- int suppress_deletions;
- ssize_t advance_index;
- };
-
-@@ reftable/merged.c: static int merged_iter_seek_void(void *it, struct reftable_record *want)
-
- static int merged_iter_next_void(void *p, struct reftable_record *rec)
- {
-- struct merged_iter *mi = p;
-- while (1) {
-- int err = merged_iter_next_entry(mi, rec);
-- if (err)
-- return err;
-- if (mi->suppress_deletions && reftable_record_is_deletion(rec))
-- continue;
-- return 0;
-- }
-+ return merged_iter_next_entry(p, rec);
- }
-
- static struct reftable_iterator_vtable merged_iter_vtable = {
-@@ reftable/merged.c: int merged_table_init_iter(struct reftable_merged_table *mt,
- goto out;
- }
- mi->advance_index = -1;
-- mi->suppress_deletions = mt->suppress_deletions;
- mi->subiters = subiters;
- mi->subiters_len = mt->tables_len;
-
-
- ## reftable/merged.h ##
-@@ reftable/merged.h: struct reftable_merged_table {
- size_t tables_len;
- enum reftable_hash hash_id;
-
-- /* If unset, produce deletions. This is useful for compaction. For the
-- * full stack, deletions should be produced. */
-- int suppress_deletions;
--
- uint64_t min;
- uint64_t max;
- };
-
## reftable/stack.c ##
@@ reftable/stack.c: static int reftable_stack_reload_once(struct reftable_stack *st,
/* Update the stack to point to the new tables. */
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 1/2] t/perf: add perf test for ref tombstone scenarios
From: Kristofer Karlsson via GitGitGadget @ 2026-07-09 12:08 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2166.v2.git.1783598912.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Add performance tests for update-ref when many tombstones are present
in a reftable.
The first test exercises the scenario where all refs are deleted
(creating tombstones) and then re-created with the same names, which
currently exhibits quadratic behavior.
The second test uses a separate repository with an asymmetric variant
where refs are deleted and then new, differently-named refs are
created. When the tombstones sort after the new refs, every create
scans all tombstones, making this case even worse than re-creating
the same refs.
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
t/perf/p1401-ref-store-tombstones.sh | 46 ++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
create mode 100755 t/perf/p1401-ref-store-tombstones.sh
diff --git a/t/perf/p1401-ref-store-tombstones.sh b/t/perf/p1401-ref-store-tombstones.sh
new file mode 100755
index 0000000000..9e3d8031aa
--- /dev/null
+++ b/t/perf/p1401-ref-store-tombstones.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+
+test_description="Tests performance of ref operations with many tombstones"
+
+. ./perf-lib.sh
+
+test_expect_success "setup" '
+ git init --ref-format=reftable repo &&
+ blob=$(echo foo | git -C repo hash-object -w --stdin) &&
+ for i in $(test_seq 8000)
+ do
+ printf "create refs/tags/tag-%d %s\n" "$i" "$blob" ||
+ return 1
+ done >repo/input &&
+ git -C repo update-ref --stdin <repo/input &&
+ git -C repo for-each-ref --format="delete %(refname)" |
+ git -C repo update-ref --stdin
+'
+
+test_perf "recreate refs after mass delete" '
+ git -C repo update-ref --stdin <repo/input &&
+ git -C repo for-each-ref --format="delete %(refname)" |
+ git -C repo update-ref --stdin
+'
+
+test_expect_success "setup asymmetric" '
+ git init --ref-format=reftable repo2 &&
+ blob=$(echo foo | git -C repo2 hash-object -w --stdin) &&
+ for i in $(test_seq 8000)
+ do
+ printf "create refs/tags/old-%d %s\n" "$i" "$blob" ||
+ return 1
+ done >repo2/input-old &&
+ sed "s/old-/new-/" <repo2/input-old >repo2/input-new &&
+ git -C repo2 update-ref --stdin <repo2/input-old &&
+ git -C repo2 for-each-ref --format="delete %(refname)" |
+ git -C repo2 update-ref --stdin
+'
+
+test_perf "create new refs after deleting differently-named refs" '
+ git -C repo2 update-ref --stdin <repo2/input-new &&
+ git -C repo2 for-each-ref --format="delete %(refname)" refs/tags/ |
+ git -C repo2 update-ref --stdin
+'
+
+test_done
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/2] reftable: fix quadratic behavior in the presence of tombstones
From: Kristofer Karlsson via GitGitGadget @ 2026-07-09 12:08 UTC (permalink / raw)
To: git; +Cc: Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2166.v2.git.1783598912.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
When many tombstones are present in a reftable, operations that need
to look up or iterate over refs exhibit quadratic behavior. With
8000 refs deleted and re-created, update-ref takes ~15s, quadrupling
for each doubling of input size.
The root cause is the merged iterator's suppress_deletions flag.
When set, merged_iter_next_void() silently consumes tombstone records
in a tight internal loop before returning to the caller. This
prevents higher-level code from checking iteration bounds (such as
prefix or refname comparisons) until after all tombstones have been
scanned.
This affects any code path that seeks into a range containing
tombstones, including:
- refs_verify_refnames_available() seeks to "refs/tags/foo-1/" to
check for D/F conflicts and must scan through all subsequent
tombstones before the caller can see that they are past the prefix
of interest.
- reftable_backend_read_ref() seeks to a specific refname and must
scan through all subsequent tombstones before returning "not
found", because the merged iterator skips the matching tombstone
and searches for the next live record.
Fix this by no longer setting suppress_deletions on the stack's
merged table and instead handling deletion records at each call site
in the reftable backend, where prefix and refname bounds are
available. Tombstones are now returned to callers, which skip them
after their existing bounds checks. This allows iteration to
terminate as soon as a tombstone past the relevant bound is
encountered.
The suppress_deletions flag and its logic in the merged iterator are
retained for downstream users of the reftable library (e.g. libgit2).
This also requires adding deletion checks to the log iteration paths,
since suppress_deletions applied to both ref and log iterators.
Both tests in p1401 go from ~14s to ~0.2s with this change.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
refs/reftable-backend.c | 54 ++++++++++++++++++++++++++++++++---------
reftable/stack.c | 1 -
2 files changed, 43 insertions(+), 12 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 212408c769..028f0211af 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -84,7 +84,8 @@ static int reftable_backend_read_ref(struct reftable_backend *be,
if (ret)
goto done;
- if (strcmp(ref.refname, refname)) {
+ if (strcmp(ref.refname, refname) ||
+ reftable_ref_record_is_deletion(&ref)) {
ret = 1;
goto done;
}
@@ -110,7 +111,6 @@ static int reftable_backend_read_ref(struct reftable_backend *be,
oidread(oid, reftable_ref_record_val1(&ref),
&hash_algos[hash_id]);
} else {
- /* We got a tombstone, which should not happen. */
BUG("unhandled reference value type %d", ref.value_type);
}
@@ -652,6 +652,9 @@ static int reftable_ref_iterator_advance(struct ref_iterator *ref_iterator)
break;
}
+ if (iter->ref.value_type == REFTABLE_REF_DELETION)
+ continue;
+
if (iter->exclude_patterns && should_exclude_current_ref(iter))
continue;
@@ -1532,6 +1535,8 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&log))
+ continue;
ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
tombstone = &logs[logs_nr++];
@@ -1929,6 +1934,8 @@ static int write_copy_table(struct reftable_writer *writer, void *cb_data)
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&old_log))
+ continue;
free(old_log.refname);
@@ -2061,6 +2068,9 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)
if (iter->err)
break;
+ if (reftable_log_record_is_deletion(&iter->log))
+ continue;
+
/*
* We want the refnames that we have reflogs for, so we skip if
* we've already produced this name. This could be faster by
@@ -2220,6 +2230,8 @@ static int reftable_be_for_each_reflog_ent_reverse(struct ref_store *ref_store,
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&log))
+ continue;
ret = yield_log_record(refs, &log, fn, cb_data);
if (ret)
@@ -2272,6 +2284,10 @@ static int reftable_be_for_each_reflog_ent(struct ref_store *ref_store,
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&log)) {
+ reftable_log_record_release(&log);
+ continue;
+ }
ALLOC_GROW(logs, logs_nr + 1, logs_alloc);
logs[logs_nr++] = log;
@@ -2318,18 +2334,26 @@ static int reftable_be_reflog_exists(struct ref_store *ref_store,
goto done;
/*
- * Check whether we get at least one log record for the given ref name.
- * If so, the reflog exists, otherwise it doesn't.
+ * Check whether we get at least one non-deleted log record for the
+ * given ref name. If so, the reflog exists, otherwise it doesn't.
*/
- ret = reftable_iterator_next_log(&it, &log);
- if (ret < 0)
- goto done;
- if (ret > 0) {
- ret = 0;
- goto done;
+ while (1) {
+ ret = reftable_iterator_next_log(&it, &log);
+ if (ret < 0)
+ goto done;
+ if (ret > 0) {
+ ret = 0;
+ goto done;
+ }
+ if (strcmp(log.refname, refname)) {
+ ret = 0;
+ goto done;
+ }
+ if (!reftable_log_record_is_deletion(&log))
+ break;
}
- ret = strcmp(log.refname, refname) == 0;
+ ret = 1;
done:
reftable_iterator_destroy(&it);
@@ -2442,6 +2466,8 @@ static int write_reflog_delete_table(struct reftable_writer *writer, void *cb_da
ret = 0;
break;
}
+ if (reftable_log_record_is_deletion(&log))
+ continue;
tombstone.refname = (char *)arg->refname;
tombstone.value_type = REFTABLE_LOG_DELETION;
@@ -2625,6 +2651,10 @@ static int reftable_be_reflog_expire(struct ref_store *ref_store,
reftable_log_record_release(&log);
break;
}
+ if (reftable_log_record_is_deletion(&log)) {
+ reftable_log_record_release(&log);
+ continue;
+ }
oidread(&old_oid, log.value.update.old_hash,
ref_store->repo->hash_algo);
@@ -2791,6 +2821,8 @@ static int reftable_be_fsck(struct ref_store *ref_store, struct fsck_options *o,
report.path = refname.buf;
switch (ref.value_type) {
+ case REFTABLE_REF_DELETION:
+ continue;
case REFTABLE_REF_VAL1:
case REFTABLE_REF_VAL2: {
struct object_id oid;
diff --git a/reftable/stack.c b/reftable/stack.c
index ab12926708..fd7d8f3f1e 100644
--- a/reftable/stack.c
+++ b/reftable/stack.c
@@ -337,7 +337,6 @@ static int reftable_stack_reload_once(struct reftable_stack *st,
/* Update the stack to point to the new tables. */
if (st->merged)
reftable_merged_table_free(st->merged);
- new_merged->suppress_deletions = 1;
st->merged = new_merged;
if (st->tables)
--
gitgitgadget
^ permalink raw reply related
* Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite [and 1 more messages]
From: Phillip Wood @ 2026-07-09 13:19 UTC (permalink / raw)
To: Ian Jackson, Colin Stagner; +Cc: git, Johannes Schindelin
In-Reply-To: <27215.27575.968985.583226@chiark.greenend.org.uk>
Hi Ian
On 09/07/2026 10:36, Ian Jackson wrote:
>
> Colin Stagner writes ("Re: [PATCH 2/2] git-subtree: Bail out if we find output from Rust rewrite (test)"):
>> It may be slightly faster to create only one repo and just make orphan
>> branches, like `test_create_subtree_add()` does.
> ...
>> `test_commit()` from test-lib-functions.sh may be superior to manually
>> writing and committing this file.
>
> Thanks for the suggestions. I'll take a look.
I think
test_commit --no-tag sabotage .git-subtree/config "# sabotage"
is the equivalent of what you have in the test at the moment
> TBH I found this test framework quite awkward to work with. Maybe
> folks here have some tips:
>
> One thing I was missing was a primitive for "check this fails *and
> produces an error message matching this regexp*". test_must_fail
> makes it easy for a slips in the command (or some kinds of regression)
> to go undetected: the test then passes because the command *does* fail
> with a usage error or whatever. And AFAICT there isn't a way to
> manually inspect the output when the tests pass? I resorted to
> sabotaging the test by adding `&& false` to the end of the shell
> snippet string, and eyeballing t/test-results/t7900-subtree.out.
The usual approach to checking that a command fails for the expected
reason is
test_must_fail git ... 2>err &&
test_grep regexp err
which prints the contents of err if it does not match regexp. To see the
output of the tests run them with "-v". I frequently use "-v -i -x" to
debug test failures. "-i" stops the test run at the first failure so you
can inspect the test repository and "-x" turns on tracing so you can see
which command failed which is useful when I test has not been written
with debugging in mind.
Thanks
Phillip
> Colin Stagner writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite"):
>>> +reject_if_v2_config () {
>>> + local config=.git-subtree/config
>>
>> This is a nit, but `local` is not specified by POSIX. I know it is used
>> elsewhere within git-subtree, but it is specifically discouraged.
>
> There are 7 existing uses of `local`. I think I prefer to use it here
> too. In practice I think there are no shells we might want to use
> that don't have local. The alternative is to change all the variable
> names to be obviously globally unique, which is clumsy and also seems
> to me to put us at greater risk of bugs.
>
>>> + if git rev-parse --verify -q "$rev:$config"; then
>>
>> For subtree split, should we also test for this file in tree you are
>> splitting: i.e., "$dir/$config"? The answer might be no.
>
> You're right that we should consider this question. The answer is:
> no, we should not. Briefly, whether to use the new or old algorithms
> depends on whether the downstream has adopted the new git-subtree, not
> on whether the upstream has added some optional config.
>
> https://codeberg.org/diziet/git-subtree/src/branch/main/DATA-MODEL.md#control-of-unmarked-subtree-merges-guessing-config
>
>> I think that subtree merge should only test the top-level project, as
>> this patch does now.
>
> By "top-level" I think you mean what I've taken to calling the
> "downstream": the project where the subtree is in a subdir, and whose
> top-level has other stuff. In which case I agree.
>
>> On 7/6/26 06:58, Ian Jackson wrote:
>>> Another, bigger, reason is that current git-subtree generates unmarked
>>> subtree merges (ie, without any git-subtree trailers)
>>
>> Subtree merges can be performed without git-subtree, via the `-X
>> subtree` merge strategy option. While the design of RIIR git-subtree is
>> outside the scope of this patch series, this may be worth thinking about
>> in your rewrite.
>
> This is what I'm calling an "unmarked subtree merge". My rewrite is
> not going to support this user behaviour. The problem is that it is
> not possible to reliably determine whetheer something is an unmarked
> subtree merge.
>
> It is possible to guess based on tree similarity, but that's a
> heuristic. It's also possible to guess based on root commits.
> Both of these approaches can go wrong in some cases. I prefer to
> write reliable software, which doesn't guess.
>
> I'll advise against this practice in the documentation, but I'm
> reasonably confident that if a user does this anyway the results won't
> be terrible. The upstream input to an unmarked subtree merge in a
> downstream that has already used my rewrite, will be treated as if it
> were a downstream branch that predates the subtree addition. The
> effect on split (in most cases) is a missing parent relationship,
> which is undesirable but not catastrophic.I've made a note to add a
> test case for this scenario.
>
> Combining manual -X subtree merges with git-subtree --squash merges
> could easily produce quite weird and wrong results in the tree (even
> before anyone tries split, or something). I don't think I can even
> reliably detect this situation after the user has done it, and of
> course since that user is using plain git, I certainly can't prevent
> it. This is another reason why manual use of -X subtree should be
> discouraged.
>
> Regards,
> Ian.
>
^ permalink raw reply
* Re: [PATCH 2/2] commit-graph: propagate topo_levels slab to all chain layers
From: Patrick Steinhardt @ 2026-07-09 13:43 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
In-Reply-To: <f9c1482a76493520b948a2e918de7a5481fa1043.1783418384.git.gitgitgadget@gmail.com>
On Tue, Jul 07, 2026 at 09:59:43AM +0000, Kristofer Karlsson via GitGitGadget wrote:
> diff --git a/commit-graph.c b/commit-graph.c
> index 4e39a048c4..c2a711cceb 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -2610,7 +2610,7 @@ int write_commit_graph(struct odb_source *source,
>
> g = prepare_commit_graph(ctx.r);
> for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
> - g->topo_levels = &topo_levels;
> + chain->topo_levels = &topo_levels;
>
> if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
> ctx.changed_paths = 1;
Oops, that's an embarrassing bug indeed. Thanks for finding and fixing
it!
> diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
> index f9c57760f4..9e5ab7dbd0 100755
> --- a/t/t5324-split-commit-graph.sh
> +++ b/t/t5324-split-commit-graph.sh
> @@ -738,11 +738,7 @@ test_expect_success 'incremental write reads topo levels from all layers' '
> GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
> git commit-graph write --reachable --split=no-merge &&
>
> - # BUG: topo levels from lower graph layers are not
> - # propagated, so the DFS re-walks from base-3 down to
> - # the root (7 steps) instead of reading topo levels
> - # from the existing graph (1 step).
> - test_trace2_data commit-graph generation-dfs-steps 7 <trace.txt
> + test_trace2_data commit-graph generation-dfs-steps 1 <trace.txt
> )
> '
Makes sense.
Patrick
^ permalink raw reply
* Re: [PATCH v2 2/2] reftable: fix quadratic behavior in the presence of tombstones
From: Patrick Steinhardt @ 2026-07-09 13:53 UTC (permalink / raw)
To: Kristofer Karlsson via GitGitGadget; +Cc: git, Kristofer Karlsson
In-Reply-To: <c13f15ddc20f721443fa1d462ea1b7c2356fbffc.1783598912.git.gitgitgadget@gmail.com>
On Thu, Jul 09, 2026 at 12:08:31PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> diff --git a/reftable/stack.c b/reftable/stack.c
> index ab12926708..fd7d8f3f1e 100644
> --- a/reftable/stack.c
> +++ b/reftable/stack.c
> @@ -337,7 +337,6 @@ static int reftable_stack_reload_once(struct reftable_stack *st,
> /* Update the stack to point to the new tables. */
> if (st->merged)
> reftable_merged_table_free(st->merged);
> - new_merged->suppress_deletions = 1;
> st->merged = new_merged;
>
> if (st->tables)
Okay, we still retain the field after this patch. But the question is:
how would libgit2 now set it? I think we should rather extend the
`struct reftable_stack_options` so that the caller can control whether
or not to suppress deletions at stack creation time.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v3 06/11] odb/transaction: propagate begin errors
From: Justin Tobler @ 2026-07-09 14:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, ps
In-Reply-To: <xmqqjyr4rg78.fsf@gitster.g>
On 26/07/08 08:32PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
>
> > When `odb_transaction_begin()` is invoked, the function returns the
> > transaction pointer directly. There is no way for the backend to
> > signal that it failed to set up its state, such as when creating the
> > temporary object directory backing the transaction.
> >
> > In a subsequent commit, git-receive-pack(1) starts using ODB
> > transactions and needs to be able to report such failures rather
> > than silently ignore them. Refactor `odb_transaction_begin()` to
> > return an int error code and write the resulting transaction into an
> > out parameter. Also introduce `odb_transaction_begin_or_die()` as a
> > convenience for callsites that do not need to handle errors
> > explicitly.
> >
> > Note that `odb_transaction_begin()` now returns an error when the ODB
> > already has an inflight transaction pending. ODB transaction call sites
> > that may encounter an inflight transaction are updated to explicitly
> > handle this case.
> >
> > Signed-off-by: Justin Tobler <jltobler@gmail.com>
> > ---
> > ...
> > diff --git a/odb/transaction.c b/odb/transaction.c
> > index b16e07aebf..a5fba7f908 100644
> > --- a/odb/transaction.c
> > +++ b/odb/transaction.c
> > @@ -1,15 +1,20 @@
> > #include "git-compat-util.h"
> > +#include "gettext.h"
> > #include "odb/source.h"
> > #include "odb/transaction.h"
> >
> > -struct odb_transaction *odb_transaction_begin(struct object_database *odb)
> > +int odb_transaction_begin(struct object_database *odb,
> > + struct odb_transaction **out)
> > {
> > + int ret;
> > +
> > if (odb->transaction)
> > - return NULL;
> > + return error(_("object database transaction already pending"));
> >
> > - odb_source_begin_transaction(odb->sources, &odb->transaction);
> > + ret = odb_source_begin_transaction(odb->sources, out);
> > + odb->transaction = *out;
>
> Can odb_source_begin_transaction() ever fail? If so, and when it
> fails, would *out be left untouched?
In this patch there it not yet a way for it to fail, but it can return
an error later in the series. When an error is encountered though, *out
_is_ left untouched.
> I am wondering if we want
>
> if (!(ret = odb_source_begin_transaction(odb->sources, out)))
> odb->transaction = *out;
>
> or something like that.
I think it is a good idea to for `odb_transaction_begin()` to ensure the
repository transaction is only set on success though. Will update in the
next version. Thanks
-Justin
^ permalink raw reply
* Re: [PATCH v2 2/2] reftable: fix quadratic behavior in the presence of tombstones
From: Kristofer Karlsson @ 2026-07-09 14:48 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <ak-n6K4heV2kHviZ@pks.im>
On Thu, 9 Jul 2026 at 15:53, Patrick Steinhardt <ps@pks.im> wrote:
>
> On Thu, Jul 09, 2026 at 12:08:31PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> > diff --git a/reftable/stack.c b/reftable/stack.c
> > index ab12926708..fd7d8f3f1e 100644
> > --- a/reftable/stack.c
> > +++ b/reftable/stack.c
> > @@ -337,7 +337,6 @@ static int reftable_stack_reload_once(struct reftable_stack *st,
> > /* Update the stack to point to the new tables. */
> > if (st->merged)
> > reftable_merged_table_free(st->merged);
> > - new_merged->suppress_deletions = 1;
> > st->merged = new_merged;
> >
> > if (st->tables)
>
> Okay, we still retain the field after this patch. But the question is:
> how would libgit2 now set it? I think we should rather extend the
> `struct reftable_stack_options` so that the caller can control whether
> or not to suppress deletions at stack creation time.
You are right, I (still) missed the compatibility problem here.
I started thinking about a way to make it fully backwards compatible,
but then I looked at the libgit2 repo and realized it will need
updating anyway since it predates the reftable_stack_options split.
I will add suppress_deletions to reftable_stack_options as you
suggested.
Thanks,
Kristofer
^ permalink raw reply
* Re: [PATCH v2 2/2] reftable: fix quadratic behavior in the presence of tombstones
From: Patrick Steinhardt @ 2026-07-09 14:54 UTC (permalink / raw)
To: Kristofer Karlsson; +Cc: Kristofer Karlsson via GitGitGadget, git
In-Reply-To: <CAL71e4PrtZwB8TMg3eBj=LzC7ik+C8yxLYEEEP7SDgMPiWSs0Q@mail.gmail.com>
On Thu, Jul 09, 2026 at 04:48:43PM +0200, Kristofer Karlsson wrote:
> On Thu, 9 Jul 2026 at 15:53, Patrick Steinhardt <ps@pks.im> wrote:
> >
> > On Thu, Jul 09, 2026 at 12:08:31PM +0000, Kristofer Karlsson via GitGitGadget wrote:
> > > diff --git a/reftable/stack.c b/reftable/stack.c
> > > index ab12926708..fd7d8f3f1e 100644
> > > --- a/reftable/stack.c
> > > +++ b/reftable/stack.c
> > > @@ -337,7 +337,6 @@ static int reftable_stack_reload_once(struct reftable_stack *st,
> > > /* Update the stack to point to the new tables. */
> > > if (st->merged)
> > > reftable_merged_table_free(st->merged);
> > > - new_merged->suppress_deletions = 1;
> > > st->merged = new_merged;
> > >
> > > if (st->tables)
> >
> > Okay, we still retain the field after this patch. But the question is:
> > how would libgit2 now set it? I think we should rather extend the
> > `struct reftable_stack_options` so that the caller can control whether
> > or not to suppress deletions at stack creation time.
>
> You are right, I (still) missed the compatibility problem here.
>
> I started thinking about a way to make it fully backwards compatible,
> but then I looked at the libgit2 repo and realized it will need
> updating anyway since it predates the reftable_stack_options split.
Yeah, that's something I'll handle soon(ish).
> I will add suppress_deletions to reftable_stack_options as you
> suggested.
Thanks!
Patrick
^ permalink raw reply
* Re: [PATCH v3 08/11] odb/transaction: add transaction env interface
From: Justin Tobler @ 2026-07-09 15:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, ps
In-Reply-To: <xmqqbjcgrg0e.fsf@gitster.g>
On 26/07/08 08:36PM, Junio C Hamano wrote:
> Justin Tobler <jltobler@gmail.com> writes:
>
> > +static int odb_transaction_files_env(struct odb_transaction *base,
> > + struct strvec *env)
> > +{
> > + struct odb_transaction_files *transaction =
> > + container_of(base, struct odb_transaction_files, base);
> > +
> > + odb_transaction_files_prepare(&transaction->base);
>
> Can this fail? The caller of us would not notice that something
> went wrong, and ...
>
> > + strvec_pushv(env, tmp_objdir_env(transaction->objdir));
>
> ... happily ends up using transaction->objdir that may not be
> appropriate for it to use if it fails, no?
Ya, `odb_transaction_files_prepare()` can fail here. In practice,
failure results in no temporary directory being created which
`tmp_objdir_env()` does handle gracefully, but we should ideally still
be reported the failure back to callers. Will update in the next
version.
-Justin
^ permalink raw reply
* [PATCH v2 0/2] commit-graph: fix topo_levels slab propagation regression
From: Kristofer Karlsson via GitGitGadget @ 2026-07-09 15:02 UTC (permalink / raw)
To: git; +Cc: Taylor Blau, Kristofer Karlsson, Patrick Steinhardt,
Kristofer Karlsson
In-Reply-To: <pull.2170.git.1783418384.gitgitgadget@gmail.com>
When fetch.writeCommitGraph is enabled (or git maintenance runs after
fetch), an incremental commit-graph write computes generation numbers for
the newly added commits. For commits already in the graph, their topo levels
should be read from the existing layers, making the DFS proportional to the
number of new commits.
199d452758 (commit-graph: return the prepared commit graph from
prepare_commit_graph(), 2025-09-04), part of the ps/commit-graph-via-source
series [1], refactored the loop that propagates the topo_levels slab to each
layer of the commit-graph chain. The original code used a single variable
that advanced through the chain:
while (g) {
g->topo_levels = &topo_levels;
g = g->base_graph;
}
The refactored code introduced a separate iteration variable but did not
update the loop body to match:
for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
g->topo_levels = &topo_levels;
This always assigns to the topmost layer instead of the current one. Commits
from lower layers appear to have no generation numbers, so the DFS re-walks
the entire ancestry.
On a repo with a multi-layer split commit-graph, an incremental commit-graph
write triggered by git fetch drops from ~3.5 seconds to ~0.2 seconds after
the fix.
[1]
https://lore.kernel.org/git/aMNTELw0Wk8jWoPc@nand.local/T/#mb55b5f0e1ccf82d969ac1d8144c56ecf87b833e8
Changes since v1:
* Fixed wrong commit title and date in the reference (Junio, Taylor).
* use test_expect_failure with the correct assertion instead of a # BUG
comment (Taylor).
* Simplified commit messages.
Kristofer Karlsson (2):
commit-graph: add trace2 instrumentation for generation DFS
commit-graph: propagate topo_levels slab to all chain layers
commit-graph.c | 7 ++++++-
t/t5324-split-commit-graph.sh | 24 ++++++++++++++++++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
base-commit: f85a7e662054a7b0d9070e432508831afa214b47
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2170%2Fspkrka%2Fkrka%2Ffix-topo-levels-slab-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2170/spkrka/krka/fix-topo-levels-slab-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2170
Range-diff vs v1:
1: b865c2bcff ! 1: 100efa22a9 commit-graph: add trace2 instrumentation for generation DFS
@@ Metadata
## Commit message ##
commit-graph: add trace2 instrumentation for generation DFS
- Add a step counter and trace2_data_intmax call to
- compute_reachable_generation_numbers() to make the cost of
- the generation number DFS observable. This exposes a
- regression introduced in 199d452758 (commit-graph: fix
- "filling in" topological levels, 2025-04-07) where
- incremental commit-graph writes re-walk the entire commit
- ancestry instead of reading topo levels from lower graph
- layers.
+ Count the number of steps taken in
+ compute_reachable_generation_numbers() and expose it via
+ trace2 to make it easier to detect performance regressions.
- Add a test that demonstrates the problem: with a two-layer
- split commit-graph, writing a new incremental layer for a
- commit whose parent is in the base layer walks all the way
- down to the root (7 steps for 5 base commits) instead of
- reading the existing topo level and stopping immediately
- (1 step).
+ Add a failing test for such a regression, introduced in
+ 199d452758 (commit-graph: return the prepared commit graph
+ from `prepare_commit_graph()`, 2025-09-04), where incremental
+ commit-graph writes do not see existing generation numbers
+ from lower graph layers and fall back to walking the full
+ ancestry.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
@@ t/t5324-split-commit-graph.sh: test_expect_success 'write generation data chunk
)
'
-+test_expect_success 'incremental write reads topo levels from all layers' '
++test_expect_failure 'incremental write reads topo levels from all layers' '
+ git init topo-from-lower &&
+ (
+ cd topo-from-lower &&
@@ t/t5324-split-commit-graph.sh: test_expect_success 'write generation data chunk
+ GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
+ git commit-graph write --reachable --split=no-merge &&
+
-+ # BUG: topo levels from lower graph layers are not
-+ # propagated, so the DFS re-walks from base-3 down to
-+ # the root (7 steps) instead of reading topo levels
-+ # from the existing graph (1 step).
-+ test_trace2_data commit-graph generation-dfs-steps 7 <trace.txt
++ test_trace2_data commit-graph generation-dfs-steps 1 <trace.txt
+ )
+'
+
2: f9c1482a76 ! 2: 679dd2e392 commit-graph: propagate topo_levels slab to all chain layers
@@ Metadata
## Commit message ##
commit-graph: propagate topo_levels slab to all chain layers
- Fix a regression introduced in 199d452758 (commit-graph: fix
- "filling in" topological levels, 2025-04-07) where the loop
- propagating the topo_levels slab to each layer of the
- commit-graph chain always assigned to `g->topo_levels`
- (the topmost layer) instead of `chain->topo_levels` (the
- current iteration variable).
+ The topo_levels slab is only propagated to the topmost graph
+ layer instead of all layers in the chain. Commits from lower
+ layers appear to have no generation numbers, so the DFS
+ re-walks the entire ancestry.
- This meant only the topmost layer had its topo_levels pointer
- set. When compute_reachable_generation_numbers() ran for an
- incremental write, commits parsed from lower layers had their
- topo levels left at zero in the slab, since
- fill_commit_graph_info() could not store them without the
- pointer. The DFS then re-walked the entire commit ancestry
- instead of stopping at commits with known levels.
-
- On a repository with 2.78M commits and a multi-layer split
- commit-graph, this caused a single incremental commit-graph
- write to spend ~3.7 seconds in the generation DFS instead of
- microseconds.
+ Fix by making topo_levels visible to all layers, not just
+ the first one.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
@@ commit-graph.c: int write_commit_graph(struct odb_source *source,
ctx.changed_paths = 1;
## t/t5324-split-commit-graph.sh ##
-@@ t/t5324-split-commit-graph.sh: test_expect_success 'incremental write reads topo levels from all layers' '
- GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
- git commit-graph write --reachable --split=no-merge &&
-
-- # BUG: topo levels from lower graph layers are not
-- # propagated, so the DFS re-walks from base-3 down to
-- # the root (7 steps) instead of reading topo levels
-- # from the existing graph (1 step).
-- test_trace2_data commit-graph generation-dfs-steps 7 <trace.txt
-+ test_trace2_data commit-graph generation-dfs-steps 1 <trace.txt
+@@ t/t5324-split-commit-graph.sh: test_expect_success 'write generation data chunk when commit-graph chain is repl
)
'
+-test_expect_failure 'incremental write reads topo levels from all layers' '
++test_expect_success 'incremental write reads topo levels from all layers' '
+ git init topo-from-lower &&
+ (
+ cd topo-from-lower &&
--
gitgitgadget
^ permalink raw reply
* [PATCH v2 1/2] commit-graph: add trace2 instrumentation for generation DFS
From: Kristofer Karlsson via GitGitGadget @ 2026-07-09 15:03 UTC (permalink / raw)
To: git
Cc: Taylor Blau, Kristofer Karlsson, Patrick Steinhardt,
Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2170.v2.git.1783609382.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
Count the number of steps taken in
compute_reachable_generation_numbers() and expose it via
trace2 to make it easier to detect performance regressions.
Add a failing test for such a regression, introduced in
199d452758 (commit-graph: return the prepared commit graph
from `prepare_commit_graph()`, 2025-09-04), where incremental
commit-graph writes do not see existing generation numbers
from lower graph layers and fall back to walking the full
ancestry.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-graph.c | 5 +++++
t/t5324-split-commit-graph.sh | 24 ++++++++++++++++++++++++
2 files changed, 29 insertions(+)
diff --git a/commit-graph.c b/commit-graph.c
index c6d9c5c740..702ba9731b 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1653,6 +1653,7 @@ static void compute_reachable_generation_numbers(
{
int i;
struct commit_list *list = NULL;
+ intmax_t steps = 0;
for (i = 0; i < info->commits->nr; i++) {
struct commit *c = info->commits->items[i];
@@ -1671,6 +1672,7 @@ static void compute_reachable_generation_numbers(
int all_parents_computed = 1;
timestamp_t max_gen = 0;
+ steps++;
for (parent = current->parents; parent; parent = parent->next) {
repo_parse_commit(info->r, parent->item);
gen = info->get_generation(parent->item, info->data);
@@ -1694,6 +1696,9 @@ static void compute_reachable_generation_numbers(
}
}
}
+
+ trace2_data_intmax("commit-graph", info->r,
+ "generation-dfs-steps", steps);
}
static timestamp_t get_topo_level(struct commit *c, void *data)
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index 49a057cc2e..b41331e3dd 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -718,6 +718,30 @@ test_expect_success 'write generation data chunk when commit-graph chain is repl
)
'
+test_expect_failure 'incremental write reads topo levels from all layers' '
+ git init topo-from-lower &&
+ (
+ cd topo-from-lower &&
+
+ for i in $(test_seq 5)
+ do
+ test_commit base-$i || return 1
+ done &&
+ git commit-graph write --reachable &&
+
+ test_commit extra &&
+ git commit-graph write --reachable --split=no-merge &&
+
+ git checkout base-3 &&
+ test_commit new-branch &&
+
+ GIT_TRACE2_EVENT="$(pwd)/trace.txt" \
+ git commit-graph write --reachable --split=no-merge &&
+
+ test_trace2_data commit-graph generation-dfs-steps 1 <trace.txt
+ )
+'
+
test_expect_success 'temporary graph layer is discarded upon failure' '
git init layer-discard &&
(
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 2/2] commit-graph: propagate topo_levels slab to all chain layers
From: Kristofer Karlsson via GitGitGadget @ 2026-07-09 15:03 UTC (permalink / raw)
To: git
Cc: Taylor Blau, Kristofer Karlsson, Patrick Steinhardt,
Kristofer Karlsson, Kristofer Karlsson
In-Reply-To: <pull.2170.v2.git.1783609382.gitgitgadget@gmail.com>
From: Kristofer Karlsson <krka@spotify.com>
The topo_levels slab is only propagated to the topmost graph
layer instead of all layers in the chain. Commits from lower
layers appear to have no generation numbers, so the DFS
re-walks the entire ancestry.
Fix by making topo_levels visible to all layers, not just
the first one.
Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
commit-graph.c | 2 +-
t/t5324-split-commit-graph.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/commit-graph.c b/commit-graph.c
index 702ba9731b..a0bca248ac 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -2610,7 +2610,7 @@ int write_commit_graph(struct odb_source *source,
g = prepare_commit_graph(ctx.r);
for (struct commit_graph *chain = g; chain; chain = chain->base_graph)
- g->topo_levels = &topo_levels;
+ chain->topo_levels = &topo_levels;
if (flags & COMMIT_GRAPH_WRITE_BLOOM_FILTERS)
ctx.changed_paths = 1;
diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh
index b41331e3dd..9e5ab7dbd0 100755
--- a/t/t5324-split-commit-graph.sh
+++ b/t/t5324-split-commit-graph.sh
@@ -718,7 +718,7 @@ test_expect_success 'write generation data chunk when commit-graph chain is repl
)
'
-test_expect_failure 'incremental write reads topo levels from all layers' '
+test_expect_success 'incremental write reads topo levels from all layers' '
git init topo-from-lower &&
(
cd topo-from-lower &&
--
gitgitgadget
^ permalink raw reply related
* [PATCH v9 0/9] migrate more variables into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git; +Cc: cirnovskyv, szeder.dev, Tian Yuchen
In-Reply-To: <20260708160300.8852-1-cat@malon.dev>
Hi everyone,
This patch series continues the ongoing libification effort by migrating
a batch of global configuration variables into struct repo_config_values.
What does this series do:
infrastructure & strings (commits 1-6):
Introduce 'repo_config_values_clear()' to manage the lifecycle
of heap-allocated configuration strings. This infrastructure is utilized
to migrate string variables, including 'excludes_file', 'apply' whitespace
configs, and external programs including 'editor', 'pager', 'askpass'.
enums (commits 7-9):
Migrate enumerations 'push_default', 'autorebase', and
'object_creation_mode'. Care was taken to make these types available
to the configuration structure without triggering circular header
dependencies.
RFC:
Commit 3~5. Is it really necessary to migrate _program variables?
https://lore.kernel.org/git/8e657184-ee0b-453a-9f2d-a98080d3582e@gmail.com/
Commit 6~9. Previous related discussions on 'git_branch_track'.
https://lore.kernel.org/git/CAD=f0L-mPX+KECUjXk-WBzEbTP7wCa8sB56GySQT0yh9mfUOWw@mail.gmail.com/
Note:
Since a new getter 'repo_excludes_file()' is introduced, as previously
promised, once it is finally merged into 'master', there will be a patch to
update and squash the comments.
Similarly, I've noticed that the classification and sorting of variables in
'repo_config_values' don't seem to be correct. There will also be a patch
to fix this, and I think it will form a commit series along with the comment
patch?
Change since v8:
Fixed a memory leak in pager.c.
Thanks!
Tian Yuchen (9):
repository: introduce repo_config_values_clear()
environment: move excludes_file into repo_config_values
environment: move editor_program into repo_config_values
environment: move pager_program into repo_config_values
environment: move askpass_program into repo_config_values
environment: migrate apply_default_whitespace and
apply_default_ignorewhitespace
environment: move push_default into repo_config_values
environment: move autorebase into repo_config_values
environment: move object_creation_mode into repo_config_values
apply.c | 20 +++++++-----
branch.c | 2 +-
builtin/push.c | 8 ++---
dir.c | 4 +--
editor.c | 4 +--
environment.c | 87 +++++++++++++++++++++++++++++++++++---------------
environment.h | 75 +++++++++++++++++++++++++++----------------
object-file.c | 2 +-
pager.c | 26 +++++++++------
prompt.c | 3 +-
remote.c | 2 +-
repository.c | 1 +
12 files changed, 152 insertions(+), 82 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v9 1/9] repository: introduce repo_config_values_clear()
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
As part of the ongoing libification effort, dynamically allocated
global configuration variables are being moved into
'struct repo_config_values'. To prevent memory leaks, we need a
destructor to free these heap-allocated variables when a repository
instance is torn down.
Introduce 'repo_config_values_clear()' in environment.c and invoke it
from 'repo_clear()' in repository.c. As a starting point, update this
new function to handle the cleanup of 'attributes_file'.
Note:
Submodules are currently not supported by repo_config_values(), which
explicitly BUG()s out if 'repo != the_repository'. Since repo_clear()
cleans up all repository instances, we must bypass them to prevent
crashing.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
environment.c | 19 +++++++++++++++++++
environment.h | 9 +++++++++
repository.c | 1 +
3 files changed, 29 insertions(+)
diff --git a/environment.c b/environment.c
index ba2c60103f..13677484de 100644
--- a/environment.c
+++ b/environment.c
@@ -726,3 +726,22 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->sparse_expect_files_outside_of_patterns = 0;
cfg->warn_on_object_refname_ambiguity = 1;
}
+
+void repo_config_values_clear(struct repository *repo)
+{
+ struct repo_config_values *cfg;
+
+ /*
+ * NEEDSWORK: Submodules are currently not supported by
+ * repo_config_values(), which explicitly BUG()s out if
+ * repo != the_repository. Since repo_clear() cleans up all
+ * repository instances, we must bypass them here to prevent
+ * crashing.
+ */
+ if (repo != the_repository)
+ return;
+
+ cfg = repo_config_values(repo);
+
+ FREE_AND_NULL(cfg->attributes_file);
+}
diff --git a/environment.h b/environment.h
index 6f18286955..c4a6a45704 100644
--- a/environment.h
+++ b/environment.h
@@ -135,6 +135,15 @@ int git_default_core_config(const char *var, const char *value,
void repo_config_values_init(struct repo_config_values *cfg);
+/*
+ * Frees memory allocated for dynamically loaded configuration values
+ * inside `repo_config_values`.
+ *
+ * As dynamically allocated variables are migrated into this struct,
+ * their FREE_AND_NULL() calls should be appended here.
+ */
+void repo_config_values_clear(struct repository *repo);
+
/*
* TODO: All the below state either explicitly or implicitly relies on
* `the_repository`. We should eventually get rid of these and make the
diff --git a/repository.c b/repository.c
index 187dd471c4..b31f1b7852 100644
--- a/repository.c
+++ b/repository.c
@@ -388,6 +388,7 @@ void repo_clear(struct repository *repo)
FREE_AND_NULL(repo->parsed_objects);
repo_settings_clear(repo);
+ repo_config_values_clear(repo);
if (repo->config) {
git_configset_clear(repo->config);
--
2.43.0
^ permalink raw reply related
* [PATCH v9 2/9] environment: move excludes_file into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
The global variable 'excludes_file' is used to track the path to the
global ignore file. If this variable is NULL,
'setup_standard_excludes()'
in 'dir.c' forcefully evaluates and assigns the XDG default path to it.
Continue the libification effort by encapsulating this lazy-loading
fallback logic into a proper getter and moving the variable into
'struct repo_config_values'.
Since 'excludes_file' is a dynamically allocated string, it requires
proper heap memory management. It is safely freed using the newly
introduced `repo_config_values_clear()` function when the repository
is torn down.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
dir.c | 4 ++--
environment.c | 15 ++++++++++++---
environment.h | 4 +++-
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/dir.c b/dir.c
index 7a73690fbc..4f87a52b3c 100644
--- a/dir.c
+++ b/dir.c
@@ -3481,11 +3481,11 @@ static GIT_PATH_FUNC(git_path_info_exclude, "info/exclude")
void setup_standard_excludes(struct dir_struct *dir)
{
+ const char *excludes_file = repo_excludes_file(the_repository);
+
dir->exclude_per_dir = ".gitignore";
/* core.excludesfile defaulting to $XDG_CONFIG_HOME/git/ignore */
- if (!excludes_file)
- excludes_file = xdg_config_home("ignore");
if (excludes_file && !access_or_warn(excludes_file, R_OK, 0))
add_patterns_from_file_1(dir, excludes_file,
dir->untracked ? &dir->internal.ss_excludes_file : NULL);
diff --git a/environment.c b/environment.c
index 13677484de..5950592d63 100644
--- a/environment.c
+++ b/environment.c
@@ -57,7 +57,6 @@ enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
char *editor_program;
char *askpass_program;
-char *excludes_file;
enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
enum eol core_eol = EOL_UNSET;
int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
@@ -134,6 +133,14 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !repo_get_work_tree(the_repository);
}
+const char *repo_excludes_file(struct repository *repo)
+{
+ if (!repo_config_values(repo)->excludes_file)
+ repo_config_values(repo)->excludes_file = xdg_config_home("ignore");
+
+ return repo_config_values(repo)->excludes_file;
+}
+
int have_git_dir(void)
{
return startup_info->have_repository
@@ -461,8 +468,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.excludesfile")) {
- FREE_AND_NULL(excludes_file);
- return git_config_pathname(&excludes_file, var, value);
+ FREE_AND_NULL(cfg->excludes_file);
+ return git_config_pathname(&cfg->excludes_file, var, value);
}
if (!strcmp(var, "core.whitespace")) {
@@ -715,6 +722,7 @@ int git_default_config(const char *var, const char *value,
void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
+ cfg->excludes_file = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -744,4 +752,5 @@ void repo_config_values_clear(struct repository *repo)
cfg = repo_config_values(repo);
FREE_AND_NULL(cfg->attributes_file);
+ FREE_AND_NULL(cfg->excludes_file);
}
diff --git a/environment.h b/environment.h
index c4a6a45704..2e8352de7f 100644
--- a/environment.h
+++ b/environment.h
@@ -90,6 +90,7 @@ struct repository;
struct repo_config_values {
/* section "core" config values */
char *attributes_file;
+ char *excludes_file;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -133,6 +134,8 @@ int git_default_config(const char *, const char *,
int git_default_core_config(const char *var, const char *value,
const struct config_context *ctx, void *cb);
+const char *repo_excludes_file(struct repository *repo);
+
void repo_config_values_init(struct repo_config_values *cfg);
/*
@@ -217,7 +220,6 @@ extern char *git_log_output_encoding;
extern char *editor_program;
extern char *askpass_program;
-extern char *excludes_file;
/*
* The character that begins a commented line in user-editable file
--
2.43.0
^ permalink raw reply related
* [PATCH v9 3/9] environment: move editor_program into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
The global variable 'editor_program' holds the path to the user's
preferred editor. Move 'editor_program' into
'struct repo_config_values' to continue the libification effort.
There have been discussions on whether external programs like
editors truly need to be configured on a per-repository basis within
the same process. While a single process might rarely invoke
different editors, this migration is necessary for two reasons:
1. Developers frequently use different toolchains for different
projects. Per-repo configuration respects this.
2. Moving this string into 'repo_config_values' eliminates mutable
global state. As the codebase moves toward becoming a long-running
processes managing multiple repositories concurrently must
not overwrite each other's program configurations.
No standalone getter function is introduced. Callers directly access
the field via 'repo_config_values()'. Heap memory is safely reclaimed
in 'repo_config_values_clear()'.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
editor.c | 4 ++--
environment.c | 7 ++++---
environment.h | 2 +-
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/editor.c b/editor.c
index fd174e6a03..07d264cba0 100644
--- a/editor.c
+++ b/editor.c
@@ -29,8 +29,8 @@ const char *git_editor(void)
const char *editor = getenv("GIT_EDITOR");
int terminal_is_dumb = is_terminal_dumb();
- if (!editor && editor_program)
- editor = editor_program;
+ if (!editor && repo_config_values(the_repository)->editor_program)
+ editor = repo_config_values(the_repository)->editor_program;
if (!editor && !terminal_is_dumb)
editor = getenv("VISUAL");
if (!editor)
diff --git a/environment.c b/environment.c
index 5950592d63..0a01f4761a 100644
--- a/environment.c
+++ b/environment.c
@@ -55,7 +55,6 @@ int fsync_object_files = -1;
int use_fsync = -1;
enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
enum fsync_component fsync_components = FSYNC_COMPONENTS_DEFAULT;
-char *editor_program;
char *askpass_program;
enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
enum eol core_eol = EOL_UNSET;
@@ -435,8 +434,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.editor")) {
- FREE_AND_NULL(editor_program);
- return git_config_string(&editor_program, var, value);
+ FREE_AND_NULL(cfg->editor_program);
+ return git_config_string(&cfg->editor_program, var, value);
}
if (!strcmp(var, "core.commentchar") ||
@@ -723,6 +722,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
{
cfg->attributes_file = NULL;
cfg->excludes_file = NULL;
+ cfg->editor_program = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -753,4 +753,5 @@ void repo_config_values_clear(struct repository *repo)
FREE_AND_NULL(cfg->attributes_file);
FREE_AND_NULL(cfg->excludes_file);
+ FREE_AND_NULL(cfg->editor_program);
}
diff --git a/environment.h b/environment.h
index 2e8352de7f..1ec19149cb 100644
--- a/environment.h
+++ b/environment.h
@@ -91,6 +91,7 @@ struct repo_config_values {
/* section "core" config values */
char *attributes_file;
char *excludes_file;
+ char *editor_program;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -218,7 +219,6 @@ const char *get_commit_output_encoding(void);
extern char *git_commit_encoding;
extern char *git_log_output_encoding;
-extern char *editor_program;
extern char *askpass_program;
/*
--
2.43.0
^ permalink raw reply related
* [PATCH v9 4/9] environment: move pager_program into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
The 'pager_program' variable is currently defined as a file-scoped
static string in pager.c. Move it into 'struct repo_config_values'.
The configuration parsing logic remains strictly within pager.c to
respect subsystem boundaries. The read/write operations are simply
redirected to the repository-specific structure using
'repo_config_values()'.
Similar to the recent editor_program migration, no standalone getter
is introduced to keep the code minimal. The dynamically allocated
memory is now managed by 'repo_config_values_clear()'.
On top of that, fix a memory leak in pager.c while we are at it.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
environment.c | 2 ++
environment.h | 1 +
| 26 +++++++++++++++++---------
3 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/environment.c b/environment.c
index 0a01f4761a..a1204fdcb2 100644
--- a/environment.c
+++ b/environment.c
@@ -723,6 +723,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->attributes_file = NULL;
cfg->excludes_file = NULL;
cfg->editor_program = NULL;
+ cfg->pager_program = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -754,4 +755,5 @@ void repo_config_values_clear(struct repository *repo)
FREE_AND_NULL(cfg->attributes_file);
FREE_AND_NULL(cfg->excludes_file);
FREE_AND_NULL(cfg->editor_program);
+ FREE_AND_NULL(cfg->pager_program);
}
diff --git a/environment.h b/environment.h
index 1ec19149cb..22f6697c52 100644
--- a/environment.h
+++ b/environment.h
@@ -92,6 +92,7 @@ struct repo_config_values {
char *attributes_file;
char *excludes_file;
char *editor_program;
+ char *pager_program;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
--git a/pager.c b/pager.c
index 35b210e048..bc55546670 100644
--- a/pager.c
+++ b/pager.c
@@ -5,6 +5,8 @@
#include "run-command.h"
#include "sigchain.h"
#include "alias.h"
+#include "repository.h"
+#include "environment.h"
int pager_use_color = 1;
@@ -13,7 +15,6 @@ int pager_use_color = 1;
#endif
static struct child_process pager_process;
-static char *pager_program;
static int old_fd1 = -1, old_fd2 = -1;
/* Is the value coming back from term_columns() just a guess? */
@@ -75,10 +76,15 @@ static void wait_for_pager_signal(int signo)
static int core_pager_config(const char *var, const char *value,
const struct config_context *ctx UNUSED,
- void *data UNUSED)
+ void *data)
{
- if (!strcmp(var, "core.pager"))
- return git_config_string(&pager_program, var, value);
+ struct repository *r = data;
+
+ if (!strcmp(var, "core.pager")) {
+ FREE_AND_NULL(repo_config_values(r)->pager_program);
+ return git_config_string(&repo_config_values(r)->pager_program, var, value);
+ }
+
return 0;
}
@@ -91,10 +97,10 @@ const char *git_pager(struct repository *r, int stdout_is_tty)
pager = getenv("GIT_PAGER");
if (!pager) {
- if (!pager_program)
+ if (!repo_config_values(r)->pager_program)
read_early_config(r,
- core_pager_config, NULL);
- pager = pager_program;
+ core_pager_config, r);
+ pager = repo_config_values(r)->pager_program;
}
if (!pager)
pager = getenv("PAGER");
@@ -302,7 +308,9 @@ int check_pager_config(struct repository *r, const char *cmd)
read_early_config(r, pager_command_config, &data);
- if (data.value)
- pager_program = data.value;
+ if (data.value) {
+ free(repo_config_values(r)->pager_program);
+ repo_config_values(r)->pager_program = data.value;
+ }
return data.want;
}
--
2.43.0
^ permalink raw reply related
* [PATCH v9 5/9] environment: move askpass_program into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
The global variable 'askpass_program' stores the path to the program
used to prompt the user for credentials. Move it into repo_config_values
to continue the libification effort.
While it is uncommon for a single process to require different askpass
programs for different repositories, maintaining this value as a mutable
global string is a blocker for libification. Global heap-allocated
strings introduce thread-safety issues in a multi-repo environment.
Move 'askpass_program' into 'struct repo_config_values' to eliminate
this global state. The memory is now safely managed and freed via
'repo_config_values_clear()'.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
environment.c | 6 ++++--
environment.h | 1 +
prompt.c | 3 ++-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/environment.c b/environment.c
index a1204fdcb2..3782bf68aa 100644
--- a/environment.c
+++ b/environment.c
@@ -462,8 +462,8 @@ int git_default_core_config(const char *var, const char *value,
}
if (!strcmp(var, "core.askpass")) {
- FREE_AND_NULL(askpass_program);
- return git_config_string(&askpass_program, var, value);
+ FREE_AND_NULL(cfg->askpass_program);
+ return git_config_string(&cfg->askpass_program, var, value);
}
if (!strcmp(var, "core.excludesfile")) {
@@ -724,6 +724,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->excludes_file = NULL;
cfg->editor_program = NULL;
cfg->pager_program = NULL;
+ cfg->askpass_program = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -756,4 +757,5 @@ void repo_config_values_clear(struct repository *repo)
FREE_AND_NULL(cfg->excludes_file);
FREE_AND_NULL(cfg->editor_program);
FREE_AND_NULL(cfg->pager_program);
+ FREE_AND_NULL(cfg->askpass_program);
}
diff --git a/environment.h b/environment.h
index 22f6697c52..d55b1ba073 100644
--- a/environment.h
+++ b/environment.h
@@ -93,6 +93,7 @@ struct repo_config_values {
char *excludes_file;
char *editor_program;
char *pager_program;
+ char *askpass_program;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
diff --git a/prompt.c b/prompt.c
index 706fba2a50..d8d74c7e37 100644
--- a/prompt.c
+++ b/prompt.c
@@ -3,6 +3,7 @@
#include "git-compat-util.h"
#include "parse.h"
#include "environment.h"
+#include "repository.h"
#include "run-command.h"
#include "strbuf.h"
#include "prompt.h"
@@ -51,7 +52,7 @@ char *git_prompt(const char *prompt, int flags)
askpass = getenv("GIT_ASKPASS");
if (!askpass)
- askpass = askpass_program;
+ askpass = repo_config_values(the_repository)->askpass_program;
if (!askpass)
askpass = getenv("SSH_ASKPASS");
if (askpass && *askpass)
--
2.43.0
^ permalink raw reply related
* [PATCH v9 6/9] environment: migrate apply_default_whitespace and apply_default_ignorewhitespace
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
The global variables 'apply_default_whitespace' and
'apply_default_ignorewhitespace' are used to store the default
whitespace configuration for 'git apply'. Move these variables
into 'struct repo_config_values' to continue the libification
effort.
Dynamically allocated strings fetched via 'repo_config_get_string()'
are now tracked per-repository and safely freed in
'repo_config_values_clear()'.
As part of this transition, update 'git_apply_config()' to accept a
'struct repository *' argument rather than relying on the
'the_repository' global.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
apply.c | 20 ++++++++++++--------
environment.c | 6 ++++--
environment.h | 4 ++--
3 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/apply.c b/apply.c
index 249248d4f2..66db9b7678 100644
--- a/apply.c
+++ b/apply.c
@@ -47,11 +47,13 @@ struct gitdiff_data {
int p_value;
};
-static void git_apply_config(void)
+static void git_apply_config(struct repository *repo)
{
- repo_config_get_string(the_repository, "apply.whitespace", &apply_default_whitespace);
- repo_config_get_string(the_repository, "apply.ignorewhitespace", &apply_default_ignorewhitespace);
- repo_config(the_repository, git_xmerge_config, NULL);
+ repo_config_get_string(repo, "apply.whitespace",
+ &repo_config_values(repo)->apply_default_whitespace);
+ repo_config_get_string(repo, "apply.ignorewhitespace",
+ &repo_config_values(repo)->apply_default_ignorewhitespace);
+ repo_config(repo, git_xmerge_config, NULL);
}
static int parse_whitespace_option(struct apply_state *state, const char *option)
@@ -126,10 +128,12 @@ int init_apply_state(struct apply_state *state,
strset_init(&state->kept_symlinks);
strbuf_init(&state->root, 0);
- git_apply_config();
- if (apply_default_whitespace && parse_whitespace_option(state, apply_default_whitespace))
+ git_apply_config(repo);
+ if (repo_config_values(repo)->apply_default_whitespace &&
+ parse_whitespace_option(state, repo_config_values(repo)->apply_default_whitespace))
return -1;
- if (apply_default_ignorewhitespace && parse_ignorewhitespace_option(state, apply_default_ignorewhitespace))
+ if (repo_config_values(repo)->apply_default_ignorewhitespace &&
+ parse_ignorewhitespace_option(state, repo_config_values(repo)->apply_default_ignorewhitespace))
return -1;
return 0;
}
@@ -192,7 +196,7 @@ int check_apply_state(struct apply_state *state, int force_apply)
static void set_default_whitespace_mode(struct apply_state *state)
{
- if (!state->whitespace_option && !apply_default_whitespace)
+ if (!state->whitespace_option && !repo_config_values(state->repo)->apply_default_whitespace)
state->ws_error_action = (state->apply ? warn_on_ws_error : nowarn_ws_error);
}
diff --git a/environment.c b/environment.c
index 3782bf68aa..8744790219 100644
--- a/environment.c
+++ b/environment.c
@@ -49,8 +49,6 @@ int assume_unchanged;
int is_bare_repository_cfg = -1; /* unspecified */
char *git_commit_encoding;
char *git_log_output_encoding;
-char *apply_default_whitespace;
-char *apply_default_ignorewhitespace;
int fsync_object_files = -1;
int use_fsync = -1;
enum fsync_method fsync_method = FSYNC_METHOD_DEFAULT;
@@ -725,6 +723,8 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->editor_program = NULL;
cfg->pager_program = NULL;
cfg->askpass_program = NULL;
+ cfg->apply_default_whitespace = NULL;
+ cfg->apply_default_ignorewhitespace = NULL;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
@@ -758,4 +758,6 @@ void repo_config_values_clear(struct repository *repo)
FREE_AND_NULL(cfg->editor_program);
FREE_AND_NULL(cfg->pager_program);
FREE_AND_NULL(cfg->askpass_program);
+ FREE_AND_NULL(cfg->apply_default_whitespace);
+ FREE_AND_NULL(cfg->apply_default_ignorewhitespace);
}
diff --git a/environment.h b/environment.h
index d55b1ba073..9aecd64152 100644
--- a/environment.h
+++ b/environment.h
@@ -94,6 +94,8 @@ struct repo_config_values {
char *editor_program;
char *pager_program;
char *askpass_program;
+ char *apply_default_whitespace;
+ char *apply_default_ignorewhitespace;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -182,8 +184,6 @@ extern int has_symlinks;
extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
-extern char *apply_default_whitespace;
-extern char *apply_default_ignorewhitespace;
extern unsigned long pack_size_limit_cfg;
extern int protect_hfs;
--
2.43.0
^ permalink raw reply related
* [PATCH v9 7/9] environment: move push_default into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
The global variable 'push_default' specifies the default behavior of
'git push' when no explicit refspec is provided. Move 'push_default'
into 'struct repo_config_values' to continue the libification effort.
While 'enum push_default_type' ideally belongs in 'remote.h', moving it
there introduces a circular dependency chain:
remote.h -> hash.h -> repository.h -> environment.h.
Therefore, the enum definition is kept in 'environment.h' just above
'struct repo_config_values' with a NEEDSWORK comment for future cleanup.
Modify the configuration parsing in environment.c to update the
per-repository structure directly, and update caller across the
codebase to access the value via 'repo_config_values()'.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
builtin/push.c | 8 ++++----
environment.c | 16 +++++++++-------
environment.h | 26 ++++++++++++++++----------
remote.c | 2 +-
4 files changed, 30 insertions(+), 22 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 6021b71d66..6dc3224b60 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -88,7 +88,7 @@ static void refspec_append_mapped(struct refspec *refspec, const char *ref,
}
}
- if (push_default == PUSH_DEFAULT_UPSTREAM &&
+ if (repo_config_values(the_repository)->push_default == PUSH_DEFAULT_UPSTREAM &&
skip_prefix(matched->name, "refs/heads/", &branch_name)) {
struct branch *branch = branch_get(branch_name);
if (branch->merge_nr == 1 && branch->merge[0]->src) {
@@ -160,7 +160,7 @@ static NORETURN void die_push_simple(struct branch *branch,
* Don't show advice for people who explicitly set
* push.default.
*/
- if (push_default == PUSH_DEFAULT_UNSPECIFIED)
+ if (cfg->push_default == PUSH_DEFAULT_UNSPECIFIED)
advice_pushdefault_maybe = _("\n"
"To choose either option permanently, "
"see push.default in 'git help config'.\n");
@@ -232,7 +232,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
const char *dst;
int same_remote;
- switch (push_default) {
+ switch (repo_config_values(the_repository)->push_default) {
case PUSH_DEFAULT_MATCHING:
refspec_append(&rs, ":");
return;
@@ -252,7 +252,7 @@ static void setup_default_push_refspecs(int *flags, struct remote *remote)
dst = branch->refname;
same_remote = !strcmp(remote->name, remote_for_branch(branch, NULL));
- switch (push_default) {
+ switch (repo_config_values(the_repository)->push_default) {
default:
case PUSH_DEFAULT_UNSPECIFIED:
case PUSH_DEFAULT_SIMPLE:
diff --git a/environment.c b/environment.c
index 8744790219..09de2fee87 100644
--- a/environment.c
+++ b/environment.c
@@ -59,7 +59,6 @@ enum eol core_eol = EOL_UNSET;
int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
char *check_roundtrip_encoding;
enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
-enum push_default_type push_default = PUSH_DEFAULT_UNSPECIFIED;
#ifndef OBJECT_CREATION_MODE
#define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
#endif
@@ -619,21 +618,23 @@ static int git_default_branch_config(const char *var, const char *value)
static int git_default_push_config(const char *var, const char *value)
{
+ struct repo_config_values *cfg = repo_config_values(the_repository);
+
if (!strcmp(var, "push.default")) {
if (!value)
return config_error_nonbool(var);
else if (!strcmp(value, "nothing"))
- push_default = PUSH_DEFAULT_NOTHING;
+ cfg->push_default = PUSH_DEFAULT_NOTHING;
else if (!strcmp(value, "matching"))
- push_default = PUSH_DEFAULT_MATCHING;
+ cfg->push_default = PUSH_DEFAULT_MATCHING;
else if (!strcmp(value, "simple"))
- push_default = PUSH_DEFAULT_SIMPLE;
+ cfg->push_default = PUSH_DEFAULT_SIMPLE;
else if (!strcmp(value, "upstream"))
- push_default = PUSH_DEFAULT_UPSTREAM;
+ cfg->push_default = PUSH_DEFAULT_UPSTREAM;
else if (!strcmp(value, "tracking")) /* deprecated */
- push_default = PUSH_DEFAULT_UPSTREAM;
+ cfg->push_default = PUSH_DEFAULT_UPSTREAM;
else if (!strcmp(value, "current"))
- push_default = PUSH_DEFAULT_CURRENT;
+ cfg->push_default = PUSH_DEFAULT_CURRENT;
else {
error(_("malformed value for %s: %s"), var, value);
return error(_("must be one of nothing, matching, simple, "
@@ -725,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->askpass_program = NULL;
cfg->apply_default_whitespace = NULL;
cfg->apply_default_ignorewhitespace = NULL;
+ cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 9aecd64152..72859b5d76 100644
--- a/environment.h
+++ b/environment.h
@@ -87,6 +87,21 @@ extern const char * const local_repo_env[];
struct strvec;
struct repository;
+
+/*
+ * NEEDSWORK: It would be better if these definitions could be moved to
+ * other more specific files, but care is needed to avoid circular
+ * inclusion issues.
+ */
+enum push_default_type {
+ PUSH_DEFAULT_NOTHING = 0,
+ PUSH_DEFAULT_MATCHING,
+ PUSH_DEFAULT_SIMPLE,
+ PUSH_DEFAULT_UPSTREAM,
+ PUSH_DEFAULT_CURRENT,
+ PUSH_DEFAULT_UNSPECIFIED
+};
+
struct repo_config_values {
/* section "core" config values */
char *attributes_file;
@@ -96,6 +111,7 @@ struct repo_config_values {
char *askpass_program;
char *apply_default_whitespace;
char *apply_default_ignorewhitespace;
+ enum push_default_type push_default;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -197,16 +213,6 @@ enum rebase_setup_type {
};
extern enum rebase_setup_type autorebase;
-enum push_default_type {
- PUSH_DEFAULT_NOTHING = 0,
- PUSH_DEFAULT_MATCHING,
- PUSH_DEFAULT_SIMPLE,
- PUSH_DEFAULT_UPSTREAM,
- PUSH_DEFAULT_CURRENT,
- PUSH_DEFAULT_UNSPECIFIED
-};
-extern enum push_default_type push_default;
-
enum object_creation_mode {
OBJECT_CREATION_USES_HARDLINKS = 0,
OBJECT_CREATION_USES_RENAMES = 1
diff --git a/remote.c b/remote.c
index 00723b385e..d48c01d375 100644
--- a/remote.c
+++ b/remote.c
@@ -1933,7 +1933,7 @@ static char *branch_get_push_1(struct repository *repo,
if (remote->mirror)
return tracking_for_push_dest(remote, branch->refname, err);
- switch (push_default) {
+ switch (repo_config_values(repo)->push_default) {
case PUSH_DEFAULT_NOTHING:
return error_buf(err, _("push has no destination (push.default is 'nothing')"));
--
2.43.0
^ permalink raw reply related
* [PATCH v9 8/9] environment: move autorebase into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
The global variable 'autorebase' dictates whether a newly created
branch should be configured to automatically rebase by default.
Move it into 'struct repo_config_values' to continue the
libification effort.
The 'enum rebase_setup_type' definition is moved higher up in
'environment.h' so that it is visible to the repository-specific
structure. The default state AUTOREBASE_NEVER is now correctly
initialized in 'repo_config_values_init()'.
Configuration parsing in 'git_default_branch_config()' is updated to
write directly to the repository's configuration instance.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
branch.c | 2 +-
environment.c | 10 +++++-----
environment.h | 16 ++++++++--------
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/branch.c b/branch.c
index 243db7d0fc..e1c1f8c89d 100644
--- a/branch.c
+++ b/branch.c
@@ -61,7 +61,7 @@ static int find_tracked_branch(struct remote *remote, void *priv)
static int should_setup_rebase(const char *origin)
{
- switch (autorebase) {
+ switch (repo_config_values(the_repository)->autorebase) {
case AUTOREBASE_NEVER:
return 0;
case AUTOREBASE_LOCAL:
diff --git a/environment.c b/environment.c
index 09de2fee87..7701aa3bc0 100644
--- a/environment.c
+++ b/environment.c
@@ -58,7 +58,6 @@ enum auto_crlf auto_crlf = AUTO_CRLF_FALSE;
enum eol core_eol = EOL_UNSET;
int global_conv_flags_eol = CONV_EOL_RNDTRP_WARN;
char *check_roundtrip_encoding;
-enum rebase_setup_type autorebase = AUTOREBASE_NEVER;
#ifndef OBJECT_CREATION_MODE
#define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
#endif
@@ -600,13 +599,13 @@ static int git_default_branch_config(const char *var, const char *value)
if (!value)
return config_error_nonbool(var);
else if (!strcmp(value, "never"))
- autorebase = AUTOREBASE_NEVER;
+ cfg->autorebase = AUTOREBASE_NEVER;
else if (!strcmp(value, "local"))
- autorebase = AUTOREBASE_LOCAL;
+ cfg->autorebase = AUTOREBASE_LOCAL;
else if (!strcmp(value, "remote"))
- autorebase = AUTOREBASE_REMOTE;
+ cfg->autorebase = AUTOREBASE_REMOTE;
else if (!strcmp(value, "always"))
- autorebase = AUTOREBASE_ALWAYS;
+ cfg->autorebase = AUTOREBASE_ALWAYS;
else
return error(_("malformed value for %s"), var);
return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->apply_default_whitespace = NULL;
cfg->apply_default_ignorewhitespace = NULL;
cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
+ cfg->autorebase = AUTOREBASE_NEVER;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 72859b5d76..464ff73136 100644
--- a/environment.h
+++ b/environment.h
@@ -102,6 +102,13 @@ enum push_default_type {
PUSH_DEFAULT_UNSPECIFIED
};
+enum rebase_setup_type {
+ AUTOREBASE_NEVER = 0,
+ AUTOREBASE_LOCAL,
+ AUTOREBASE_REMOTE,
+ AUTOREBASE_ALWAYS
+};
+
struct repo_config_values {
/* section "core" config values */
char *attributes_file;
@@ -112,6 +119,7 @@ struct repo_config_values {
char *apply_default_whitespace;
char *apply_default_ignorewhitespace;
enum push_default_type push_default;
+ enum rebase_setup_type autorebase;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -205,14 +213,6 @@ extern unsigned long pack_size_limit_cfg;
extern int protect_hfs;
extern int protect_ntfs;
-enum rebase_setup_type {
- AUTOREBASE_NEVER = 0,
- AUTOREBASE_LOCAL,
- AUTOREBASE_REMOTE,
- AUTOREBASE_ALWAYS
-};
-extern enum rebase_setup_type autorebase;
-
enum object_creation_mode {
OBJECT_CREATION_USES_HARDLINKS = 0,
OBJECT_CREATION_USES_RENAMES = 1
--
2.43.0
^ permalink raw reply related
* [PATCH v9 9/9] environment: move object_creation_mode into repo_config_values
From: Tian Yuchen @ 2026-07-09 16:11 UTC (permalink / raw)
To: git
Cc: cirnovskyv, szeder.dev, Tian Yuchen, Christian Couder,
Ayush Chandekar, Olamide Caleb Bello
In-Reply-To: <20260709161145.13349-1-cat@malon.dev>
The global variable 'object_creation_mode' controls how Git creates
object files, specifically determining whether to use hardlinks or
renames when moving temporary files into the object database. Move
it into 'struct repo_config_values' to continue the libification
effort.
Move the 'enum object_creation_mode' definition higher up in
'environment.h' to ensure it is visible to the structure. Initialize
the per-repository value to its default macro value
OBJECT_CREATION_MODE inside 'repo_config_values_init()'.
Update configuration parsing in 'git_default_core_config()' to write
directly to the repository-specific configuration structure.
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Ayush Chandekar <ayu.chandekar@gmail.com>
Mentored-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Tian Yuchen <cat@malon.dev>
---
environment.c | 6 +++---
environment.h | 12 ++++++------
object-file.c | 2 +-
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/environment.c b/environment.c
index 7701aa3bc0..e50beda918 100644
--- a/environment.c
+++ b/environment.c
@@ -61,7 +61,6 @@ char *check_roundtrip_encoding;
#ifndef OBJECT_CREATION_MODE
#define OBJECT_CREATION_MODE OBJECT_CREATION_USES_HARDLINKS
#endif
-enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
int grafts_keep_true_parents;
unsigned long pack_size_limit_cfg;
@@ -511,9 +510,9 @@ int git_default_core_config(const char *var, const char *value,
if (!value)
return config_error_nonbool(var);
if (!strcmp(value, "rename"))
- object_creation_mode = OBJECT_CREATION_USES_RENAMES;
+ cfg->object_creation_mode = OBJECT_CREATION_USES_RENAMES;
else if (!strcmp(value, "link"))
- object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
+ cfg->object_creation_mode = OBJECT_CREATION_USES_HARDLINKS;
else
die(_("invalid mode for object creation: %s"), value);
return 0;
@@ -727,6 +726,7 @@ void repo_config_values_init(struct repo_config_values *cfg)
cfg->apply_default_ignorewhitespace = NULL;
cfg->push_default = PUSH_DEFAULT_UNSPECIFIED;
cfg->autorebase = AUTOREBASE_NEVER;
+ cfg->object_creation_mode = OBJECT_CREATION_MODE;
cfg->apply_sparse_checkout = 0;
cfg->branch_track = BRANCH_TRACK_REMOTE;
cfg->trust_ctime = 1;
diff --git a/environment.h b/environment.h
index 464ff73136..eaa0aba7bc 100644
--- a/environment.h
+++ b/environment.h
@@ -109,6 +109,11 @@ enum rebase_setup_type {
AUTOREBASE_ALWAYS
};
+enum object_creation_mode {
+ OBJECT_CREATION_USES_HARDLINKS = 0,
+ OBJECT_CREATION_USES_RENAMES = 1
+};
+
struct repo_config_values {
/* section "core" config values */
char *attributes_file;
@@ -120,6 +125,7 @@ struct repo_config_values {
char *apply_default_ignorewhitespace;
enum push_default_type push_default;
enum rebase_setup_type autorebase;
+ enum object_creation_mode object_creation_mode;
int apply_sparse_checkout;
int trust_ctime;
int check_stat;
@@ -213,12 +219,6 @@ extern unsigned long pack_size_limit_cfg;
extern int protect_hfs;
extern int protect_ntfs;
-enum object_creation_mode {
- OBJECT_CREATION_USES_HARDLINKS = 0,
- OBJECT_CREATION_USES_RENAMES = 1
-};
-extern enum object_creation_mode object_creation_mode;
-
extern int grafts_keep_true_parents;
const char *get_log_output_encoding(void);
diff --git a/object-file.c b/object-file.c
index 9afa842da2..cbbfc8f1dc 100644
--- a/object-file.c
+++ b/object-file.c
@@ -415,7 +415,7 @@ int finalize_object_file_flags(struct repository *repo,
retry:
ret = 0;
- if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)
+ if (repo_config_values(repo)->object_creation_mode == OBJECT_CREATION_USES_RENAMES)
goto try_rename;
else if (link(tmpfile, filename))
ret = errno;
--
2.43.0
^ 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