From 92006c696779b179bc12da91457412a041f70f1c Mon Sep 17 00:00:00 2001 From: Thomas Bachem Date: Mon, 14 Sep 2026 08:04:20 +0000 Subject: [PATCH] rerere: wait for MERGE_RR.lock, and let the gc skip it 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. 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 Signed-off-by: Junio C Hamano --- Documentation/config/rerere.adoc | 9 +++++ Documentation/git-rerere.adoc | 4 +- rerere.c | 39 ++++++++++++++++--- rerere.h | 2 + t/t4200-rerere.sh | 67 ++++++++++++++++++++++++++++++++ t/t7900-maintenance.sh | 8 ++++ 6 files changed, 122 insertions(+), 7 deletions(-) diff --git a/Documentation/config/rerere.adoc b/Documentation/config/rerere.adoc index 3a78b5ebb1..cc9dd0c37b 100644 --- a/Documentation/config/rerere.adoc +++ b/Documentation/config/rerere.adoc @@ -10,3 +10,12 @@ 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. `git rerere gc` never + waits and skips its run while the lock is held. diff --git a/Documentation/git-rerere.adoc b/Documentation/git-rerere.adoc index 4e6ab9a27c..4df653367e 100644 --- a/Documentation/git-rerere.adoc +++ b/Documentation/git-rerere.adoc @@ -70,7 +70,9 @@ occurred a long time ago. By default, unresolved conflicts older than 15 days and resolved conflicts older than 60 days are pruned. These defaults are controlled via the `gc.rerereUnresolved` and `gc.rerereResolved` configuration -variables respectively. +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 diff --git a/rerere.c b/rerere.c index 3d3bd0db16..7d44f3937c 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,34 @@ 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) && (flags & RERERE_NOWAIT)) + BUG("RERERE_READONLY takes no lock, so RERERE_NOWAIT does not apply"); + 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 { + const char *path = git_path_merge_rr(r); + int lock_flags = LOCK_DIE_ON_ERROR; + long 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. The gc itself never + * waits: skipping one of its runs costs nothing. + */ + 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'"), path); + return -1; + } + } read_rr(r, merge_rr); return fd; } @@ -1211,7 +1238,7 @@ void rerere_gc(struct repository *r, struct string_list *rr) timestamp_t cutoff_resolve = now - 60 * 86400; struct strbuf buf = STRBUF_INIT; - if (setup_rerere(r, rr, 0) < 0) + if (setup_rerere(r, rr, RERERE_NOWAIT) < 0) return; repo_config_get_expiry_in_days(the_repository, "gc.rerereresolved", diff --git a/rerere.h b/rerere.h index d4b5f7c932..a2712d543e 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 +/* Never wait for MERGE_RR.lock, and skip the run when it is held */ +#define RERERE_NOWAIT 010 /* * Marks paths that have been hand-resolved and added to the diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 7bb601e117..27082a7676 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -242,6 +242,73 @@ test_expect_success 'old records rest in peace' ' test_path_is_missing $rr2/preimage ' +test_expect_success 'gc 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 2>err && + test_grep "MERGE_RR.lock" err && + test_path_is_file $rr2/preimage && + + rm .git/MERGE_RR.lock && + git rerere gc && + 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 and clear 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_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)" ' diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 5fbb16f0f0..4a27767817 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -1051,6 +1051,14 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut test_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc ' +test_expect_success 'rerere-gc task succeeds while MERGE_RR is locked' ' + test_when_finished "rm -rf .git/rr-cache .git/MERGE_RR.lock" && + mkdir .git/rr-cache && + : >.git/rr-cache/entry && + >.git/MERGE_RR.lock && + test_expect_rerere_gc git maintenance run --task=rerere-gc +' + 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