maintenance: extract platform-specific scheduling

The existing schedule mechanism using 'cron' is supported by POSIX
platforms, but not Windows. It also works slightly differently on
macOS to significant detriment of the user experience. To allow for
new implementations on these platforms, extract a method that
performs the platform-specific scheduling mechanism. This will be
swapped at compile time with new implementations on specialized
platforms.

As we add this generality, rename GIT_TEST_CRONTAB to
GIT_TEST_MAINT_SCHEDULER. Further, this variable is now parsed as
"<scheduler>:<command>" so we can test platform-specific scheduling
logic even when not on the correct platform. By specifying the
<scheduler> in this string, we will be able to test all three sets of
Git logic from a Linux machine.

Co-authored-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/gc.c b/builtin/gc.c
index e3098ef6a12896e4f4890a9f6d12c6ac7bed3eac..18ae7f7138a393923daed71c3fa593782e8459d7 100644 (file)
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1494,35 +1494,23 @@ static int maintenance_unregister(void)
 #define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE"
 #define END_LINE "# END GIT MAINTENANCE SCHEDULE"
 
-static int update_background_schedule(int run_maintenance)
+static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd)
 {
        int result = 0;
        int in_old_region = 0;
        struct child_process crontab_list = CHILD_PROCESS_INIT;
        struct child_process crontab_edit = CHILD_PROCESS_INIT;
        FILE *cron_list, *cron_in;
-       const char *crontab_name;
        struct strbuf line = STRBUF_INIT;
-       struct lock_file lk;
-       char *lock_path = xstrfmt("%s/schedule", the_repository->objects->odb->path);
 
-       if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
-               return error(_("another process is scheduling background maintenance"));
-
-       crontab_name = getenv("GIT_TEST_CRONTAB");
-       if (!crontab_name)
-               crontab_name = "crontab";
-
-       strvec_split(&crontab_list.args, crontab_name);
+       strvec_split(&crontab_list.args, cmd);
        strvec_push(&crontab_list.args, "-l");
        crontab_list.in = -1;
-       crontab_list.out = dup(lk.tempfile->fd);
+       crontab_list.out = dup(fd);
        crontab_list.git_cmd = 0;
 
-       if (start_command(&crontab_list)) {
-               result = error(_("failed to run 'crontab -l'; your system might not support 'cron'"));
-               goto cleanup;
-       }
+       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);
@@ -1531,17 +1519,15 @@ static int update_background_schedule(int run_maintenance)
         * Read from the .lock file, filtering out the old
         * schedule while appending the new schedule.
         */
-       cron_list = fdopen(lk.tempfile->fd, "r");
+       cron_list = fdopen(fd, "r");
        rewind(cron_list);
 
-       strvec_split(&crontab_edit.args, crontab_name);
+       strvec_split(&crontab_edit.args, cmd);
        crontab_edit.in = -1;
        crontab_edit.git_cmd = 0;
 
-       if (start_command(&crontab_edit)) {
-               result = error(_("failed to run 'crontab'; your system might not support 'cron'"));
-               goto cleanup;
-       }
+       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) {
@@ -1586,14 +1572,44 @@ static int update_background_schedule(int run_maintenance)
        close(crontab_edit.in);
 
 done_editing:
-       if (finish_command(&crontab_edit)) {
+       if (finish_command(&crontab_edit))
                result = error(_("'crontab' died"));
-               goto cleanup;
+       else
+               fclose(cron_list);
+       return result;
+}
+
+static const char platform_scheduler[] = "crontab";
+
+static int update_background_schedule(int enable)
+{
+       int result;
+       const char *scheduler = platform_scheduler;
+       const char *cmd = scheduler;
+       char *testing;
+       struct lock_file lk;
+       char *lock_path = xstrfmt("%s/schedule", the_repository->objects->odb->path);
+
+       testing = xstrdup_or_null(getenv("GIT_TEST_MAINT_SCHEDULER"));
+       if (testing) {
+               char *sep = strchr(testing, ':');
+               if (!sep)
+                       die("GIT_TEST_MAINT_SCHEDULER unparseable: %s", testing);
+               *sep = '\0';
+               scheduler = testing;
+               cmd = sep + 1;
        }
-       fclose(cron_list);
 
-cleanup:
+       if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
+               return error(_("another process is scheduling background maintenance"));
+
+       if (!strcmp(scheduler, "crontab"))
+               result = crontab_update_schedule(enable, lk.tempfile->fd, cmd);
+       else
+               die("unknown background scheduler: %s", scheduler);
+
        rollback_lock_file(&lk);
+       free(testing);
        return result;
 }
 

diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index 20184e96e1addf424f1fdeb951a30928a4319ec5..eeb939168da0413dd71605ce49c7dfd20bb1741a 100755 (executable)
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -368,7 +368,7 @@ test_expect_success 'register and unregister' '
 '
 
 test_expect_success 'start from empty cron table' '
-       GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
+       GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start &&
 
        # start registers the repo
        git config --get --global maintenance.repo "$(pwd)" &&
@@ -379,19 +379,19 @@ test_expect_success 'start from empty cron table' '
 '
 
 test_expect_success 'stop from existing schedule' '
-       GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
+       GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance stop &&
 
        # stop does not unregister the repo
        git config --get --global maintenance.repo "$(pwd)" &&
 
        # Operation is idempotent
-       GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
+       GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance stop &&
        test_must_be_empty cron.txt
 '
 
 test_expect_success 'start preserves existing schedule' '
        echo "Important information!" >cron.txt &&
-       GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
+       GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start &&
        grep "Important information!" cron.txt
 '
 

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 4a60d1ed76632ecc1d24c5f6b52c3c5962449499..ddbeee1f5eb9b71f202d322b79b419803dc490b3 100644 (file)
--- 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"