Merge branch 'jk/gc-pre-detach-under-hook' into maint

We run an early part of "git gc" that deals with refs before
daemonising (and not under lock) even when running a background
auto-gc, which caused multiple gc processes attempting to run the
early part at the same time.  This is now prevented by running the
early part also under the GC lock.

* jk/gc-pre-detach-under-hook:
gc: run pre-detach operations under lock

diff --git a/builtin/gc.c b/builtin/gc.c
index 91f7696a85ec974d0ff5a1591cc404ed99efb7d7..2d2027d8b09913d30ce103c8c0ae4332fb729997 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -413,8 +413,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
                        if (report_last_gc_error())
                                return -1;
 
+                       if (lock_repo_for_gc(force, &pid))
+                               return 0;
                        if (gc_before_repack())
                                return -1;
+                       delete_tempfile(&pidfile);
+
                        /*
                         * failure to daemonize is ok, we'll continue
                         * in foreground

diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh
index cc7acd101db082302b351a81929f3f571c1a4db0..41b0be575d523071152c19a224a7ef914ce2ee1a 100755 (executable)
--- a/t/t6500-gc.sh
+++ b/t/t6500-gc.sh
@@ -95,6 +95,27 @@ test_expect_success 'background auto gc does not run if gc.log is present and re
        test_line_count = 1 packs
 '
 
+test_expect_success 'background auto gc respects lock for all operations' '
+       # make sure we run a background auto-gc
+       test_commit make-pack &&
+       git repack &&
+       test_config gc.autopacklimit 1 &&
+       test_config gc.autodetach true &&
+
+       # create a ref whose loose presence we can use to detect a pack-refs run
+       git update-ref refs/heads/should-be-loose HEAD &&
+       test_path_is_file .git/refs/heads/should-be-loose &&
+
+       # now fake a concurrent gc that holds the lock; we can use our
+       # shell pid so that it looks valid.
+       hostname=$(hostname || echo unknown) &&
+       printf "$$ %s" "$hostname" >.git/gc.pid &&
+
+       # our gc should exit zero without doing anything
+       run_and_wait_for_auto_gc &&
+       test_path_is_file .git/refs/heads/should-be-loose
+'
+
 # DO NOT leave a detached auto gc process running near the end of the
 # test script: it can run long enough in the background to racily
 # interfere with the cleanup in 'test_done'.