[PATCH v7 0/2] rerere: wait for MERGE_RR.lock, but not in auto maintenance

A rebase dies at a conflict when a background "git rerere gc" holds MERGE_RR.lock at that moment. With the first patch, rerere waits for the lock instead of dying at once. With the second, the "git rerere gc" started by auto maintenance does nothing while the lock is held.

For v7 I took Patrick's suggestions on patches 2 and 3. Changes since v6:

 * Patch 3, which let a command that stops at a conflict go on without
   rerere, is gone. It can come back if users still run into the lock
   (Patrick).

 * Patch 2's log message is Patrick's, plus a last paragraph on why the
   option is hidden.

 * rerere_gc() takes its own flags, enum rerere_gc_flags, with
   RERERE_GC_NOWAIT (Patrick).

 * The comment in setup_rerere() no longer says who passes RERERE_NOWAIT,
   and the one on RERERE_NOWAIT now says that setup_rerere() then returns -1
   as if rerere were disabled (Patrick).

 * "git rerere gc --skip-locked" without a held lock is a test of its own
   (Patrick).

Thomas Bachem (2):
  rerere: wait for MERGE_RR.lock before giving up
  rerere: add "gc --skip-locked" for auto maintenance

 Documentation/config/rerere.adoc | 10 ++++
 builtin/gc.c                     |  4 +-
 builtin/rerere.c                 | 10 +++-
 rerere.c                         | 38 ++++++++++++---
 rerere.h                         | 11 ++++-
 t/t4200-rerere.sh                | 79 ++++++++++++++++++++++++++++++++
 t/t7900-maintenance.sh           | 25 +++++++++-
 7 files changed, 165 insertions(+), 12 deletions(-)

base-commit: 34f06850c16c7f7ac822b1adc71354f11b0f2ca3
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2214%2Fthomasbachem%2Frerere-gc-lock-v7
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2214/thomasbachem/rerere-gc-lock-v7
Pull-Request: https://github.com/gitgitgadget/git/pull/2214

Range-diff vs v6:

 1:  3dc3d02f12 = 1:  3dc3d02f12 rerere: wait for MERGE_RR.lock before giving up
 2:  2ef141410a ! 2:  0002881196 rerere: add "gc --skip-locked" for auto maintenance
     @@ Metadata
       ## Commit message ##
          rerere: add "gc --skip-locked" for auto maintenance
      
     -    Since the previous commit, "git rerere gc" waits for MERGE_RR.lock
     -    like every other command that takes it, and fails only if the wait
     -    times out. That suits a user who runs it by hand and wants to know
     -    when nothing was pruned. But the user did not ask for the gc that auto
     -    maintenance starts after a commit, and the next commit starts another
     -    one.
     +    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.
      
     -    So add "--skip-locked", with which "git rerere gc" quietly does
     -    nothing while the lock is held, and pass it from
     -    "git maintenance run --auto" and "git gc --auto". Only these two need
     -    the option, so hide it and leave it undocumented, like the
     -    "--skip-foreground-tasks" that "git maintenance run" passes to
     -    "git gc". A run without it, from the command line or a maintenance
     -    schedule, still waits for the lock and fails if the wait times out.
     +    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 <ps@pks.im>
          Assisted-by: Claude Fable 5.1
          Signed-off-by: Thomas Bachem <mail@thomasbachem.com>
      
     @@ builtin/rerere.c: int cmd_rerere(int argc,
       	} else if (!strcmp(argv[0], "gc"))
      -		rerere_gc(the_repository, &merge_rr);
      +		rerere_gc(the_repository, &merge_rr,
     -+			  skip_locked ? RERERE_NOWAIT : 0);
     ++			  skip_locked ? RERERE_GC_NOWAIT : 0);
       	else if (!strcmp(argv[0], "status")) {
       		if (setup_rerere(the_repository, &merge_rr,
       				 flags | RERERE_READONLY) < 0)
     @@ rerere.c: int setup_rerere(struct repository *r, struct string_list *merge_rr, i
       		/*
       		 * 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.
     -+		 * it instead of dying right away.  The gc of an automatic
     -+		 * maintenance run does not wait, since skipping one of
     -+		 * its runs costs nothing.
     + 		 * it instead of dying right away.
       		 */
      +		if (flags & RERERE_NOWAIT) {
      +			lock_flags = 0;
     @@ rerere.c: out:
       }
       
      -void rerere_gc(struct repository *r, struct string_list *rr)
     -+void rerere_gc(struct repository *r, struct string_list *rr, int flags)
     ++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;
     @@ rerere.c: void rerere_gc(struct repository *r, struct string_list *rr)
       	struct strbuf buf = STRBUF_INIT;
       
      -	if (setup_rerere(r, rr, 0) < 0)
     -+	if (setup_rerere(r, rr, flags) < 0)
     ++	if (setup_rerere(r, rr,
     ++			 (flags & RERERE_GC_NOWAIT) ? RERERE_NOWAIT : 0) < 0)
       		return;
       
       	rerere_gc_cutoffs(r, &cutoff_resolve, &cutoff_noresolve);
     @@ rerere.h: struct repository;
       #define RERERE_AUTOUPDATE   01
       #define RERERE_NOAUTOUPDATE 02
       #define RERERE_READONLY     04
     -+/* Take MERGE_RR.lock only if it is free, and return quietly otherwise */
     ++/* If MERGE_RR.lock is taken, return -1 as if rerere were disabled */
      +#define RERERE_NOWAIT       010
       
       /*
     @@ rerere.h: const char *rerere_path(struct strbuf *buf, const struct rerere_id *,
       int rerere_remaining(struct repository *, struct string_list *);
       void rerere_clear(struct repository *, struct string_list *);
      -void rerere_gc(struct repository *, struct string_list *);
     -+void rerere_gc(struct repository *, struct string_list *, int);
     ++
     ++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
     @@ t/t4200-rerere.sh: test_expect_success 'old records rest in peace' '
      +	>.git/MERGE_RR.lock &&
      +	git rerere gc --skip-locked 2>err &&
      +	test_must_be_empty err &&
     -+	test_path_is_file $rr2/preimage &&
     ++	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 &&
      +
     -+	rm .git/MERGE_RR.lock &&
      +	git rerere gc --skip-locked &&
      +	test_path_is_missing $rr2/preimage
      +'
 3:  cd018289bb < -:  ---------- rerere: go on at a conflict when the lock stays busy