diff --git a/Documentation/config/rerere.adoc b/Documentation/config/rerere.adoc index 3a78b5ebb1dc02..14ef193545fbf0 100644 --- a/Documentation/config/rerere.adoc +++ b/Documentation/config/rerere.adoc @@ -10,3 +10,13 @@ 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 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. diff --git a/Documentation/git-rerere.adoc b/Documentation/git-rerere.adoc index 4e6ab9a27c9168..05935b06030986 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 lock on the +recorded resolutions, for example a merge or rebase that is recording +a conflict, `gc` does nothing and reports so. DISCUSSION diff --git a/builtin/am.c b/builtin/am.c index e9623b8307793f..32f11161b489a5 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -2112,7 +2112,7 @@ 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); } diff --git a/builtin/rebase.c b/builtin/rebase.c index fa4f5d9306b856..363d177472d39e 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -367,7 +367,7 @@ 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: { @@ -1382,7 +1382,7 @@ 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) @@ -1396,7 +1396,7 @@ 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)) diff --git a/builtin/rerere.c b/builtin/rerere.c index a056cb791b4c80..70a4bd16835f22 100644 --- a/builtin/rerere.c +++ b/builtin/rerere.c @@ -74,7 +74,7 @@ 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; @@ -85,14 +85,15 @@ 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")) { diff --git a/rerere.c b/rerere.c index 8232542585cad4..bae780f584fef2 100644 --- a/rerere.c +++ b/rerere.c @@ -32,6 +32,7 @@ static int rerere_enabled = -1; /* automatically update cleanly resolved paths to the index */ static int rerere_autoupdate; +static int rerere_lock_timeout_ms = 1000; #define RR_HAS_POSTIMAGE 1 #define RR_HAS_PREIMAGE 2 @@ -876,6 +877,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); } @@ -908,12 +911,36 @@ 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_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) { fd = 0; - else - fd = hold_lock_file_for_update(&write_lock, - git_path_merge_rr(r), - LOCK_DIE_ON_ERROR); + } else { + int lock_flags = 0; + 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. + */ + fd = hold_lock_file_for_update_timeout(&write_lock, + git_path_merge_rr(r), + lock_flags, timeout_ms); + if (fd < 0) { + warning_errno(_("skipping rerere, unable to create '%s.lock'"), + git_path_merge_rr(r)); + return -1; + } + } read_rr(r, merge_rr); return fd; } @@ -1124,7 +1151,7 @@ static int rerere_forget_one_path(struct index_state *istate, 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; @@ -1133,7 +1160,7 @@ 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; @@ -1237,7 +1264,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", @@ -1289,11 +1316,11 @@ 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++) { diff --git a/rerere.h b/rerere.h index d4b5f7c932006a..3a9f58acd93e85 100644 --- a/rerere.h +++ b/rerere.h @@ -10,6 +10,10 @@ 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 */ +#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 @@ -34,9 +38,9 @@ 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), \ diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh index 1717f407c80d86..243b3ebed3863f 100755 --- a/t/t4200-rerere.sh +++ b/t/t4200-rerere.sh @@ -242,6 +242,102 @@ 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 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 "skipping rerere" 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 && + 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 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 && + 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_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 d7f82e1bec163f..a55ca2e829d0b0 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -885,6 +885,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