From 4ec42e1d809513c551fcf9b5b75db88b6598ff0b Mon Sep 17 00:00:00 2001 From: Thomas Bachem Date: Mon, 14 Sep 2026 08:04:21 +0000 Subject: [PATCH] rerere: go on at a conflict when the lock stays busy When a merge, rebase, cherry-pick, revert, am, stash or apply stops at a conflict, it runs rerere right before it returns to the user. If MERGE_RR.lock is still held when rerere.lockTimeout runs out, the command dies there. For a rebase that is worse than a lost recording: the sequencer has not yet written the state that "git rebase --continue" needs, so the rebase cannot continue, and following the "git commit --amend" advice folds the conflicted pick into the previous commit. So print a warning and go on instead. The conflict is still in place, and the warning tells the user to run "git rerere" before resolving it. That records the preimage, or replays a known resolution, just as the command would have done. All other callers still fail when the timeout runs out. "git commit" and "git am --continue" record the resolution and then move on to the next commit or patch. A warning would come too late there, and the next rerere run would record whatever the file contains by then. The "rerere clear" that am and rebase run for --skip and --abort would leave the same stale entry behind. "git rerere", "git rerere forget" and "git rerere clear" fail because the user asked for that state explicitly. Assisted-by: Claude Fable 5.1 Signed-off-by: Thomas Bachem Signed-off-by: Junio C Hamano --- Documentation/config/rerere.adoc | 10 ++- apply.c | 2 +- builtin/am.c | 3 +- builtin/merge.c | 2 +- builtin/stash.c | 2 +- rerere.c | 16 ++++- rerere.h | 2 + sequencer.c | 4 +- t/t4200-rerere.sh | 105 ++++++++++++++++++++++++++++++- 9 files changed, 132 insertions(+), 14 deletions(-) diff --git a/Documentation/config/rerere.adoc b/Documentation/config/rerere.adoc index cc9dd0c37b..81deefa006 100644 --- a/Documentation/config/rerere.adoc +++ b/Documentation/config/rerere.adoc @@ -16,6 +16,10 @@ rerere.lockTimeout:: 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. + second). When the time is up, a command that stops at a + conflict, such as `git merge` or `git rebase`, prints a + warning and goes on without rerere; run `git rerere` before + resolving the conflict to record it after all. Any other + 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/apply.c b/apply.c index f00b7ba4d3..3b8502535b 100644 --- a/apply.c +++ b/apply.c @@ -4865,7 +4865,7 @@ static int write_out_results(struct apply_state *state, struct patch *list) * tree with conflict markers, but that isn't written with --cached. */ if (!state->cached) - repo_rerere(state->repo, 0); + repo_rerere(state->repo, RERERE_SKIP_LOCKED); } return errs; diff --git a/builtin/am.c b/builtin/am.c index e9623b8307..bcb93db515 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1649,7 +1649,8 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa o.verbosity = 0; if (merge_ort_generic(&o, &our_tree, &their_tree, 1, bases, &result)) { - repo_rerere(the_repository, state->allow_rerere_autoupdate); + repo_rerere(the_repository, state->allow_rerere_autoupdate | + RERERE_SKIP_LOCKED); free(their_tree_name); return error(_("Failed to merge in the changes.")); } diff --git a/builtin/merge.c b/builtin/merge.c index 5b4eb23a83..1ed5959bfd 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1061,7 +1061,7 @@ static int suggest_conflicts(void) fputs(msgbuf.buf, fp); strbuf_release(&msgbuf); fclose(fp); - repo_rerere(the_repository, allow_rerere_auto); + repo_rerere(the_repository, allow_rerere_auto | RERERE_SKIP_LOCKED); printf(_("Automatic merge failed; " "fix conflicts and then commit the result.\n")); return 1; diff --git a/builtin/stash.c b/builtin/stash.c index 72c52571f8..83b6e1be72 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -729,7 +729,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info, ret = error(_("could not write index")); if (ret) { - repo_rerere(the_repository, 0); + repo_rerere(the_repository, RERERE_SKIP_LOCKED); if (index) fprintf_ln(stderr, _("Index was not unstashed.")); diff --git a/rerere.c b/rerere.c index 7d44f3937c..a996d39159 100644 --- a/rerere.c +++ b/rerere.c @@ -3,6 +3,7 @@ #include "git-compat-util.h" #include "abspath.h" +#include "advice.h" #include "config.h" #include "copy.h" #include "environment.h" @@ -887,8 +888,9 @@ 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_READONLY takes no lock, so RERERE_NOWAIT does not apply"); + if ((flags & RERERE_READONLY) && + (flags & (RERERE_NOWAIT | RERERE_SKIP_LOCKED))) + BUG("RERERE_READONLY takes no lock, so no lock flag applies"); if (flags & RERERE_READONLY) { fd = 0; } else { @@ -900,18 +902,26 @@ int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags) * 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. + * waits: skipping one of its runs costs nothing. A + * command that stops at a conflict must not die here + * either, so it warns and goes on without rerere. */ if (flags & RERERE_NOWAIT) { lock_flags = 0; timeout_ms = 0; } + if (flags & RERERE_SKIP_LOCKED) + lock_flags = 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); + if (flags & RERERE_SKIP_LOCKED) + advise(_("run \"git rerere\" before resolving " + "the conflict to record or replay " + "its resolution")); return -1; } } diff --git a/rerere.h b/rerere.h index a2712d543e..e91f3f4afa 100644 --- a/rerere.h +++ b/rerere.h @@ -12,6 +12,8 @@ struct repository; #define RERERE_READONLY 04 /* Never wait for MERGE_RR.lock, and skip the run when it is held */ #define RERERE_NOWAIT 010 +/* Warn and go on without rerere if MERGE_RR.lock cannot be taken in time */ +#define RERERE_SKIP_LOCKED 020 /* * Marks paths that have been hand-resolved and added to the diff --git a/sequencer.c b/sequencer.c index 65afd100d9..49776e7c5a 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2519,7 +2519,7 @@ static enum pick_result do_pick_commit(struct repository *r, : _("could not apply %s... %s"), short_commit_name(r, commit), msg.subject); print_advice(r, res == 1, opts); - repo_rerere(r, opts->allow_rerere_auto); + repo_rerere(r, opts->allow_rerere_auto | RERERE_SKIP_LOCKED); goto leave; } @@ -4448,7 +4448,7 @@ static int do_merge(struct repository *r, rollback_lock_file(&lock); if (ret) - repo_rerere(r, opts->allow_rerere_auto); + repo_rerere(r, opts->allow_rerere_auto | RERERE_SKIP_LOCKED); else /* * In case of problems, we now want to return a positive diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 27082a7676..d0a9363a1c 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -272,17 +272,118 @@ test_expect_success 'a held lock is waited out within rerere.lockTimeout' ' test_grep "^=======\$" $rr/preimage ' -test_expect_success 'merge fails once rerere.lockTimeout is up' ' +test_expect_success 'merge goes on without rerere 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 "skipping rerere" err && + test_grep "hint: .*git rerere" err && test_grep "^=======\$" a1 && test_path_is_missing $rr/preimage ' +test_expect_success 'rerere run at the stop records what was skipped' ' + git reset --hard && + rm -rf $rr && + git checkout -b lock-held-catch-up third && + test_when_finished "git checkout third && git branch -D lock-held-catch-up" && + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + test_must_fail git -c rerere.lockTimeout=0 merge first && + test_path_is_missing $rr/preimage && + rm .git/MERGE_RR.lock && + git rerere && + test_grep "^=======\$" $rr/preimage && + echo resolved >a1 && + git add a1 && + git commit -qm resolved && + test_path_is_file $rr/postimage +' + +test_expect_success 'rebase goes on without rerere once rerere.lockTimeout is up' ' + 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 && + rm .git/MERGE_RR.lock && + echo resolved >a1 && + git add a1 && + git rebase --continue && + test_path_is_missing .git/rebase-merge && + test_path_is_missing $rr/preimage +' + +test_expect_success 'commit fails on a lock it cannot take' ' + 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 && + echo resolved >a1 && + git add a1 && + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + test_must_fail git -c rerere.lockTimeout=0 commit -qm resolved 2>err && + test_grep "Unable to create" err && + test_path_is_missing $rr/postimage +' + +test_expect_success 'am goes on without rerere once rerere.lockTimeout is up' ' + git reset --hard && + rm -rf $rr && + git checkout -b lock-held-am third && + test_when_finished "test_might_fail git am --abort && + git checkout third && git branch -D lock-held-am" && + git format-patch -1 --stdout first >first.patch && + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + test_must_fail git -c rerere.lockTimeout=0 am --3way first.patch 2>err && + test_grep "skipping rerere" err && + test_path_is_dir .git/rebase-apply && + rm .git/MERGE_RR.lock && + git am --abort && + test_path_is_missing .git/rebase-apply +' + +test_expect_success 'stash pop goes on without rerere once rerere.lockTimeout is up' ' + git reset --hard && + rm -rf $rr && + git checkout -b lock-held-stash third && + test_when_finished "git reset --hard && git stash drop && + git checkout third && git branch -D lock-held-stash" && + echo stashed >>a1 && + git stash && + echo committed >>a1 && + git commit -qam committed && + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + test_must_fail git -c rerere.lockTimeout=0 stash pop 2>err && + test_grep "skipping rerere" err && + test_grep "^=======\$" a1 +' + +test_expect_success 'apply --3way goes on without rerere once rerere.lockTimeout is up' ' + git reset --hard && + rm -rf $rr && + git checkout -b lock-held-apply third && + test_when_finished "git reset --hard && + git checkout third && git branch -D lock-held-apply" && + git format-patch -1 --stdout first >first.patch && + test_when_finished "rm -f .git/MERGE_RR.lock" && + >.git/MERGE_RR.lock && + test_must_fail git -c rerere.lockTimeout=0 apply --3way first.patch 2>err && + test_grep "skipping rerere" err && + test_grep "^=======\$" a1 +' + 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 &&