* [PATCH v2 3/4] xprepare: simplify error handling
From: Phillip Wood @ 2026-05-04 14:06 UTC (permalink / raw)
To: git; +Cc: Ezekiel Newren, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1777903579.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If either of the two allocations fail we want to take the same action
so use a single if statement. This saves a few lines and makes it
easier for the next commit to add a couple more allocations.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
xdiff/xprepare.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 81de412875a..7a29e5fc474 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -282,11 +282,8 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
* Create temporary arrays that will help us decide if
* changed[i] should remain false, or become true.
*/
- if (!XDL_CALLOC_ARRAY(action1, len1)) {
- ret = -1;
- goto cleanup;
- }
- if (!XDL_CALLOC_ARRAY(action2, len2)) {
+ if (!XDL_CALLOC_ARRAY(action1, len1) ||
+ !XDL_CALLOC_ARRAY(action2, len2)) {
ret = -1;
goto cleanup;
}
--
2.54.0.rc1.174.gd833f386ac5.dirty
^ permalink raw reply related
* [PATCH v2 2/4] xdiff: cleanup xdl_clean_mmatch()
From: Phillip Wood @ 2026-05-04 14:06 UTC (permalink / raw)
To: git; +Cc: Ezekiel Newren, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1777903579.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Remove the "s" parameter as, since the last commit, this function
is always called with s == 0. Also change parameter "e" to expect a
length, rather than the index of the last line to simplify the caller.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
xdiff/xprepare.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index 3b6bae0d158..81de412875a 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -197,8 +197,9 @@ void xdl_free_env(xdfenv_t *xe) {
}
-static bool xdl_clean_mmatch(uint8_t const *action, ptrdiff_t i, ptrdiff_t s, ptrdiff_t e) {
+static bool xdl_clean_mmatch(uint8_t const *action, ptrdiff_t i, ptrdiff_t len) {
ptrdiff_t r, rdis0, rpdis0, rdis1, rpdis1;
+ ptrdiff_t s = 0, e = len - 1;
/*
* Limits the window that is examined during the similar-lines
@@ -342,7 +343,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
uint8_t action = action1[i];
if (action == INVESTIGATE) {
- if (!xdl_clean_mmatch(action1, i, 0, len1 - 1))
+ if (!xdl_clean_mmatch(action1, i, len1))
action = KEEP;
else
action = DISCARD;
@@ -363,7 +364,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
uint8_t action = action2[i];
if (action == INVESTIGATE) {
- if (!xdl_clean_mmatch(action2, i, 0, len2 - 1))
+ if (!xdl_clean_mmatch(action2, i, len2))
action = KEEP;
else
action = DISCARD;
--
2.54.0.rc1.174.gd833f386ac5.dirty
^ permalink raw reply related
* [PATCH v2 1/4] xdiff: reduce size of action arrays
From: Phillip Wood @ 2026-05-04 14:06 UTC (permalink / raw)
To: git; +Cc: Ezekiel Newren, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1777903579.git.phillip.wood@dunelm.org.uk>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
When the myers algorithm is selected the input files are pre-processed
to remove any common prefix and suffix. Then any lines that appear
only in one side of the diff are marked as changed and frequently
occurring lines are marked as changed if they are adjacent to a
changed line. This step requires a couple of temporary arrays. As as
the common prefix and suffix have already been removed, the arrays
only need to be big enough to hold the lines between them, not the
whole file. Reduce the size of the arrays and adjust the loops that
use them accordingly while taking care to keep indexing the arrays
in xdfile_t with absolute line numbers.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
xdiff/xprepare.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/xdiff/xprepare.c b/xdiff/xprepare.c
index beef711067b..3b6bae0d158 100644
--- a/xdiff/xprepare.c
+++ b/xdiff/xprepare.c
@@ -273,16 +273,19 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
uint8_t *action1 = NULL, *action2 = NULL;
bool need_min = !!(cf->flags & XDF_NEED_MINIMAL);
int ret = 0;
+ ptrdiff_t off = xdf1->dstart;
+ ptrdiff_t len1 = xdf1->dend - off + 1;
+ ptrdiff_t len2 = xdf2->dend - off + 1;
/*
* Create temporary arrays that will help us decide if
* changed[i] should remain false, or become true.
*/
- if (!XDL_CALLOC_ARRAY(action1, xdf1->nrec + 1)) {
+ if (!XDL_CALLOC_ARRAY(action1, len1)) {
ret = -1;
goto cleanup;
}
- if (!XDL_CALLOC_ARRAY(action2, xdf2->nrec + 1)) {
+ if (!XDL_CALLOC_ARRAY(action2, len2)) {
ret = -1;
goto cleanup;
}
@@ -298,8 +301,8 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
if (mlim1 > XDL_MAX_EQLIMIT)
mlim1 = XDL_MAX_EQLIMIT;
}
- for (i = xdf1->dstart; i <= xdf1->dend; i++) {
- size_t mph1 = xdf1->recs[i].minimal_perfect_hash;
+ for (i = 0; i < len1; i++) {
+ size_t mph1 = xdf1->recs[i + off].minimal_perfect_hash;
rcrec = cf->rcrecs[mph1];
nm = rcrec ? rcrec->len2 : 0;
if (nm == 0)
@@ -318,8 +321,8 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
if (mlim2 > XDL_MAX_EQLIMIT)
mlim2 = XDL_MAX_EQLIMIT;
}
- for (i = xdf2->dstart; i <= xdf2->dend; i++) {
- size_t mph2 = xdf2->recs[i].minimal_perfect_hash;
+ for (i = 0; i < len2; i++) {
+ size_t mph2 = xdf2->recs[i + off].minimal_perfect_hash;
rcrec = cf->rcrecs[mph2];
nm = rcrec ? rcrec->len1 : 0;
if (nm == 0)
@@ -335,42 +338,42 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
* false, or become true.
*/
xdf1->nreff = 0;
- for (i = xdf1->dstart; i <= xdf1->dend; i++) {
+ for (i = 0; i < len1; i++) {
uint8_t action = action1[i];
if (action == INVESTIGATE) {
- if (!xdl_clean_mmatch(action1, i, xdf1->dstart, xdf1->dend))
+ if (!xdl_clean_mmatch(action1, i, 0, len1 - 1))
action = KEEP;
else
action = DISCARD;
}
if (action == KEEP) {
- xdf1->reference_index[xdf1->nreff++] = i;
+ xdf1->reference_index[xdf1->nreff++] = i + off;
/* changed[i] remains false */
} else if (action == DISCARD) {
- xdf1->changed[i] = true;
+ xdf1->changed[i + off] = true;
} else {
BUG("Illegal state for action");
}
}
xdf2->nreff = 0;
- for (i = xdf2->dstart; i <= xdf2->dend; i++) {
+ for (i = 0; i < len2; i++) {
uint8_t action = action2[i];
if (action == INVESTIGATE) {
- if (!xdl_clean_mmatch(action2, i, xdf2->dstart, xdf2->dend))
+ if (!xdl_clean_mmatch(action2, i, 0, len2 - 1))
action = KEEP;
else
action = DISCARD;
}
if (action == KEEP) {
- xdf2->reference_index[xdf2->nreff++] = i;
+ xdf2->reference_index[xdf2->nreff++] = i + off;
/* changed[i] remains false */
} else if (action == DISCARD) {
- xdf2->changed[i] = true;
+ xdf2->changed[i + off] = true;
} else {
BUG("Illegal state for action");
}
--
2.54.0.rc1.174.gd833f386ac5.dirty
^ permalink raw reply related
* [PATCH v2 0/4] xdiff: reduce the size of a couple of arrays
From: Phillip Wood @ 2026-05-04 14:06 UTC (permalink / raw)
To: git; +Cc: Ezekiel Newren, Junio C Hamano, Phillip Wood
In-Reply-To: <cover.1775141855.git.phillip.wood@dunelm.org.uk>
When the myers algorithm is selected the input files are pre-processed
to remove any common prefix and suffix. There are a couple of places
where we allocate arrays large enough to hold the whole file when
they only need to be big enough to hold the remaining lines after the
common prefix and suffix have been removed. This series adjusts those
allocations to avoid allocating space for the common lines.
These patches are based on 'en/xdiff-cleanup-3'
Changes since V1:
- rebased onto updated upstream
Base-Commit: f87808b7014cf06db4a7e19b193cf9aa7e965ebc
Published-As: https://github.com/phillipwood/git/releases/tag/pw%2Fxdiff-reduce-array-sizes%2Fv2
View-Changes-At: https://github.com/phillipwood/git/compare/f87808b70...d7cb49a7c
Fetch-It-Via: git fetch https://github.com/phillipwood/git pw/xdiff-reduce-array-sizes/v2
Phillip Wood (4):
xdiff: reduce size of action arrays
xdiff: cleanup xdl_clean_mmatch()
xprepare: simplify error handling
xdiff: reduce the size of array
xdiff/xprepare.c | 46 ++++++++++++++++++++++------------------------
1 file changed, 22 insertions(+), 24 deletions(-)
Range-diff against v1:
1: 447b8c0af17 ! 1: ec692cabfec xdiff: reduce size of action arrays
@@ xdiff/xprepare.c: static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *
goto cleanup;
}
@@ xdiff/xprepare.c: static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
- /*
- * Initialize temporary arrays with DISCARD, KEEP, or INVESTIGATE.
- */
+ if (mlim1 > XDL_MAX_EQLIMIT)
+ mlim1 = XDL_MAX_EQLIMIT;
+ }
- for (i = xdf1->dstart; i <= xdf1->dend; i++) {
- size_t mph1 = xdf1->recs[i].minimal_perfect_hash;
+ for (i = 0; i < len1; i++) {
@@ xdiff/xprepare.c: static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *
nm = rcrec ? rcrec->len2 : 0;
if (nm == 0)
@@ xdiff/xprepare.c: static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
- action1[i] = INVESTIGATE;
+ if (mlim2 > XDL_MAX_EQLIMIT)
+ mlim2 = XDL_MAX_EQLIMIT;
}
-
- for (i = xdf2->dstart; i <= xdf2->dend; i++) {
- size_t mph2 = xdf2->recs[i].minimal_perfect_hash;
+ for (i = 0; i < len2; i++) {
@@ xdiff/xprepare.c: static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *
xdf1->nreff = 0;
- for (i = xdf1->dstart; i <= xdf1->dend; i++) {
+ for (i = 0; i < len1; i++) {
- if (action1[i] == INVESTIGATE) {
+ uint8_t action = action1[i];
+
+ if (action == INVESTIGATE) {
- if (!xdl_clean_mmatch(action1, i, xdf1->dstart, xdf1->dend))
+ if (!xdl_clean_mmatch(action1, i, 0, len1 - 1))
- action1[i] = KEEP;
+ action = KEEP;
else
- action1[i] = DISCARD;
+ action = DISCARD;
}
- if (action1[i] == KEEP) {
+ if (action == KEEP) {
- xdf1->reference_index[xdf1->nreff++] = i;
+ xdf1->reference_index[xdf1->nreff++] = i + off;
/* changed[i] remains false */
- } else if (action1[i] == DISCARD)
+ } else if (action == DISCARD) {
- xdf1->changed[i] = true;
+ xdf1->changed[i + off] = true;
- else
- BUG("Illegal state for action1[i]");
+ } else {
+ BUG("Illegal state for action");
+ }
}
xdf2->nreff = 0;
- for (i = xdf2->dstart; i <= xdf2->dend; i++) {
+ for (i = 0; i < len2; i++) {
- if (action2[i] == INVESTIGATE) {
+ uint8_t action = action2[i];
+
+ if (action == INVESTIGATE) {
- if (!xdl_clean_mmatch(action2, i, xdf2->dstart, xdf2->dend))
+ if (!xdl_clean_mmatch(action2, i, 0, len2 - 1))
- action2[i] = KEEP;
+ action = KEEP;
else
- action2[i] = DISCARD;
+ action = DISCARD;
}
- if (action2[i] == KEEP) {
+ if (action == KEEP) {
- xdf2->reference_index[xdf2->nreff++] = i;
+ xdf2->reference_index[xdf2->nreff++] = i + off;
/* changed[i] remains false */
- } else if (action2[i] == DISCARD)
+ } else if (action == DISCARD) {
- xdf2->changed[i] = true;
+ xdf2->changed[i + off] = true;
- else
- BUG("Illegal state for action2[i]");
- }
+ } else {
+ BUG("Illegal state for action");
+ }
2: 78e9313fd44 ! 2: 977f4577521 xdiff: cleanup xdl_clean_mmatch()
@@ xdiff/xprepare.c: void xdl_free_env(xdfenv_t *xe) {
/*
* Limits the window that is examined during the similar-lines
@@ xdiff/xprepare.c: static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
- xdf1->nreff = 0;
- for (i = 0; i < len1; i++) {
- if (action1[i] == INVESTIGATE) {
+ uint8_t action = action1[i];
+
+ if (action == INVESTIGATE) {
- if (!xdl_clean_mmatch(action1, i, 0, len1 - 1))
+ if (!xdl_clean_mmatch(action1, i, len1))
- action1[i] = KEEP;
+ action = KEEP;
else
- action1[i] = DISCARD;
+ action = DISCARD;
@@ xdiff/xprepare.c: static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd
- xdf2->nreff = 0;
- for (i = 0; i < len2; i++) {
- if (action2[i] == INVESTIGATE) {
+ uint8_t action = action2[i];
+
+ if (action == INVESTIGATE) {
- if (!xdl_clean_mmatch(action2, i, 0, len2 - 1))
+ if (!xdl_clean_mmatch(action2, i, len2))
- action2[i] = KEEP;
+ action = KEEP;
else
- action2[i] = DISCARD;
+ action = DISCARD;
3: cdcad99edc4 = 3: 24e65d42b72 xprepare: simplify error handling
4: a3438dc0933 = 4: d7cb49a7c99 xdiff: reduce the size of array
--
2.54.0.rc1.174.gd833f386ac5.dirty
^ permalink raw reply
* Re: What's cooking in git.git (May 2026, #01)
From: Phillip Wood @ 2026-05-04 13:16 UTC (permalink / raw)
To: Junio C Hamano, git
In-Reply-To: <xmqqmryhtar8.fsf@gitster.g>
On 03/05/2026 04:47, Junio C Hamano wrote:
>
> * rs/grep-column-only-match-fix (2026-04-24) 1 commit
> - grep: fix --column --only-match for 2nd and later matches
>
> "git grep" update.
>
> Will merge to 'next'?
> source: <9bd69678-f04b-41d2-ad74-a386820d34c8@web.de>
I've just read this patch and it looked good. It is surprising no one
has complained about the strange column numbers before
> * hn/git-checkout-m-with-stash (2026-04-28) 5 commits
> - checkout -m: autostash when switching branches
> - checkout: rollback lock on early returns in merge_working_tree
> - sequencer: teach autostash apply to take optional conflict marker labels
> - sequencer: allow create_autostash to run silently
> - stash: add --label-ours, --label-theirs, --label-base for apply
>
> "git checkout -m another-branch" was invented to deal with local
> changes to paths that are different between the current and the new
> branch, but it gave only one chance to resolve conflicts. The command
> was taught to create a stash to save the local changes.
>
> Will merge to 'next'?
> source: <pull.2234.v16.git.git.1777401552.gitgitgadget@gmail.com>
I'm happy with the latest re-roll - this looks ready to me
> * pw/xdiff-shrink-memory-consumption (2026-04-02) 4 commits
> . xdiff: reduce the size of array
> . xprepare: simplify error handling
> . xdiff: cleanup xdl_clean_mmatch()
> . xdiff: reduce size of action arrays
>
> Shrink wasted memory in Myers diff that does not account for common
> prefix and suffix removal.
>
> Needs to be rebased on updated en/xdiff-cleanup-3.
Will do
> * en/xdiff-cleanup-3 (2026-04-29) 6 commits
> - xdiff/xdl_cleanup_records: make execution of action easier to follow
> - xdiff/xdl_cleanup_records: make setting action easier to follow
> - xdiff/xdl_cleanup_records: make limits more clear
> - xdiff/xdl_cleanup_records: use unambiguous types
> - xdiff: use unambiguous types in xdl_bogo_sqrt()
> - xdiff/xdl_cleanup_records: delete local recs pointer
>
> Preparation of the xdiff/ codebase to work with Rust.
>
> Will merge to 'next'?
Yes, I think this is ready as well
Thanks
Phillip
^ permalink raw reply
* Re: [PATCH] grep: fix --column --only-match for 2nd and later matches
From: Phillip Wood @ 2026-05-04 13:10 UTC (permalink / raw)
To: René Scharfe, Brandon Chinn, git
In-Reply-To: <9bd69678-f04b-41d2-ad74-a386820d34c8@web.de>
Hi René
On 24/04/2026 22:04, René Scharfe wrote:
> diff --git a/grep.c b/grep.c
> index c7e1dc1e0e..a54e5d86a9 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -1267,6 +1267,7 @@ static void show_line(struct grep_opt *opt,
> regmatch_t match;
> enum grep_context ctx = GREP_CONTEXT_BODY;
> int eflags = 0;
> + const char *start = bol;
Here we save a pointer to the start of the line
> if (want_color(opt->color)) {
> if (sign == ':')
> @@ -1285,6 +1286,7 @@ static void show_line(struct grep_opt *opt,
> if (match.rm_so == match.rm_eo)
> break;
>
> + cno = bol - start + match.rm_so + 1;
and then we calculate the column number relative to that.
That looks good, thanks for fixing it
Phillip
> if (opt->only_matching)
> show_line_header(opt, name, lno, cno, sign);
> else
> @@ -1294,7 +1296,6 @@ static void show_line(struct grep_opt *opt,
> if (opt->only_matching)
> opt->output(opt, "\n", 1);
> bol += match.rm_eo;
> - cno += match.rm_eo;
> rest -= match.rm_eo;
> eflags = REG_NOTBOL;
> }
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 64ac4f04ee..bd439563d6 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -322,11 +322,11 @@ do
> ${HC}file:1:5:mmap
> ${HC}file:2:5:mmap
> ${HC}file:3:5:mmap
> - ${HC}file:3:13:mmap
> + ${HC}file:3:14:mmap
> ${HC}file:4:5:mmap
> - ${HC}file:4:13:mmap
> + ${HC}file:4:14:mmap
> ${HC}file:5:5:mmap
> - ${HC}file:5:13:mmap
> + ${HC}file:5:14:mmap
> EOF
> git grep --column -n -o -e mmap $H >actual &&
> test_cmp expected actual
^ permalink raw reply
* Re: [PATCH v5] revision.c: implement --max-count-oldest
From: Mirko Faina @ 2026-05-04 13:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Jeff King, Jean-Noël Avila, Patrick Steinhardt,
Tian Yuchen, Ben Knoble, Johannes Sixt, Chris Torek, Mirko Faina
In-Reply-To: <xmqqik93px8s.fsf@gitster.g>
On Mon, May 04, 2026 at 02:19:47PM +0900, Junio C Hamano wrote:
> Mirko Faina <mroik@delayed.space> writes:
>
> > --max-count is a commit limiting option sets a maximum amount of commits
> > to be shown. If a user wants to see only the first N commits of the
> > history (the oldest commits) they'd have to combine --max-count with
> > --skip. This is not very user-friendly.
>
> To use "--skip=<n>" for this purose, you'd need to know how many
> records are going to be omitted to begin with, but that means you'd
> run the command without count limitation once only to find out how
> many records there are. "not very user-friendly" sounds like an
> understatement of the year.
>
> They can do with something silly like
>
> git rev-list ... |
> tail -n N |
> xargs -n1 git show ...
>
> and that does count as "not very user-friendly", I would think.
Will reword in v6.
> > Teach get_revision() the --max-count-oldest option.
> >
> > Signed-off-by: Mirko Faina <mroik@delayed.space>
> > ---
> > Documentation/rev-list-options.adoc | 3 ++
> > revision.c | 77 +++++++++++++++++++++++++++--
> > revision.h | 2 +
> > t/t4202-log.sh | 14 ++++++
> > 4 files changed, 93 insertions(+), 3 deletions(-)
>
> It looks like this needs measurably smaller damage to the codebase
> than the other --reverse=before approach ;-).
Yes, the control flow of the program is also easier to read.
Thanks
^ permalink raw reply
* Re: [RFC PATCH 3/7] path-walk: support `object:type` filter
From: Derrick Stolee @ 2026-05-04 12:32 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <db46c1248ece57476b369a9bff920facab24be04.1777853408.git.me@ttaylorr.com>
On 5/3/2026 8:11 PM, Taylor Blau wrote:
> The `object:type` filter accepts only objects of a single type; it is
> the second member of the object-info-only filter family that bitmap
> traversal already supports.
...
> But there are a couple of side effects of the "trees off, blobs on" case
> that need fixing:
>
> 1. 'setup_pending_objects()' previously skipped pending trees as soon
> as `info->trees` was zero. For 'object:type=blob' the call site
> needs those pending trees: a lightweight tag pointing to a tree, or
> an annotated tag whose peeled target is a tree, can both reach
> blobs that are otherwise unreachable from any commit's root tree.
> Loosen the gate to "if (!info->trees && !info->blobs) continue" and
> similarly retrieve the root_tree_list whenever either trees or
> blobs are wanted.
>
> 2. The revision machinery's `handle_commit()` drops pending trees when
> `revs->tree_objects` is zero (see the 'OBJ_TREE' handler in
> revision.c), so by the time path-walk sees the pending list
> after `prepare_revision_walk()` the tree-bearing pendings would
> already be gone. Fix this by setting
>
> revs->tree_objects = info->trees || info->blobs
>
> so pending trees survive `prepare_revision_walk()` whenever we
> need to walk into them. Path-walk still resets tree_objects to
> zero immediately after `prepare_revision_walk()` returns, so the
> rev-walk itself never enumerates trees redundantly with
> path-walk's own descent.
Both of these changes are very valuable bug fixes for the path-walk API!
Thanks for catching the distinction here where we should still be
walking trees in order to find the blobs we want.
Thanks,
-Stolee
^ permalink raw reply
* Re: [RFC PATCH 2/7] path-walk: support `tree:0` filter
From: Derrick Stolee @ 2026-05-04 12:30 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <e1b7fd3cb2a2bba5f6404ac5f8ac3487a46d51b5.1777853408.git.me@ttaylorr.com>
On 5/3/2026 8:11 PM, Taylor Blau wrote:
> The `tree:0` object filter omits all trees and blobs from the result,
> keeping only commits and tags. Consequently, this filter type should
> has a fairly straightforward integration with path-walk, as the decision
> to include an object depends only on its type and does not depend on any
> path-sensitive state.
I agree that the implementation here is straight-forward. It's something
where I could easily see wanting to disable the path-walk API because it
is no longer contributing much value, but perhaps the caller wants a
consistent callback that provides all commits and tags in different
chunks.
> Non-zero tree-depth filters are not supported. Those depend on the depth
> at which a tree is visited, which is a path-walk concept the filter
> machinery doesn't currently share with the path-walk API. Reject them in
> `prepare_filters()` with a helpful error and let pack-objects fall back
> to the regular traversal, the same way it already does for unsupported
> filters.
I think that this could be remedied with some tweaks to the internal
methods and data within the path-walk API to track a depth. This could
be handled later, if there was enough demand for nonzero tree-depth.
The diff itself looks good.
Thanks,
-Stolee
^ permalink raw reply
* Re: [RFC PATCH 1/7] pack-objects: update `--path-walk`'s existing incompatibilities
From: Derrick Stolee @ 2026-05-04 12:22 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <babe1596161365209c226d374db70a1bdc284a1c.1777853408.git.me@ttaylorr.com>
On 5/3/2026 8:11 PM, Taylor Blau wrote:
> The documentation in git-pack-objects(1) claims that `--path-walk` is
> incompatible with `-shallow`. However, commit c178b02e29f (pack-objects:
> allow --shallow and --path-walk, 2025-05-16) resolves this
> incompatibility, leaving the documentation stale.
>
> Likewise, this documentation claims that `--filter` is incompatible, but
> `blob:none`, `blob:limit=<n>`, and `sparse:oid=<blob>` already work via
> path-walk.
>
> List the supported `--filter` forms explicitly and note that other forms
> fall back to the regular object traversal. Also remove the
> incompatibility notice with `--shallow`.
Thanks for pointing out that I didn't update the docs in my series.
I should incorporate the appropriate language for these changes in my
patches and give you co-authorship. That will also help avoid using
commit references that are impermanent until the series lands.
Thanks,
-Stolee
^ permalink raw reply
* Re: [RFC PATCH 0/7] pack-bitmap: resolve various `--path-walk` incompatibilities
From: Derrick Stolee @ 2026-05-04 12:13 UTC (permalink / raw)
To: Taylor Blau, git; +Cc: Junio C Hamano, Jeff King, Elijah Newren
In-Reply-To: <cover.1777853408.git.me@ttaylorr.com>
On 5/3/2026 8:11 PM, Taylor Blau wrote:
> (Note to the maintainer, this is built on top of 'ds/path-walk-filters').
>
> Between other tasks, I have been working on trying to integrate
> `--path-walk` within GitHub's infrastructure. In order to do this,
> `--path-walk` must work with features that GitHub depends on, such as
> reachability bitmaps and delta-islands (along with filters, shallow,
> etc., though more on that below).
>
> I had been sitting on these patches for a few days in my fork before
> Stolee sent his series in [1] which resolves incompatibilities between
> the `--path-walk` option and various filter types. Since I figured that
> others are working in this area I wanted to send a reworked version of
> my series for a couple of reasons:
>
> 1. Since reviewers are already looking at this area as a consequence of
> Stolee's series, this topic should be slightly easier to review
> while the area is fresh.
I agree that we should review both series together. It's been a while
since the original path-walk API series, so it may require some refresh
of all its nuances.
> 2. In case Stolee (or others) are working on resolving the
> incompatibility between `--path-walk` and either delta-islands or
> reachability bitmaps, this series can either combine with those (if
> any) or serve as inspiration (if others are in the process of
> writing such series).
I was _not_ working on bitmap compatibility, but I'm grateful to see it!
> When writing this originally, I had borrowed the same filter-application
> mechanism from bitmaps, which supports trivial filters (e.g., blob:none,
> tree:0, and combinations therein). Stolee's series is a strict
> improvement on that approach supporting sparse:<oid> filters as well, so
> I reworked my filtering-related patches based on that.
You have some new filters that I had not considered, so they are welcome
additions. If you don't mind, I could add them into my series, as they
may be more appropriate grouped with the other filter changes.
> The patches surrounding bitmaps and delta-islands are largely
> unchanged from when I had originally written them:
>
> * Supporting bitmaps with `--path-walk` is mostly straightforward, and
> boils down to ensuring that the path-walk-specific object callback
> indexes any commit(s) it sees for bitmapping.
>
> * Supporting delta-islands with `--path-walk` required a bit more
> surgery, and involves propagating island marks for commits in the
> path-walk-specific callback, as well as recording tree depth
> information in the same spot.
>
> I'm submitting these patches as an RFC, since (a) I haven't thought
> deeply about the approach taken here and could very well be on the wrong
> track, and (b) in case Stolee or others want to combine forces here
> and/or coordinate around each other.
I'll definitely take a very close read of these patches, as there are
some interesting interactions here.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH 4/7] backfill: die on incompatible filter options
From: Derrick Stolee @ 2026-05-04 12:09 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee via GitGitGadget
Cc: git, christian.couder, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps
In-Reply-To: <xmqqbjewrtek.fsf@gitster.g>
On 5/3/2026 6:59 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> The 'git backfill' command uses the path-walk API in a critical way: it
>> uses the objects output from the command to find the batches of missing
>> objects that should be requested from the server. Unlike 'git
>> pack-objects', we cannot fall back to another mechanism.
>>
>> The previous change added the path_walk_filter_compatible() method that
>> we can reuse here. Use it during argument validation in cmd_backfill().
>>
>> Signed-off-by: Derrick Stolee <stolee@gmail.com>
>> ---
>> builtin/backfill.c | 2 ++
>> t/t5620-backfill.sh | 8 ++++++++
>> 2 files changed, 10 insertions(+)
>
> Another topic adds a helper function to check for many incompatible
> options and calls it from here. When I merged this topic, I made an
> semi-evil merge to move this call to that function (with necessary
> adjustment to the parameter). Please sanity check the resolution I
> made in 'seen'. Thanks.
Thanks for alerting me about this. I do think there is an error in
your merge. Hopefully the tests I wrote in this series caught the
mistake.
Here are the last lines in your copy of
reject_unsupported_rev_list_options():
if (revs->filter.choice)
die(_("'%s' cannot be used with 'git backfill'"),
"--filter");
if (!path_walk_filter_compatible(&revs->filter))
die(_("cannot backfill with these filter options"));
if (revs->filter.blob_limit_value)
die(_("cannot backfill with blob size limits"));
The last two options are correct, but they can't do anything
because the first one causes the command to fail immediately.
This should have caused failures in t5620-backfill.sh, specifically
the test 'backfill rejects incompatible filter options'. Indeed, I
get this error output when running on that commit:
+ test_grep cannot backfill with these filter options err
+ eval last_arg=${2}
+ last_arg=err
+ test -f err
+ test 2 -lt 2
+ test x! = xcannot backfill with these filter options
+ test x! = xcannot backfill with these filter options
+ grep cannot backfill with these filter options err
+ echo error: 'grep cannot backfill with these filter options err' didn't find a match in:
error: 'grep cannot backfill with these filter options err' didn't find a match in:
+ test -s err
+ cat err
fatal: '--filter' cannot be used with 'git backfill'
+ return 1
This does make it clear that I should add a new test in t5620 that
tests the 'sparse:<oid>' filter now that it is compatible, which I
missed in v1.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH 1/7] pack-objects: pass --objects with --path-walk
From: Derrick Stolee @ 2026-05-04 12:01 UTC (permalink / raw)
To: Junio C Hamano, Derrick Stolee via GitGitGadget
Cc: git, christian.couder, johannes.schindelin, johncai86,
karthik.188, kristofferhaugsbakk, me, newren, peff, ps
In-Reply-To: <xmqqo6iwq9qs.fsf@gitster.g>
On 5/3/2026 8:49 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> When 'git pack-objects' has the --path-walk option enabled, it uses a
>> different set of revision walk parameters than normal. For once,
>
> "once" -> "one" (or "instance")?
Yes, "one". Sorry for the typo.
>> --objects was previously assumed by the path-walk API and was not needed
>> to be added. We also needed --boundary to allow discovering
>> UNINTERESTING objects to use as delta bases.
>>
>> We will be updating the path-walk API soon to work with some filter
>> options. However, the revision machinery will trigger a fatal error:
>>
>> fatal: object filtering requires --objects
>>
>> The fix is easy: add the --objects option as an argument. This has no
>> effect on the path-walk API but does simplify the revision option
>> parsing for the objects filter.
>>
>> We can remove the comment about "removing" the options because they were
>> never removed and instead not added. We still need to disable using
>> bitmaps.
>
> In the old code, there was a valid reason why bitmaps were not used
> (i.e., "--objects" not enabled), but that no longer holds (i.e., now
> we add "--objects" ourselves). Do we need to give an updated
> rationale to keep bitmap disabled?
>> if (path_walk) {
>> strvec_push(&rp, "--boundary");
>> - /*
>> - * We must disable the bitmaps because we are removing
>> - * the --objects / --objects-edge[-aggressive] options.
>> - */
>> + strvec_push(&rp, "--objects");
>> use_bitmap_index = 0;
>> } else if (thin) {
This old comment is perhaps confusing things. The important thing here
is to disable bitmaps with 'use_bitmap_index = 0;' (though perhaps not
for long [1]).
[1] https://lore.kernel.org/git/f50f8df01a9f216d5b4388b2fe4ff58077b574f3.1777853408.git.me@ttaylorr.com/
The path-walk API itself disables the objects walk for the revision
machinery in walk_objects_by_path():
info->revs->blob_objects = info->revs->tree_objects = 0;
This allows the path-walk API to rely on the revision walk for a
_commits only_ walk and then have the path-walk API handle the trees
and blobs.
The reason we need to add "--objects" now is to allow for parsing the
"--filter" option without the revision logic complaining.
Thanks,
-Stolee
^ permalink raw reply
* Re: [PATCH v2 4/8] promisor-remote: add 'local_name' to 'struct promisor_info'
From: Toon Claes @ 2026-05-04 11:46 UTC (permalink / raw)
To: Christian Couder, git
Cc: Junio C Hamano, Patrick Steinhardt, Taylor Blau, Karthik Nayak,
Elijah Newren, Christian Couder, Christian Couder
In-Reply-To: <20260427124108.3524129-5-christian.couder@gmail.com>
Christian Couder <christian.couder@gmail.com> writes:
> In a following commit, we will store promisor remote information under
> a remote name different than the one the server advertised.
>
> To prepare for this change, let's add a new 'char *local_name' member
> to 'struct promisor_info', and let's update the related functions.
>
> While at it, let's also add a small promisor_info_internal_name()
> helper that returns `local_name` when set, `name` otherwise, and let's
> use this small helper in promisor_store_advertised_fields() and in the
> post-loop of filter_promisor_remote() so that lookups against the local
> repo configuration use the right name.
It seems the `local_name` doesn't get filled in yet, so because
promisor_info_internal_name() falls back to `name` there is no
functional change in this commit. Okay.
--
Cheers,
Toon
^ permalink raw reply
* Re: git clone with --dissociate sometimes fails to check out target commit
From: Rasmus Villemoes @ 2026-05-04 11:36 UTC (permalink / raw)
To: Jeff King; +Cc: git, emkan
In-Reply-To: <20260504095110.GA599780@coredump.intra.peff.net>
On Mon, May 04 2026, Jeff King <peff@peff.net> wrote:
> On Mon, May 04, 2026 at 10:20:32AM +0200, Rasmus Villemoes wrote:
>
>> Are we using --dissociate wrongly, or are we perhaps not maintaining
>> those local mirror repos properly? They are essentially just created
>> with 'git clone --mirror', with 'git remote update' run periodically.
>>
>> Naively, I'd expect the effects of --dissociate to only happen after
>> everything else the clone command does has been done, but it seems that
>> the ties to the reference repo are cut too soon.
>
> No, you're using it correctly. The dissociate step should copy all of
> the shared objects into the new repo, so it shouldn't matter whether we
> do it before or after checkout. The objects are there either way.
>
[snip]
>
> It's kind of ugly, but I think may be the least-bad solution. See that
> earlier thread for more discussion of alternatives.
>
> In the meantime, doing your dissociate clone with:
>
> git -c core.commitGraph=false clone ...
>
> should work around the problem.
Thanks for the extremely fast reply, analysis, patch and workaround!
I can confirm that the commit graph disabling workaround works on both
the Debian and Arch machines.
I can also confirm that the patch applied on top of v2.54.0 works,
although the build does throw this warning:
commit.c: In function ‘get_commit_tree_oid’:
commit.c:481:66: warning: passing argument 2 of ‘repo_get_commit_tree’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
481 | struct tree *tree = repo_get_commit_tree(the_repository, commit);
| ^~~~~~
commit.c:459:50: note: expected ‘struct commit *’ but argument is of type ‘const struct commit *’
459 | struct commit *commit)
| ~~~~~~~~~~~~~~~^~~~~~
Thanks again,
Rasmus
^ permalink raw reply
* [PATCH v2] Reintegrate: send "Huh?" warnings to stderr, not stdout
From: erik @ 2026-05-04 10:28 UTC (permalink / raw)
To: git; +Cc: gitster, cat, Erik Cervin-Edin
In-Reply-To: <ae896PlyiYeqldFN@mbp>
From: Erik Cervin-Edin <erik@cervined.in>
In show_merge(), the "Huh?: $msg" warning -- emitted when a
first-parent merge subject does not match either "Merge branch '...'"
or "Merge remote branch '...'" -- is meant to go to stderr, but the
redirect
echo 2>&1 "Huh?: $msg"
goes the wrong way and sends the message to stdout instead.
In the common Reintegrate invocation that captures stdout, e.g.
Meta/Reintegrate next..seen >Meta/redo-seen.sh
this means the warning is silently embedded in the generated heredoc
body instead of being printed to the maintainer's terminal, with no
diagnostic that something went wrong. Worse, the resulting redo-*
script is corrupted with a "Huh?:..." line.
Switch the redirection to ">&2", matching every other diagnostic in
this script.
Signed-off-by: Erik Cervin-Edin <erik@cervined.in>
---
Reintegrate | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Reintegrate b/Reintegrate
index a1e67a0330..6fdc7c5f41 100755
--- a/Reintegrate
+++ b/Reintegrate
@@ -327,7 +327,7 @@ show_merge () {
merge_hier=
;;
*)
- echo 2>&1 "Huh?: $msg"
+ echo >&2 "Huh?: $msg"
return
;;
esac &&
--
2.53.0
^ permalink raw reply related
* [PATCH] t7703: ignore 'total' line when comparing ls -l output
From: Joerg Thalheim @ 2026-05-04 10:14 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Jörg Thalheim
From: Jörg Thalheim <joerg@thalheim.io>
The 'total N' header from ls -l reports the block count, which on
copy-on-write or compressing filesystems such as ZFS can change between
two back-to-back invocations even when the directory contents are
identical. The MIDX retention checks introduced in 6ce9d558ce
(midx-write: skip rewriting MIDX with --stdin-packs unless needed,
2025-12-11) compare full ls -l output and thus fail spuriously on such
filesystems. Strip the header line before comparing.
Signed-off-by: Jörg Thalheim <joerg@thalheim.io>
---
t/t7703-repack-geometric.sh | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/t/t7703-repack-geometric.sh b/t/t7703-repack-geometric.sh
index 04d5d8fc33..9b5a428620 100755
--- a/t/t7703-repack-geometric.sh
+++ b/t/t7703-repack-geometric.sh
@@ -299,9 +299,9 @@ test_expect_success '--geometric --write-midx retains up-to-date MIDX without bi
test_path_is_file .git/objects/pack/multi-pack-index &&
test-tool chmtime =0 .git/objects/pack/multi-pack-index &&
- ls -l .git/objects/pack/ >expect &&
+ ls -l .git/objects/pack/ | sed 1d >expect &&
git repack --geometric=2 --write-midx --no-write-bitmap-index &&
- ls -l .git/objects/pack/ >actual &&
+ ls -l .git/objects/pack/ | sed 1d >actual &&
test_cmp expect actual
)
'
@@ -316,9 +316,9 @@ test_expect_success '--geometric --write-midx retains up-to-date MIDX with bitma
test_path_is_file repo/.git/objects/pack/multi-pack-index &&
test-tool chmtime =0 repo/.git/objects/pack/multi-pack-index &&
- ls -l repo/.git/objects/pack/ >expect &&
+ ls -l repo/.git/objects/pack/ | sed 1d >expect &&
git -C repo repack --geometric=2 --write-midx --write-bitmap-index &&
- ls -l repo/.git/objects/pack/ >actual &&
+ ls -l repo/.git/objects/pack/ | sed 1d >actual &&
test_cmp expect actual
'
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
2.53.0
^ permalink raw reply related
* Re: [PATCH] Reintegrate: send "Huh?" warnings to stderr, not stdout
From: Erik Cervin Edin @ 2026-05-04 10:03 UTC (permalink / raw)
To: Tian Yuchen; +Cc: git, gitster
In-Reply-To: <5a821f71-3d6e-4942-9bdb-257617484a6b@malon.dev>
On 26/04/29 02:05AM, Tian Yuchen wrote:
> It would be better for me if this sentence (i.e., what was the code
> originally *intended* to do before the patch?) were placed right at the
> beginning. Something similar to:
>
> In show_merge(), the warning "Huh?: $msg" is emitted to stdout because it
> uses the erroneous redirect `echo 2>&1`. The intent was clearly to use `>&2`
> to print to stderr...
>
> Of course, it’s up to you ;)
Good point! I hoisted the intent up to the opening paragraph; took the
spirit of your wording rather than copying it verbatim, and tightened
the rest while I was at it.
^ permalink raw reply
* Re: git rename/moved status unreliable in ruby
From: Jeff King @ 2026-05-04 10:00 UTC (permalink / raw)
To: sebastien.stettler
Cc: phillip.wood@dunelm.org.uk, chris.torek@gmail.com,
git@vger.kernel.org, j6t@kdbg.org
In-Reply-To: <IC7a4NnSKMdvXlVyaSDYEtU7iRlKdJGzCwrXNCFKrtFfnBJTMrwY522rHF8PfzYxFs43huo0KFGrqB6f4IQjmvYi2B8Ehh0cwfjHHOYW_RU=@proton.me>
On Sat, May 02, 2026 at 09:34:18AM +0000, sebastien.stettler wrote:
> Has there been explorations of ignoring white space for the similarity checker, i would
> assume that majority of white space movements across many languages would result in a
> semantically similar document in most cases.
I don't think anybody has ever looked into it. We do have "-w" and
friends for diffs, and it makes sense that there might be some mode to
soften renames in the same way (especially if you are doing a "-w"
diff, or a merge that ignores whitespace).
The line you need to touch is probably this:
diff --git a/diffcore-delta.c b/diffcore-delta.c
index 2b7db39983..379f6010d3 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -147,6 +147,8 @@ static struct spanhash_top *hash_chars(struct repository *r,
/* Ignore CR in CRLF sequence if text */
if (is_text && c == '\r' && sz && *buf == '\n')
continue;
+ if (is_text && (c == ' ' || c == '\t'))
+ continue;
accum1 = (accum1 << 7) ^ (accum2 >> 25);
accum2 = (accum2 << 7) ^ (old_1 >> 25);
but:
1. The option to ignore whitespace would need to be plumbed through
the rest of the diffcore code.
2. This concept probably throws off some other rename heuristics.
E.g., I think we do a rough check that the sizes of the objects are
not too far apart before even looking at the content. So you could
construct a pathological case where the line "a\n" was changed to
have a million spaces, and the files would look like they couldn't
possibly be similar, even though they are identical when ignoring
whitespace. I think in practice you could just ignore this, as sane
cases would tend to have a reasonable ratio of content to
whitespace changes.
-Peff
^ permalink raw reply related
* Re: git clone with --dissociate sometimes fails to check out target commit
From: Jeff King @ 2026-05-04 9:51 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, emkan
In-Reply-To: <87h5onsi0f.fsf@prevas.dk>
On Mon, May 04, 2026 at 10:20:32AM +0200, Rasmus Villemoes wrote:
> Are we using --dissociate wrongly, or are we perhaps not maintaining
> those local mirror repos properly? They are essentially just created
> with 'git clone --mirror', with 'git remote update' run periodically.
>
> Naively, I'd expect the effects of --dissociate to only happen after
> everything else the clone command does has been done, but it seems that
> the ties to the reference repo are cut too soon.
No, you're using it correctly. The dissociate step should copy all of
the shared objects into the new repo, so it shouldn't matter whether we
do it before or after checkout. The objects are there either way.
But there's an interesting bug here with commit graphs. What happens is
this:
1. During the initial part of the clone, we may load a commit object
using the commit-graph file from the --reference repo. The commit
struct is left with a blank "maybe_tree" field, because we know we
can load it from the commit graph later (and don't want to spend
the effort to make a "struct tree" unless somebody asks for it).
2. During the dissociate step, we call odb_close(), since we're
throwing away the link to the reference repo, and we don't want to
use our in-process structs that point to it. That step also throws
away our open reference to the commit-graph file, and the
in-process slab that holds the graph positions we've loaded.
3. The checkout process needs the tree, so it calls
repo_get_commit_tree(). That sees that maybe_commit is NULL, so we
check whether it might be loaded from the graph file. But when we
ask about the graph position, we don't have one! It was in the slab
we threw away. So we return NULL, and the caller thinks the commit
is corrupt.
This is a bug that we theorized existed in a thread a while ago:
https://lore.kernel.org/git/20240110113914.GE16674@coredump.intra.peff.net/
but we didn't have a way to trigger it. Now we do. Hooray, I guess? ;)
The fallback load suggested in that message fixes it (modulo the fact
that it forgot to return commit->maybe_tree at the end of the function).
Below is a slightly safer version of the same concept that likewise
fixes the problem.
It's kind of ugly, but I think may be the least-bad solution. See that
earlier thread for more discussion of alternatives.
In the meantime, doing your dissociate clone with:
git -c core.commitGraph=false clone ...
should work around the problem.
---
diff --git a/commit.c b/commit.c
index 80d8d07875..50d736b339 100644
--- a/commit.c
+++ b/commit.c
@@ -434,16 +434,46 @@ static inline void set_commit_tree(struct commit *c, struct tree *t)
c->maybe_tree = t;
}
+static void load_tree_from_commit_contents(struct repository *r, struct commit *commit)
+{
+ enum object_type type;
+ unsigned long size;
+ char *buf;
+ const char *p;
+ struct object_id tree_oid;
+
+ buf = odb_read_object(r->objects, &commit->object.oid, &type, &size);
+ if (!buf)
+ return;
+
+ if (type == OBJ_COMMIT &&
+ skip_prefix(buf, "tree ", &p) &&
+ !parse_oid_hex(p, &tree_oid, &p) &&
+ *p == '\n')
+ commit->maybe_tree = lookup_tree(r, &tree_oid);
+
+ free(buf);
+}
+
struct tree *repo_get_commit_tree(struct repository *r,
- const struct commit *commit)
+ struct commit *commit)
{
if (commit->maybe_tree || !commit->object.parsed)
return commit->maybe_tree;
if (commit_graph_position(commit) != COMMIT_NOT_FROM_GRAPH)
return get_commit_tree_in_graph(r, commit);
- return NULL;
+ /*
+ * This is either a corrupt commit, or one which we partially loaded
+ * from a graph file but then subsequently threw away the graph data.
+ *
+ * Optimistically assume it's the latter and try to reload from
+ * scratch. This gives a performance penalty if it really is a corrupt
+ * commit, but presumably that happens rarely.
+ */
+ load_tree_from_commit_contents(r, commit);
+ return commit->maybe_tree;
}
struct object_id *get_commit_tree_oid(const struct commit *commit)
diff --git a/commit.h b/commit.h
index 58150045af..5eb1264077 100644
--- a/commit.h
+++ b/commit.h
@@ -163,7 +163,7 @@ void repo_unuse_commit_buffer(struct repository *r,
*/
void free_commit_buffer(struct parsed_object_pool *pool, struct commit *);
-struct tree *repo_get_commit_tree(struct repository *, const struct commit *);
+struct tree *repo_get_commit_tree(struct repository *, struct commit *);
struct object_id *get_commit_tree_oid(const struct commit *);
/*
^ permalink raw reply related
* Re: git clone with --dissociate sometimes fails to check out target commit
From: Jeff King @ 2026-05-04 9:54 UTC (permalink / raw)
To: Rasmus Villemoes; +Cc: git, emkan
In-Reply-To: <20260504095110.GA599780@coredump.intra.peff.net>
On Mon, May 04, 2026 at 05:51:10AM -0400, Jeff King wrote:
> No, you're using it correctly. The dissociate step should copy all of
> the shared objects into the new repo, so it shouldn't matter whether we
> do it before or after checkout. The objects are there either way.
>
> But there's an interesting bug here with commit graphs. What happens is
> this:
Oh, and ironically dissociating later _would_ fix this bug, like so:
diff --git a/builtin/clone.c b/builtin/clone.c
index fba3c9c508..7b7c83c717 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -1616,11 +1616,6 @@ int cmd_clone(int argc,
transport_unlock_pack(transport, 0);
transport_disconnect(transport);
- if (option_dissociate) {
- odb_close(the_repository->objects);
- dissociate_from_references();
- }
-
if (option_sparse_checkout && git_sparse_checkout_init(dir))
return 1;
@@ -1630,6 +1625,11 @@ int cmd_clone(int argc,
filter_submodules,
ref_storage_format);
+ if (option_dissociate) {
+ odb_close(the_repository->objects);
+ dissociate_from_references();
+ }
+
list_objects_filter_release(&filter_options);
string_list_clear(&option_not, 0);
But only because we are working around it: if we dissociate at the very
end, then there is no in-process code that will look at the objects
after that odb_close() call, and thus the bug cannot be triggered. It
would still potentially be lurking for other odb_close() callers,
though.
-Peff
^ permalink raw reply related
* Re: 10.26
From: Aa Kk @ 2026-05-04 9:38 UTC (permalink / raw)
To: git
In-Reply-To: <2D2D91D2-C309-42BE-BC1C-FDC5CCB8BF3F@icloud.com>
524152
Sent from my iPhone
> On 4 May 2569 BE, at 4:38 PM, Aa Kk <ahya0000@icloud.com> wrote:
>
>
> Sent from my iPhone
>
>
>> On 4 May 2569 BE, at 4:38 PM, Aa Kk <ahya0000@icloud.com> wrote:
>>
>>
>> Sent from my iPhone
>>
>>
>>>> On 4 May 2569 BE, at 4:37 PM, Aa Kk <ahya0000@icloud.com> wrote:
>>>
>>> 524152
>>> Sent from my iPhone
>>>
>>>
>>>>> On 4 May 2569 BE, at 12:05 AM, Aa Kk <ahya0000@icloud.com> wrote:
>>>>
>>>>
>>>> Sent from my iPhone
>> <Contact 3.vcf>
>>>>
>>>>
^ permalink raw reply
* Re: 10.26
From: Aa Kk @ 2026-05-04 9:38 UTC (permalink / raw)
To: git
In-Reply-To: <8277F598-127D-4629-9EAE-3B0D690B4209@icloud.com>
Sent from my iPhone
> On 4 May 2569 BE, at 4:37 PM, Aa Kk <ahya0000@icloud.com> wrote:
>
> 524152
> Sent from my iPhone
>
>
>> On 4 May 2569 BE, at 12:05 AM, Aa Kk <ahya0000@icloud.com> wrote:
>>
>>
>> Sent from my iPhone
>>
^ permalink raw reply
* Re: 10.26
From: Aa Kk @ 2026-05-04 9:38 UTC (permalink / raw)
To: git
In-Reply-To: <05948FB0-CE3E-4111-8B20-623AE2924267@icloud.com>
Sent from my iPhone
> On 4 May 2569 BE, at 4:38 PM, Aa Kk <ahya0000@icloud.com> wrote:
>
>
> Sent from my iPhone
>
>
>> On 4 May 2569 BE, at 4:37 PM, Aa Kk <ahya0000@icloud.com> wrote:
>>
>> 524152
>> Sent from my iPhone
>>
>>
>>>> On 4 May 2569 BE, at 12:05 AM, Aa Kk <ahya0000@icloud.com> wrote:
>>>
>>>
>>> Sent from my iPhone
> <Contact 3.vcf>
>>>
>>>
^ permalink raw reply
* Re: 10.26
From: Aa Kk @ 2026-05-04 9:38 UTC (permalink / raw)
To: git
In-Reply-To: <05948FB0-CE3E-4111-8B20-623AE2924267@icloud.com>
Sent from my iPhone
> On 4 May 2569 BE, at 4:38 PM, Aa Kk <ahya0000@icloud.com> wrote:
>
>
> Sent from my iPhone
>
>
>> On 4 May 2569 BE, at 4:37 PM, Aa Kk <ahya0000@icloud.com> wrote:
>>
>> 524152
>> Sent from my iPhone
>>
>>
>>>> On 4 May 2569 BE, at 12:05 AM, Aa Kk <ahya0000@icloud.com> wrote:
>>>
>>>
>>> Sent from my iPhone
> <Contact 3.vcf>
>>>
>>>
^ 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