From 997c1daf1dce036cd52ba2a4a3d4060b74dd9d54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 25 Aug 2026 20:03:47 +0200 Subject: [PATCH 01/18] worktree add: don't read out of bounds in worktree_basename() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we search for the start of the basename and `len` is zero, `name` ends up being `path` - 1, out of bounds. Avoid that by checking before decrementing. Fixes https://github.com/git-for-windows/git/issues/6346. Original-patch-by: Matthias Aßhauer Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/worktree.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index d21c43fde38b5e..1d827c4eae1360 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -303,11 +303,9 @@ static const char *worktree_basename(const char *path, int *olen) while (len && is_dir_sep(path[len - 1])) len--; - for (name = path + len - 1; name > path; name--) - if (is_dir_sep(*name)) { - name++; - break; - } + name = path + len; + while (name > path && !is_dir_sep(name[-1])) + name--; *olen = len; return name; From 382f88157754097baad9fb0b45513d9e9135a63e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 25 Aug 2026 20:03:48 +0200 Subject: [PATCH 02/18] worktree add: reject separator-only path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit worktree_basename() extracts an empty basename from a path consisting only of zero or more path separators. We can't use that as a worktree name. Properly report such a path as invalid instead of triggering a BUG that asks the user what just happened. Original-patch-by: Matthias Aßhauer Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/worktree.c | 2 ++ t/t2400-worktree-add.sh | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/builtin/worktree.c b/builtin/worktree.c index 1d827c4eae1360..214de50d4c38d4 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -491,6 +491,8 @@ static int add_worktree(const char *path, const char *refname, name = worktree_basename(path, &len); strbuf_add(&sb, name, path + len - name); + if (!sb.len) + die(_("invalid path '%s'"), path); sanitize_refname_component(sb.buf, &sb_name); if (!sb_name.len) BUG("How come '%s' becomes empty after sanitization?", sb.buf); diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index 58b4445cc441a8..4ffdd56fb4859b 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -46,6 +46,10 @@ test_expect_success '"add" refuses to checkout locked branch' ' test_path_is_missing .git/worktrees/zere ' +test_expect_success '"add" rejects an empty path' ' + test_must_fail git worktree add "" HEAD +' + test_expect_success 'checking out paths not complaining about linked checkouts' ' ( cd existing_empty && From a6de316efde124a132d38e50c6407477df47f9cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 25 Aug 2026 20:03:49 +0200 Subject: [PATCH 03/18] worktree add: trim slashes when deriving branch name from path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit worktree_basename() sets `n` to the length of `path` without trailing path separators, not to the length of the basename. This matters when deriving a branch name from a path with more than one component. E.g.: path: /new/worktree/ s: ^ n: |-----------| So here xstrndup(s, n) copies up to 13 characters from "worktree/", effectively to the end of the string, including the trailing dash. Path separators are not allowed at the end of branch names, so strip them off by calculating the basename length and extracting just that part. Signed-off-by: René Scharfe Signed-off-by: Junio C Hamano --- builtin/worktree.c | 4 ++-- t/t2400-worktree-add.sh | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 214de50d4c38d4..e5b7d6f5ec623b 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -768,7 +768,7 @@ static char *dwim_branch(const char *path, char **new_branch) int n; int branch_exists; const char *s = worktree_basename(path, &n); - char *branchname = xstrndup(s, n); + char *branchname = xmemdupz(s, path + n - s); struct strbuf ref = STRBUF_INIT; branch_exists = !check_branch_ref(&ref, branchname) && @@ -877,7 +877,7 @@ static int add(int ac, const char **av, const char *prefix, if (opts.orphan && !new_branch) { int n; const char *s = worktree_basename(path, &n); - new_branch = new_branch_to_free = xstrndup(s, n); + new_branch = new_branch_to_free = xmemdupz(s, path + n - s); } else if (opts.orphan) { ; /* no-op */ } else if (opts.detach) { diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index 4ffdd56fb4859b..9542a17cf38ef3 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -298,6 +298,11 @@ test_expect_success '"add" with omitted' ' test_cmp_rev HEAD bat ' +test_expect_success '"add" with trailing slash and omitted' ' + git worktree add waffle/bit/ && + test_cmp_rev HEAD bit +' + test_expect_success '"add" checks out existing branch of dwimd name' ' git branch dwim HEAD~1 && git worktree add dwim && @@ -388,6 +393,14 @@ test_expect_success '"add --orphan (no -b)"' ' test_cmp expected actual ' +test_expect_success '"add --orphan with trailing slash (no -b)"' ' + test_when_finished "git worktree remove -f -f neworphan" && + git worktree add --orphan ./neworphan/ && + echo refs/heads/neworphan >expected && + git -C neworphan symbolic-ref HEAD >actual && + test_cmp expected actual +' + test_expect_success '"add --orphan --quiet"' ' test_when_finished "git worktree remove -f -f orphandir" && git worktree add --quiet --orphan -b neworphan orphandir 2>log.actual && From 2e8a9d94b009c69628e29bc7c50d9a3fc12e14ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Scharfe?= Date: Tue, 25 Aug 2026 20:03:50 +0200 Subject: [PATCH 04/18] worktree add: let worktree_basename() return string copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit worktree_basename() requires callers to do pointer arithmetic to get the actual basename. Simplify them by doing the calculations in the function and returning a copy of the basename directly. Remind programmers to free the result by renaming the function to worktree_basename_dup(). Among the three callers of the original function, two immediately make copies of the returned string before using and freeing it, which makes for an easy conversion. Convert the other one from resetting a shared strbuf to freeing the allocated string, which requires the same number of lines, but no arithmetic. The added allocation is negligible because it's small and there's only one per run of "git worktree add". Signed-off-by: René Scharfe [jc: rephrased the second paragraph a bit.] Signed-off-by: Junio C Hamano --- builtin/worktree.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index e5b7d6f5ec623b..e5a4196f7f501c 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -294,7 +294,7 @@ static void remove_junk_on_signal(int signo) raise(signo); } -static const char *worktree_basename(const char *path, int *olen) +static char *worktree_basename_dup(const char *path) { const char *name; int len; @@ -307,8 +307,7 @@ static const char *worktree_basename(const char *path, int *olen) while (name > path && !is_dir_sep(name[-1])) name--; - *olen = len; - return name; + return xmemdupz(name, path + len - name); } /* check that path is viable location for worktree */ @@ -462,6 +461,7 @@ static int add_worktree(const char *path, const char *refname, struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; const char *name; + char *name_to_free = NULL; struct strvec child_env = STRVEC_INIT; unsigned int counter = 0; int len, ret; @@ -489,14 +489,12 @@ static int add_worktree(const char *path, const char *refname, if (!commit && !opts->orphan) die(_("invalid reference: %s"), refname); - name = worktree_basename(path, &len); - strbuf_add(&sb, name, path + len - name); - if (!sb.len) + name = name_to_free = worktree_basename_dup(path); + if (!*name) die(_("invalid path '%s'"), path); - sanitize_refname_component(sb.buf, &sb_name); + sanitize_refname_component(name, &sb_name); if (!sb_name.len) - BUG("How come '%s' becomes empty after sanitization?", sb.buf); - strbuf_reset(&sb); + BUG("How come '%s' becomes empty after sanitization?", name); name = sb_name.buf; repo_git_path_replace(the_repository, &sb_repo, "worktrees/%s", name); len = sb_repo.len; @@ -629,6 +627,7 @@ static int add_worktree(const char *path, const char *refname, strbuf_release(&sb_git); strbuf_release(&sb_name); free_worktree(wt); + free(name_to_free); return ret; } @@ -765,10 +764,8 @@ static int dwim_orphan(const struct add_opts *opts, int opt_track, int remote) static char *dwim_branch(const char *path, char **new_branch) { - int n; int branch_exists; - const char *s = worktree_basename(path, &n); - char *branchname = xmemdupz(s, path + n - s); + char *branchname = worktree_basename_dup(path); struct strbuf ref = STRBUF_INIT; branch_exists = !check_branch_ref(&ref, branchname) && @@ -875,9 +872,7 @@ static int add(int ac, const char **av, const char *prefix, } if (opts.orphan && !new_branch) { - int n; - const char *s = worktree_basename(path, &n); - new_branch = new_branch_to_free = xmemdupz(s, path + n - s); + new_branch = new_branch_to_free = worktree_basename_dup(path); } else if (opts.orphan) { ; /* no-op */ } else if (opts.detach) { From 1260b4661c181884e2069ac9adb6189b3d6cc2f1 Mon Sep 17 00:00:00 2001 From: Nikolaus Schuetz Date: Wed, 26 Aug 2026 16:51:47 +0000 Subject: [PATCH 05/18] t1401: check symbolic-ref failure and --quiet silence on a non-symbolic ref git-symbolic-ref(1) documents that reading a name that is not a symbolic ref fails, and that --quiet does so silently. Tests such as t2020 and t5621 already rely on "symbolic-ref -q HEAD" failing on a detached HEAD, but none checks that the plain form reports the error or that --quiet stays silent. Assert that a non-symbolic ref fails with the "is not a symbolic ref" message, and that --quiet fails with no output. Use test_must_fail rather than pinning the exact exit codes, which are documented but not worth freezing in the test. Signed-off-by: Nikolaus Schuetz Signed-off-by: Junio C Hamano --- t/t1401-symbolic-ref.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh index a2a7e947164c2a..fd3aa89a91cbc0 100755 --- a/t/t1401-symbolic-ref.sh +++ b/t/t1401-symbolic-ref.sh @@ -38,6 +38,18 @@ test_expect_success 'symbolic-ref refuses bare sha1' ' reset_to_sane +test_expect_success 'symbolic-ref reports a non-symbolic ref' ' + test_must_fail git symbolic-ref refs/heads/foo >out 2>err && + test_must_be_empty out && + test_grep "is not a symbolic ref" err +' + +test_expect_success 'symbolic-ref -q is silent on a non-symbolic ref' ' + test_must_fail git symbolic-ref -q refs/heads/foo >out 2>err && + test_must_be_empty out && + test_must_be_empty err +' + test_expect_success 'HEAD cannot be removed' ' test_must_fail git symbolic-ref -d HEAD ' From f27e711b7bd69406ce1ddc51a8239a894e6b88cc Mon Sep 17 00:00:00 2001 From: Nikolaus Schuetz Date: Thu, 20 Aug 2026 22:20:17 +0000 Subject: [PATCH 06/18] t1402: test forbidden characters in refnames git-check-ref-format(1) documents that a refname cannot contain a space, tilde, caret, colon, question-mark, asterisk, open-bracket or backslash, nor the sequence "..", and cannot be the single character "@". Of these, only "?", "\" and ".." were tested embedded in an otherwise-valid refname; "*" was checked only as a lone character or with --refspec-pattern. Test all of them in that embedded form with a single loop, and check that "@" alone is rejected even with --allow-onelevel -- where "@" is otherwise a valid refname component, as "refs/@" confirms. Signed-off-by: Nikolaus Schuetz Signed-off-by: Junio C Hamano --- t/t1402-check-ref-format.sh | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh index cabc516ae9a4fa..9dd64662b25be9 100755 --- a/t/t1402-check-ref-format.sh +++ b/t/t1402-check-ref-format.sh @@ -49,16 +49,19 @@ invalid_ref 'foo/./bar' invalid_ref 'foo/bar/.' invalid_ref '.refs/foo' invalid_ref 'refs/heads/foo.' -invalid_ref 'heads/foo..bar' -invalid_ref 'heads/foo?bar' +for c in '?' '~' '^' ':' '*' '[' ' ' '\' '..' +do + invalid_ref "heads/foo${c}bar" +done valid_ref 'foo./bar' invalid_ref 'heads/foo.lock' invalid_ref 'heads///foo.lock' invalid_ref 'foo.lock/bar' invalid_ref 'foo.lock///bar' valid_ref 'heads/foo@bar' +valid_ref 'refs/@' +invalid_ref '@' --allow-onelevel invalid_ref 'heads/v@{ation' -invalid_ref 'heads/foo\bar' invalid_ref "$(printf 'heads/foo\t')" invalid_ref "$(printf 'heads/foo\177')" valid_ref "$(printf 'heads/fu\303\237')" From b6f5e80fdf32c270ff812c1743e720dedca7d5e2 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 29 Aug 2026 07:00:28 +0000 Subject: [PATCH 07/18] replay: fail gracefully when a merge input is unreadable When objects involved in the merge cannot be read, the merge machinery will return early with result.clean = -1, and result.tree left as NULL. pick_regular_commit() tested only "if (!result->clean)", ignoring the case where "clean < 0". That causes the code to try to use result->tree, resulting in a SIGSEGV. Handle clean < 0 explicitly; the merge machinery will already have printed messages such as "Could not read " and "collecting merge info failed for trees...", so we don't need to add much detail beyond the fact that the merge failed. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- replay.c | 7 +++++++ t/t3650-replay-basics.sh | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/replay.c b/replay.c index 463c900d6c7c56..33e21b20320e01 100644 --- a/replay.c +++ b/replay.c @@ -327,6 +327,13 @@ static struct commit *pick_regular_commit(struct repository *repo, merge_opt->ancestor = NULL; merge_opt->branch2 = NULL; + if (result->clean < 0) { + error(_("merge of %s onto %s failed"), + oid_to_hex(&pickme->object.oid), + oid_to_hex(&replayed_base->object.oid)); + return NULL; + } + if (!result->clean) return NULL; diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index 3353bc4a4dc6ed..12348b4a5f2e64 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -565,4 +565,38 @@ test_expect_success '--onto with --ref rejects multiple revision ranges' ' test_grep "cannot be used with multiple revision ranges" err ' +test_expect_success 'replay fails without segfault when objects are missing' ' + test_when_finished "rm -fr unreadable" && + git init unreadable && + ( + cd unreadable && + + test_write_lines l1 l2 l3 l4 l5 l6 l7 l8 >f && + git add f && + git commit -m base && + git branch base && + + test_write_lines l1 l2 l3 l4 l5 l6 l7 CHANGED >f && + git commit -am side && + git branch side && + + git switch -c onto base && + test_write_lines CHANGED l2 l3 l4 l5 l6 l7 l8 >f && + git commit -am onto && + + # The replay works while every object is readable. + git replay --onto onto base..side && + + # Removing the onto tree makes parse_tree() fail during the + # incore merge, driving clean < 0 with a NULL result tree. + onto_tree=$(git rev-parse onto^{tree}) && + obj=$(test_oid_to_path "$onto_tree") && + mv .git/objects/${obj} saved-tree && + + # Ensure replay gracefully handles the missing object + test_must_fail git replay --onto onto base..side 2>err && + test_grep -e "Could not read" -e "collecting merge info failed" err + ) +' + test_done From 6272f3bd174fcd1b394d8b5e05b2ba384bd9f63e Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 29 Aug 2026 07:00:29 +0000 Subject: [PATCH 08/18] mktree: plug per-tree leak in --batch mode In --batch mode "git mktree" reuses its entry buffer across trees, resetting `used` to 0 after writing each tree. It never frees the `treeent` structures the previous tree appended, though, so once the next tree overwrites those slots the earlier allocations are leaked. A single-tree invocation hides this, as the entries stay reachable through the `entries` global until exit. Free each entry when resetting the buffer, and free the buffer itself before returning. Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/mktree.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/builtin/mktree.c b/builtin/mktree.c index 4084e324768b94..dc2d293c3d9b4e 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -200,8 +200,11 @@ int cmd_mktree(int ac, puts(oid_to_hex(&oid)); fflush(stdout); } + for (int i = 0; i < used; i++) + free(entries[i]); used=0; /* reset tree entry buffer for re-use in batch mode */ } + free(entries); strbuf_release(&sb); return 0; From 22eef58fba36a6dd9cadfe606b9f711e89266ae3 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 29 Aug 2026 07:00:30 +0000 Subject: [PATCH 09/18] mktree: do not use OBJECT_INFO_QUICK when checking objects mktree_line() checks each referenced object's type with odb_read_object_info_extended() under OBJECT_INFO_QUICK. QUICK skips the reprepare-and-retry that reloads the on-disk pack set, so a resident "git mktree --batch" reader reports an object that a concurrent repack just relocated into a new pack as missing, and rejects the entry. QUICK entered this lookup in 817b0f602710 (mktree: do not check type of remote objects, 2022-06-21) only to avoid lazily fetching promisor objects; OBJECT_INFO_SKIP_FETCH_OBJECT already provides that. Drop OBJECT_INFO_QUICK and keep OBJECT_INFO_SKIP_FETCH_OBJECT, so mktree still avoids a promisor fetch but recovers an object that was merely repacked. Add a regression test driving a resident mktree --batch reader across a concurrent repack that retires a pack. Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/mktree.c | 1 - t/t1010-mktree.sh | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/builtin/mktree.c b/builtin/mktree.c index dc2d293c3d9b4e..45ae2af3b5f13e 100644 --- a/builtin/mktree.c +++ b/builtin/mktree.c @@ -125,7 +125,6 @@ static void mktree_line(struct repository *repo, char *buf, int nul_term_line, i oi.typep = &obj_type; if (odb_read_object_info_extended(repo->objects, &oid, &oi, OBJECT_INFO_LOOKUP_REPLACE | - OBJECT_INFO_QUICK | OBJECT_INFO_SKIP_FETCH_OBJECT) < 0) obj_type = -1; diff --git a/t/t1010-mktree.sh b/t/t1010-mktree.sh index 312fe6717a622f..cecba55d451f2d 100755 --- a/t/t1010-mktree.sh +++ b/t/t1010-mktree.sh @@ -69,4 +69,52 @@ test_expect_success 'mktree refuses to read ls-tree -r output (2)' ' test_must_fail git mktree pack-a && + echo "$b" | git pack-objects .git/objects/pack/pack >pack-b && + + # Drop the loose copies so the blobs resolve only through the + # packs the multi-pack-index names. + git prune-packed && + git multi-pack-index write && + printf "100644 blob %s\ta\n" "$a" >tree-a && + printf "100644 blob %s\tb\n" "$b" >tree-b && + + victim=".git/objects/pack/pack-$(cat pack-b)" && + mkfifo in out && + + # mktree --batch stays resident, so its pack view predates the + # repack below; feed it one tree at a time over a fifo. The + # subshell exit closes the fifos, letting mktree see EOF and quit. + (git mktree --batch out 2>err &) && + exec 9>in && + exec 8&9 && echo >&9 && read tree_a <&8 && + + # Mimic a concurrent repack: a replacement pack holds every + # object, and the pack for b loses its .idx (its .pack lingers), + # matching the order in which unlink_pack_path() removes files. + git cat-file --batch-all-objects --batch-check="%(objectname)" >oids && + git pack-objects .git/objects/pack/pack /dev/null && + rm -f "$victim.idx" && + + # Resolving b used to fail, as its QUICK lookup accepted the + # miss; without QUICK the reader repreps and finds b in the + # replacement pack. + cat tree-b >&9 && echo >&9 && read tree_b <&8 && + exec 9>&- && + + test -n "$tree_b" + ) +' + test_done From 8f909ff4e9e883bf4938c0f0f67b9e1d48cc0167 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 29 Aug 2026 07:00:31 +0000 Subject: [PATCH 10/18] packfile: recover when a multi-pack-index names a removed pack A geometric repack writes a new pack and multi-pack-index and then deletes the packs the new one subsumes. A process still using the previous MIDX keeps seeing a removed pack listed as the owner of some objects. Since a MIDX attributes each object to exactly one pack, such an object is served only through its recorded owner; if that owner was just removed, find_pack_entry() cannot serve it -- the MIDX lookup routes to the missing pack, and the regular pack fallback deliberately skips every MIDX-covered pack, so a surviving copy in another covered pack (e.g. a kept base pack) is never consulted. Unlike the ordinary "a pack's .idx is mapped but its .pack is gone" race, the second read does not rescue us. Reloading the on-disk pack set does not reload the borrowed, cached MIDX (freeing it under the code that caches the "struct multi_pack_index *" would be a use-after-free), so the stale MIDX keeps routing to the removed pack and the surviving copy stays hidden behind the covered-pack skip. cat-file, rev-list and pack-objects can thus all spuriously fail with "unable to read object". Teach find_pack_entry() to recover. The MIDX lookup now returns a tri-state, distinguishing an object absent from the MIDX from one it owns via a pack that can no longer be opened; in the latter case, once the regular fallback has also missed, scan the MIDX's packs directly for a surviving copy. Because the return value is no longer a boolean, rename fill_midx_entry() to midx_fill_entry() so callers must reckon with the new enum rather than silently treat MIDX_FILL_OWNER_UNAVAILABLE as a hit. Do the scan only on the second read (OBJECT_INFO_SECOND_READ): by then the cheaper on-disk reload has run, so an object merely relocated into a new (uncovered) pack has already been found by the regular fallback, and only a genuine hidden duplicate reaches the rescan. A QUICK caller that skips the second read simply accepts the false negative, as QUICK is designed to. Reloading the stale MIDX would be a more complete fix but is much more involved (the borrowers above need proper invalidation), so leave that for later. Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol Helped-by: Jeff King Signed-off-by: Elijah Newren Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 2 +- midx.c | 20 +++++++++--------- midx.h | 21 +++++++++++++++++-- odb/source-packed.c | 42 ++++++++++++++++++++++++++++++++----- t/helper/test-read-midx.c | 2 +- t/t5319-multi-pack-index.sh | 40 +++++++++++++++++++++++++++++++++++ 6 files changed, 108 insertions(+), 19 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 399acd0f225d93..751d5d34498c88 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1786,7 +1786,7 @@ static int want_object_in_pack_mtime(const struct object_id *oid, struct multi_pack_index *m = get_multi_pack_index(files->packed); struct pack_entry e; - if (m && fill_midx_entry(m, oid, &e, NULL)) { + if (m && midx_fill_entry(m, oid, &e, NULL) == MIDX_FILL_HIT) { want = want_object_in_pack_one(e.p, oid, exclude, found_pack, found_offset, found_mtime); if (want != -1) return want; diff --git a/midx.c b/midx.c index 37f082dbdd5558..6d1c548e3dae8e 100644 --- a/midx.c +++ b/midx.c @@ -589,23 +589,23 @@ uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos) (off_t)pos * MIDX_CHUNK_OFFSET_WIDTH); } -int fill_midx_entry(struct multi_pack_index *m, - const struct object_id *oid, - struct pack_entry *e, - struct packed_git **bad_pack) +enum midx_fill_result midx_fill_entry(struct multi_pack_index *m, + const struct object_id *oid, + struct pack_entry *e, + struct packed_git **bad_pack) { uint32_t pos; uint32_t pack_int_id; struct packed_git *p; if (!bsearch_midx(oid, m, &pos)) - return 0; + return MIDX_FILL_MISS; midx_for_object(&m, pos); pack_int_id = nth_midxed_pack_int_id(m, pos); if (prepare_midx_pack(m, pack_int_id)) - return 0; + return MIDX_FILL_OWNER_UNAVAILABLE; p = m->packs[pack_int_id - m->num_packs_in_base]; /* @@ -616,19 +616,19 @@ int fill_midx_entry(struct multi_pack_index *m, * loaded! */ if (!is_pack_valid(p)) - return 0; + return MIDX_FILL_OWNER_UNAVAILABLE; if (oidset_size(&p->bad_objects) && oidset_contains(&p->bad_objects, oid)) { if (bad_pack && !*bad_pack) *bad_pack = p; - return 0; + return MIDX_FILL_MISS; } e->offset = nth_midxed_offset(m, pos); e->p = p; - return 1; + return MIDX_FILL_HIT; } /* Match "foo.idx" against either "foo.pack" _or_ "foo.idx". */ @@ -1032,7 +1032,7 @@ int verify_midx_file(struct odb_source_packed *source, unsigned flags) nth_midxed_object_oid(&oid, m, pairs[i].pos); - if (!fill_midx_entry(m, &oid, &e, NULL)) { + if (midx_fill_entry(m, &oid, &e, NULL) != MIDX_FILL_HIT) { midx_report(_("failed to load pack entry for oid[%d] = %s"), pairs[i].pos, oid_to_hex(&oid)); continue; diff --git a/midx.h b/midx.h index 1f2f2d53214da5..4b768769b98a10 100644 --- a/midx.h +++ b/midx.h @@ -117,8 +117,25 @@ uint32_t nth_midxed_pack_int_id(struct multi_pack_index *m, uint32_t pos); struct object_id *nth_midxed_object_oid(struct object_id *oid, struct multi_pack_index *m, uint32_t n); -int fill_midx_entry(struct multi_pack_index *m, const struct object_id *oid, - struct pack_entry *e, struct packed_git **bad_pack); +/* + * Result of looking an object up in a multi-pack-index. MIDX_FILL_HIT means + * "e was filled in"; the two miss variants distinguish an object the midx does + * not know about (MIDX_FILL_MISS) from one it does know about but whose owning + * pack we can no longer open (MIDX_FILL_OWNER_UNAVAILABLE -- the signature of a + * concurrent repack having removed that pack). A known-bad (corrupt) object + * reports MIDX_FILL_MISS but also sets *bad_pack, if provided, to the owning + * pack so the caller can tell "corrupt" apart from "absent". + */ +enum midx_fill_result { + MIDX_FILL_MISS = 0, + MIDX_FILL_HIT, + MIDX_FILL_OWNER_UNAVAILABLE, +}; + +enum midx_fill_result midx_fill_entry(struct multi_pack_index *m, + const struct object_id *oid, + struct pack_entry *e, + struct packed_git **bad_pack); int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name); int midx_layer_contains_pack(struct multi_pack_index *m, diff --git a/odb/source-packed.c b/odb/source-packed.c index 1a12a605dbc62e..90d88c0a121c5e 100644 --- a/odb/source-packed.c +++ b/odb/source-packed.c @@ -17,13 +17,18 @@ static int find_pack_entry(struct odb_source_packed *store, const struct object_id *oid, struct pack_entry *e, + enum object_info_flags flags, struct packed_git **bad_pack) { struct packfile_list_entry *l; + enum midx_fill_result midx_result = MIDX_FILL_MISS; odb_source_prepare(&store->base, 0); - if (store->midx && fill_midx_entry(store->midx, oid, e, bad_pack)) - return 1; + if (store->midx) { + midx_result = midx_fill_entry(store->midx, oid, e, bad_pack); + if (midx_result == MIDX_FILL_HIT) + return 1; + } for (l = store->packs.head; l; l = l->next) { struct packed_git *p = l->pack; @@ -35,6 +40,33 @@ static int find_pack_entry(struct odb_source_packed *store, } } + /* + * Recovery for a concurrent-repack race: a stale MIDX may still name a + * vanished owning pack even though the object survives in another pack + * the same MIDX covers. The regular fallback above skips MIDX-covered + * packs, and repreparing the on-disk pack set does not reload the + * borrowed, cached MIDX, so scan its packs directly for the survivor. + * + * Do this only on the second read, by which point repreparing packs has + * already had a chance to find an object merely relocated into a new, + * uncovered pack; only a genuine hidden duplicate reaches here. + */ + if (midx_result == MIDX_FILL_OWNER_UNAVAILABLE && + (flags & OBJECT_INFO_SECOND_READ)) { + struct multi_pack_index *m = store->midx; + uint32_t i; + + for (i = 0; i < m->num_packs + m->num_packs_in_base; i++) { + struct packed_git *p; + + if (prepare_midx_pack(m, i)) + continue; + p = nth_midxed_pack(m, i); + if (p && packfile_fill_entry(p, oid, e, bad_pack)) + return 1; + } + } + return 0; } @@ -57,7 +89,7 @@ static enum odb_read_status odb_source_packed_read_object_info(struct odb_source if (flags & OBJECT_INFO_SECOND_READ) odb_source_prepare(source, ODB_PREPARE_FLUSH_CACHES); - if (!find_pack_entry(packed, oid, &e, &bad_pack)) { + if (!find_pack_entry(packed, oid, &e, flags, &bad_pack)) { /* * The lookup may have failed because the object is known to be * corrupt in one of the packfiles. Report the object as @@ -105,7 +137,7 @@ static int odb_source_packed_read_object_stream(struct odb_read_stream **out, struct odb_source_packed *packed = odb_source_packed_downcast(source); struct pack_entry e; - if (!find_pack_entry(packed, oid, &e, NULL)) + if (!find_pack_entry(packed, oid, &e, 0, NULL)) return -1; return packfile_read_object_stream(out, oid, e.p, e.offset); @@ -611,7 +643,7 @@ static int odb_source_packed_freshen_object(struct odb_source *source, timesp = × } - if (!find_pack_entry(packed, oid, &e, NULL)) + if (!find_pack_entry(packed, oid, &e, 0, NULL)) return 0; if (e.p->is_cruft) return 0; diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c index 27a05da957afc2..9c5e30876102eb 100644 --- a/t/helper/test-read-midx.c +++ b/t/helper/test-read-midx.c @@ -82,7 +82,7 @@ static int read_midx_file(const char *object_dir, const char *checksum, for (i = 0; i < m->num_objects; i++) { nth_midxed_object_oid(&oid, m, i + m->num_objects_in_base); - fill_midx_entry(m, &oid, &e, NULL); + midx_fill_entry(m, &oid, &e, NULL); printf("%s %"PRIu64"\t%s\n", oid_to_hex(&oid), e.offset, e.p->pack_name); diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index 68143cb5b76952..2b8ff6f3ed28cd 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -1393,4 +1393,44 @@ test_expect_success 'pack.preferBitmapTips interprets patterns as hierarchy' ' ) ' +test_expect_success 'lookup recovers object whose midx-owning pack was removed' ' + test_when_finished "rm -fr repo" && + git init repo && + ( + cd repo && + + # "keep" ends up only in the big pack; "dup" is deliberately + # placed in two packs so the midx has to choose an owner. + test_commit keep && + echo duplicated-content >dup && + git add dup && + git commit -m dup && + dup_oid=$(git rev-parse HEAD:dup) && + + # Roll every object, including dup, into a single big pack. + git repack -adq && + + # Build a second, "moderate" pack that also contains dup, so dup + # now lives in two packs that the midx will cover. + moderate=$(echo "$dup_oid" | + git pack-objects --quiet $objdir/pack/pack) && + + # Attribute dup to the moderate pack in the midx. + git multi-pack-index write \ + --preferred-pack="pack-$moderate.idx" && + + # Simulate a concurrent "git repack" retiring the moderate pack: + # its files disappear, but the now-stale midx still names it as + # the owner of dup. A valid copy of dup survives in the big pack. + rm -f $objdir/pack/pack-$moderate.* && + + # The midx routes the lookup to the deleted pack, and the regular + # pack fallback skips midx-covered packs, so without recovery dup + # would appear missing even though it is physically present. + echo blob >expect && + git cat-file -t "$dup_oid" >actual && + test_cmp expect actual + ) +' + test_done From 98b33a62f604c608ae2648750de767b23e961e67 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 15:13:47 +0200 Subject: [PATCH 11/18] replay: add helper to put entry into replayed_commits The function replay_revisions() in replay.c is rather lengthy. Extract the logic to put a commit entry into a `struct mapped_commits` into a helper function put_mapped_commit(). While at it, rename mapped_commit() to get_mapped_commit() to pair with this new function. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- replay.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/replay.c b/replay.c index 463c900d6c7c56..860e194ba064c9 100644 --- a/replay.c +++ b/replay.c @@ -254,9 +254,9 @@ static void set_up_replay_mode(struct repository *repo, strset_clear(&rinfo.positive_refs); } -static struct commit *mapped_commit(kh_oid_map_t *replayed_commits, - struct commit *commit, - struct commit *fallback) +static struct commit *get_mapped_commit(kh_oid_map_t *replayed_commits, + struct commit *commit, + struct commit *fallback) { khint_t pos; if (!commit) @@ -267,6 +267,21 @@ static struct commit *mapped_commit(kh_oid_map_t *replayed_commits, return kh_value(replayed_commits, pos); } +static void put_mapped_commit(kh_oid_map_t *replayed_commits, + struct commit *commit, + struct commit *new_commit) +{ + khint_t pos; + int ret; + + pos = kh_put_oid_map(replayed_commits, commit->object.oid, &ret); + if (ret == 0) + BUG("Duplicate rewritten commit: %s", + oid_to_hex(&commit->object.oid)); + + kh_value(replayed_commits, pos) = new_commit; +} + static struct commit *pick_regular_commit(struct repository *repo, struct commit *pickme, kh_oid_map_t *replayed_commits, @@ -287,7 +302,7 @@ static struct commit *pick_regular_commit(struct repository *repo, base_tree = lookup_tree(repo, repo->hash_algo->empty_tree); } - replayed_base = mapped_commit(replayed_commits, base, onto); + replayed_base = get_mapped_commit(replayed_commits, base, onto); replayed_base_tree = repo_get_commit_tree(repo, replayed_base); pickme_tree = repo_get_commit_tree(repo, pickme); @@ -427,8 +442,6 @@ int replay_revisions(struct rev_info *revs, replayed_commits = kh_init_oid_map(); while ((commit = get_revision(revs))) { const struct name_decoration *decoration; - khint_t pos; - int hr; if (commit->parents && commit->parents->next) die(_("replaying merge commits is not supported yet!")); @@ -440,11 +453,7 @@ int replay_revisions(struct rev_info *revs, break; /* Record commit -> last_commit mapping */ - pos = kh_put_oid_map(replayed_commits, commit->object.oid, &hr); - if (hr == 0) - BUG("Duplicate rewritten commit: %s\n", - oid_to_hex(&commit->object.oid)); - kh_value(replayed_commits, pos) = last_commit; + put_mapped_commit(replayed_commits, commit, last_commit); /* Update any necessary branches */ if (ref) From 446099ef7c6106a73143a651967a6424328c53dd Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 15:13:48 +0200 Subject: [PATCH 12/18] replay: resolve the replay base outside pick_regular_commit() Depending on what gets passed into the function pick_regular_commit(), it decides the new base for the replayed commit. It first tries to find the replayed results of `pickme`'s parent in the `replayed_commits` map. If not found, it falls back to `onto`. When using git-replay(1) with --onto, the fallback is the revision passed in with this option, but when using --revert, the fallback is `last_commit`. It's rather confusing the base is decided partly inside pick_regular_commit() and partly by its caller. Move the base selection completely into the caller: replay_revisions(). This bundles all the logic of deciding on the base together. Also, this reduces the number of parameters of pick_regular_commit(), making its interface cleaner. This refactoring doesn't bring any behavior changes. Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- replay.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/replay.c b/replay.c index 860e194ba064c9..7e35f40d379e84 100644 --- a/replay.c +++ b/replay.c @@ -284,25 +284,19 @@ static void put_mapped_commit(kh_oid_map_t *replayed_commits, static struct commit *pick_regular_commit(struct repository *repo, struct commit *pickme, - kh_oid_map_t *replayed_commits, - struct commit *onto, + struct commit *replayed_base, struct merge_options *merge_opt, struct merge_result *result, enum replay_mode mode, enum replay_empty_commit_action empty) { - struct commit *base, *replayed_base; struct tree *pickme_tree, *base_tree, *replayed_base_tree; - if (pickme->parents) { - base = pickme->parents->item; - base_tree = repo_get_commit_tree(repo, base); - } else { - base = NULL; + if (pickme->parents) + base_tree = repo_get_commit_tree(repo, pickme->parents->item); + else base_tree = lookup_tree(repo, repo->hash_algo->empty_tree); - } - replayed_base = get_mapped_commit(replayed_commits, base, onto); replayed_base_tree = repo_get_commit_tree(repo, replayed_base); pickme_tree = repo_get_commit_tree(repo, pickme); @@ -443,12 +437,26 @@ int replay_revisions(struct rev_info *revs, while ((commit = get_revision(revs))) { const struct name_decoration *decoration; + /* + * Decide where to replay this commit on. + * If the parent commit was replayed already, the replayed result + * can be found in `replayed_commits`. Otherwise fall back to `onto`. + * When reverting, commits are replayed in reverse order and thus + * its parent isn't replayed yet. Therefore revert commits are + * always replayed onto `last_commit`. + */ + struct commit *parent = commit->parents ? commit->parents->item : NULL; + struct commit *base = get_mapped_commit(replayed_commits, parent, onto); + + if (mode == REPLAY_MODE_REVERT) + base = last_commit; + if (commit->parents && commit->parents->next) die(_("replaying merge commits is not supported yet!")); - last_commit = pick_regular_commit(revs->repo, commit, replayed_commits, - mode == REPLAY_MODE_REVERT ? last_commit : onto, - &merge_opt, &result, mode, opts->empty); + last_commit = pick_regular_commit(revs->repo, commit, base, + &merge_opt, &result, + mode, opts->empty); if (!last_commit) break; From 354c736978bdc7a1d0bb0f19b81060182a374be6 Mon Sep 17 00:00:00 2001 From: Toon Claes Date: Mon, 31 Aug 2026 15:13:49 +0200 Subject: [PATCH 13/18] replay: offer an option to linearize the commit topology One of the stated goals of git-replay(1) is to allow implementing the git-rebase(1) functionality on the server side. The default mode of git-rebase(1) is to act as if `--no-rebase-merges` was given. This mode drops merge commits instead of replaying them, and linearizes the history into a sequence of regular (single-parent) commits. Add option `--linearize` to git-replay(1) to do the same. Each replayed commit is stacked on top of the previously replayed one. When a merge is encountered, the commits reachable from all of its sides are replayed into the single line and the merge itself is dropped. If a ref was pointing to a merge commit, that ref is updated to the merge's last replayed ancestor. git-replay(1) accepts multiple branches, for example: $ git replay --onto main topic1 topic2 Without `--linearize` this replays 'topic1' and 'topic2' onto 'main' (keeping shared portions of history shared and divergent parts divergent) and updates both refs. Due to current implementation limitations, replaying multiple branches with `--linearize` is disallowed to avoid concatenating unrelated histories into a single line. For the same reason disallow the use of `--contained` with `--linearize`. Users who want to linearize multiple branches are advised to do this in separate git-replay(1) invocations. Linearizing multiple branches at once might be added later. Note that `--linearize` is not modeled after git-rebase(1)'s `--rebase-merges[=]` interface. Recreating merges, by preserving their topology, is a distinct operation that would be a separate mode. `--linearize` only drops merges and replays commits linearly. So git-replay(1) uses its own option rather than reusing that interface. Based-on-patches-by: Johannes Schindelin Signed-off-by: Toon Claes Signed-off-by: Junio C Hamano --- Documentation/git-replay.adoc | 17 +++++- builtin/replay.c | 6 +- replay.c | 60 ++++++++++++------- replay.h | 5 ++ t/t3650-replay-basics.sh | 109 +++++++++++++++++++++++++++++++++- 5 files changed, 174 insertions(+), 23 deletions(-) diff --git a/Documentation/git-replay.adoc b/Documentation/git-replay.adoc index a32f72aead3750..84a54babf35e08 100644 --- a/Documentation/git-replay.adoc +++ b/Documentation/git-replay.adoc @@ -10,7 +10,7 @@ SYNOPSIS -------- [verse] (EXPERIMENTAL!) 'git replay' ([--contained] --onto= | --advance= | --revert=) - [--ref=] [--ref-action=] + [--ref=] [--ref-action=] [--linearize] DESCRIPTION ----------- @@ -88,6 +88,21 @@ incompatible with `--contained` (which is a modifier for `--onto` only). + The default mode can be configured via the `replay.refAction` configuration variable. +--linearize:: + In this mode, each replayed commit is stacked on top of the + previously replayed one, so all replayed commits are flattened into + a single linear history. ++ +When a merge commit is encountered, all commits in the range reachable +from the merge commit are replayed into the linear history, and the +merge commit itself is dropped. A ref that pointed to a merge commit is +updated to the merge's last replayed ancestor. (This matches the +behavior of git-rebase(1)'s `--no-rebase-merges` option.) ++ +`--linearize` cannot be combined with multiple branches or with +`--contained`. To linearize several branches, replay them in separate +`git replay` invocations. + :: Range of commits to replay; see "Specifying Ranges" in linkgit:git-rev-parse[1]. In `--advance=` or diff --git a/builtin/replay.c b/builtin/replay.c index 39e3a86f6c10ab..d39626a37d055c 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -85,7 +85,7 @@ int cmd_replay(int argc, const char *const replay_usage[] = { N_("(EXPERIMENTAL!) git replay " "([--contained] --onto= | --advance= | --revert=)\n" - "[--ref=] [--ref-action=] "), + "[--ref=] [--ref-action=] [--linearize] "), NULL }; struct option replay_options[] = { @@ -111,6 +111,8 @@ int cmd_replay(int argc, N_("mode"), N_("control ref update behavior (update|print)"), PARSE_OPT_NONEG), + OPT_BOOL(0, "linearize", &opts.linearize, + N_("drop merge commits, replaying only non-merge commits")), OPT_END() }; @@ -132,6 +134,8 @@ int cmd_replay(int argc, opts.contained, "--contained"); die_for_incompatible_opt2(!!opts.ref, "--ref", !!opts.contained, "--contained"); + die_for_incompatible_opt2(opts.linearize, "--linearize", + !!opts.contained, "--contained"); /* Parse ref action mode from command line or config */ ref_mode = get_ref_action_mode(repo, ref_action); diff --git a/replay.c b/replay.c index 7e35f40d379e84..6565e4d2152362 100644 --- a/replay.c +++ b/replay.c @@ -404,6 +404,12 @@ int replay_revisions(struct rev_info *revs, set_up_replay_mode(revs->repo, &revs->cmdline, opts->onto, &detached_head, &advance, &revert, &onto, &update_refs); + if (opts->linearize && + update_refs && strset_get_size(update_refs) > 1) { + ret = error(_("'--linearize' cannot be used with multiple branches")); + goto out; + } + if (opts->ref) { struct object_id oid; @@ -437,26 +443,40 @@ int replay_revisions(struct rev_info *revs, while ((commit = get_revision(revs))) { const struct name_decoration *decoration; - /* - * Decide where to replay this commit on. - * If the parent commit was replayed already, the replayed result - * can be found in `replayed_commits`. Otherwise fall back to `onto`. - * When reverting, commits are replayed in reverse order and thus - * its parent isn't replayed yet. Therefore revert commits are - * always replayed onto `last_commit`. - */ - struct commit *parent = commit->parents ? commit->parents->item : NULL; - struct commit *base = get_mapped_commit(replayed_commits, parent, onto); - - if (mode == REPLAY_MODE_REVERT) - base = last_commit; - - if (commit->parents && commit->parents->next) - die(_("replaying merge commits is not supported yet!")); - - last_commit = pick_regular_commit(revs->repo, commit, base, - &merge_opt, &result, - mode, opts->empty); + if (commit->parents && commit->parents->next) { + if (!opts->linearize) + die(_("replaying merge commits is not supported yet!")); + /* + * Drop the merge commit: do not pick it, leave + * `last_commit` unchanged, and fall through to the + * rest of the loop. As a result: + * - refs pointing to the merge commit will be updated + * to `last_commit`. + * - the next replayed commit uses `last_commit` as its + * `base`. + */ + } else { + /* + * Decide where to replay this commit onto. + * If the parent commit was replayed already, the replayed result + * can be found in `replayed_commits`. Otherwise fall back to `onto`. + * When reverting, commits are replayed in reverse order and thus + * its parent isn't replayed yet. Therefore revert commits are + * always replayed onto `last_commit`. + * Also when opts->linearize is true, set the base to + * `last_commit` to create a single linear history. + */ + struct commit *parent = commit->parents ? commit->parents->item : NULL; + struct commit *base = get_mapped_commit(replayed_commits, parent, onto); + + if (opts->linearize || mode == REPLAY_MODE_REVERT) + base = last_commit; + + last_commit = pick_regular_commit(revs->repo, commit, base, + &merge_opt, &result, + mode, opts->empty); + } + if (!last_commit) break; diff --git a/replay.h b/replay.h index 491db145e30151..2c71afbfde05b4 100644 --- a/replay.h +++ b/replay.h @@ -62,6 +62,11 @@ struct replay_revisions_options { * Defaults to REPLAY_EMPTY_COMMIT_DROP. */ enum replay_empty_commit_action empty; + + /* + * Whether to linearize the commits (i.e. drop merge commits). + */ + int linearize; }; /* This struct is used as an out-parameter by `replay_revisions()`. */ diff --git a/t/t3650-replay-basics.sh b/t/t3650-replay-basics.sh index 3353bc4a4dc6ed..d3409b9bb121c3 100755 --- a/t/t3650-replay-basics.sh +++ b/t/t3650-replay-basics.sh @@ -52,8 +52,19 @@ test_expect_success 'setup' ' test_merge P O --no-ff && git switch main && + git switch --orphan unrelated && + test_commit unrelated-root && + git switch -c conflict B && - test_commit C.conflict C.t conflict + test_commit C.conflict C.t conflict && + git branch -D unrelated && + + git switch -c divergent-x main && + test_commit X && + git switch -c divergent-y main && + test_commit Y && + git switch divergent-x && + test_merge Z divergent-y --no-ff ' test_expect_success 'setup bare' ' @@ -565,4 +576,100 @@ test_expect_success '--onto with --ref rejects multiple revision ranges' ' test_grep "cannot be used with multiple revision ranges" err ' +test_expect_success 'replay to rebase merge commit with --linearize' ' + git replay --ref-action=print --linearize \ + --onto main I..topic-with-merge >result && + + test_line_count = 1 result && + + git log --format=%s $(cut -f 3 -d " " result) >actual && + test_write_lines O N J M L B A >expect && + test_cmp expect actual +' + +test_expect_success 'replay to rebase merge commit with --linearize down to the root commit' ' + git replay --ref-action=print --linearize \ + --onto unrelated-root topic-with-merge >result && + + test_line_count = 1 result && + + git log --format=%s $(cut -f 3 -d " " result) >actual && + test_write_lines O N J I B A unrelated-root >expect && + test_cmp expect actual +' + +test_expect_success 'replay to cherry-pick merge commit with --linearize' ' + git replay --ref-action=print --linearize \ + --advance main I..topic-with-merge >result && + + test_line_count = 1 result && + + git log --format=%s $(cut -f 3 -d " " result) >actual && + test_write_lines O N J M L B A >expect && + test_cmp expect actual && + + printf "update refs/heads/main " >expect && + printf "%s " $(cut -f 3 -d " " result) >>expect && + git rev-parse main >>expect && + test_cmp expect result +' + +test_expect_success 'replay --linearize produces the same patches' ' + git replay --ref-action=print --linearize \ + --onto main I..topic-with-merge >result && + + test_line_count = 1 result && + tip=$(cut -f 3 -d " " result) && + + # range-diff does not care about the dropped merge, + # so the original commits (I..topic-with-merge) + # and the replayed chain (main..tip) must produce identical patches. + git range-diff I..topic-with-merge main..$tip >out && + test_file_not_empty out && + test_grep ! -v "=" out && + + git log --oneline main..$tip >out && + test_line_count = 3 out +' + +test_expect_success '--linearize rejects multiple branches' ' + test_must_fail git replay --ref-action=print --linearize \ + --onto main ^B topic2 topic3 topic4 2>err && + test_grep "cannot be used with multiple branches" err +' + +test_expect_success 'replay with --linearize of a divergent merge keeps both sides' ' + git replay --ref-action=print --linearize \ + --onto main main..divergent-x >result && + test_line_count = 1 result && + tip=$(cut -f 3 -d " " result) && + + # The merge Z is dropped, but both X and Y are linearized onto main; + # neither side is lost. + git log --format=%s main..$tip >actual && + test_write_lines Y X >expect && + test_cmp expect actual +' + +test_expect_success '--linearize and --contained cannot be used together' ' + test_must_fail git replay --ref-action=print --linearize --contained \ + --onto main ^B topic-with-merge 2>err && + test_grep "cannot be used together" err +' + +test_expect_success 'replay --revert with --linearize reverts a range containing a merge' ' + git replay --ref-action=print --revert=divergent-x --linearize \ + main..divergent-x >result && + test_line_count = 1 result && + tip=$(cut -f 3 -d " " result) && + + git log --format=%s $tip >actual && + test_write_lines \ + "Revert \"X\"" "Revert \"Y\"" Z Y X M L B A >expect && + test_cmp expect actual && + + test_must_fail git cat-file -e $tip:X.t && + test_must_fail git cat-file -e $tip:Y.t +' + test_done From a251b1bd213cc2991ad17c78f2cfe0fad826a159 Mon Sep 17 00:00:00 2001 From: Harald Nordgren Date: Mon, 31 Aug 2026 16:18:15 +0000 Subject: [PATCH 14/18] ci: cancel stale pull request workflow runs The CI workflow groups all runs by commit hash using `group: ${{ github.sha }}`. This means every push to a pull request starts a separate workflow run, and all workflows triggered by the same commit share the same concurrency group. With this change, pull request runs are grouped by pull request number instead of commit hash, and runs superseded by a newer push are canceled. The concurrency group becomes `${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }}` and `cancel-in-progress` is set to true for pull request events. For pull request events, the group is `-` (e.g., "main-workflow-42"). If you push a new commit to an existing pull request before the CI working on it finishes, the new request will be placed in the same group and cancel the currently running run. For non-pull-request events, the group is `${{ github.workflow }}-${{ github.sha }}` and `cancel-in-progress` defaults to false, so there is no regression in behavior. Note that the previous configuration used `group: ${{ github.sha }}`, which meant all workflows sharing the same commit hash were in the same group. The new configuration includes the workflow name in the group, so each workflow has its own concurrency group per commit/PR. Signed-off-by: Harald Nordgren Signed-off-by: Junio C Hamano --- .github/workflows/main.yml | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index cf341d74dbff21..6b56c36996950f 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -5,18 +5,20 @@ on: [push, pull_request] env: DEVELOPER: 1 -# If more than one workflow run is triggered for the very same commit hash -# (which happens when multiple branches pointing to the same commit), only -# the first one is allowed to run, the second will be kept in the "queued" -# state. This allows a successful completion of the first run to be reused -# in the second run via the `skip-if-redundant` logic in the `config` job. +# For pull requests, only the latest workflow run is allowed to proceed. +# Older runs are canceled when a new revision is pushed. # -# The only caveat is that if a workflow run is triggered for the same commit -# hash that another run is already being held, that latter run will be -# canceled. For more details about the `concurrency` attribute, see: +# For pushes, if more than one workflow run is triggered for the very same +# commit hash (which happens when multiple branches point to the same commit), +# only the first one is allowed to run. This allows a successful completion of +# the first run to be reused in the second run via the `skip-if-redundant` +# logic in the `config` job. +# +# For more details about the `concurrency` attribute, see: # https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#concurrency concurrency: - group: ${{ github.sha }} + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.sha }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} jobs: ci-config: From c486c1df728652ee3c87d395bbe5217f7fc07ce8 Mon Sep 17 00:00:00 2001 From: Hardik Kumar Date: Tue, 1 Sep 2026 00:59:13 +0530 Subject: [PATCH 15/18] versioncmp: fix typo in versioncmp.c, t/t0022-crlf-rename.sh The patch fixes two typos in two places. versioncmp.c: "fractionnal" -> "fractional" t/t0022-crlf-rename.sh: "similiarity" -> "similarity" No functional changes, only update a comment and a test_description. Signed-off-by: Hardik Kumar Signed-off-by: Junio C Hamano --- t/t0022-crlf-rename.sh | 2 +- versioncmp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t0022-crlf-rename.sh b/t/t0022-crlf-rename.sh index 9bd863a970d2b1..328c6e5903ba4d 100755 --- a/t/t0022-crlf-rename.sh +++ b/t/t0022-crlf-rename.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='ignore CR in CRLF sequence while computing similiarity' +test_description='ignore CR in CRLF sequence while computing similarity' . ./test-lib.sh diff --git a/versioncmp.c b/versioncmp.c index 3a81b17bc1b8d1..f1e451755a7800 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -15,7 +15,7 @@ /* * states: S_N: normal, S_I: comparing integral part, S_F: comparing - * fractionnal parts, S_Z: idem but with leading Zeroes only + * fractional parts, S_Z: idem but with leading Zeroes only */ #define S_N 0x0 #define S_I 0x3 From 66f4856110a7577c12f97ae905c95a6f38adba9d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 1 Sep 2026 02:28:15 -0400 Subject: [PATCH 16/18] revision: hang on to "freed" argv elements In setup_revisions() we rewrite the incoming argv array, losing references to the strings it contains. For a synthetic argv array constructed from heap strings, that traditionally meant we leaked those allocated strings. We fixed the leak in cd43948798 (revision: manage memory ownership of argv in setup_revisions(), 2025-09-19). Now callers can tell the revision code that argv entries are allocated and should be freed, which it will do before overwriting them. But this introduced a new bug! The overwritten entries go away as soon as option parsing is finished, but a few options may actually create new references to those strings. And once we free the strings, those stale references become use-after-free bugs. For example, running: git stash show --src-prefix=foo/ demonstrates the problem: 1. The stash command generates its own synthetic argv (because it has to treat the stash specifiers specially) which it then passes to setup_revisions(). 2. Parsing will create a reference to the partial string "foo/" in revs.diffopt.a_prefix. 3. When setup_revisions() finishes, we rewrite argv to throw away parsed strings. This frees the entry holding "--src-prefix=foo", at which point we have a dangling reference in revs.diffopt. 4. We generate an actual diff, accessing garbage memory via revs.diffopt.a_prefix. The output is usually garbled, but ASan also detects this reliably. One obvious fix here is to allocate new strings when we pull data out of the argv array. But doing so is error prone (every string option must remember to do it or risk a subtle bug), and creates more questions about memory ownership (e.g., some callers assign string literals directly to a_prefix, and we would not want to free those). Instead we can fix this centrally by delaying the free() calls. We'll collect any "freed" strings in a new array, hold on to it for the life of the rev_info struct, and then release it at the end. We can easily use a strvec for this, since it handles growth and cleanup for us. This fixes the prefix case above (which is now tested in t3903), and should fix any other stray cases. Though I could not find any; we use OPT_STRING only in the prefix diff options, and very few revision opts store strings. Those that do (like --format and --encoding) already make a copy of the string. They do not need for us to hold on to the memory longer, but it does not hurt them if we do. One may note that combined with cd43948798 we have approached a simpler solution in a roundabout way. We are still hacking up argv, but now carefully constructing a parallel argv of old strings we've overwritten (and will eventually free). In an alternate universe, we could instead leave the original argv pristine and return a new reduced-size argv. This is conceptually simpler, though it does mean that every caller must free that new argv array itself (not the entries). That's not something they traditionally had to do, so it would mean tweaking every caller. So even though the combination of this cd43948798 and this patch is a little convoluted, it should make things just work (no leaks and no use-after-free) without modifying any callers. Reported-by: Nicolas Le Cam Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 36 ++++++++++++++++++++++++++++-------- revision.h | 9 +++++++++ t/t3903-stash.sh | 17 +++++++++++++++++ 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/revision.c b/revision.c index 50dc8b199137c7..7aee96bd8ec35c 100644 --- a/revision.c +++ b/revision.c @@ -2307,9 +2307,27 @@ static timestamp_t parse_age(const char *arg) return num; } +/* + * When asked to free argv strings, we should not do so immediately. Some + * option parsing may have stored a reference to the string (either the whole + * thing, or a substring inside it). We should keep it valid until the rev_info + * struct itself is freed. + * + * Note that we take a const str for the convenience of callers (who have the + * usual const argv array, even when opt->free_removed_argv_elements is set). + * We cast away the const on their behalf. + */ +static void mark_argv_for_free(struct rev_info *revs, const char *str) +{ + if (!str) + return; + strvec_push_nodup(&revs->argv_to_free, (char *)str); +} + static void overwrite_argv(int *argc, const char **argv, const char **value, - const struct setup_revision_opt *opt) + const struct setup_revision_opt *opt, + struct rev_info *revs) { /* * Detect the case when we are overwriting ourselves. The assignment @@ -2318,7 +2336,7 @@ static void overwrite_argv(int *argc, const char **argv, */ if (*value != argv[*argc]) { if (opt && opt->free_removed_argv_elements) - free((char *)argv[*argc]); + mark_argv_for_free(revs, argv[*argc]); argv[*argc] = *value; *value = NULL; } @@ -2346,7 +2364,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg starts_with(arg, "--branches=") || starts_with(arg, "--tags=") || starts_with(arg, "--remotes=") || starts_with(arg, "--no-walk=")) { - overwrite_argv(unkc, unkv, &argv[0], opt); + overwrite_argv(unkc, unkv, &argv[0], opt, revs); return 1; } @@ -2738,7 +2756,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg } else { int opts = diff_opt_parse(&revs->diffopt, argv, argc, revs->prefix); if (!opts) - overwrite_argv(unkc, unkv, &argv[0], opt); + overwrite_argv(unkc, unkv, &argv[0], opt, revs); return opts; } @@ -3038,7 +3056,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s if (strcmp(arg, "--")) continue; if (opt && opt->free_removed_argv_elements) - free((char *)argv[i]); + mark_argv_for_free(revs, argv[i]); argv[i] = NULL; argc = i; if (argv[i + 1]) @@ -3068,7 +3086,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s if (!strcmp(arg, "--stdin")) { if (revs->disable_stdin) { - overwrite_argv(&left, argv, &argv[i], opt); + overwrite_argv(&left, argv, &argv[i], + opt, revs); continue; } if (revs->read_from_stdin++) @@ -3242,7 +3261,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s if (argv) { if (opt && opt->free_removed_argv_elements) - free((char *)argv[left]); + mark_argv_for_free(revs, argv[left]); argv[left] = NULL; } @@ -3264,7 +3283,7 @@ void setup_revisions_from_strvec(struct strvec *argv, struct rev_info *revs, ret = setup_revisions(argv->nr, argv->v, revs, opt); for (size_t i = ret; i < argv->nr; i++) - free((char *)argv->v[i]); + mark_argv_for_free(revs, argv->v[i]); argv->nr = ret; } @@ -3326,6 +3345,7 @@ void release_revisions(struct rev_info *revs) oidset_clear(&revs->missing_commits); release_revisions_bloom_keyvecs(revs); release_follow_pathspec_slab(revs); + strvec_clear(&revs->argv_to_free); } static void add_child(struct rev_info *revs, struct commit *parent, struct commit *child) diff --git a/revision.h b/revision.h index acf6d06b24126c..e5dabd18ce1ecb 100644 --- a/revision.h +++ b/revision.h @@ -396,6 +396,14 @@ struct rev_info { /* Missing commits to be tracked without failing traversal. */ struct oidset missing_commits; + + /* + * Strings whose ownership has been handed over to us, but which + * we may be referencing in any of the above options (including + * within the diffopt struct). These will remain valid until + * release_revisions() is called. + */ + struct strvec argv_to_free; }; /** @@ -433,6 +441,7 @@ struct rev_info { .commit_format = CMIT_FMT_DEFAULT, \ .expand_tabs_in_log_default = 8, \ .rdiff_log_arg = STRVEC_INIT, \ + .argv_to_free = STRVEC_INIT, \ } /** diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh index da27a6599a6a79..260c809f994bc6 100755 --- a/t/t3903-stash.sh +++ b/t/t3903-stash.sh @@ -780,6 +780,23 @@ test_expect_success 'stash show --patience shows diff' ' diff_cmp expected actual ' +test_expect_success 'stash show supports prefixes' ' + git reset --hard && + echo foo >>file && + git stash && + cat >expected <<-\EOF && + diff --git foo/file bar/file + index 7601807..71b52c4 100644 + --- foo/file + +++ bar/file + @@ -1 +1,2 @@ + baz + +foo + EOF + git stash show --src-prefix=foo/ --dst-prefix=bar/ >actual && + diff_cmp expected actual +' + test_expect_success 'drop: fail early if specified stash is not a stash ref' ' git stash clear && test_when_finished "git reset --hard HEAD && git stash clear" && From 0e96176af4261824b363df146b1f673b14a1fed5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 1 Sep 2026 02:36:45 -0400 Subject: [PATCH 17/18] revision: simplify mark_argv_for_free() callers You do not want to mark an argv element for freeing unless the caller has given us the free_removed_argv_elements flag. Originally we just called free() in this case, so each caller checked the flag itself. Now that we mark them via a helper function, we can push the check down into the helper. This saves a little bit of duplicated code, but also hopefully makes the result conceptually simpler. Every caller but one was already checking this flag. The exception is setup_revisions_from_strvec(), but it always sets the flag explicitly (since its whole purpose is managing argv memory). So even though it was not checking the flag, doing so is OK (it will always be set). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/revision.c b/revision.c index 7aee96bd8ec35c..59d63725066eb9 100644 --- a/revision.c +++ b/revision.c @@ -2317,8 +2317,11 @@ static timestamp_t parse_age(const char *arg) * usual const argv array, even when opt->free_removed_argv_elements is set). * We cast away the const on their behalf. */ -static void mark_argv_for_free(struct rev_info *revs, const char *str) +static void mark_argv_for_free(const struct setup_revision_opt *opt, + struct rev_info *revs, const char *str) { + if (!opt || !opt->free_removed_argv_elements) + return; if (!str) return; strvec_push_nodup(&revs->argv_to_free, (char *)str); @@ -2335,8 +2338,7 @@ static void overwrite_argv(int *argc, const char **argv, * cases around the free() and NULL operations. */ if (*value != argv[*argc]) { - if (opt && opt->free_removed_argv_elements) - mark_argv_for_free(revs, argv[*argc]); + mark_argv_for_free(opt, revs, argv[*argc]); argv[*argc] = *value; *value = NULL; } @@ -3055,8 +3057,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s const char *arg = argv[i]; if (strcmp(arg, "--")) continue; - if (opt && opt->free_removed_argv_elements) - mark_argv_for_free(revs, argv[i]); + mark_argv_for_free(opt, revs, argv[i]); argv[i] = NULL; argc = i; if (argv[i + 1]) @@ -3260,8 +3261,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s } if (argv) { - if (opt && opt->free_removed_argv_elements) - mark_argv_for_free(revs, argv[left]); + mark_argv_for_free(opt, revs, argv[left]); argv[left] = NULL; } @@ -3283,7 +3283,7 @@ void setup_revisions_from_strvec(struct strvec *argv, struct rev_info *revs, ret = setup_revisions(argv->nr, argv->v, revs, opt); for (size_t i = ret; i < argv->nr; i++) - mark_argv_for_free(revs, argv->v[i]); + mark_argv_for_free(opt, revs, argv->v[i]); argv->nr = ret; } From fa7f9290efe2bd22dd736689597b474b93798e11 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 10 Sep 2026 05:36:03 -0700 Subject: [PATCH 18/18] Git 2.56-rc0 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.56.0.adoc | 26 ++++++++++++++++++++++++++ GIT-VERSION-GEN | 2 +- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/Documentation/RelNotes/2.56.0.adoc b/Documentation/RelNotes/2.56.0.adoc index f86d3eb2635cb5..c72c2ddac6ff86 100644 --- a/Documentation/RelNotes/2.56.0.adoc +++ b/Documentation/RelNotes/2.56.0.adoc @@ -144,6 +144,10 @@ UI, Workflows & Features remote-tracking branch cannot be uniquely identified, which has been corrected. + * The 'git replay' command has been taught the '--linearize' option to + drop merge commits and linearize the replayed history, mimicking 'git + rebase --no-rebase-merges'. + Performance, Internal Implementation, Development Support etc. -------------------------------------------------------------- @@ -498,6 +502,16 @@ Performance, Internal Implementation, Development Support etc. refactored to use the internal apply API directly, avoiding the need to spawn a 'git apply' subprocess. + * The memory ownership of argv elements passed to the revision + machinery has been made more robust by keeping logically "freed" + elements alive until the rev_info struct is released, preventing + use-after-free bugs when options store references to them. + + * The object lookup machinery has been taught to gracefully recover + when a multi-pack-index points to an owning pack that was removed + during a concurrent geometric repack, and 'git replay' has been + fixed to not segfault when reading such missing objects. + Fixes since v2.55 ----------------- @@ -779,6 +793,18 @@ Fixes since v2.55 the path originally recorded in the file was absolute, and this new capability is used to correctly detect such mismatches. + * GitHub Actions CI workflow runs triggered by pull requests have + been configured to cancel older runs when a new push is made to the + same pull request. + (merge a251b1bd21 hn/ci-cancel-stale-pr-runs later to maint). + + * The string extraction logic for the branch name and worktree name + from the given path in 'git worktree add' has been corrected and + simplified to avoid out-of-bounds reads and improper handling of + trailing slashes. + (merge 2e8a9d94b0 rs/worktree-add-basename-fixes later to maint). + * Other code cleanup, docfix, build fix, etc. (merge 026636128f ss/submittingpatches-typofix later to maint). (merge d2af22cc21 jc/rerere-doc-typofix later to maint). + (merge c486c1df72 hk/typofix later to maint). diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index a72f090fe2b8d2..272bdc6e8c7682 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,6 +1,6 @@ #!/bin/sh -DEF_VER=v2.55.GIT +DEF_VER=v2.56.0-rc0 LF=' '