From: Thomas Bachem via GitGitGadget Date: Mon, 14 Sep 2026 08:04:19 GMT Subject: [PATCH v4 0/2] rerere: wait for MERGE_RR.lock, and go on at a conflict Message-ID: In-Reply-To: This is now two patches, as Patrick asked: the wait first, the skip on top, so that the second can be discussed on its own. Patch 1 makes every writer wait rerere.lockTimeout for MERGE_RR.lock before it fails as it does today, and lets "git rerere gc" skip its run instead of waiting. That alone keeps a rebase alive through the gc that a commit's auto maintenance starts: the gc gives way whenever it comes second, and the one second default is longer than pruning a few thousand entries takes. Patch 2 is what I proposed in my 6 September reply to Junio: a command that stops at a conflict warns and goes on when the lock is still held after the wait, and the warning tells the user to run "git rerere" before resolving. Everything else keeps failing: the recordings after a resolution, the user's own rerere commands, and the clears that am and rebase run. Changes since v3: * Based on master instead of maint. On master setup_rerere() already uses the repository-scoped lock helpers, so the series merges cleanly into next and seen, including ps/tune-rerere-gc (which makes the gc rarer, not the race) and tb/rerere-lock-grace (which only stops the commands a rebase spawns from starting the gc). * The first log message says that the race is as old as the lock and that the 2.54 maintenance default only made it easy to hit (Patrick). * Only the six callers that stop at a conflict go on without rerere: the sequencer's pick and merge, "git merge", the 3-way fallback of "git am", "git stash" and "git apply --3way". "git commit" and "git am --continue" fail after the wait, as in 2.55 (Junio). Merge, rebase, am, stash and apply each have a test for this. * The config entry describes the outcome per command and no longer says "retry" (Junio). * The "rerere clear" that am and rebase run for --skip and --abort waits and then fails like every other caller, so a held lock cannot leave a stale MERGE_RR entry behind. The signatures of rerere_clear() and rerere_forget() are back to what upstream has. * A new test runs "git rerere" at the stop after a skipped recording and checks that the preimage, and after the resolution the postimage, get recorded. Thomas Bachem (2): rerere: wait for MERGE_RR.lock, and let the gc skip it rerere: go on at a conflict when the lock stays busy Documentation/config/rerere.adoc | 13 +++ Documentation/git-rerere.adoc | 4 +- apply.c | 2 +- builtin/am.c | 3 +- builtin/merge.c | 2 +- builtin/stash.c | 2 +- rerere.c | 49 +++++++-- rerere.h | 4 + sequencer.c | 4 +- t/t4200-rerere.sh | 168 +++++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 8 ++ 11 files changed, 246 insertions(+), 13 deletions(-) base-commit: 47ce80527c56f462cb97db4ca8125342204d3783 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2214%2Fthomasbachem%2Frerere-gc-lock-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2214/thomasbachem/rerere-gc-lock-v4 Pull-Request: https://github.com/gitgitgadget/git/pull/2214 Range-diff vs v3: 1: 5bfda65baa ! 1: 8a7a74d6aa rerere: keep a background gc from killing a rebase @@ Metadata Author: Thomas Bachem ## Commit message ## - rerere: keep a background gc from killing a rebase + rerere: wait for MERGE_RR.lock, and let the gc skip it - A "git rerere gc" holds MERGE_RR.lock for as long as pruning rr-cache - takes, and since 2.54 the auto maintenance after every commit runs - one whenever rr-cache has an entry. The commit a rebase spawns for a - resolved pick starts it too, and the sequencer's repo_rerere() at the - next conflict wants the lock a few milliseconds later. Both take it - with LOCK_DIE_ON_ERROR, so whichever comes second dies. When it is - the rebase, the index is written but the state for "git rebase - --continue" is not, and every later continue refuses with "you have - staged changes". + setup_rerere() takes MERGE_RR.lock with LOCK_DIE_ON_ERROR. When two + processes want the lock at the same time, the second one dies. This + was always the case, but since 452b12c2e0 (builtin/maintenance: use + "geometric" strategy by default, 2026-02-24) it is easy to hit: auto + maintenance now runs "git rerere gc" after every commit whenever + rr-cache contains at least one entry, and the gc holds the lock + while it prunes. - The gc needs the lock, since a rerere that has just created its - directory looks like the empty ones it prunes. So wait for it - instead, rerere.lockTimeout milliseconds, 1000 by default with the - semantics of core.packedRefsTimeout, then warn and go on without - rerere: a lost recording or replay is nothing next to a rebase that - cannot continue. The gc itself never waits, and "git rerere", "git - rerere forget" and "git rerere clear" wait but then die, since the - state behind the lock is all they are for. The clearing "am" and - "rebase" do on --abort, --skip and --quit goes on without it, and - leaves the entries for the gc. + A rebase whose next pick conflicts while the gc holds the lock dies + inside repo_rerere(). That runs before the sequencer writes the state + that "git rebase --continue" needs, so every later "git rebase + --continue" fails with "you have staged changes". + + Instead of dying right away, wait for the lock for up to + rerere.lockTimeout milliseconds, 1000 by default, and only then fail + as before. The gc itself does not wait: when the lock is held, it + skips this run and leaves the pruning to the next one. Assisted-by: Claude Fable 5.1 Signed-off-by: Thomas Bachem @@ Documentation/config/rerere.adoc: rerere.enabled:: repository. + +rerere.lockTimeout:: -+ The length of time, in milliseconds, to retry when trying to -+ take the rerere lock while another process holds it, typically -+ a background `git rerere gc`. When the time is up, the command -+ warns and goes on without rerere. Value 0 means not to retry -+ at all; -1 means to try indefinitely. Default is 1000 (i.e., -+ retry for 1 second). `git rerere gc` does not retry at all. -+ `git rerere`, `git rerere forget` and `git rerere clear` retry -+ the same way, but fail when the time is up instead of going on. ++ The length of time, in milliseconds, to wait for the rerere ++ lock when another process holds it, typically a background ++ `git rerere gc`. Value 0 means not to wait at all; -1 means ++ to wait indefinitely. Default is 1000 (i.e., wait for 1 ++ second). When the time is up, the command fails as it does ++ for any other lock it cannot take. `git rerere gc` never ++ waits and skips its run while the lock is held. ## Documentation/git-rerere.adoc ## @@ Documentation/git-rerere.adoc: occurred a long time ago. By default, unresolved conflicts older @@ Documentation/git-rerere.adoc: occurred a long time ago. By default, unresolved days are pruned. These defaults are controlled via the `gc.rerereUnresolved` and `gc.rerereResolved` configuration -variables respectively. -+variables respectively. If another process holds the lock on the -+recorded resolutions, for example a merge or rebase that is recording -+a conflict, `gc` does nothing and reports so. ++variables respectively. If another process holds the rerere lock, ++for example a merge or rebase that is recording a conflict, `gc` ++does nothing and says so. DISCUSSION - ## builtin/am.c ## -@@ builtin/am.c: static int clean_index(const struct object_id *head, const struct object_id *rem - static void am_rerere_clear(void) - { - struct string_list merge_rr = STRING_LIST_INIT_DUP; -- rerere_clear(the_repository, &merge_rr); -+ rerere_clear(the_repository, &merge_rr, 0); - string_list_clear(&merge_rr, 1); - } - - - ## builtin/rebase.c ## -@@ builtin/rebase.c: static int run_sequencer_rebase(struct rebase_options *opts) - case ACTION_SKIP: { - struct string_list merge_rr = STRING_LIST_INIT_DUP; - -- rerere_clear(the_repository, &merge_rr); -+ rerere_clear(the_repository, &merge_rr, 0); - } - /* fallthrough */ - case ACTION_CONTINUE: { -@@ builtin/rebase.c: int cmd_rebase(int argc, - case ACTION_SKIP: { - struct string_list merge_rr = STRING_LIST_INIT_DUP; - -- rerere_clear(the_repository, &merge_rr); -+ rerere_clear(the_repository, &merge_rr, 0); - string_list_clear(&merge_rr, 1); - ropts.flags = RESET_HEAD_HARD; - if (reset_head(the_repository, &ropts) < 0) -@@ builtin/rebase.c: int cmd_rebase(int argc, - struct string_list merge_rr = STRING_LIST_INIT_DUP; - struct strbuf head_msg = STRBUF_INIT; - -- rerere_clear(the_repository, &merge_rr); -+ rerere_clear(the_repository, &merge_rr, 0); - string_list_clear(&merge_rr, 1); - - if (read_basic_state(&options)) - - ## builtin/rerere.c ## -@@ builtin/rerere.c: int cmd_rerere(int argc, - flags = RERERE_NOAUTOUPDATE; - - if (argc < 1) -- return repo_rerere(the_repository, flags); -+ return repo_rerere(the_repository, flags | RERERE_LOCK_OR_DIE); - - if (!strcmp(argv[0], "forget")) { - struct pathspec pathspec; -@@ builtin/rerere.c: int cmd_rerere(int argc, - parse_pathspec(&pathspec, 0, PATHSPEC_PREFER_CWD, - prefix, argv + 1); - -- ret = rerere_forget(the_repository, &pathspec); -+ ret = rerere_forget(the_repository, &pathspec, -+ RERERE_LOCK_OR_DIE); - - clear_pathspec(&pathspec); - return ret; - } - - if (!strcmp(argv[0], "clear")) { -- rerere_clear(the_repository, &merge_rr); -+ rerere_clear(the_repository, &merge_rr, RERERE_LOCK_OR_DIE); - } else if (!strcmp(argv[0], "gc")) - rerere_gc(the_repository, &merge_rr); - else if (!strcmp(argv[0], "status")) { - ## rerere.c ## @@ rerere.c: static int rerere_enabled = -1; - /* automatically update cleanly resolved paths to the index */ static int rerere_autoupdate; -+static int rerere_lock_timeout_ms = 1000; ++/* how long to wait for MERGE_RR.lock, in milliseconds */ ++static int rerere_lock_timeout_ms = 1000; ++ #define RR_HAS_POSTIMAGE 1 #define RR_HAS_PREIMAGE 2 + struct rerere_dir { @@ rerere.c: static void git_rerere_config(void) { repo_config_get_bool(the_repository, "rerere.enabled", &rerere_enabled); @@ rerere.c: int setup_rerere(struct repository *r, struct string_list *merge_rr, i if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE)) rerere_autoupdate = !!(flags & RERERE_AUTOUPDATE); - if (flags & RERERE_READONLY) -+ if ((flags & RERERE_NOWAIT) && (flags & RERERE_LOCK_OR_DIE)) -+ BUG("RERERE_NOWAIT and RERERE_LOCK_OR_DIE are mutually exclusive"); -+ if ((flags & RERERE_READONLY) && -+ (flags & (RERERE_NOWAIT | RERERE_LOCK_OR_DIE))) -+ BUG("RERERE_READONLY takes no lock, so no lock flag applies"); ++ if ((flags & RERERE_READONLY) && (flags & RERERE_NOWAIT)) ++ BUG("RERERE_READONLY takes no lock, so RERERE_NOWAIT does not apply"); + if (flags & RERERE_READONLY) { fd = 0; - else -- fd = hold_lock_file_for_update(&write_lock, -- git_path_merge_rr(r), -- LOCK_DIE_ON_ERROR); +- fd = repo_hold_lock_file_for_update(r, &write_lock, +- git_path_merge_rr(r), +- LOCK_DIE_ON_ERROR); + } else { -+ int lock_flags = 0; ++ const char *path = git_path_merge_rr(r); ++ int lock_flags = LOCK_DIE_ON_ERROR; + long timeout_ms = rerere_lock_timeout_ms; + -+ if (flags & RERERE_LOCK_OR_DIE) -+ lock_flags = LOCK_DIE_ON_ERROR; -+ if (flags & RERERE_NOWAIT) -+ timeout_ms = 0; + /* -+ * A background "rerere gc" holds the lock for as long as it -+ * takes to prune rr-cache, so wait it out rather than fail -+ * at once. The gc itself has nothing to lose from a skipped -+ * run and never waits. ++ * Another process may hold the lock for a while, e.g. ++ * "git rerere gc" while it prunes rr-cache, so wait for ++ * it instead of dying right away. The gc itself never ++ * waits: skipping one of its runs costs nothing. + */ -+ fd = hold_lock_file_for_update_timeout(&write_lock, -+ git_path_merge_rr(r), -+ lock_flags, timeout_ms); ++ if (flags & RERERE_NOWAIT) { ++ lock_flags = 0; ++ timeout_ms = 0; ++ } ++ fd = repo_hold_lock_file_for_update_timeout(r, &write_lock, ++ path, lock_flags, ++ timeout_ms); + if (fd < 0) { -+ warning_errno(_("skipping rerere, unable to create '%s.lock'"), -+ git_path_merge_rr(r)); ++ warning_errno(_("skipping rerere, " ++ "unable to create '%s.lock'"), path); + return -1; + } + } read_rr(r, merge_rr); return fd; } -@@ rerere.c: fail_exit: - return -1; - } - --int rerere_forget(struct repository *r, struct pathspec *pathspec) -+int rerere_forget(struct repository *r, struct pathspec *pathspec, int flags) - { - int i, fd, ret; - struct string_list conflict = STRING_LIST_INIT_DUP; -@@ rerere.c: int rerere_forget(struct repository *r, struct pathspec *pathspec) - if (repo_read_index(r) < 0) - return error(_("index file corrupt")); - -- fd = setup_rerere(r, &merge_rr, RERERE_NOAUTOUPDATE); -+ fd = setup_rerere(r, &merge_rr, RERERE_NOAUTOUPDATE | flags); - if (fd < 0) - return 0; - @@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr) timestamp_t cutoff_resolve = now - 60 * 86400; struct strbuf buf = STRBUF_INIT; @@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr) return; repo_config_get_expiry_in_days(the_repository, "gc.rerereresolved", -@@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr) - * - * NEEDSWORK: shouldn't we be calling this from "reset --hard"? - */ --void rerere_clear(struct repository *r, struct string_list *merge_rr) -+void rerere_clear(struct repository *r, struct string_list *merge_rr, int flags) - { - int i; - -- if (setup_rerere(r, merge_rr, 0) < 0) -+ if (setup_rerere(r, merge_rr, flags) < 0) - return; - - for (i = 0; i < merge_rr->nr; i++) { ## rerere.h ## @@ rerere.h: struct repository; #define RERERE_AUTOUPDATE 01 #define RERERE_NOAUTOUPDATE 02 #define RERERE_READONLY 04 -+/* Do not wait for the lock when another process holds it */ ++/* Never wait for MERGE_RR.lock, and skip the run when it is held */ +#define RERERE_NOWAIT 010 -+/* Die on a lock that cannot be taken instead of going on without rerere */ -+#define RERERE_LOCK_OR_DIE 020 /* * Marks paths that have been hand-resolved and added to the -@@ rerere.h: int repo_rerere(struct repository *, int); - */ - const char *rerere_path(struct strbuf *buf, const struct rerere_id *, - const char *file); --int rerere_forget(struct repository *, struct pathspec *); -+int rerere_forget(struct repository *, struct pathspec *, int); - int rerere_remaining(struct repository *, struct string_list *); --void rerere_clear(struct repository *, struct string_list *); -+void rerere_clear(struct repository *, struct string_list *, int); - void rerere_gc(struct repository *, struct string_list *); - - #define OPT_RERERE_AUTOUPDATE(v) OPT_UYN(0, "rerere-autoupdate", (v), \ ## t/t4200-rerere.sh ## @@ t/t4200-rerere.sh: test_expect_success 'old records rest in peace' ' @@ t/t4200-rerere.sh: test_expect_success 'old records rest in peace' ' + test_grep "^=======\$" $rr/preimage +' + -+test_expect_success 'merge goes on without rerere once rerere.lockTimeout is up' ' ++test_expect_success 'merge fails once rerere.lockTimeout is up' ' + git reset --hard && + rm -rf $rr && + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + test_must_fail git -c rerere.lockTimeout=0 merge first 2>err && -+ test_grep "skipping rerere" err && ++ test_grep "Unable to create" err && + test_grep "^=======\$" a1 && + test_path_is_missing $rr/preimage +' + -+test_expect_success 'commit goes on without rerere once rerere.lockTimeout is up' ' -+ git reset --hard && -+ rm -rf $rr && -+ git checkout -b lock-held-commit third && -+ test_when_finished "git checkout third && git branch -D lock-held-commit" && -+ test_must_fail git merge first && -+ test_path_is_file $rr/preimage && -+ test_when_finished "rm -f .git/MERGE_RR.lock" && -+ >.git/MERGE_RR.lock && -+ echo resolved >a1 && -+ git add a1 && -+ git -c rerere.lockTimeout=0 commit -qm resolved 2>err && -+ test_grep "skipping rerere" err && -+ test_path_is_missing $rr/postimage -+' -+ +test_expect_success 'rerere, forget and clear fail on a lock they cannot take' ' + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && @@ t/t4200-rerere.sh: test_expect_success 'old records rest in peace' ' + test_grep "Unable to create" err +' + -+test_expect_success 'rebase goes on without rerere once rerere.lockTimeout is up' ' ++test_expect_success 'rebase --abort fails on a lock it cannot take' ' + git reset --hard && -+ rm -rf $rr && -+ git checkout -b lock-held third && -+ test_when_finished "git checkout third && git branch -D lock-held" && -+ test_when_finished "rm -f .git/MERGE_RR.lock" && -+ >.git/MERGE_RR.lock && -+ test_must_fail git -c rerere.lockTimeout=0 rebase first 2>err && -+ test_grep "skipping rerere" err && -+ test_path_is_file .git/rebase-merge/stopped-sha && -+ echo resolved >a1 && -+ git add a1 && -+ git -c rerere.lockTimeout=0 rebase --continue && -+ test_path_is_missing .git/rebase-merge && -+ test_path_is_missing $rr/preimage -+' -+ -+test_expect_success 'rebase --abort goes on without rerere on a held lock' ' + git checkout -b lock-held-abort third && + test_when_finished "git checkout third && git branch -D lock-held-abort" && + test_must_fail git rebase first && + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && -+ git -c rerere.lockTimeout=0 rebase --abort 2>err && -+ test_grep "skipping rerere" err && ++ test_must_fail git -c rerere.lockTimeout=0 rebase --abort 2>err && ++ test_grep "Unable to create" err && ++ test_path_is_dir .git/rebase-merge && ++ rm .git/MERGE_RR.lock && ++ git rebase --abort && + test_path_is_missing .git/rebase-merge +' + -: ---------- > 2: 1cce403113 rerere: go on at a conflict when the lock stays busy -- gitgitgadget