From 3dc3d02f12a3118ac9e270c19960815f6b8170cb Mon Sep 17 00:00:00 2001 From: Thomas Bachem Date: Mon, 28 Sep 2026 11:47:43 +0200 Subject: [PATCH 1/2] rerere: wait for MERGE_RR.lock before giving up setup_rerere() takes MERGE_RR.lock with LOCK_DIE_ON_ERROR, so of two processes that want it at the same time the second one dies. That used to be rare. Since 452b12c2e0 (builtin/maintenance: use "geometric" strategy by default, 2026-02-24) the auto maintenance after a commit runs "git rerere gc" whenever rr-cache has enough stale entries, and the gc holds the lock while it prunes. A rebase whose next pick conflicts while that happens dies inside repo_rerere(), before the sequencer has written the state that "git rebase --continue" needs. Every later "git rebase --continue" then fails with "you have staged changes in your working tree". Wait for the lock for up to rerere.lockTimeout milliseconds, 1000 by default, and only then fail as before. Pruning a few thousand entries takes well under a second, so the default covers a rebase that runs into the gc. Assisted-by: Claude Fable 5.1 Signed-off-by: Thomas Bachem --- Documentation/config/rerere.adoc | 8 +++++ rerere.c | 22 ++++++++++--- t/t4200-rerere.sh | 53 ++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 5 deletions(-) diff --git a/Documentation/config/rerere.adoc b/Documentation/config/rerere.adoc index 3a78b5ebb1dc02..30e827f32b289b 100644 --- a/Documentation/config/rerere.adoc +++ b/Documentation/config/rerere.adoc @@ -10,3 +10,11 @@ rerere.enabled:: enabled if there is an `rr-cache` directory under the `$GIT_DIR`, e.g. if "rerere" was previously used in the repository. + +rerere.lockTimeout:: + 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. diff --git a/rerere.c b/rerere.c index 1c3745d9e3279a..64fac07c71d9d8 100644 --- a/rerere.c +++ b/rerere.c @@ -33,6 +33,9 @@ static int rerere_enabled = -1; /* automatically update cleanly resolved paths to the index */ static int rerere_autoupdate; +/* 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 { @@ -850,6 +853,8 @@ static void git_rerere_config(void) { repo_config_get_bool(the_repository, "rerere.enabled", &rerere_enabled); repo_config_get_bool(the_repository, "rerere.autoupdate", &rerere_autoupdate); + repo_config_get_int(the_repository, "rerere.locktimeout", + &rerere_lock_timeout_ms); repo_config(the_repository, git_default_config, NULL); } @@ -882,12 +887,19 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags) if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE)) rerere_autoupdate = !!(flags & RERERE_AUTOUPDATE); - if (flags & RERERE_READONLY) + if (flags & RERERE_READONLY) { fd = 0; - else - fd = repo_hold_lock_file_for_update(r, &write_lock, - git_path_merge_rr(r), - LOCK_DIE_ON_ERROR); + } else { + /* + * 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. + */ + fd = repo_hold_lock_file_for_update_timeout(r, &write_lock, + git_path_merge_rr(r), + LOCK_DIE_ON_ERROR, + rerere_lock_timeout_ms); + } read_rr(r, merge_rr); return fd; } diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 7bb601e117ec35..7bd92235dcfed6 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -242,6 +242,59 @@ test_expect_success 'old records rest in peace' ' test_path_is_missing $rr2/preimage ' +test_expect_success 'a held lock is waited out within rerere.lockTimeout' ' + git reset --hard && + rm -rf $rr && + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + { + ( sleep 1 && rm -f .git/MERGE_RR.lock ) & + } && + test_must_fail git -c rerere.lockTimeout=5000 merge first 2>err && + wait && + test_grep ! "MERGE_RR" err && + test_grep "^=======\$" $rr/preimage +' + +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 "Unable to create" err && + test_grep "^=======\$" a1 && + test_path_is_missing $rr/preimage +' + +test_expect_success 'rerere, forget, clear and gc fail on a lock they cannot take' ' + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + test_must_fail git -c rerere.lockTimeout=0 rerere 2>err && + test_grep "Unable to create" err && + test_must_fail git -c rerere.lockTimeout=0 rerere forget a1 2>err && + test_grep "Unable to create" err && + test_must_fail git -c rerere.lockTimeout=0 rerere clear 2>err && + test_grep "Unable to create" err && + test_must_fail git -c rerere.lockTimeout=0 rerere gc 2>err && + test_grep "Unable to create" err +' + +test_expect_success 'rebase --abort fails on a lock it cannot take' ' + git reset --hard && + 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 && + 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 +' + rerere_gc_custom_expiry_test () { five_days="$1" right_now="$2" test_expect_success "rerere gc with custom expiry ($five_days, $right_now)" ' From 000288119671bad19e5e9e37419766b364daaacc Mon Sep 17 00:00:00 2001 From: Thomas Bachem Date: Mon, 28 Sep 2026 11:51:17 +0200 Subject: [PATCH 2/2] rerere: add "gc --skip-locked" for auto maintenance Starting with the preceding commit, processes that want to acquire the rerere cache's MERGE_RR.lock by default know to wait up to one second until that lock has been released. This is a sensible default for many commands that happen to write rerere entries, as we would otherwise die immediately when the lock is taken by another process. But for repository maintenance it's a bit more complicated, as there are two cases that we have to care about. When the user explicitly asks us to garbage collect rerere entries via `git rerere gc` they probably want us to try our best to perform this operation. It's thus sensible to wait for the lock and then die if we weren't able to acquire it. But we also prune rerere entries as part of auto-maintenance, which is only executed on a best-effort basis anyway. Delaying the whole operation to acquire the lock is somewhat heavy-handed, and neither does it make sense to die in case we haven't been able to garbage collect rerere entries as that would impede other housekeeping tasks. Furthermore, it's totally fine to skip the operation when the rerere cache is locked already, as we will retry during the next run anyway. But we do not have an easy way to tell `git rerere gc` to skip the operation in case the cache is locked already. Add a new "--skip-locked" flag to plug that gap and have auto-maintenance pass that flag. Only auto-maintenance needs that flag, so hide it, like the "--skip-foreground-tasks" flag that `git maintenance run` passes to `git gc`. Helped-by: Patrick Steinhardt Assisted-by: Claude Fable 5.1 Signed-off-by: Thomas Bachem --- Documentation/config/rerere.adoc | 4 +++- builtin/gc.c | 4 +++- builtin/rerere.c | 10 ++++++++-- rerere.c | 20 ++++++++++++++++---- rerere.h | 11 ++++++++++- t/t4200-rerere.sh | 26 ++++++++++++++++++++++++++ t/t7900-maintenance.sh | 25 ++++++++++++++++++++++++- 7 files changed, 90 insertions(+), 10 deletions(-) diff --git a/Documentation/config/rerere.adoc b/Documentation/config/rerere.adoc index 30e827f32b289b..80c38ee951dd0f 100644 --- a/Documentation/config/rerere.adoc +++ b/Documentation/config/rerere.adoc @@ -17,4 +17,6 @@ rerere.lockTimeout:: `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. + for any other lock it cannot take. A `git rerere gc` run by + `git maintenance run --auto` or `git gc --auto` does not wait + and does nothing while the lock is held. diff --git a/builtin/gc.c b/builtin/gc.c index 57a3520263d7be..7ad3987b717ff5 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -385,12 +385,14 @@ static int worktree_prune_condition(struct gc_config *cfg) return should_prune; } -static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts UNUSED, +static int maintenance_task_rerere_gc(struct maintenance_run_opts *opts, struct gc_config *cfg UNUSED) { struct child_process rerere_cmd = CHILD_PROCESS_INIT; rerere_cmd.git_cmd = 1; strvec_pushl(&rerere_cmd.args, "rerere", "gc", NULL); + if (opts->auto_flag) + strvec_push(&rerere_cmd.args, "--skip-locked"); return run_command(&rerere_cmd); } diff --git a/builtin/rerere.c b/builtin/rerere.c index a056cb791b4c80..91cb3c86682c15 100644 --- a/builtin/rerere.c +++ b/builtin/rerere.c @@ -56,16 +56,21 @@ int cmd_rerere(int argc, struct repository *repo UNUSED) { struct string_list merge_rr = STRING_LIST_INIT_DUP; - int autoupdate = -1, flags = 0; + int autoupdate = -1, skip_locked = 0, flags = 0; struct option options[] = { OPT_SET_INT(0, "rerere-autoupdate", &autoupdate, N_("register clean resolutions in index"), 1), + OPT_HIDDEN_BOOL(0, "skip-locked", &skip_locked, + N_("skip gc while another process holds the lock")), OPT_END(), }; argc = parse_options(argc, argv, prefix, options, rerere_usage, 0); + if (skip_locked && (argc < 1 || strcmp(argv[0], "gc"))) + die(_("the option '%s' requires '%s'"), "--skip-locked", "gc"); + repo_config(the_repository, git_xmerge_config, NULL); if (autoupdate == 1) @@ -94,7 +99,8 @@ int cmd_rerere(int argc, if (!strcmp(argv[0], "clear")) { rerere_clear(the_repository, &merge_rr); } else if (!strcmp(argv[0], "gc")) - rerere_gc(the_repository, &merge_rr); + rerere_gc(the_repository, &merge_rr, + skip_locked ? RERERE_GC_NOWAIT : 0); else if (!strcmp(argv[0], "status")) { if (setup_rerere(the_repository, &merge_rr, flags | RERERE_READONLY) < 0) diff --git a/rerere.c b/rerere.c index 64fac07c71d9d8..889ec700dd9e8e 100644 --- a/rerere.c +++ b/rerere.c @@ -887,18 +887,28 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags) if (flags & (RERERE_AUTOUPDATE|RERERE_NOAUTOUPDATE)) rerere_autoupdate = !!(flags & RERERE_AUTOUPDATE); + if ((flags & RERERE_READONLY) && (flags & RERERE_NOWAIT)) + BUG("RERERE_NOWAIT does not apply with RERERE_READONLY"); if (flags & RERERE_READONLY) { fd = 0; } else { + int lock_flags = LOCK_DIE_ON_ERROR; + int timeout_ms = rerere_lock_timeout_ms; + /* * 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. */ + if (flags & RERERE_NOWAIT) { + lock_flags = 0; + timeout_ms = 0; + } fd = repo_hold_lock_file_for_update_timeout(r, &write_lock, git_path_merge_rr(r), - LOCK_DIE_ON_ERROR, - rerere_lock_timeout_ms); + lock_flags, timeout_ms); + if (fd < 0) + return -1; } read_rr(r, merge_rr); return fd; @@ -1284,7 +1294,8 @@ bool rerere_gc_needed(struct repository *r, size_t limit) return needed; } -void rerere_gc(struct repository *r, struct string_list *rr) +void rerere_gc(struct repository *r, struct string_list *rr, + enum rerere_gc_flags flags) { struct string_list to_remove = STRING_LIST_INIT_DUP; DIR *dir; @@ -1294,7 +1305,8 @@ void rerere_gc(struct repository *r, struct string_list *rr) timestamp_t cutoff_resolve; struct strbuf buf = STRBUF_INIT; - if (setup_rerere(r, rr, 0) < 0) + if (setup_rerere(r, rr, + (flags & RERERE_GC_NOWAIT) ? RERERE_NOWAIT : 0) < 0) return; rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve); diff --git a/rerere.h b/rerere.h index feeb0e2c9fe61b..61e986475b82d6 100644 --- a/rerere.h +++ b/rerere.h @@ -10,6 +10,8 @@ struct repository; #define RERERE_AUTOUPDATE 01 #define RERERE_NOAUTOUPDATE 02 #define RERERE_READONLY 04 +/* If MERGE_RR.lock is taken, return -1 as if rerere were disabled */ +#define RERERE_NOWAIT 010 /* * Marks paths that have been hand-resolved and added to the @@ -37,7 +39,14 @@ const char *rerere_path(struct strbuf *buf, const struct rerere_id *, int rerere_forget(struct repository *, struct pathspec *); int rerere_remaining(struct repository *, struct string_list *); void rerere_clear(struct repository *, struct string_list *); -void rerere_gc(struct repository *, struct string_list *); + +enum rerere_gc_flags { + /* Skip the operation in case the MERGE_RR.lock is already taken. */ + RERERE_GC_NOWAIT = (1 << 0), +}; + +void rerere_gc(struct repository *, struct string_list *, + enum rerere_gc_flags flags); /* * Check whether garbage collection for rerere entries is needed, which is diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 7bd92235dcfed6..527a872c00844d 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -242,6 +242,32 @@ test_expect_success 'old records rest in peace' ' test_path_is_missing $rr2/preimage ' +test_expect_success 'gc --skip-locked does nothing while MERGE_RR is locked' ' + mkdir -p $rr2 && + echo Hello >$rr2/preimage && + test-tool chmtime =$just_over_15_days_ago $rr2/preimage && + + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + git rerere gc --skip-locked 2>err && + test_must_be_empty err && + test_path_is_file $rr2/preimage +' + +test_expect_success 'gc --skip-locked prunes while MERGE_RR is not locked' ' + mkdir -p $rr2 && + echo Hello >$rr2/preimage && + test-tool chmtime =$just_over_15_days_ago $rr2/preimage && + + git rerere gc --skip-locked && + test_path_is_missing $rr2/preimage +' + +test_expect_success '--skip-locked is only accepted by gc' ' + test_must_fail git rerere --skip-locked clear 2>err && + test_grep "option .--skip-locked. requires .gc." err +' + test_expect_success 'a held lock is waited out within rerere.lockTimeout' ' git reset --hard && rm -rf $rr && diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 4f65fa9439c0b8..f0f9b37d4ff7d5 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -1007,9 +1007,17 @@ test_expect_rerere_gc () { shift fi + # An automatic run passes --skip-locked to "git rerere gc". + skip_locked= + case " $* " in + *" --auto "*) + skip_locked=--skip-locked + ;; + esac + rm -f "rerere-gc.txt" && GIT_TRACE2_EVENT="$(pwd)/rerere-gc.txt" "$@" && - test_subcommand $negate git rerere gc .git/MERGE_RR.lock && + test_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc +' + +test_expect_success 'rerere-gc task without --auto fails while MERGE_RR is locked' ' + test_when_finished "rm -rf .git/rr-cache .git/MERGE_RR.lock" && + mkdir .git/rr-cache && + >.git/MERGE_RR.lock && + test_must_fail git -c rerere.lockTimeout=0 maintenance run --task=rerere-gc 2>err && + test_grep "Unable to create" err +' + test_expect_success '--auto and --schedule incompatible' ' test_must_fail git maintenance run --auto --schedule=daily 2>err && test_grep "cannot be used together" err