* [PATCH] clone: accept DEPTH env var as fallback for --depth
From: h8d13 via GitGitGadget @ 2026-06-13 1:39 UTC (permalink / raw)
To: git; +Cc: h8d13, h8d13
From: h8d13 <hadean-eon-dev@proton.me>
When git clone is run by a tool the user does not control directly
(CI runners, package build scripts such as makepkg, or any wrapper
that spawns nested clones), there is no way to request a shallow
clone: --depth only exists as a command-line option on the process
that invokes git clone, and unlike url.*.insteadOf there is no
configuration key that could be injected via GIT_CONFIG_* to achieve
the same effect.
Teach git clone to read a DEPTH environment variable when --depth is
not given on the command line. Since environment variables propagate
to child processes, exporting DEPTH=1 once makes every nested clone
underneath shallow, which is useful in CI pipelines and recursive
build tools. An explicit --depth on the command line still takes
precedence, and the value goes through the existing validation, so a
non-positive DEPTH dies with the same error as a non-positive
--depth.
Signed-off-by: h8d13 <hadean-eon-dev@proton.me>
---
clone: accept DEPTH env var as fallback for --depth
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2333%2Fh8d13%2Fdepth-env-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2333/h8d13/depth-env-v1
Pull-Request: https://github.com/git/git/pull/2333
builtin/clone.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/builtin/clone.c b/builtin/clone.c
index d60d1b60bc..549506f672 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1022,6 +1022,12 @@ int cmd_clone(int argc,
usage_msg_opt(_("You must specify a repository to clone."),
builtin_clone_usage, builtin_clone_options);
+ if (!option_depth) {
+ const char *env_depth = getenv("DEPTH");
+ if (env_depth && *env_depth)
+ option_depth = xstrdup(env_depth);
+ }
+
if (option_depth || option_since || option_not.nr)
deepen = 1;
if (option_single_branch == -1)
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
--
gitgitgadget
^ permalink raw reply related
* [PATCH v4 3/3] commit-reach: die on contains walk errors
From: Tamir Duberstein @ 2026-06-12 21:49 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260612-ref-filter-memoized-contains-v4-0-5ed39fd001dd@gmail.com>
Without generation numbers, repo_is_descendant_of() can return -1 when
it cannot read commit ancestry. commit_contains() exposes that result
through a Boolean interface, so ref-filter treats it as true. This can
include a ref for --contains or exclude it for --no-contains without
failing the command.
Die when repo_is_descendant_of() reports an error. The memoized walk
already dies when it cannot parse a commit, so callers of the
non-memoized path no longer turn a failed walk into a match.
Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 8 +++++++-
t/t6301-for-each-ref-errors.sh | 22 ++++++++++++++++++++++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/commit-reach.c b/commit-reach.c
index 18fcd69113..37b66b6b21 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -805,10 +805,16 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
int commit_contains(struct ref_filter *filter, struct commit *commit,
struct commit_list *list, struct contains_cache *cache)
{
+ int result;
+
if (filter->with_commit_tag_algo ||
generation_numbers_enabled(the_repository))
return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
- return repo_is_descendant_of(the_repository, commit, list);
+
+ result = repo_is_descendant_of(the_repository, commit, list);
+ if (result < 0)
+ die(_("failed to check reachability"));
+ return result;
}
int can_all_from_reach_with_flag(struct object_array *from,
diff --git a/t/t6301-for-each-ref-errors.sh b/t/t6301-for-each-ref-errors.sh
index e06feb06e9..72b27c8be3 100755
--- a/t/t6301-for-each-ref-errors.sh
+++ b/t/t6301-for-each-ref-errors.sh
@@ -52,6 +52,28 @@ test_expect_success 'Missing objects are reported correctly' '
test_must_be_empty brief-err
'
+test_expect_success 'missing ancestors are reported by contains filters' '
+ test_when_finished "git update-ref -d refs/heads/missing-parent" &&
+ {
+ echo "tree $(git rev-parse HEAD^{tree})" &&
+ echo "parent $MISSING" &&
+ git cat-file commit HEAD |
+ sed -n -e "/^author /p" -e "/^committer /p" &&
+ echo &&
+ echo "missing parent"
+ } >commit &&
+ broken=$(git hash-object -t commit -w commit) &&
+ git update-ref refs/heads/missing-parent "$broken" &&
+ for option in --contains --no-contains
+ do
+ test_must_fail git for-each-ref "$option=HEAD" \
+ refs/heads/missing-parent >out 2>err &&
+ test_must_be_empty out &&
+ test_grep "parse commit $MISSING" err ||
+ return 1
+ done
+'
+
test_expect_success 'ahead-behind requires an argument' '
test_must_fail git for-each-ref \
--format="%(ahead-behind)" 2>err &&
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH v4 2/3] ref-filter: memoize --contains with generations
From: Tamir Duberstein @ 2026-06-12 21:49 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260612-ref-filter-memoized-contains-v4-0-5ed39fd001dd@gmail.com>
git branch and git for-each-ref run a separate reachability walk for
each ref considered by --contains and --no-contains. Refs with shared
history therefore traverse the same commits repeatedly.
git tag instead uses a depth-first walk that caches results across
refs. That walk can perform poorly without generation numbers: a
negative check may walk to the root instead of stopping at a nearby
divergence. Generation numbers let it stop below the oldest target.
Use the memoized walk for all ref-filter callers when generation
numbers are available. Keep git tag on its existing path without
generations. Caching still helps when many tags share deep history:
ffc4b8012d (tag: speed up --contains calculation, 2011-06-11) reduced
git tag --contains HEAD~200 in linux-2.6 from 15.417 to 5.329 seconds.
The new shared-history perf test improves from 0.72 to 0.03 seconds. In
a repository with 62,174 remote-tracking refs, running:
git branch -r --contains c78ae85f3ce7e
improves from 104.365 seconds to 468 milliseconds.
Suggested-by: Jeff King <peff@peff.net>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 3 ++-
t/perf/p1500-graph-walks.sh | 28 +++++++++++++++++++++++++++-
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index e1bedc596d..18fcd69113 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -805,7 +805,8 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
int commit_contains(struct ref_filter *filter, struct commit *commit,
struct commit_list *list, struct contains_cache *cache)
{
- if (filter->with_commit_tag_algo)
+ if (filter->with_commit_tag_algo ||
+ generation_numbers_enabled(the_repository))
return contains_tag_algo(commit, list, cache) == CONTAINS_YES;
return repo_is_descendant_of(the_repository, commit, list);
}
diff --git a/t/perf/p1500-graph-walks.sh b/t/perf/p1500-graph-walks.sh
index 5b23ce5db9..d167b4f7e1 100755
--- a/t/perf/p1500-graph-walks.sh
+++ b/t/perf/p1500-graph-walks.sh
@@ -32,7 +32,16 @@ test_expect_success 'setup' '
echo "X:$line" >>test-tool-tags || return 1
done &&
- commit=$(git commit-tree $(git rev-parse HEAD^{tree})) &&
+ git rev-list --first-parent --max-count=8192 HEAD >contains-commits &&
+ test_file_not_empty contains-commits &&
+ git update-ref refs/contains-perf-base "$(tail -n 1 contains-commits)" &&
+ awk "{
+ printf \"update refs/contains-perf/%04d %s\\n\", NR, \$1
+ }" contains-commits |
+ git update-ref --stdin &&
+ git pack-refs --include "refs/contains-perf/*" &&
+
+ commit=$(git commit-tree HEAD^{tree}) &&
git update-ref refs/heads/disjoint-base $commit &&
git commit-graph write --reachable
@@ -62,6 +71,23 @@ test_perf 'contains: git tag --merged' '
xargs git tag --merged=HEAD <tags
'
+test_perf 'contains: git for-each-ref' '
+ git for-each-ref --contains=refs/contains-perf-base --stdin <refs
+'
+
+test_perf 'contains: git branch' '
+ xargs git branch --contains=refs/contains-perf-base <branches
+'
+
+test_perf 'contains: git tag' '
+ xargs git tag --contains=refs/contains-perf-base <tags
+'
+
+test_perf 'contains: synthetic shared history' '
+ git for-each-ref --contains=refs/contains-perf-base \
+ refs/contains-perf/ >/dev/null
+'
+
test_perf 'is-base check: test-tool reach (refs)' '
test-tool reach get_branch_base_for_tip <test-tool-refs
'
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH v4 1/3] commit-reach: reject cycles in contains walk
From: Tamir Duberstein @ 2026-06-12 21:49 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260612-ref-filter-memoized-contains-v4-0-5ed39fd001dd@gmail.com>
The memoized contains traversal used by git tag assumes that commit
ancestry is acyclic. Replacement refs can violate that assumption,
causing it to keep pushing an already active commit until memory is
exhausted.
Mark commits while they are active and die if the traversal encounters
an active commit. Other failures in this walk already die through
parse_commit_or_die(); using a second reachability walk would only add
a separate policy for malformed history.
Suggested-by: Kristofer Karlsson <krka@spotify.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
commit-reach.c | 12 +++++++++---
commit-reach.h | 3 ++-
t/t7004-tag.sh | 18 ++++++++++++++++++
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/commit-reach.c b/commit-reach.c
index 9b3ea46d6f..e1bedc596d 100644
--- a/commit-reach.c
+++ b/commit-reach.c
@@ -708,7 +708,8 @@ static int in_commit_list(const struct commit_list *want, struct commit *c)
/*
* Test whether the candidate is contained in the list.
- * Do not recurse to find out, though, but return -1 if inconclusive.
+ * Do not recurse to find out, though, but return CONTAINS_UNKNOWN if
+ * inconclusive.
*/
static enum contains_result contains_test(struct commit *candidate,
const struct commit_list *want,
@@ -765,6 +766,7 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
if (result != CONTAINS_UNKNOWN)
return result;
+ *contains_cache_at(cache, candidate) = CONTAINS_IN_PROGRESS;
push_to_contains_stack(candidate, &contains_stack);
while (contains_stack.nr) {
struct contains_stack_entry *entry = &contains_stack.contains_stack[contains_stack.nr - 1];
@@ -776,8 +778,8 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
contains_stack.nr--;
}
/*
- * If we just popped the stack, parents->item has been marked,
- * therefore contains_test will return a meaningful yes/no.
+ * A parent may have just been popped and marked, or may still
+ * be active when replacement refs create a cycle.
*/
else switch (contains_test(parents->item, want, cache, cutoff)) {
case CONTAINS_YES:
@@ -787,7 +789,11 @@ static enum contains_result contains_tag_algo(struct commit *candidate,
case CONTAINS_NO:
entry->parents = parents->next;
break;
+ case CONTAINS_IN_PROGRESS:
+ die(_("commit ancestry contains a cycle"));
case CONTAINS_UNKNOWN:
+ *contains_cache_at(cache, parents->item) =
+ CONTAINS_IN_PROGRESS;
push_to_contains_stack(parents->item, &contains_stack);
break;
}
diff --git a/commit-reach.h b/commit-reach.h
index 3f3a563d8a..f908d305b1 100644
--- a/commit-reach.h
+++ b/commit-reach.h
@@ -73,7 +73,8 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid);
enum contains_result {
CONTAINS_UNKNOWN = 0,
CONTAINS_NO,
- CONTAINS_YES
+ CONTAINS_YES,
+ CONTAINS_IN_PROGRESS
};
define_commit_slab(contains_cache, enum contains_result);
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index d918005dd9..67309494d2 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1611,6 +1611,24 @@ test_expect_success 'checking that first commit is in all tags (hash)' '
test_cmp expected actual
'
+test_expect_success 'tag --contains rejects cyclic replacement histories' '
+ first=$(git rev-parse HEAD~2) &&
+ second=$(git rev-parse HEAD~) &&
+ third=$(git rev-parse HEAD) &&
+ test_when_finished "
+ git replace -d $first &&
+ git replace -d $third &&
+ git tag -d cycle-a cycle-b
+ " &&
+ git tag cycle-a "$first" &&
+ git tag cycle-b "$third" &&
+ git replace --graft "$first" "$third" "$second" &&
+ git replace --graft "$third" "$first" &&
+ test_must_fail git tag --contains="$second" --list "cycle-*" \
+ >/dev/null 2>err &&
+ test_grep "fatal: commit ancestry contains a cycle" err
+'
+
# other ways of specifying the commit
test_expect_success 'checking that first commit is in all tags (tag)' '
cat >expected <<-\EOF &&
--
2.54.0.548.gbe7bb2469c
^ permalink raw reply related
* [PATCH v4 0/3] Reuse --contains traversal results
From: Tamir Duberstein @ 2026-06-12 21:49 UTC (permalink / raw)
To: git
Cc: Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren, Kristofer Karlsson,
Tamir Duberstein
In-Reply-To: <20260611-ref-filter-memoized-contains-v3-0-b26af3dba285@gmail.com>
git tag uses a memoized traversal for --contains, while git branch
and git for-each-ref repeat a reachability walk for each ref. Reuse
the memoized traversal when generation numbers can bound the walk.
The first patch makes the memoized traversal reject cyclic replacement
histories. The last makes the non-memoized path report reachability
errors.
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Changes in v4:
- Die on cyclic ancestry instead of retrying another reachability walk.
- Update the cycle test and credit Kristofer Karlsson.
- Remove unexplained links to review messages.
- Link to v3: https://patch.msgid.link/20260611-ref-filter-memoized-contains-v3-0-b26af3dba285@gmail.com
Changes in v3:
- Split missing-ancestor error handling into its own patch.
- Use die() for reachability errors, remove redundant cache setup, and
chain cycle-test cleanup.
- Drop the unrelated empty-target-list behavior change.
- Explain why git tag retains memoization without generation numbers.
- Add p1500 coverage for all three frontends and a shared-history
case.
- Remove correctness checks from p1500 and drop output hashes.
- Link to v2: https://patch.msgid.link/20260608-ref-filter-memoized-contains-v2-0-e72720344a7c@gmail.com
Changes in v2:
- Split cycle handling into a preparatory patch.
- Exercise cycle handling through the existing git tag path.
- Move perf result verification out of setup.
- Link to v1: https://patch.msgid.link/20260607-ref-filter-memoized-contains-v1-1-a1972dde9c76@gmail.com
---
Tamir Duberstein (3):
commit-reach: reject cycles in contains walk
ref-filter: memoize --contains with generations
commit-reach: die on contains walk errors
commit-reach.c | 23 ++++++++++++++++++-----
commit-reach.h | 3 ++-
t/perf/p1500-graph-walks.sh | 28 +++++++++++++++++++++++++++-
t/t6301-for-each-ref-errors.sh | 22 ++++++++++++++++++++++
t/t7004-tag.sh | 18 ++++++++++++++++++
5 files changed, 87 insertions(+), 7 deletions(-)
---
base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
change-id: 20260607-ref-filter-memoized-contains-7cb6b3bccad1
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply
* [PATCH v4] ref-filter: restore prefix-scoped iteration
From: Tamir Duberstein @ 2026-06-12 21:27 UTC (permalink / raw)
To: git
Cc: Karthik Nayak, Patrick Steinhardt, Junio C Hamano, Victoria Dye,
ZheNing Hu, Tamir Duberstein
In-Reply-To: <20260610-fix-git-branch-regression-v3-1-6fd48fad7a53@gmail.com>
dabecb9db2 (for-each-ref: introduce a '--start-after' option,
2025-07-15) changed branch, remote-tracking branch, and tag enumeration
from constructing an iterator with the namespace prefix to constructing
an unscoped iterator and seeking to the prefix.
Review of --start-after noted that the construction prefix and seek
position represent different state and are easy to conflate [1]. It also
noted that future branch or tag support would need to retain the
namespace prefix while moving the cursor [2].
The files backend constructs its loose-ref iterator with cache priming
enabled. cache_ref_iterator_begin() immediately applies the construction
prefix through cache_ref_iterator_set_prefix(), reading loose refs
beneath it before packed refs are opened. An empty prefix therefore
reads every loose ref, and a later seek cannot undo that I/O.
For the current single-kind filters, construct the iterator with the
namespace prefix when start_after is not set. Leave the existing
start_after path unchanged; no current command combines it with these
filters, and future support must carry the prefix separately from the
cursor.
With 10,000 unrelated loose refs in the files backend, the p6300 tests
improve as follows:
before after
branch 2.74 s 0.11 s
branch --remotes 2.81 s 0.12 s
tag 3.01 s 0.11 s
[1] https://lore.kernel.org/r/aGZidwwlToWThkn8@pks.im/
[2] https://lore.kernel.org/r/xmqqikjq7s16.fsf@gitster.g/
Fixes: dabecb9db2b2 ("for-each-ref: introduce a '--start-after' option")
Suggested-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
The series is based on a89346e34a (maint) because the regression has
been present in released versions since Git 2.51.0.
---
Changes in v4:
- Explain the historical references in the commit message.
- Run the new performance cases with both ref backends.
- Drop the Assisted-by trailer.
- Link to v3: https://patch.msgid.link/20260610-fix-git-branch-regression-v3-1-6fd48fad7a53@gmail.com
Changes in v3:
- Construct the iterator directly with the namespace prefix.
- Explain when the files backend primes its loose-ref cache.
- Condense the commit message and performance results.
- Link to v2: https://patch.msgid.link/20260608-fix-git-branch-regression-v2-1-fd82075a8520@gmail.com
Changes in v2:
- Extract local variable `store`.
- Link to v1: https://patch.msgid.link/20260605-fix-git-branch-regression-v1-1-02f40ad40929@gmail.com
---
ref-filter.c | 13 ++++++-------
t/perf/p6300-for-each-ref.sh | 39 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 44 insertions(+), 8 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 1da4c0e60d..9b04e3af85 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -3316,15 +3316,14 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
if (prefix) {
struct ref_iterator *iter;
+ struct ref_store *store = get_main_ref_store(the_repository);
- iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
- "", NULL, 0, 0);
-
- if (filter->start_after)
+ if (filter->start_after) {
+ iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
ret = start_ref_iterator_after(iter, filter->start_after);
- else
- ret = ref_iterator_seek(iter, prefix,
- REF_ITERATOR_SEEK_SET_PREFIX);
+ } else {
+ iter = refs_ref_iterator_begin(store, prefix, NULL, 0, 0);
+ }
if (!ret)
ret = do_for_each_ref_iterator(iter, fn, cb_data);
diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh
index fa7289c752..25ffa5e84c 100755
--- a/t/perf/p6300-for-each-ref.sh
+++ b/t/perf/p6300-for-each-ref.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='performance of for-each-ref'
+test_description='performance of ref-filter users'
. ./perf-lib.sh
test_perf_fresh_repo
@@ -84,4 +84,41 @@ test_expect_success 'pack refs' '
'
run_tests "packed"
+test_expect_success 'setup many unrelated refs' '
+ git init scoped &&
+ test_commit -C scoped --no-tag base &&
+ test_seq $ref_count_per_type |
+ sed "s,.*,update refs/custom/unrelated_& HEAD," |
+ git -C scoped update-ref --stdin &&
+ git -C scoped update-ref refs/remotes/origin/main HEAD &&
+ git -C scoped update-ref refs/tags/only HEAD
+'
+
+test_perf "branch (many unrelated refs)" "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git branch --format='%(refname)' >/dev/null
+ done
+ )
+"
+
+test_perf "branch --remotes (many unrelated refs)" "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git branch --remotes --format='%(refname)' >/dev/null
+ done
+ )
+"
+
+test_perf "tag (many unrelated refs)" "
+ (
+ cd scoped &&
+ for i in \$(test_seq $test_iteration_count); do
+ git tag --format='%(refname)' >/dev/null
+ done
+ )
+"
+
test_done
---
base-commit: a89346e34a937f001e5d397ee62224e3e9852040
change-id: 20260605-fix-git-branch-regression-9e4236f18091
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
^ permalink raw reply related
* Re: [PATCH v3 1/3] commit-reach: handle cycles in contains walk
From: Tamir Duberstein @ 2026-06-12 21:26 UTC (permalink / raw)
To: Kristofer Karlsson
Cc: git, Jeff King, Karthik Nayak, Junio C Hamano, Victoria Dye,
Derrick Stolee, Elijah Newren
In-Reply-To: <CAL71e4PRqN9iPCzvgwC1Vtj-kzn4Udv+v1LTFSUXtGnC5KGrpA@mail.gmail.com>
On Fri, Jun 12, 2026 at 2:53 AM Kristofer Karlsson <krka@spotify.com> wrote:
>
> On Fri, 12 Jun 2026 at 05:00, Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > The memoized contains traversal used by git tag assumes that commit
> > ancestry is acyclic. Replacement refs can violate that assumption,
> > causing it to keep pushing an already active commit until memory is
> > exhausted.
> >
>
> The cycle detection itself makes sense, but would it be simpler to
> just die() when a cycle is found rather than falling back to a
> second reachability walk?
>
> A cycle in the commit graph means replacement refs are
> misconfigured. The existing code already loops forever when it
> hits one, so detecting and dying is strictly an improvement. The
> fallback adds a second codepath through the function, discards all
> cached results (so later candidates redo work), and papers over
> what is really a broken invariant.
>
> do_lookup_replace_object() already dies when replacement refs
> chain deeper than MAXREPLACEDEPTH (which covers cycles), so the
> existing contract treats this as a fatal configuration error.
> parse_commit_or_die() sets the same precedent within the walk
> itself.
Yes. The test creates an ancestry cycle through replacement commit
parents, so MAXREPLACEDEPTH does not catch this particular cycle. But I
agree with the design conclusion: the history is malformed and the
fallback only adds complexity.
Done in v4.
Thanks!
^ permalink raw reply
* Re: [PATCH v3] ref-filter: restore prefix-scoped iteration
From: Tamir Duberstein @ 2026-06-12 21:24 UTC (permalink / raw)
To: Patrick Steinhardt
Cc: git, Karthik Nayak, Junio C Hamano, Victoria Dye, ZheNing Hu
In-Reply-To: <aivx-7VOKE_TC50R@pks.im>
On Fri, Jun 12, 2026 at 7:48 AM Patrick Steinhardt <ps@pks.im> wrote:
>
> On Wed, Jun 10, 2026 at 05:29:49AM -0700, Tamir Duberstein wrote:
> > dabecb9db2 (for-each-ref: introduce a '--start-after' option,
> > 2025-07-15) changed branch, remote-tracking branch, and tag enumeration
> > from constructing an iterator with the namespace prefix to constructing
> > an unscoped iterator and seeking to the prefix.
> >
> > The files backend constructs its loose-ref iterator with cache priming
> > enabled. cache_ref_iterator_begin() immediately applies the construction
> > prefix through cache_ref_iterator_set_prefix(), reading loose refs
> > beneath it before packed refs are opened. An empty prefix therefore
> > reads every loose ref, and a later seek cannot undo that I/O.
> >
> > For these single-kind filters, construct the iterator with the namespace
> > prefix when start_after is not set. Keep the existing unscoped
> > construction for start_after, whose seek position may differ from the
> > namespace prefix.
> >
> > With 10,000 unrelated loose refs, the p6300 tests improve as follows:
> >
> > before after
> > branch 2.74 s 0.11 s
> > branch --remotes 2.81 s 0.12 s
> > tag 3.01 s 0.11 s
> >
> > Link: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
> > Link: https://lore.kernel.org/git/xmqqikjq7s16.fsf@gitster.g/
> > Link: https://lore.kernel.org/r/CAOLa=ZRHKNNymXGk31YgECjUmF9nZ8GsPUdQb7aKBH5DKMz7=w@mail.gmail.com
>
> I honestly have no idea what you want to say with these links, as they
> seem to just link to random reviews mails when the above mentioned
> commit was reviewed. In general, we typically try to embed references
> like this into the explanation, like:
>
> In [1], we discussed... and this is relevant because of ...
>
> [1]: https://lore.kernel.org/git/aGZidwwlToWThkn8@pks.im/
>
> Just dropping the links as-is without much of an explanation isn't
> helpful.
Will be numbered references in next spin.
>
> > diff --git a/ref-filter.c b/ref-filter.c
> > index 1da4c0e60d..9b04e3af85 100644
> > --- a/ref-filter.c
> > +++ b/ref-filter.c
> > @@ -3316,15 +3316,14 @@ static int do_filter_refs(struct ref_filter *filter, unsigned int type, refs_for
> >
> > if (prefix) {
> > struct ref_iterator *iter;
> > + struct ref_store *store = get_main_ref_store(the_repository);
> >
> > - iter = refs_ref_iterator_begin(get_main_ref_store(the_repository),
> > - "", NULL, 0, 0);
> > -
> > - if (filter->start_after)
> > + if (filter->start_after) {
> > + iter = refs_ref_iterator_begin(store, "", NULL, 0, 0);
> > ret = start_ref_iterator_after(iter, filter->start_after);
> > - else
> > - ret = ref_iterator_seek(iter, prefix,
> > - REF_ITERATOR_SEEK_SET_PREFIX);
> > + } else {
> > + iter = refs_ref_iterator_begin(store, prefix, NULL, 0, 0);
> > + }
> >
> > if (!ret)
> > ret = do_for_each_ref_iterator(iter, fn, cb_data);
>
> The patch itself seems sensible to me.
>
> > diff --git a/t/perf/p6300-for-each-ref.sh b/t/perf/p6300-for-each-ref.sh
> > index fa7289c752..ed9c1c6a19 100755
> > --- a/t/perf/p6300-for-each-ref.sh
> > +++ b/t/perf/p6300-for-each-ref.sh
> > @@ -1,6 +1,6 @@
> > #!/bin/sh
> >
> > -test_description='performance of for-each-ref'
> > +test_description='performance of ref-filter users'
> > . ./perf-lib.sh
> >
> > test_perf_fresh_repo
> > @@ -84,4 +84,41 @@ test_expect_success 'pack refs' '
> > '
> > run_tests "packed"
> >
> > +test_expect_success REFFILES 'setup many unrelated loose refs' '
> > + git init scoped &&
> > + test_commit -C scoped --no-tag base &&
> > + test_seq $ref_count_per_type |
> > + sed "s,.*,update refs/custom/unrelated_& HEAD," |
> > + git -C scoped update-ref --stdin &&
> > + git -C scoped update-ref refs/remotes/origin/main HEAD &&
> > + git -C scoped update-ref refs/tags/only HEAD
> > +'
>
> I've already called this out before on other patches, but the REFFILES
> prerequisite just doesn't make any sense here as this test logic is
> generic.
You're right. Removed in v4.
^ permalink raw reply
* Re: [PATCH v2] log: improve --follow following renames for non-linear history
From: Junio C Hamano @ 2026-06-12 20:10 UTC (permalink / raw)
To: Miklos Vajna; +Cc: Jeff King, git
In-Reply-To: <aipTOsH8LKTSwglj@collabora.com>
Miklos Vajna <vmiklos@collabora.com> writes:
> Documentation/config/log.adoc | 3 +-
> log-tree.c | 133 ++++++++++++++++++++++++++++++++++
> log-tree.h | 1 +
> revision.c | 2 +
> revision.h | 4 +
> t/meson.build | 1 +
> t/t4218-log-follow-merge.sh | 119 ++++++++++++++++++++++++++++++
t4218 seems to be taken by another topic in-flight, so this needs
renumbering.
^ permalink raw reply
* Re: [PATCH v4 06/16] midx: support custom `--base` for incremental MIDX writes
From: Taylor Blau @ 2026-06-12 20:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: SZEDER Gábor, git, Jeff King, Elijah Newren,
Patrick Steinhardt
In-Reply-To: <aixNXOxfPZnAVLgK@nand.local>
On Fri, Jun 12, 2026 at 02:18:04PM -0400, Taylor Blau wrote:
> On Fri, Jun 12, 2026 at 06:21:48AM -0700, Junio C Hamano wrote:
> > SZEDER Gábor <szeder.dev@gmail.com> writes:
> >
> > >> + layer="$(git multi-pack-index write --bitmap --incremental \
> > >> + --no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
> > >
> > > There is no 'nth_line' helper function in this test script.
> >
> > Good eyes. It has been there in the file next door t5335 since
> > February, but not available here in t5334.
>
> Good spotting indeed. Fortunately or unfortunately for us, pulling on
> this thread revealed a bit of a rabbit hole. Patches forthcoming..
https://lore.kernel.org/git/cover.1781294771.git.me@ttaylorr.com/
Thanks,
Taylor
^ permalink raw reply
* [PATCH 3/3] midx-write: include packs above custom incremental base
From: Taylor Blau @ 2026-06-12 20:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt,
SZEDER Gábor
In-Reply-To: <cover.1781294771.git.me@ttaylorr.com>
The previous commit made '--base' take effect on the normal incremental
write path, which exposed an existing assumption in our helper function
`should_include_pack()`, which is that any pack already present in
`ctx->m` was skipped.
That is only correct for non-incremental writes. For incremental writes,
`ctx->base_midx` is the boundary that should be excluded from the new
layer. If the caller selects an older base, or no base at all, then
packs from layers above that base have to be included in the detached
layer so that its bitmap has reachability closure.
Teach `should_include_pack()` to choose the MIDX used for pack exclusion
based on whether or not we are performing an incremental write. When
doing so, use `ctx->base_midx`, and use `ctx->m` otherwise.
The t5334 cases from the previous commit can now be marked as
successful.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
midx-write.c | 16 +++++++++++-----
t/t5334-incremental-multi-pack-index.sh | 4 ++--
2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/midx-write.c b/midx-write.c
index aa438775ebd..c50fdb5c6d1 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -133,8 +133,17 @@ static uint32_t midx_pack_perm(struct write_midx_context *ctx,
static int should_include_pack(const struct write_midx_context *ctx,
const char *file_name)
{
+ struct multi_pack_index *m = ctx->m;
/*
- * Note that at most one of ctx->m and ctx->to_include are set,
+ * When writing incrementally, ctx->m may contain layers above
+ * the selected base MIDX, which must be included in the new
+ * layer.
+ */
+ if (ctx->incremental)
+ m = ctx->base_midx;
+
+ /*
+ * Note that at most one of m and ctx->to_include are set,
* so we are testing midx_contains_pack() and
* string_list_has_string() independently (guarded by the
* appropriate NULL checks).
@@ -148,10 +157,7 @@ static int should_include_pack(const struct write_midx_context *ctx,
* should be performed independently (likely checking
* to_include before the existing MIDX).
*/
- if (ctx->m && midx_contains_pack(ctx->m, file_name))
- return 0;
- else if (ctx->base_midx && midx_contains_pack(ctx->base_midx,
- file_name))
+ if (m && midx_contains_pack(m, file_name))
return 0;
else if (ctx->to_include &&
!string_list_has_string(ctx->to_include, file_name))
diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index 69e96bf8d93..84ff6120978 100755
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -119,7 +119,7 @@ test_expect_success 'write MIDX layer with --base without --no-write-chain-file'
test_grep "cannot use --base without --no-write-chain-file" err
'
-test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file' '
+test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file' '
test_commit base-none &&
git repack -d &&
@@ -136,7 +136,7 @@ test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file
cp "$midx_chain.bak" "$midx_chain"
'
-test_expect_failure 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
+test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
test_commit base-hash &&
git repack -d &&
--
2.55.0.rc0.3.g7bf7c87b605
^ permalink raw reply related
* [PATCH 2/3] midx: pass custom '--base' through incremental writes
From: Taylor Blau @ 2026-06-12 20:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt,
SZEDER Gábor
In-Reply-To: <cover.1781294771.git.me@ttaylorr.com>
The 'multi-pack-index' builtin parses '--base' for incremental writes,
but the normal write path does not pass that value through to
`write_midx_file()`.
As a result, something like:
$ git multi-pack-index write --incremental --base=<base>
behaves as if no custom base had been given (unless the caller used the
'--stdin-packs' path).
Thread the parsed base through `write_midx_file()`, and update the
repack caller to pass NULL for the new argument where no custom base
selection is needed.
This exposes a pre-existing problem in incremental writes with custom
bases: the writer skips packs from the full existing MIDX chain, even
when the caller selected an older base or no base at all.
The affected t5334 cases fail while trying to write MIDX bitmaps. The
detached layer omits packs above the selected base, and thus the
resulting MIDX does not have a reachability closure, making it
impossible to generate reachability bitmaps.
Mark those tests as expected failures accordingly. The following commit
will fix the broken behavior and restore these tests.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
builtin/multi-pack-index.c | 3 ++-
builtin/repack.c | 2 +-
midx-write.c | 2 ++
midx.h | 2 +-
t/t5334-incremental-multi-pack-index.sh | 24 +++++++++++++++++++-----
5 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 00ffb36394d..949bfa796b2 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -224,7 +224,8 @@ static int cmd_multi_pack_index_write(int argc, const char **argv,
}
ret = write_midx_file(source, opts.preferred_pack,
- opts.refs_snapshot, opts.flags);
+ opts.refs_snapshot, opts.incremental_base,
+ opts.flags);
free(opts.refs_snapshot);
return ret;
diff --git a/builtin/repack.c b/builtin/repack.c
index 1524a9c13ad..0092a72a996 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -629,7 +629,7 @@ int cmd_repack(int argc,
unsigned flags = 0;
if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX_WRITE_INCREMENTAL, 0))
flags |= MIDX_WRITE_INCREMENTAL;
- write_midx_file(existing.source, NULL, NULL, flags);
+ write_midx_file(existing.source, NULL, NULL, NULL, flags);
}
cleanup:
diff --git a/midx-write.c b/midx-write.c
index 561e9eedc0e..aa438775ebd 100644
--- a/midx-write.c
+++ b/midx-write.c
@@ -1850,12 +1850,14 @@ static int write_midx_internal(struct write_midx_opts *opts)
int write_midx_file(struct odb_source *source,
const char *preferred_pack_name,
const char *refs_snapshot,
+ const char *incremental_base,
unsigned flags)
{
struct write_midx_opts opts = {
.source = source,
.preferred_pack_name = preferred_pack_name,
.refs_snapshot = refs_snapshot,
+ .incremental_base = incremental_base,
.flags = flags,
};
diff --git a/midx.h b/midx.h
index 63853a03a47..92ed29d913d 100644
--- a/midx.h
+++ b/midx.h
@@ -131,7 +131,7 @@ int prepare_multi_pack_index_one(struct odb_source *source);
*/
int write_midx_file(struct odb_source *source,
const char *preferred_pack_name, const char *refs_snapshot,
- unsigned flags);
+ const char *incremental_base, unsigned flags);
int write_midx_file_only(struct odb_source *source,
struct string_list *packs_to_include,
const char *preferred_pack_name,
diff --git a/t/t5334-incremental-multi-pack-index.sh b/t/t5334-incremental-multi-pack-index.sh
index 68a103d13d2..69e96bf8d93 100755
--- a/t/t5334-incremental-multi-pack-index.sh
+++ b/t/t5334-incremental-multi-pack-index.sh
@@ -119,7 +119,7 @@ test_expect_success 'write MIDX layer with --base without --no-write-chain-file'
test_grep "cannot use --base without --no-write-chain-file" err
'
-test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file' '
+test_expect_failure 'write MIDX layer with --base=none and --no-write-chain-file' '
test_commit base-none &&
git repack -d &&
@@ -128,19 +128,33 @@ test_expect_success 'write MIDX layer with --base=none and --no-write-chain-file
--no-write-chain-file --base=none)" &&
test_cmp "$midx_chain.bak" "$midx_chain" &&
- test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
+ test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
+
+ echo "$layer" >"$midx_chain" &&
+ test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
+ test_grep "^$(git rev-parse 2.2) " midx.objects &&
+ cp "$midx_chain.bak" "$midx_chain"
'
-test_expect_success 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
+test_expect_failure 'write MIDX layer with --base=<hash> and --no-write-chain-file' '
test_commit base-hash &&
git repack -d &&
cp "$midx_chain" "$midx_chain.bak" &&
+ base="$(nth_line 1 "$midx_chain")" &&
layer="$(git multi-pack-index write --bitmap --incremental \
- --no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
+ --no-write-chain-file --base="$base")" &&
test_cmp "$midx_chain.bak" "$midx_chain" &&
- test_path_is_file "$midxdir/multi-pack-index-$layer.midx"
+ test_path_is_file "$midxdir/multi-pack-index-$layer.midx" &&
+
+ {
+ echo "$base" &&
+ echo "$layer"
+ } >"$midx_chain" &&
+ test-tool read-midx --show-objects "$objdir" "$layer" >midx.objects &&
+ test_grep "^$(git rev-parse 2.2) " midx.objects &&
+ cp "$midx_chain.bak" "$midx_chain"
'
for reuse in false single multi
--
2.55.0.rc0.3.g7bf7c87b605
^ permalink raw reply related
* [PATCH 1/3] t5334: expose shared `nth_line()` helper
From: Taylor Blau @ 2026-06-12 20:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt,
SZEDER Gábor
In-Reply-To: <cover.1781294771.git.me@ttaylorr.com>
Since commit 0cd2255e64b (midx: support custom `--base` for incremental
MIDX writes, 2026-05-19), t5334 has referred to a non-existent helper
function 'nth_line', which is defined in t5335, but not here.
Move the helper to lib-midx.sh so that both tests can use the same
implementation. Ensure likewise that `nth_line()` remains visible from
within t5335 by sourcing lib-midx.sh there appropriately.
Curiously, t5334 passes both before and after this change. Before this
change, the failed command substitution leaves '--base' with an empty
value, and after this change, the custom base value is still ignored by
the normal incremental write path. The following commits will explain
and address that behavior.
Noticed-by: SZEDER Gábor <szeder.dev@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
t/lib-midx.sh | 6 ++++++
t/t5335-compact-multi-pack-index.sh | 7 +------
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/t/lib-midx.sh b/t/lib-midx.sh
index e38c609604c..b522dbdb0f4 100644
--- a/t/lib-midx.sh
+++ b/t/lib-midx.sh
@@ -34,3 +34,9 @@ compare_results_with_midx () {
midx_git_two_modes "cat-file --batch-all-objects --batch-check --unordered" sorted
'
}
+
+nth_line() {
+ local n="$1"
+ shift
+ awk "NR==$n" "$@"
+}
diff --git a/t/t5335-compact-multi-pack-index.sh b/t/t5335-compact-multi-pack-index.sh
index ec1dafe89fc..6a4b799b9c9 100755
--- a/t/t5335-compact-multi-pack-index.sh
+++ b/t/t5335-compact-multi-pack-index.sh
@@ -3,6 +3,7 @@
test_description='multi-pack-index compaction'
. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-midx.sh
GIT_TEST_MULTI_PACK_INDEX=0
GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=0
@@ -13,12 +14,6 @@ packdir=$objdir/pack
midxdir=$packdir/multi-pack-index.d
midx_chain=$midxdir/multi-pack-index-chain
-nth_line() {
- local n="$1"
- shift
- awk "NR==$n" "$@"
-}
-
write_packs () {
for c in "$@"
do
--
2.55.0.rc0.3.g7bf7c87b605
^ permalink raw reply related
* [PATCH 0/3] midx: honor custom bases for incremental writes
From: Taylor Blau @ 2026-06-12 20:07 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jeff King, Elijah Newren, Patrick Steinhardt,
SZEDER Gábor
SZEDER noticed[1] that t5334 was trying to call `nth_line()`, despite
that helper living only in t5335.
Fixing that should have made the test exercise `git multi-pack-index
write --incremental --base=...`. Instead, it uncovered another wrinkle,
which is that the normal MIDX write path parsed "--bases" without
actually passing it down to the MIDX writer.
This short series fixes both issues. It is structured as follows:
* The first patch moves `nth_line()` to lib-midx.sh so that t5334 and
t5335 use the same helper.
* The second patch threads the parsed `--base` value through
`write_midx_file()`, and consequently marks two t5334 cases as known
breakages.
* The final patch fixes the pack inclusion check and marks the tests
successful again.
The result is that `--base=none` and `--base=<hash>` now correctly
produce detached incremental layers that include any packs above the
selected base, preserving reachability closure for bitmaps.
Thanks in advance for your review!
[1]: https://lore.kernel.org/git/aiuaf3fKJ6kIITrf@szeder.dev/
Taylor Blau (3):
t5334: expose shared `nth_line()` helper
midx: pass custom '--base' through incremental writes
midx-write: include packs above custom incremental base
builtin/multi-pack-index.c | 3 ++-
builtin/repack.c | 2 +-
midx-write.c | 18 +++++++++++++-----
midx.h | 2 +-
t/lib-midx.sh | 6 ++++++
t/t5334-incremental-multi-pack-index.sh | 20 +++++++++++++++++---
t/t5335-compact-multi-pack-index.sh | 7 +------
7 files changed, 41 insertions(+), 17 deletions(-)
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
--
2.55.0.rc0.3.g7bf7c87b605
^ permalink raw reply
* Re: Automated reviews by AI (was Re: [PATCH 0/5] Duplicate entry hardening)
From: Junio C Hamano @ 2026-06-12 19:32 UTC (permalink / raw)
To: Christian Couder
Cc: Patrick Steinhardt, Elijah Newren via GitGitGadget, git,
Elijah Newren, Konstantin Ryabitsev, Taylor Blau
In-Reply-To: <CAP8UFD35cLP6FcEuPr+SghKae1ew4JWLWYAoMQ-fuEOu-JmZdg@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> On Tue, Jun 2, 2026 at 8:16 AM Patrick Steinhardt <ps@pks.im> wrote:
>>
>> Overall I agree that everyone who is a core contributor should also make
>> reviews part of their regular worflow. At least for corporate
>> contributors that might also make it easier to communicate this to their
>> respective employers. Regardless of that, my expectation is that there
>> will be times where it works well, and other times where it works less
>> well.
>
> Sashiko (https://github.com/sashiko-dev/sashiko) is used these days by
> Linux kernel developers and seems to work well for them.
>
> At GitLab and probably in other companies, some of us also use AI to
> review our work before sending it to the mailing list. And yeah, it
> helps find issues before our patches reach the mailing list.
>
> In the same way as we require that patches must pass CI, do we want to
> require that patches "pass" an AI review before they get accepted?
I do not think so. You (figuratively, not limited to Christian
Couder) are welcome to use whatever tool available to you to help
you polish your submission, and the higher quality your patches are
(e.g., fewer typos and jumps in logic flow that interferes the
thought process of human reviewers), the more helpful you are being
to the community. The use of GitHub PR initiated CI run falls into
the same category, I think, in that we do not require you to have an
account and trigger the CI there, but you are doing a good service
if you made sure you caught breakages on macOS you do not have
access to otherwise before sending your patches to the list.
But I do not think we should require you to bring your own token
budget to be able to contribute.
> The benefit would be that it would hopefully catch a lot of trivial
> things like indentation, typos/grammos, etc, and a lot of things a bit
> more difficult to spot like memory issues. Perhaps with some amount of
> prompting/configuration (for example pointing it at our
> CodingGuidelines and SubmittingPatches) it could also catch issues
> like style issues, commits that do too many things, refactoring
> opportunities, etc.
Yes.
Similarly, you are welcome to use tools including AI tools to help
you review others' patches, or help sanity check your reviews of
others' patches before you send them out. The reason why such an
effort is valuable to the community is the same.
But I personally consider that the use of the tools (not limited to
AI tools) is up to each developer. What counts a lot more is the
quality of the output. Just like PR driven CI at GitHub is offered
to everybody who wants to participate and is willing to have an
account there, it may help those aspiring developers if automated
review services are made easily available, but it is a different
story to _require_ use of such service.
^ permalink raw reply
* Re: [PATCH v3 2/2] compat/posix.h: simplify GIT_GNUC_PREREQ() comparison
From: Dominik Loidolt @ 2026-06-12 19:04 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, gitster, asedeno, asedeno, avarab
In-Reply-To: <aiwJSBfRbUFZ70gP@pks.im>
Thanks again for taking the time to review my contribution.
On Fri, Jun 12, 2026 at 03:27:36PM +0200, Patrick Steinhardt wrote:
> > It is also more future-proof, as it no longer assumes that GCC version
> > components stay below 65536.
>
> I feel like all the message needs to say is "let's do it for
> consistency, and it's easier to read". That would've been sufficient,
> whereas this argument here feels a bit thin.
Agreed. I'll simplify the commit message. The "future-proof" bit was a joke I
just couldn't resist, but it may cause more confusion than it is worth.
I'll drop it.
> It would've been nice to either move these changes into a preparatory
> commit or at least mention them
Agreed. I'll split the cleanup into a separate commit.
> I'm not sure myself whether this could use another reroll. It's all just
> nits, and the intent is clear enough.
I think it's worth rerolling.
Thanks,
Dominik
^ permalink raw reply
* [GSoC Patch v3 4/4] repo: add path.gitdir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-12 18:28 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kristofferhaugsbakk,
kumarayushjha123, lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260612182847.562816-1-jayatheerthkulkarni2005@gmail.com>
Scripts need a stable way to locate the git directory without
parsing rev-parse output or relying on its flag-driven path format
selection. There is no way to retrieve this path from git repo info
today.
Introduce path.gitdir.absolute and path.gitdir.relative keys,
consistent with the path.commondir keys added in the previous patch.
Reuse the test_repo_info_path helper introduced there to validate
both variants.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 6 ++++++
builtin/repo.c | 24 ++++++++++++++++++++++++
t/t1900-repo-info.sh | 7 +++++++
3 files changed, 37 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 890c34051d..ed7d80c690 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -113,6 +113,12 @@ values that they return:
The path to the Git repository's common directory relative to
the current working directory.
+`path.gitdir.absolute`::
+ The canonical absolute path to the Git repository directory (the `.git` directory).
+
+`path.gitdir.relative`::
+ The path to the Git repository directory relative to the current working directory.
+
`references.format`::
The reference storage format. The valid values are:
+
diff --git a/builtin/repo.c b/builtin/repo.c
index c4cc3bf3fc..9a312d127a 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -99,6 +99,28 @@ static int get_path_commondir_relative(struct repository *repo, struct strbuf *b
return 0;
}
+static int get_path_gitdir_absolute(struct repository *repo, struct strbuf *buf)
+{
+ const char *git_dir = repo_get_git_dir(repo);
+
+ if (!git_dir)
+ return error(_("unable to get git directory"));
+
+ append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
+static int get_path_gitdir_relative(struct repository *repo, struct strbuf *buf)
+{
+ const char *git_dir = repo_get_git_dir(repo);
+
+ if (!git_dir)
+ return error(_("unable to get git directory"));
+
+ append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
static int get_references_format(struct repository *repo, struct strbuf *buf)
{
strbuf_addstr(buf,
@@ -113,6 +135,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "object.format", get_object_format },
{ "path.commondir.absolute", get_path_commondir_absolute },
{ "path.commondir.relative", get_path_commondir_relative },
+ { "path.gitdir.absolute", get_path_gitdir_absolute },
+ { "path.gitdir.relative", get_path_gitdir_relative },
{ "references.format", get_references_format },
};
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 28fe76e25b..26acb5fe82 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -216,4 +216,11 @@ test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
'commondir-only-gitdir' '.git' '../.git' \
'GIT_DIR="../.git" && export GIT_DIR'
+test_repo_info_path 'gitdir standard' 'gitdir' 'gitdir-std' \
+ '.git' '../.git'
+
+test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
+ 'gitdir-env' '.git' '../.git' \
+ 'GIT_DIR="../.git" && export GIT_DIR'
+
test_done
--
2.54.0
^ permalink raw reply related
* [GSoC Patch v3 3/4] repo: add path.commondir with absolute and relative suffix formatting
From: K Jayatheerth @ 2026-06-12 18:28 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kristofferhaugsbakk,
kumarayushjha123, lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260612182847.562816-1-jayatheerthkulkarni2005@gmail.com>
Scripts working with worktree setups need a reliable way to discover
the common directory, which diverges from the git directory when
multiple worktrees are in use. There is no way to retrieve this path
from git repo info today.
Introduce path.commondir.absolute and path.commondir.relative keys.
Exposing explicit format variants rather than a single key with a
default avoids ambiguity for scripts that require predictable output.
Add a test helper test_repo_info_path that creates isolated
repositories per test case to prevent state leaks, captures the repo
root before changing directories to avoid eval, and accepts an optional
init_command to cover environment variable overrides such as
GIT_COMMON_DIR and GIT_DIR.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
Documentation/git-repo.adoc | 9 ++++++
builtin/repo.c | 26 ++++++++++++++++
t/t1900-repo-info.sh | 61 +++++++++++++++++++++++++++++++++++++
3 files changed, 96 insertions(+)
diff --git a/Documentation/git-repo.adoc b/Documentation/git-repo.adoc
index 42262c1983..890c34051d 100644
--- a/Documentation/git-repo.adoc
+++ b/Documentation/git-repo.adoc
@@ -104,6 +104,15 @@ values that they return:
`object.format`::
The object format (hash algorithm) used in the repository.
+`path.commondir.absolute`::
+ The canonical absolute path to the Git repository's common
+ directory (the shared `.git` directory containing objects,
+ refs, and global configuration).
+
+`path.commondir.relative`::
+ The path to the Git repository's common directory relative to
+ the current working directory.
+
`references.format`::
The reference storage format. The valid values are:
+
diff --git a/builtin/repo.c b/builtin/repo.c
index 71a5c1c29c..c4cc3bf3fc 100644
--- a/builtin/repo.c
+++ b/builtin/repo.c
@@ -7,12 +7,14 @@
#include "hex.h"
#include "odb.h"
#include "parse-options.h"
+#include "path.h"
#include "path-walk.h"
#include "progress.h"
#include "quote.h"
#include "ref-filter.h"
#include "refs.h"
#include "revision.h"
+#include "setup.h"
#include "strbuf.h"
#include "string-list.h"
#include "shallow.h"
@@ -75,6 +77,28 @@ static int get_object_format(struct repository *repo, struct strbuf *buf)
return 0;
}
+static int get_path_commondir_absolute(struct repository *repo, struct strbuf *buf)
+{
+ const char *common_dir = repo_get_common_dir(repo);
+
+ if (!common_dir)
+ return error(_("unable to get common directory"));
+
+ append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
+static int get_path_commondir_relative(struct repository *repo, struct strbuf *buf)
+{
+ const char *common_dir = repo_get_common_dir(repo);
+
+ if (!common_dir)
+ return error(_("unable to get common directory"));
+
+ append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
static int get_references_format(struct repository *repo, struct strbuf *buf)
{
strbuf_addstr(buf,
@@ -87,6 +111,8 @@ static const struct repo_info_field repo_info_field[] = {
{ "layout.bare", get_layout_bare },
{ "layout.shallow", get_layout_shallow },
{ "object.format", get_object_format },
+ { "path.commondir.absolute", get_path_commondir_absolute },
+ { "path.commondir.relative", get_path_commondir_relative },
{ "references.format", get_references_format },
};
diff --git a/t/t1900-repo-info.sh b/t/t1900-repo-info.sh
index 39bb77dda0..28fe76e25b 100755
--- a/t/t1900-repo-info.sh
+++ b/t/t1900-repo-info.sh
@@ -155,4 +155,65 @@ test_expect_success 'git repo info -h shows only repo info usage' '
test_grep ! "git repo structure" actual
'
+# Helper function to test path keys in both absolute and relative formats.
+# $1: label for the test
+# $2: field_name (e.g., commondir)
+# $3: unique repo name for isolation
+# $4: expect_absolute (suffix appended to repo root)
+# $5: expect_relative (the relative path string expected)
+# $6: init_command (extra setup like exporting env vars)
+test_repo_info_path () {
+ label=$1
+ field_name=$2
+ repo_name=$3
+ expect_absolute_suffix=$4
+ expect_relative=$5
+ init_command=$6
+
+ absolute_root="$repo_name-absolute"
+ relative_root="$repo_name-relative"
+
+ test_expect_success "setup: $label" '
+ git init "$absolute_root" &&
+ git init "$relative_root" &&
+ mkdir -p "$absolute_root/sub" "$relative_root/sub"
+ '
+
+ test_expect_success "absolute: $label" '
+ (
+ cd "$absolute_root/sub" &&
+ ROOT="$(test-tool path-utils real_path "..")" && export ROOT &&
+ eval "$init_command" &&
+ expect_path="$ROOT${expect_absolute_suffix:+/$expect_absolute_suffix}" &&
+ echo "path.$field_name.absolute=$expect_path" >expect &&
+ git repo info "path.$field_name.absolute" >actual &&
+ test_cmp expect actual
+ )
+ '
+
+ test_expect_success "relative: $label" '
+ (
+ cd "$relative_root/sub" &&
+ ROOT="$(test-tool path-utils real_path "..")" && export ROOT &&
+ eval "$init_command" &&
+ echo "path.$field_name.relative=$expect_relative" >expect &&
+ git repo info "path.$field_name.relative" >actual &&
+ test_cmp expect actual
+ )
+ '
+}
+
+test_repo_info_path 'commondir standard' 'commondir' 'commondir-std' \
+ '.git' '../.git'
+
+test_repo_info_path 'commondir with GIT_COMMON_DIR and GIT_DIR' 'commondir' \
+ 'commondir-envs' 'custom-common' '../custom-common' \
+ 'GIT_COMMON_DIR="$ROOT/custom-common" && export GIT_COMMON_DIR &&
+ GIT_DIR="../.git" && export GIT_DIR &&
+ git init --bare "$ROOT/custom-common"'
+
+test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
+ 'commondir-only-gitdir' '.git' '../.git' \
+ 'GIT_DIR="../.git" && export GIT_DIR'
+
test_done
--
2.54.0
^ permalink raw reply related
* [GSoC Patch v3 2/4] rev-parse: use append_formatted_path() for path formatting
From: K Jayatheerth @ 2026-06-12 18:28 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kristofferhaugsbakk,
kumarayushjha123, lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260612182847.562816-1-jayatheerthkulkarni2005@gmail.com>
Now that path formatting logic lives in a shared helper, keeping a
duplicate implementation in rev-parse is unnecessary and risks the
two diverging over time.
Replace the local format_type and default_type enums and the
hand-rolled formatting logic with a call to append_formatted_path().
Introduce PATH_FORMAT_DEFAULT as the initial value of arg_path_format
so that per-path fallback behavior is resolved in print_path() rather
than leaked into the shared helper.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
builtin/rev-parse.c | 103 ++++++++++----------------------------------
1 file changed, 23 insertions(+), 80 deletions(-)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 218b5f34d6..2dd35361f3 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -632,73 +632,16 @@ static void handle_ref_opt(const char *pattern, const char *prefix)
clear_ref_exclusions(&ref_excludes);
}
-enum format_type {
- /* We would like a relative path. */
- FORMAT_RELATIVE,
- /* We would like a canonical absolute path. */
- FORMAT_CANONICAL,
- /* We would like the default behavior. */
- FORMAT_DEFAULT,
-};
-
-enum default_type {
- /* Our default is a relative path. */
- DEFAULT_RELATIVE,
- /* Our default is a relative path if there's a shared root. */
- DEFAULT_RELATIVE_IF_SHARED,
- /* Our default is a canonical absolute path. */
- DEFAULT_CANONICAL,
- /* Our default is not to modify the item. */
- DEFAULT_UNMODIFIED,
-};
-
-static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def)
+static void print_path(const char *path, const char *prefix,
+ enum path_format arg_path_format, enum path_format def_format)
{
- char *cwd = NULL;
- /*
- * We don't ever produce a relative path if prefix is NULL, so set the
- * prefix to the current directory so that we can produce a relative
- * path whenever possible. If we're using RELATIVE_IF_SHARED mode, then
- * we want an absolute path unless the two share a common prefix, so don't
- * set it in that case, since doing so causes a relative path to always
- * be produced if possible.
- */
- if (!prefix && (format != FORMAT_DEFAULT || def != DEFAULT_RELATIVE_IF_SHARED))
- prefix = cwd = xgetcwd();
- if (format == FORMAT_DEFAULT && def == DEFAULT_UNMODIFIED) {
- puts(path);
- } else if (format == FORMAT_RELATIVE ||
- (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE)) {
- /*
- * In order for relative_path to work as expected, we need to
- * make sure that both paths are absolute paths. If we don't,
- * we can end up with an unexpected absolute path that the user
- * didn't want.
- */
- struct strbuf buf = STRBUF_INIT, realbuf = STRBUF_INIT, prefixbuf = STRBUF_INIT;
- if (!is_absolute_path(path)) {
- strbuf_realpath_forgiving(&realbuf, path, 1);
- path = realbuf.buf;
- }
- if (!is_absolute_path(prefix)) {
- strbuf_realpath_forgiving(&prefixbuf, prefix, 1);
- prefix = prefixbuf.buf;
- }
- puts(relative_path(path, prefix, &buf));
- strbuf_release(&buf);
- strbuf_release(&realbuf);
- strbuf_release(&prefixbuf);
- } else if (format == FORMAT_DEFAULT && def == DEFAULT_RELATIVE_IF_SHARED) {
- struct strbuf buf = STRBUF_INIT;
- puts(relative_path(path, prefix, &buf));
- strbuf_release(&buf);
- } else {
- struct strbuf buf = STRBUF_INIT;
- strbuf_realpath_forgiving(&buf, path, 1);
- puts(buf.buf);
- strbuf_release(&buf);
- }
- free(cwd);
+ struct strbuf sb = STRBUF_INIT;
+ enum path_format fmt = (arg_path_format != PATH_FORMAT_DEFAULT) ? arg_path_format : def_format;
+
+ append_formatted_path(&sb, path, prefix, fmt);
+ puts(sb.buf);
+
+ strbuf_release(&sb);
}
int cmd_rev_parse(int argc,
@@ -717,7 +660,7 @@ int cmd_rev_parse(int argc,
const char *name = NULL;
struct strbuf buf = STRBUF_INIT;
int seen_end_of_options = 0;
- enum format_type format = FORMAT_DEFAULT;
+ enum path_format arg_path_format = PATH_FORMAT_DEFAULT;
show_usage_if_asked(argc, argv, builtin_rev_parse_usage);
@@ -797,8 +740,8 @@ int cmd_rev_parse(int argc,
die(_("--git-path requires an argument"));
print_path(repo_git_path_replace(the_repository, &buf,
"%s", argv[i + 1]), prefix,
- format,
- DEFAULT_RELATIVE_IF_SHARED);
+ arg_path_format,
+ PATH_FORMAT_RELATIVE_IF_SHARED);
i++;
continue;
}
@@ -820,9 +763,9 @@ int cmd_rev_parse(int argc,
if (!arg)
die(_("--path-format requires an argument"));
if (!strcmp(arg, "absolute")) {
- format = FORMAT_CANONICAL;
+ arg_path_format = PATH_FORMAT_CANONICAL;
} else if (!strcmp(arg, "relative")) {
- format = FORMAT_RELATIVE;
+ arg_path_format = PATH_FORMAT_RELATIVE;
} else {
die(_("unknown argument to --path-format: %s"), arg);
}
@@ -985,7 +928,7 @@ int cmd_rev_parse(int argc,
if (!strcmp(arg, "--show-toplevel")) {
const char *work_tree = repo_get_work_tree(the_repository);
if (work_tree)
- print_path(work_tree, prefix, format, DEFAULT_UNMODIFIED);
+ print_path(work_tree, prefix, arg_path_format, PATH_FORMAT_UNMODIFIED);
else
die(_("this operation must be run in a work tree"));
continue;
@@ -993,7 +936,7 @@ int cmd_rev_parse(int argc,
if (!strcmp(arg, "--show-superproject-working-tree")) {
struct strbuf superproject = STRBUF_INIT;
if (get_superproject_working_tree(&superproject))
- print_path(superproject.buf, prefix, format, DEFAULT_UNMODIFIED);
+ print_path(superproject.buf, prefix, arg_path_format, PATH_FORMAT_UNMODIFIED);
strbuf_release(&superproject);
continue;
}
@@ -1028,18 +971,18 @@ int cmd_rev_parse(int argc,
const char *gitdir = getenv(GIT_DIR_ENVIRONMENT);
char *cwd;
int len;
- enum format_type wanted = format;
+ enum path_format wanted = arg_path_format;
if (arg[2] == 'g') { /* --git-dir */
if (gitdir) {
- print_path(gitdir, prefix, format, DEFAULT_UNMODIFIED);
+ print_path(gitdir, prefix, arg_path_format, PATH_FORMAT_UNMODIFIED);
continue;
}
if (!prefix) {
- print_path(".git", prefix, format, DEFAULT_UNMODIFIED);
+ print_path(".git", prefix, arg_path_format, PATH_FORMAT_UNMODIFIED);
continue;
}
} else { /* --absolute-git-dir */
- wanted = FORMAT_CANONICAL;
+ wanted = PATH_FORMAT_CANONICAL;
if (!gitdir && !prefix)
gitdir = ".git";
if (gitdir) {
@@ -1055,11 +998,11 @@ int cmd_rev_parse(int argc,
strbuf_reset(&buf);
strbuf_addf(&buf, "%s%s.git", cwd, len && cwd[len-1] != '/' ? "/" : "");
free(cwd);
- print_path(buf.buf, prefix, wanted, DEFAULT_CANONICAL);
+ print_path(buf.buf, prefix, wanted, PATH_FORMAT_CANONICAL);
continue;
}
if (!strcmp(arg, "--git-common-dir")) {
- print_path(repo_get_common_dir(the_repository), prefix, format, DEFAULT_RELATIVE_IF_SHARED);
+ print_path(repo_get_common_dir(the_repository), prefix, arg_path_format, PATH_FORMAT_RELATIVE_IF_SHARED);
continue;
}
if (!strcmp(arg, "--is-inside-git-dir")) {
@@ -1089,7 +1032,7 @@ int cmd_rev_parse(int argc,
if (the_repository->index->split_index) {
const struct object_id *oid = &the_repository->index->split_index->base_oid;
const char *path = repo_git_path_replace(the_repository, &buf, "sharedindex.%s", oid_to_hex(oid));
- print_path(path, prefix, format, DEFAULT_RELATIVE);
+ print_path(path, prefix, arg_path_format, PATH_FORMAT_RELATIVE);
}
continue;
}
--
2.54.0
^ permalink raw reply related
* [GSoC Patch v3 1/4] path: introduce append_formatted_path() for shared path formatting
From: K Jayatheerth @ 2026-06-12 18:28 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kristofferhaugsbakk,
kumarayushjha123, lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260612182847.562816-1-jayatheerthkulkarni2005@gmail.com>
The path-formatting logic in builtin/rev-parse.c is tightly coupled
to that command and writes directly to stdout, making it impossible
for other builtins to reuse.
Extract the core algorithm into append_formatted_path() in path.c
and expose a path_format enum in path.h so that any builtin can
format paths consistently without duplicating logic.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
path.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
path.h | 36 ++++++++++++++++++++++++++++++
2 files changed, 106 insertions(+)
diff --git a/path.c b/path.c
index d7e17bf174..5e83e3e4f6 100644
--- a/path.c
+++ b/path.c
@@ -1579,6 +1579,76 @@ char *xdg_cache_home(const char *filename)
return NULL;
}
+void append_formatted_path(struct strbuf *dest, const char *path,
+ const char *prefix, enum path_format format)
+{
+ switch (format) {
+ case PATH_FORMAT_DEFAULT:
+ case PATH_FORMAT_UNMODIFIED:
+ strbuf_addstr(dest, path);
+ break;
+
+ case PATH_FORMAT_RELATIVE: {
+ struct strbuf relative_buf = STRBUF_INIT;
+ struct strbuf real_path = STRBUF_INIT;
+ struct strbuf real_prefix = STRBUF_INIT;
+ char *cwd = NULL;
+
+ /*
+ * We don't ever produce a relative path if prefix is NULL,
+ * so set the prefix to the current directory so that we can
+ * produce a relative path whenever possible.
+ */
+ if (!prefix)
+ prefix = cwd = xgetcwd();
+
+ if (!is_absolute_path(path)) {
+ strbuf_realpath_forgiving(&real_path, path, 1);
+ path = real_path.buf;
+ }
+ if (!is_absolute_path(prefix)) {
+ strbuf_realpath_forgiving(&real_prefix, prefix, 1);
+ prefix = real_prefix.buf;
+ }
+
+ strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
+
+ strbuf_release(&relative_buf);
+ strbuf_release(&real_path);
+ strbuf_release(&real_prefix);
+ free(cwd);
+ break;
+ }
+
+ case PATH_FORMAT_RELATIVE_IF_SHARED: {
+ struct strbuf relative_buf = STRBUF_INIT;
+
+ /*
+ * If we're using RELATIVE_IF_SHARED mode, then we want an
+ * absolute path unless the two share a common prefix, so don't
+ * default the prefix to the current working directory. Doing so
+ * would cause a relative path to always be produced if possible.
+ */
+ strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
+ strbuf_release(&relative_buf);
+ break;
+ }
+
+ case PATH_FORMAT_CANONICAL: {
+ struct strbuf canonical_buf = STRBUF_INIT;
+
+ strbuf_realpath_forgiving(&canonical_buf, path, 1);
+ strbuf_addbuf(dest, &canonical_buf);
+
+ strbuf_release(&canonical_buf);
+ break;
+ }
+
+ default:
+ BUG("unknown path_format value %d", format);
+ }
+}
+
REPO_GIT_PATH_FUNC(squash_msg, "SQUASH_MSG")
REPO_GIT_PATH_FUNC(merge_msg, "MERGE_MSG")
REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR")
diff --git a/path.h b/path.h
index 0434ba5e07..6aca53b100 100644
--- a/path.h
+++ b/path.h
@@ -262,6 +262,42 @@ enum scld_error safe_create_leading_directories_no_share(char *path);
int safe_create_file_with_leading_directories(struct repository *repo,
const char *path);
+/**
+ * The formatting strategy to apply when writing a path into a buffer.
+ */
+enum path_format {
+ /*
+ * Represents the default formatting behavior. Treated as
+ * PATH_FORMAT_UNMODIFIED by append_formatted_path().
+ */
+ PATH_FORMAT_DEFAULT,
+
+ /* Output the path exactly as-is without any modifications. */
+ PATH_FORMAT_UNMODIFIED,
+
+ /* Output a path relative to the provided directory prefix. */
+ PATH_FORMAT_RELATIVE,
+
+ /* Output a relative path only if the path shares a root with the prefix. */
+ PATH_FORMAT_RELATIVE_IF_SHARED,
+
+ /* Output a fully resolved, absolute canonical path. */
+ PATH_FORMAT_CANONICAL
+};
+
+/**
+ * Format a path according to the specified formatting strategy and append
+ * the result to the given strbuf.
+ *
+ * `dest` : The string buffer to append the formatted path to.
+ * `path` : The path string that needs to be formatted.
+ * `prefix` : The directory prefix to calculate relative offsets against.
+ * Pass NULL to default to the current working directory where applicable.
+ * `format` : The formatting behavior rule to execute.
+ */
+void append_formatted_path(struct strbuf *dest, const char *path,
+ const char *prefix, enum path_format format);
+
# ifdef USE_THE_REPOSITORY_VARIABLE
# include "strbuf.h"
# include "repository.h"
--
2.54.0
^ permalink raw reply related
* [GSoC Patch v3 0/4] teach git repo info to handle path keys
From: K Jayatheerth @ 2026-06-12 18:28 UTC (permalink / raw)
To: jayatheerthkulkarni2005
Cc: a3205153416, git, gitster, jltobler, kristofferhaugsbakk,
kumarayushjha123, lucasseikioshiro, phillip.wood, sandals
In-Reply-To: <20260601151950.30686-1-jayatheerthkulkarni2005@gmail.com>
Hi!
This series teaches `git repo info` to handle `path.*`
keys, allowing scripts to reliably discover core
repository paths without resorting to `git rev-parse`.
The patches are structured as follows:
1. path: Extract the localized path-formatting logic
out of `rev-parse` and expose it globally via
`path.h` using clear append semantics.
2. rev-parse: Refactor the command to leverage the
newly shared path engine.
3. repo: Introduce `path.commondir.absolute` and
`path.commondir.relative` alongside a robust,
isolated test helper.
4. repo: Introduce `path.gitdir.absolute` and
`path.gitdir.relative` using the same standardized
formatting rules.
Since all the questions were answered
I have removed them from this cover letter.
Changes since v2:
* Renamed the shared helper from `format_path()` to
`append_formatted_path()`, and renamed the `buf`
parameter to `dest` to better reflect its
append-style behavior (Lucas, Phillip).
* Introduced a dedicated `PATH_FORMAT_DEFAULT`
enumerator. This removes the awkward `-1`
sentinel in `print_path()` while preserving enum
type safety (Phillip, Justin).
* Handled `PATH_FORMAT_DEFAULT` as
`PATH_FORMAT_UNMODIFIED` inside
`append_formatted_path()`, while intercepting it
in `print_path()` for rev-parse-specific fallback
behavior (Justin).
* Replaced the `else if` chain in `append_formatted_path()` with a
clean `switch` statement setup (Justin, Lucas).
* Reordered the `commondir` and `gitdir` patches so
the parameterized test helper
(`test_repo_info_path`) is introduced first,
establishing the isolated test infrastructure up
front (Justin).
* Reworked the test helper to accept a label,
`repo_name`, and `path_suffix`; moved repository
creation into the helper for isolation; and
replaced `eval` by capturing `$PWD` before
changing directories (Justin, Lucas).
* Corrected trailer ordering so `Signed-off-by`
appears after `Mentored-by` (Kristoffer).
* Cleaned up minor trailing whitespace issues across
the patch array declarations.
Tagging Justin Tobler, Lucas Seiki Oshiro, Junio,
Phillip Wood, brian m. carlson, and Ayush Jha.
Thanks for taking another look!
K Jayatheerth (4):
path: introduce append_formatted_path() for shared path formatting
rev-parse: use append_formatted_path() for path formatting
repo: add path.commondir with absolute and relative suffix formatting
repo: add path.gitdir with absolute and relative suffix formatting
Documentation/git-repo.adoc | 15 ++++++
builtin/repo.c | 50 +++++++++++++++++
builtin/rev-parse.c | 103 ++++++++----------------------------
path.c | 70 ++++++++++++++++++++++++
path.h | 36 +++++++++++++
t/t1900-repo-info.sh | 68 ++++++++++++++++++++++++
6 files changed, 262 insertions(+), 80 deletions(-)
Range-diff against v2:
1: c1f1e87fe9 ! 1: a396b4f8e6 path: introduce format_path() for centralized path formatting
@@ Metadata
Author: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
## Commit message ##
- path: introduce format_path() for centralized path formatting
+ path: introduce append_formatted_path() for shared path formatting
- The path-formatting logic inside `builtin/rev-parse.c` handles absolute,
- canonical, and relative formatting rules based on user-supplied options.
- However, this logic is tightly coupled to `rev-parse` and writes directly
- to stdout.
+ The path-formatting logic in builtin/rev-parse.c is tightly coupled
+ to that command and writes directly to stdout, making it impossible
+ for other builtins to reuse.
- To allow other builtins (such as the upcoming `git repo` path keys) to
- re-use this logic, extract the core path-formatting algorithm into a centralized
- helper function, `format_path()`, in `path.c`.
-
- Expose a single, streamlined `path_format` enum in `path.h` to let callers
- explicitly declare their formatting strategy (UNMODIFIED, RELATIVE,
- RELATIVE_IF_SHARED, or CANONICAL). This decouples the core algorithm from
- the localized fallback mechanics specific to `rev-parse`.
+ Extract the core algorithm into append_formatted_path() in path.c
+ and expose a path_format enum in path.h so that any builtin can
+ format paths consistently without duplicating logic.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
@@ path.c: char *xdg_cache_home(const char *filename)
return NULL;
}
-+void format_path(struct strbuf *buf, const char *path,
-+ const char *prefix, enum path_format format)
++void append_formatted_path(struct strbuf *dest, const char *path,
++ const char *prefix, enum path_format format)
+{
-+ if (format == PATH_FORMAT_UNMODIFIED) {
-+ strbuf_addstr(buf, path);
-+ return;
-+ }
++ switch (format) {
++ case PATH_FORMAT_DEFAULT:
++ case PATH_FORMAT_UNMODIFIED:
++ strbuf_addstr(dest, path);
++ break;
+
-+ if (format == PATH_FORMAT_RELATIVE) {
++ case PATH_FORMAT_RELATIVE: {
+ struct strbuf relative_buf = STRBUF_INIT;
+ struct strbuf real_path = STRBUF_INIT;
+ struct strbuf real_prefix = STRBUF_INIT;
@@ path.c: char *xdg_cache_home(const char *filename)
+ prefix = real_prefix.buf;
+ }
+
-+ strbuf_addstr(buf, relative_path(path, prefix, &relative_buf));
++ strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
+
+ strbuf_release(&relative_buf);
+ strbuf_release(&real_path);
+ strbuf_release(&real_prefix);
+ free(cwd);
-+ } else if (format == PATH_FORMAT_RELATIVE_IF_SHARED) {
++ break;
++ }
++
++ case PATH_FORMAT_RELATIVE_IF_SHARED: {
+ struct strbuf relative_buf = STRBUF_INIT;
+
+ /*
@@ path.c: char *xdg_cache_home(const char *filename)
+ * default the prefix to the current working directory. Doing so
+ * would cause a relative path to always be produced if possible.
+ */
-+ strbuf_addstr(buf, relative_path(path, prefix, &relative_buf));
++ strbuf_addstr(dest, relative_path(path, prefix, &relative_buf));
+ strbuf_release(&relative_buf);
-+ } else if (format == PATH_FORMAT_CANONICAL) {
++ break;
++ }
++
++ case PATH_FORMAT_CANONICAL: {
+ struct strbuf canonical_buf = STRBUF_INIT;
+
+ strbuf_realpath_forgiving(&canonical_buf, path, 1);
-+ strbuf_addbuf(buf, &canonical_buf);
++ strbuf_addbuf(dest, &canonical_buf);
+
+ strbuf_release(&canonical_buf);
++ break;
++ }
++
++ default:
++ BUG("unknown path_format value %d", format);
+ }
+}
+
@@ path.h: enum scld_error safe_create_leading_directories_no_share(char *path);
+ * The formatting strategy to apply when writing a path into a buffer.
+ */
+enum path_format {
++ /*
++ * Represents the default formatting behavior. Treated as
++ * PATH_FORMAT_UNMODIFIED by append_formatted_path().
++ */
++ PATH_FORMAT_DEFAULT,
++
+ /* Output the path exactly as-is without any modifications. */
+ PATH_FORMAT_UNMODIFIED,
+
@@ path.h: enum scld_error safe_create_leading_directories_no_share(char *path);
+ * Format a path according to the specified formatting strategy and append
+ * the result to the given strbuf.
+ *
-+ * `buf` : The string buffer to append the formatted path to.
++ * `dest` : The string buffer to append the formatted path to.
+ * `path` : The path string that needs to be formatted.
+ * `prefix` : The directory prefix to calculate relative offsets against.
+ * Pass NULL to default to the current working directory where applicable.
+ * `format` : The formatting behavior rule to execute.
+ */
-+void format_path(struct strbuf *buf, const char *path,
-+ const char *prefix, enum path_format format);
++void append_formatted_path(struct strbuf *dest, const char *path,
++ const char *prefix, enum path_format format);
+
# ifdef USE_THE_REPOSITORY_VARIABLE
# include "strbuf.h"
2: 2cc3e671af ! 2: 16198f96d1 rev-parse: use format_path for path formatting
@@ Metadata
Author: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
## Commit message ##
- rev-parse: use format_path for path formatting
+ rev-parse: use append_formatted_path() for path formatting
- Now that the core path-formatting logic has been abstracted into
- format_path() inside path.c, remove the localized duplicate formatting
- mechanics from builtin/rev-parse.c.
+ Now that path formatting logic lives in a shared helper, keeping a
+ duplicate implementation in rev-parse is unnecessary and risks the
+ two diverging over time.
- Drop the usage of the old local format_type and default_type enums,
- and update print_path() to act as a light wrapper around the new shared
- engine. Resolve user-provided formatting flags directly within rev-parse
- to pass the final determined path_format to format_path().
+ Replace the local format_type and default_type enums and the
+ hand-rolled formatting logic with a call to append_formatted_path().
+ Introduce PATH_FORMAT_DEFAULT as the initial value of arg_path_format
+ so that per-path fallback behavior is resolved in print_path() rather
+ than leaked into the shared helper.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
@@ builtin/rev-parse.c: static void handle_ref_opt(const char *pattern, const char
-
-static void print_path(const char *path, const char *prefix, enum format_type format, enum default_type def)
+static void print_path(const char *path, const char *prefix,
-+ int arg_path_format, enum path_format def_format)
++ enum path_format arg_path_format, enum path_format def_format)
{
- char *cwd = NULL;
- /*
@@ builtin/rev-parse.c: static void handle_ref_opt(const char *pattern, const char
- }
- free(cwd);
+ struct strbuf sb = STRBUF_INIT;
-+ enum path_format fmt = (arg_path_format != -1) ? arg_path_format : def_format;
++ enum path_format fmt = (arg_path_format != PATH_FORMAT_DEFAULT) ? arg_path_format : def_format;
+
-+ format_path(&sb, path, prefix, fmt);
++ append_formatted_path(&sb, path, prefix, fmt);
+ puts(sb.buf);
+
+ strbuf_release(&sb);
@@ builtin/rev-parse.c: int cmd_rev_parse(int argc,
struct strbuf buf = STRBUF_INIT;
int seen_end_of_options = 0;
- enum format_type format = FORMAT_DEFAULT;
-+ int arg_path_format = -1;
++ enum path_format arg_path_format = PATH_FORMAT_DEFAULT;
show_usage_if_asked(argc, argv, builtin_rev_parse_usage);
@@ builtin/rev-parse.c: int cmd_rev_parse(int argc,
char *cwd;
int len;
- enum format_type wanted = format;
-+ int wanted = arg_path_format;
++ enum path_format wanted = arg_path_format;
if (arg[2] == 'g') { /* --git-dir */
if (gitdir) {
- print_path(gitdir, prefix, format, DEFAULT_UNMODIFIED);
4: 61b5d69306 ! 3: 7de41faa04 repo: add path.commondir with absolute and relative suffix formatting
@@ Metadata
## Commit message ##
repo: add path.commondir with absolute and relative suffix formatting
- In standard Git repositories, the Git directory and the common directory
- are identical. However, in environments utilizing multiple worktrees, the
- local working state ($GIT_DIR) is separated from the shared central data
- ($GIT_COMMON_DIR). Scripts require a reliable way to discover this shared
- path.
+ Scripts working with worktree setups need a reliable way to discover
+ the common directory, which diverges from the git directory when
+ multiple worktrees are in use. There is no way to retrieve this path
+ from git repo info today.
- Introduce `path.commondir.absolute` and `path.commondir.relative` keys
- to `git repo info`. Similar to the `path.gitdir` keys, exposing explicit
- format variants removes the ambiguity of default fallbacks. Both keys are
- evaluated via the `format_path()` engine.
+ Introduce path.commondir.absolute and path.commondir.relative keys.
+ Exposing explicit format variants rather than a single key with a
+ default avoids ambiguity for scripts that require predictable output.
- Insert the new keys into the `repo_info_field` array in lexicographical
- order to maintain the integrity of binary search lookups.
-
- Utilize the parameterized `test_repo_info_path` helper to validate the
- worktree edge cases. This ensures that path resolution correctly respects
- $GIT_COMMON_DIR when defined and safely falls back to $GIT_DIR otherwise.
+ Add a test helper test_repo_info_path that creates isolated
+ repositories per test case to prevent state leaks, captures the repo
+ root before changing directories to avoid eval, and accepts an optional
+ init_command to cover environment variable overrides such as
+ GIT_COMMON_DIR and GIT_DIR.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
@@ Documentation/git-repo.adoc: values that they return:
+ The path to the Git repository's common directory relative to
+ the current working directory.
+
- `path.gitdir.absolute`::
- The canonical absolute path to the Git repository directory (the `.git` directory).
-
+ `references.format`::
+ The reference storage format. The valid values are:
+ +
## builtin/repo.c ##
+@@
+ #include "hex.h"
+ #include "odb.h"
+ #include "parse-options.h"
++#include "path.h"
+ #include "path-walk.h"
+ #include "progress.h"
+ #include "quote.h"
+ #include "ref-filter.h"
+ #include "refs.h"
+ #include "revision.h"
++#include "setup.h"
+ #include "strbuf.h"
+ #include "string-list.h"
+ #include "shallow.h"
@@ builtin/repo.c: static int get_object_format(struct repository *repo, struct strbuf *buf)
return 0;
}
@@ builtin/repo.c: static int get_object_format(struct repository *repo, struct str
+ if (!common_dir)
+ return error(_("unable to get common directory"));
+
-+ format_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
++ append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
@@ builtin/repo.c: static int get_object_format(struct repository *repo, struct str
+ if (!common_dir)
+ return error(_("unable to get common directory"));
+
-+ format_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
++ append_formatted_path(buf, common_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
- static int get_path_gitdir_absolute(struct repository *repo, struct strbuf *buf)
+ static int get_references_format(struct repository *repo, struct strbuf *buf)
{
- const char *git_dir = repo_get_git_dir(repo);
+ strbuf_addstr(buf,
@@ builtin/repo.c: static const struct repo_info_field repo_info_field[] = {
{ "layout.bare", get_layout_bare },
{ "layout.shallow", get_layout_shallow },
{ "object.format", get_object_format },
+ { "path.commondir.absolute", get_path_commondir_absolute },
+ { "path.commondir.relative", get_path_commondir_relative },
- { "path.gitdir.absolute", get_path_gitdir_absolute },
- { "path.gitdir.relative", get_path_gitdir_relative },
{ "references.format", get_references_format },
+ };
+
## t/t1900-repo-info.sh ##
-@@ t/t1900-repo-info.sh: test_expect_success 'setup test repository layout for path fields' '
- mkdir -p test-repo/sub
+@@ t/t1900-repo-info.sh: test_expect_success 'git repo info -h shows only repo info usage' '
+ test_grep ! "git repo structure" actual
'
-+test_expect_success 'setup custom-common for commondir tests' '
-+ git init --bare test-repo/custom-common
-+'
-+
-+test_repo_info_path 'commondir' 'echo "$(cd .. && pwd)/.git"' '../.git'
-+test_repo_info_path 'commondir' 'echo "$(cd .. && pwd)/custom-common"' '../custom-common' 'GIT_COMMON_DIR="$(cd .. && pwd)/custom-common" GIT_DIR=../.git'
-+test_repo_info_path 'commondir' 'echo "$(cd .. && pwd)/.git"' '../.git' 'GIT_DIR=../.git'
- test_repo_info_path 'gitdir' 'echo "$(cd .. && pwd)/.git"' '../.git'
-
++# Helper function to test path keys in both absolute and relative formats.
++# $1: label for the test
++# $2: field_name (e.g., commondir)
++# $3: unique repo name for isolation
++# $4: expect_absolute (suffix appended to repo root)
++# $5: expect_relative (the relative path string expected)
++# $6: init_command (extra setup like exporting env vars)
++test_repo_info_path () {
++ label=$1
++ field_name=$2
++ repo_name=$3
++ expect_absolute_suffix=$4
++ expect_relative=$5
++ init_command=$6
++
++ absolute_root="$repo_name-absolute"
++ relative_root="$repo_name-relative"
++
++ test_expect_success "setup: $label" '
++ git init "$absolute_root" &&
++ git init "$relative_root" &&
++ mkdir -p "$absolute_root/sub" "$relative_root/sub"
++ '
++
++ test_expect_success "absolute: $label" '
++ (
++ cd "$absolute_root/sub" &&
++ ROOT="$(test-tool path-utils real_path "..")" && export ROOT &&
++ eval "$init_command" &&
++ expect_path="$ROOT${expect_absolute_suffix:+/$expect_absolute_suffix}" &&
++ echo "path.$field_name.absolute=$expect_path" >expect &&
++ git repo info "path.$field_name.absolute" >actual &&
++ test_cmp expect actual
++ )
++ '
++
++ test_expect_success "relative: $label" '
++ (
++ cd "$relative_root/sub" &&
++ ROOT="$(test-tool path-utils real_path "..")" && export ROOT &&
++ eval "$init_command" &&
++ echo "path.$field_name.relative=$expect_relative" >expect &&
++ git repo info "path.$field_name.relative" >actual &&
++ test_cmp expect actual
++ )
++ '
++}
++
++test_repo_info_path 'commondir standard' 'commondir' 'commondir-std' \
++ '.git' '../.git'
++
++test_repo_info_path 'commondir with GIT_COMMON_DIR and GIT_DIR' 'commondir' \
++ 'commondir-envs' 'custom-common' '../custom-common' \
++ 'GIT_COMMON_DIR="$ROOT/custom-common" && export GIT_COMMON_DIR &&
++ GIT_DIR="../.git" && export GIT_DIR &&
++ git init --bare "$ROOT/custom-common"'
++
++test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
++ 'commondir-only-gitdir' '.git' '../.git' \
++ 'GIT_DIR="../.git" && export GIT_DIR'
++
test_done
3: ca95d51f6e ! 4: ffd6a5bb16 repo: add path.gitdir with absolute and relative suffix formatting
@@ Metadata
## Commit message ##
repo: add path.gitdir with absolute and relative suffix formatting
- Scripts often need to locate the `.git` directory. While `git rev-parse`
- provides this, it relies on command-line flags to dictate path formatting.
+ Scripts need a stable way to locate the git directory without
+ parsing rev-parse output or relying on its flag-driven path format
+ selection. There is no way to retrieve this path from git repo info
+ today.
- Introduce `path.gitdir.absolute` and `path.gitdir.relative` keys to
- `git repo info`. Exposing separate format-specific keys instead of a base
- `path.gitdir` key avoids default fallbacks and requires callers to state
- their format requirements explicitly. Both keys use `format_path()` to
- resolve paths.
-
- To test these keys, introduce the `test_repo_info_path` helper in
- `t/t1900-repo-info.sh`. The helper evaluates paths dynamically and accepts
- environment variable prefixes. This prepares the test suite for future path
- keys that depend on environment overrides, such as `commondir`.
+ Introduce path.gitdir.absolute and path.gitdir.relative keys,
+ consistent with the path.commondir keys added in the previous patch.
+ Reuse the test_repo_info_path helper introduced there to validate
+ both variants.
Mentored-by: Justin Tobler <jltobler@gmail.com>
Mentored-by: Lucas Seiki Oshiro <lucasseikioshiro@gmail.com>
@@ Commit message
## Documentation/git-repo.adoc ##
@@ Documentation/git-repo.adoc: values that they return:
- `object.format`::
- The object format (hash algorithm) used in the repository.
+ The path to the Git repository's common directory relative to
+ the current working directory.
+`path.gitdir.absolute`::
+ The canonical absolute path to the Git repository directory (the `.git` directory).
@@ Documentation/git-repo.adoc: values that they return:
+
## builtin/repo.c ##
-@@
- #include "hex.h"
- #include "odb.h"
- #include "parse-options.h"
-+#include "path.h"
- #include "path-walk.h"
- #include "progress.h"
- #include "quote.h"
- #include "ref-filter.h"
- #include "refs.h"
- #include "revision.h"
-+#include "setup.h"
- #include "strbuf.h"
- #include "string-list.h"
- #include "shallow.h"
-@@ builtin/repo.c: static int get_object_format(struct repository *repo, struct strbuf *buf)
+@@ builtin/repo.c: static int get_path_commondir_relative(struct repository *repo, struct strbuf *b
return 0;
}
@@ builtin/repo.c: static int get_object_format(struct repository *repo, struct str
+ if (!git_dir)
+ return error(_("unable to get git directory"));
+
-+ format_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
++ append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_CANONICAL);
+ return 0;
+}
+
@@ builtin/repo.c: static int get_object_format(struct repository *repo, struct str
+ if (!git_dir)
+ return error(_("unable to get git directory"));
+
-+ format_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
++ append_formatted_path(buf, git_dir, startup_info->prefix, PATH_FORMAT_RELATIVE);
+ return 0;
+}
+
@@ builtin/repo.c: static int get_object_format(struct repository *repo, struct str
{
strbuf_addstr(buf,
@@ builtin/repo.c: static const struct repo_info_field repo_info_field[] = {
- { "layout.bare", get_layout_bare },
- { "layout.shallow", get_layout_shallow },
{ "object.format", get_object_format },
+ { "path.commondir.absolute", get_path_commondir_absolute },
+ { "path.commondir.relative", get_path_commondir_relative },
+ { "path.gitdir.absolute", get_path_gitdir_absolute },
+ { "path.gitdir.relative", get_path_gitdir_relative },
{ "references.format", get_references_format },
@@ builtin/repo.c: static const struct repo_info_field repo_info_field[] = {
## t/t1900-repo-info.sh ##
-@@ t/t1900-repo-info.sh: test_expect_success 'git repo info -h shows only repo info usage' '
- test_grep ! "git repo structure" actual
- '
+@@ t/t1900-repo-info.sh: test_repo_info_path 'commondir with only GIT_DIR' 'commondir' \
+ 'commondir-only-gitdir' '.git' '../.git' \
+ 'GIT_DIR="../.git" && export GIT_DIR'
-+test_repo_info_path () {
-+ field_name=$1
-+ expect_absolute_eval=$2
-+ expect_relative=$3
-+ env_prefix=$4
-+
-+ test_expect_success "query individual key: path.$field_name.absolute${env_prefix:+ ($env_prefix)}" '
-+ (
-+ cd test-repo/sub &&
-+ expect_absolute=$(eval "$expect_absolute_eval") &&
-+ echo "path.$field_name.absolute=$expect_absolute" >expect &&
-+ eval "${env_prefix:+$env_prefix }git repo info \"path.$field_name.absolute\"" >actual &&
-+ test_cmp expect actual
-+ )
-+ '
-+
-+ test_expect_success "query individual key: path.$field_name.relative${env_prefix:+ ($env_prefix)}" '
-+ (
-+ cd test-repo/sub &&
-+ echo "path.$field_name.relative=$expect_relative" >expect &&
-+ eval "${env_prefix:+$env_prefix }git repo info \"path.$field_name.relative\"" >actual &&
-+ test_cmp expect actual
-+ )
-+ '
-+}
-+
-+test_expect_success 'setup test repository layout for path fields' '
-+ git init test-repo &&
-+ mkdir -p test-repo/sub
-+'
++test_repo_info_path 'gitdir standard' 'gitdir' 'gitdir-std' \
++ '.git' '../.git'
+
-+test_repo_info_path 'gitdir' 'echo "$(cd .. && pwd)/.git"' '../.git'
++test_repo_info_path 'gitdir with explicit GIT_DIR' 'gitdir' \
++ 'gitdir-env' '.git' '../.git' \
++ 'GIT_DIR="../.git" && export GIT_DIR'
+
test_done
--
2.54.0
^ permalink raw reply
* Re: [PATCH v4 06/16] midx: support custom `--base` for incremental MIDX writes
From: Taylor Blau @ 2026-06-12 18:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: SZEDER Gábor, git, Jeff King, Elijah Newren,
Patrick Steinhardt
In-Reply-To: <xmqqqzmbj3mb.fsf@gitster.g>
On Fri, Jun 12, 2026 at 06:21:48AM -0700, Junio C Hamano wrote:
> SZEDER Gábor <szeder.dev@gmail.com> writes:
>
> >> + layer="$(git multi-pack-index write --bitmap --incremental \
> >> + --no-write-chain-file --base="$(nth_line 1 "$midx_chain")")" &&
> >
> > There is no 'nth_line' helper function in this test script.
>
> Good eyes. It has been there in the file next door t5335 since
> February, but not available here in t5334.
Good spotting indeed. Fortunately or unfortunately for us, pulling on
this thread revealed a bit of a rabbit hole. Patches forthcoming..
Thanks,
Taylor
^ permalink raw reply
* Re: t5563-simple-http-auth failures with v2.55.0-rc0
From: Todd Zullinger @ 2026-06-12 18:02 UTC (permalink / raw)
To: Matthew John Cheetham; +Cc: git@vger.kernel.org
In-Reply-To: <VI0PR03MB1163416D5C66FAB25AECAAE21C0182@VI0PR03MB11634.eurprd03.prod.outlook.com>
Hi,
Matthew John Cheetham wrote:
> On 2026-06-11 22:04, Todd Zullinger wrote:
>> I notice that Fedora 44 (where the tests all pass) has
>> curl-8.18.0 while Fedora 45 has curl-8.21.0-rc2. The
>> version of httpd is the same between them, FWIW. I didn't
>> compare other package differences; it could be something
>> else entirely.
>
> Thanks for the report. The failure is not in Git, it is a libcurl
> behaviour change, and there is already an open upstream issue:
>
> https://github.com/curl/curl/issues/21943
> "Negotiate ignored with --anyauth" (Dan Fandrich, 2026-06-10)
>
> Dan also bisected it to the same commit I had locally,
> `8f71d0fde515` ("creds: hold credentials", curl PR #21548).
[...]
> Daniel Stenberg has acknowledged the curl issue but has not yet
> posted a fix. I will follow curl#21943 and, if the upstream answer
> is "the new behaviour is intended", come back here with a proposal
> for what Git should do about `http.emptyAuth` and test 18.
Excellent. This is it good hands all around.
With luck, curl is updated and the canary of distributions
like Fedora's Rawhide will have served a useful purpose in
flushing out issues before they affect most people. With
the help of git's excellent and thorough test suite, of
course. :)
If there is a curl update, I imagine it will be picked up
reasonably quickly in Fedora (and elsewhere that was testing
8.21.0 release candidates) and there will hopefully be no
strong need to make any changes on the git side.
Thanks!
--
Todd
^ permalink raw reply
* Re: [PATCH v2 1/1] environment.c: move 'protect_hfs' and 'protect_ntfs' into 'repo_config_values'
From: Junio C Hamano @ 2026-06-12 16:51 UTC (permalink / raw)
To: Christian Couder
Cc: Tian Yuchen, git, phillip.wood123, Ayush Chandekar,
Olamide Caleb Bello
In-Reply-To: <CAP8UFD1UbsXu_7DK2keGLUO3Yh06-YHieZP+On-yjY3SmV2Xmg@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> On Wed, Jun 10, 2026 at 6:41 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Tian Yuchen <cat@malon.dev> writes:
>>
>> > +int repo_protect_ntfs(struct repository *repo)
>> > +{
>> > + return repo->gitdir ?
>> ...
>> Shall we declare victory and mark the topic for 'next' now?
>
> I would have preferred the commit subject to start with "environment:"
> rather than "environment.c:" but it's a small nit and maybe you can
> fix it while merging.
If I remember, perhaps I'll try. But you know what happens when you
add more stuff that are not something only the maintainer can do on
my plate ;-)
^ permalink raw reply
* Re: [PATCH v2] update-ref: add --rename option
From: Junio C Hamano @ 2026-06-12 16:31 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git
In-Reply-To: <xmqqwlw4nccr.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> [Appendix]
> ... Ideally, these cherry-pickable commits
> that are stored under refs/merge-fix hierarchies SHOULD be indexable
> by a pair of topic (i.e. "when topic A and topic B first meets, apply
> this evil merge"), but this computation is cumbersome to write.
One scheme might be to use "refs/merge-fix/$A--$B" to store the
interaction between topic $A and topic $B, with the convention that
no topic is named with double-dash in its name.
We sequencially merge these in-flight topic into the integration
branch (e.g., 'seen'). When merging topic X, we roughly would need
to do the following.
(0) Skip if X is already in the integration branch.
(1) See what topic Y that would also be merged for the first time
to the integration branch. This is because a complex topic X
often is done by merging in-flight Y into then-current master
and applyng patches on top, and depending on the state of the
integration branch, such topic Y may or may not have already
been mergeed there. Enumerate all these topic Ys that would
be pulled into the integration branch as a side effect of
merging X.
(2) Enumerate all merge-fix refs that has any of the topic Ys or X.
For each such refs/merge-fix/$A--$B (where either $A or $B is X
or one of Ys), call the other side of "--" Z. If Z is already
in the integration branch, then we found the merge-fix we need
to apply.
The "topic X might pull other topics that haven't been merged
together with it when it gets merged" is what makes it cumbersome to
write. If we do not have to worry about it, it would be fairly
straight forward, but then the bulk-merge driver probably needs to
learn a safety check to make sure at the point of merging each topic
that the merge is not pulling another topic (base) into as a side
effect. It would mean that you prepare a new topic X on top of a
merge of Y into 'master', and X can never be merged into 'seen'
before Y is. Which may or may not be what we really want, and I
need to think about it a bit.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox