This is based on ds/maintenance-part-3.

After sitting with the background maintenance as it has been cooking, I
wanted to come back around and implement the background maintenance for
Windows. However, I noticed that there were some things bothering me with
background maintenance on my macOS machine. These are detailed in PATCH 3,
but the tl;dr is that 'cron' is not recommended by Apple and instead
'launchd' satisfies our needs.

This series implements the background scheduling so git maintenance
(start|stop) works on those platforms. I've been operating with these
schedules for a while now without the problems described in the patches.

There is a particularly annoying case about console windows popping up on
Windows, but PATCH 4 describes a plan to get around that.

Updates in V4
=============

 * Eric did an excellent job providing a patch that cleans up several parts
   of my series. The most impressive is his mechanism for testing the
   platform-specific Git logic in a way that is (mostly) platform-agnostic.

 * Windows doesn't have the 'id' command, so we cannot run the macOS
   platform test on Windows.

 * I noticed far too late that while my example XML files had been edited
   with UTF-8 encoding, Git is actually writing them as US-ASCII. Somehow
   xmllint and launchd are not complaining, but schtasks does complain.
   Unfortunately, I cannot find a way to catch this problem other than to
   install my tip version on all three platforms and go through the entire
   git maintenance start process, and double-check that the processes are
   running on the hour.

Here is a diff from the tip of v3 + Eric's patch to the tip of v4:

diff --git a/builtin/gc.c b/builtin/gc.c
index 955d4b3baf..1a3725429c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1642,13 +1642,13 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit
         break;
     }
     fprintf(plist, "</array>\n</dict>\n</plist>\n");
+    fclose(plist);

     /* 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);

-    fclose(plist);
     free(filename);
     free(name);
     return 0;
@@ -1707,25 +1707,27 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
     int result;
     struct child_process child = CHILD_PROCESS_INIT;
     const char *xml;
-    char *xmlpath, *tempDir;
-    FILE *xmlfp;
+    char *xmlpath;
+    struct tempfile *tfile;
     const char *frequency = get_frequency(schedule);
     char *name = schtasks_task_name(frequency);

-    tempDir = xstrfmt("%s/temp", the_repository->objects->odb->path);
-    xmlpath =  xstrfmt("%s/schedule-%s.xml", tempDir, frequency);
-    safe_create_leading_directories(xmlpath);
-    xmlfp = xfopen(xmlpath, "w");
+    xmlpath =  xstrfmt("%s/schedule-%s.xml",
+               the_repository->objects->odb->path,
+               frequency);
+    tfile = create_tempfile(xmlpath);
+    if (!tfile || !fdopen_tempfile(tfile, "w"))
+        die(_("failed to create '%s'"), xmlpath);

-    xml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+    xml = "<?xml version=\"1.0\" encoding=\"US-ASCII\"?>\n"
           "<Task version=\"1.4\" xmlns=\"http://schemas.microsoft.com/windows/2004/02/mit/task\">\n"
           "<Triggers>\n"
           "<CalendarTrigger>\n";
-    fputs(xml, xmlfp);
+    fputs(xml, tfile->fp);

     switch (schedule) {
     case SCHEDULE_HOURLY:
-        fprintf(xmlfp,
+        fprintf(tfile->fp,
             "<StartBoundary>2020-01-01T01:00:00</StartBoundary>\n"
             "<Enabled>true</Enabled>\n"
             "<ScheduleByDay>\n"
@@ -1739,7 +1741,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
         break;

     case SCHEDULE_DAILY:
-        fprintf(xmlfp,
+        fprintf(tfile->fp,
             "<StartBoundary>2020-01-01T00:00:00</StartBoundary>\n"
             "<Enabled>true</Enabled>\n"
             "<ScheduleByWeek>\n"
@@ -1756,7 +1758,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
         break;

     case SCHEDULE_WEEKLY:
-        fprintf(xmlfp,
+        fprintf(tfile->fp,
             "<StartBoundary>2020-01-01T00:00:00</StartBoundary>\n"
             "<Enabled>true</Enabled>\n"
             "<ScheduleByWeek>\n"
@@ -1771,7 +1773,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
         break;
     }

-    xml=  "</CalendarTrigger>\n"
+    xml = "</CalendarTrigger>\n"
           "</Triggers>\n"
           "<Principals>\n"
           "<Principal id=\"Author\">\n"
@@ -1795,11 +1797,10 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
           "</Exec>\n"
           "</Actions>\n"
           "</Task>\n";
-    fprintf(xmlfp, xml, exec_path, exec_path, frequency);
-    fclose(xmlfp);
-
+    fprintf(tfile->fp, xml, exec_path, exec_path, frequency);
     strvec_split(&child.args, cmd);
     strvec_pushl(&child.args, "/create", "/tn", name, "/f", "/xml", xmlpath, NULL);
+    close_tempfile_gently(tfile);

     child.no_stdout = 1;
     child.no_stderr = 1;
@@ -1808,8 +1809,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority
         die(_("failed to start schtasks"));
     result = finish_command(&child);

-    unlink(xmlpath);
-    rmdir(tempDir);
+    delete_tempfile(&tfile);
     free(xmlpath);
     free(name);
     return result;
@@ -1850,9 +1850,8 @@ static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd)
     crontab_list.out = dup(fd);
     crontab_list.git_cmd = 0;

-    if (start_command(&crontab_list)) {
+    if (start_command(&crontab_list))
         return error(_("failed to run 'crontab -l'; your system might not support 'cron'"));
-    }

     /* Ignore exit code, as an empty crontab will return error. */
     finish_command(&crontab_list);
@@ -1868,9 +1867,8 @@ static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd)
     crontab_edit.in = -1;
     crontab_edit.git_cmd = 0;

-    if (start_command(&crontab_edit)) {
+    if (start_command(&crontab_edit))
         return error(_("failed to run 'crontab'; your system might not support 'cron'"));
-    }

     cron_in = fdopen(crontab_edit.in, "w");
     if (!cron_in) {
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index e92946c10a..a26ff22541 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -408,7 +408,7 @@ test_expect_success 'start preserves existing schedule' '
     grep "Important information!" cron.txt
 '

-test_expect_success 'start and stop macOS maintenance' '
+test_expect_success !MINGW 'start and stop macOS maintenance' '
     uid=$(id -u) &&

     write_script print-args <<-\EOF &&
@@ -421,7 +421,6 @@ test_expect_success 'start and stop macOS maintenance' '
     # start registers the repo
     git config --get --global maintenance.repo "$(pwd)" &&

-    # ~/Library/LaunchAgents
     ls "$HOME/Library/LaunchAgents" >actual &&
     cat >expect <<-\EOF &&
     org.git-scm.git.daily.plist
@@ -468,12 +467,12 @@ test_expect_success 'start and stop Windows maintenance' '
     EOF

     rm -f args &&
-    GIT_TEST_MAINT_SCHEDULER="schtasks:/bin/sh print-args" git maintenance start &&
+    GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance start &&

     # start registers the repo
     git config --get --global maintenance.repo "$(pwd)" &&

-    printf "/create /tn Git Maintenance (%s) /f /xml .git/objects/temp/schedule-%s.xml\n" \
+    printf "/create /tn Git Maintenance (%s) /f /xml .git/objects/schedule-%s.xml\n" \
         hourly hourly daily daily weekly weekly >expect &&
     test_cmp expect args &&

@@ -483,7 +482,7 @@ test_expect_success 'start and stop Windows maintenance' '
     done &&

     rm -f args &&
-    GIT_TEST_MAINT_SCHEDULER="schtasks:/bin/sh print-args" git maintenance stop &&
+    GIT_TEST_MAINT_SCHEDULER="schtasks:./print-args" git maintenance stop &&

     # stop does not unregister the repo
     git config --get --global maintenance.repo "$(pwd)" &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4a60d1ed76..ddbeee1f5e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1704,7 +1704,8 @@ test_lazy_prereq REBASE_P '
 '

 # Ensure that no test accidentally triggers a Git command
-# that runs 'crontab', affecting a user's cron schedule.
-# Tests that verify the cron integration must set this locally
+# that runs the actual maintenance scheduler, affecting a user's
+# system permanently.
+# Tests that verify the scheduler integration must set this locally
 # to avoid errors.
-GIT_TEST_CRONTAB="exit 1"
+GIT_TEST_MAINT_SCHEDULER="none:exit 1"

Thanks, -Stolee

cc: jrnieder@gmail.com [jrnieder@gmail.com], jonathantanmy@google.com
[jonathantanmy@google.com], sluongng@gmail.com [sluongng@gmail.com]cc:
Derrick Stolee stolee@gmail.com [stolee@gmail.com]cc: Đoàn Trần Công Danh
congdanhqx@gmail.com [congdanhqx@gmail.com]cc: Martin Ågren
martin.agren@gmail.com [martin.agren@gmail.com]cc: Eric Sunshine
sunshine@sunshineco.com [sunshine@sunshineco.com]cc: Derrick Stolee
stolee@gmail.com [stolee@gmail.com]

Derrick Stolee (4):
  maintenance: extract platform-specific scheduling
  maintenance: include 'cron' details in docs
  maintenance: use launchctl on macOS
  maintenance: use Windows scheduled tasks

 Documentation/git-maintenance.txt | 116 ++++++++
 builtin/gc.c                      | 421 ++++++++++++++++++++++++++++--
 t/t7900-maintenance.sh            | 106 +++++++-
 t/test-lib.sh                     |   7 +-
 4 files changed, 616 insertions(+), 34 deletions(-)

base-commit: 0016b618182f642771dc589cf0090289f9fe1b4f

Submitted-As: https://lore.kernel.org/git/pull.776.v4.git.1605647598.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.776.git.1604412196.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.776.v2.git.1604520368.gitgitgadget@gmail.com
In-Reply-To: https://lore.kernel.org/git/pull.776.v3.git.1605276024.gitgitgadget@gmail.com