maintenance: skip bootout/bootstrap when plist is registered

On macOS, we use launchctl to manage the background maintenance
schedule. This uses a set of .plist files to describe the schedule, but
these files are also registered with 'launchctl bootstrap'. If multiple
'git maintenance start' commands run concurrently, then they can collide
replacing these schedule files and registering them with launchctl.

To avoid extra launchctl commands, do a check for the .plist files on
disk and check if they are registered using 'launchctl list <name>'.
This command will return with exit code 0 if it exists, or exit code 113
if it does not.

We can test this behavior using the GIT_TEST_MAINT_SCHEDULER environment
variable.

Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/gc.c b/builtin/gc.c
index 06f18a347d0b805bebe706039f7ea851f962014b..22e670b508dd7ba22629a46f46d9eb1efe2d9faa 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1600,6 +1600,29 @@ static int launchctl_remove_plists(const char *cmd)
                launchctl_remove_plist(SCHEDULE_WEEKLY, cmd);
 }
 
+static int launchctl_list_contains_plist(const char *name, const char *cmd)
+{
+       int result;
+       struct child_process child = CHILD_PROCESS_INIT;
+       char *uid = launchctl_get_uid();
+
+       strvec_split(&child.args, cmd);
+       strvec_pushl(&child.args, "list", name, NULL);
+
+       child.no_stderr = 1;
+       child.no_stdout = 1;
+
+       if (start_command(&child))
+               die(_("failed to start launchctl"));
+
+       result = finish_command(&child);
+
+       free(uid);
+
+       /* Returns failure if 'name' doesn't exist. */
+       return !result;
+}
+
 static int launchctl_schedule_plist(const char *exec_path, enum schedule_priority schedule, const char *cmd)
 {
        int i, fd;
@@ -1609,7 +1632,8 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
        char *filename = launchctl_service_filename(name);
        struct lock_file lk = LOCK_INIT;
        static unsigned long lock_file_timeout_ms = ULONG_MAX;
-       struct strbuf plist = STRBUF_INIT;
+       struct strbuf plist = STRBUF_INIT, plist2 = STRBUF_INIT;
+       struct stat st;
 
        preamble = "<?xml version=\"1.0\"?>\n"
                   "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\" \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
@@ -1676,18 +1700,30 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
        fd = hold_lock_file_for_update_timeout(&lk, filename, LOCK_DIE_ON_ERROR,
                                               lock_file_timeout_ms);
 
-       if (write_in_full(fd, plist.buf, plist.len) < 0 ||
-           commit_lock_file(&lk))
-               die_errno(_("could not write '%s'"), filename);
-
-       /* bootout might fail if not already running, so ignore */
-       launchctl_boot_plist(0, filename, cmd);
-       if (launchctl_boot_plist(1, filename, cmd))
-               die(_("failed to bootstrap service %s"), filename);
+       /*
+        * Does this file already exist? With the intended contents? Is it
+        * registered already? Then it does not need to be re-registered.
+        */
+       if (!stat(filename, &st) && st.st_size == plist.len &&
+           strbuf_read_file(&plist2, filename, plist.len) == plist.len &&
+           !strbuf_cmp(&plist, &plist2) &&
+           launchctl_list_contains_plist(name, cmd))
+               rollback_lock_file(&lk);
+       else {
+               if (write_in_full(fd, plist.buf, plist.len) < 0 ||
+                   commit_lock_file(&lk))
+                       die_errno(_("could not write '%s'"), filename);
+
+               /* bootout might fail if not already running, so ignore */
+               launchctl_boot_plist(0, filename, cmd);
+               if (launchctl_boot_plist(1, filename, cmd))
+                       die(_("failed to bootstrap service %s"), filename);
+       }
 
        free(filename);
        free(name);
        strbuf_release(&plist);
+       strbuf_release(&plist2);
        return 0;
 }
 

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 58f46c77e66604020b15806453719374cfddd18e..fc16ac22585b90911e9ec6e6a62fdad407a99ba1 100755 (executable)
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -582,6 +582,23 @@ test_expect_success 'start and stop macOS maintenance' '
        test_line_count = 0 actual
 '
 
+test_expect_success 'use launchctl list to prevent extra work' '
+       # ensure we are registered
+       GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start &&
+
+       # do it again on a fresh args file
+       rm -f args &&
+       GIT_TEST_MAINT_SCHEDULER=launchctl:./print-args git maintenance start &&
+
+       ls "$HOME/Library/LaunchAgents" >actual &&
+       cat >expect <<-\EOF &&
+       list org.git-scm.git.hourly
+       list org.git-scm.git.daily
+       list org.git-scm.git.weekly
+       EOF
+       test_cmp expect args
+'
+
 test_expect_success 'start and stop Windows maintenance' '
        write_script print-args <<-\EOF &&
        echo $* >>args