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>
Derrick Stolee committed
Nov 24, 2020 at 04:16 UTC
31345d5545e755bf79143841f3840ae5bcd0ac0b
3 files changed
+51
-34
builtin/gc.c
+43
-27
@@ -1494,35 +1494,23 @@ static int maintenance_unregister(void)
1494
#define BEGIN_LINE "# BEGIN GIT MAINTENANCE SCHEDULE"
1495
#define END_LINE "# END GIT MAINTENANCE SCHEDULE"
1496
1497
-static int update_background_schedule(int run_maintenance)
1497
+static int crontab_update_schedule(int run_maintenance, int fd, const char *cmd)
1498
{
1499
int result = 0;
1500
int in_old_region = 0;
1501
struct child_process crontab_list = CHILD_PROCESS_INIT;
1502
struct child_process crontab_edit = CHILD_PROCESS_INIT;
1503
FILE *cron_list, *cron_in;
1504
- const char *crontab_name;
1504
struct strbuf line = STRBUF_INIT;
1506
- struct lock_file lk;
1507
- char *lock_path = xstrfmt("%s/schedule", the_repository->objects->odb->path);
1505
1509
- if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
1510
- return error(_("another process is scheduling background maintenance"));
1511
-
1512
- crontab_name = getenv("GIT_TEST_CRONTAB");
1513
- if (!crontab_name)
1514
- crontab_name = "crontab";
1515
-
1516
- strvec_split(&crontab_list.args, crontab_name);
1506
+ strvec_split(&crontab_list.args, cmd);
1507
strvec_push(&crontab_list.args, "-l");
1508
crontab_list.in = -1;
1519
- crontab_list.out = dup(lk.tempfile->fd);
1509
+ crontab_list.out = dup(fd);
1510
crontab_list.git_cmd = 0;
1511
1522
- if (start_command(&crontab_list)) {
1523
- result = error(_("failed to run 'crontab -l'; your system might not support 'cron'"));
1524
- goto cleanup;
1525
- }
1512
+ if (start_command(&crontab_list))
1513
+ return error(_("failed to run 'crontab -l'; your system might not support 'cron'"));
1514
1515
/* Ignore exit code, as an empty crontab will return error. */
1516
finish_command(&crontab_list);
@@ -1531,17 +1519,15 @@ static int update_background_schedule(int run_maintenance)
1519
* Read from the .lock file, filtering out the old
1520
* schedule while appending the new schedule.
1521
*/
1534
- cron_list = fdopen(lk.tempfile->fd, "r");
1522
+ cron_list = fdopen(fd, "r");
1523
rewind(cron_list);
1524
1537
- strvec_split(&crontab_edit.args, crontab_name);
1525
+ strvec_split(&crontab_edit.args, cmd);
1526
crontab_edit.in = -1;
1527
crontab_edit.git_cmd = 0;
1528
1541
- if (start_command(&crontab_edit)) {
1542
- result = error(_("failed to run 'crontab'; your system might not support 'cron'"));
1543
- goto cleanup;
1544
- }
1529
+ if (start_command(&crontab_edit))
1530
+ return error(_("failed to run 'crontab'; your system might not support 'cron'"));
1531
1532
cron_in = fdopen(crontab_edit.in, "w");
1533
if (!cron_in) {
@@ -1586,14 +1572,44 @@ static int update_background_schedule(int run_maintenance)
1572
close(crontab_edit.in);
1573
1574
done_editing:
1589
- if (finish_command(&crontab_edit)) {
1575
+ if (finish_command(&crontab_edit))
1576
result = error(_("'crontab' died"));
1591
- goto cleanup;
1577
+ else
1578
+ fclose(cron_list);
1579
+ return result;
1580
+}
1581
+
1582
+static const char platform_scheduler[] = "crontab";
1583
+
1584
+static int update_background_schedule(int enable)
1585
+{
1586
+ int result;
1587
+ const char *scheduler = platform_scheduler;
1588
+ const char *cmd = scheduler;
1589
+ char *testing;
1590
+ struct lock_file lk;
1591
+ char *lock_path = xstrfmt("%s/schedule", the_repository->objects->odb->path);
1592
+
1593
+ testing = xstrdup_or_null(getenv("GIT_TEST_MAINT_SCHEDULER"));
1594
+ if (testing) {
1595
+ char *sep = strchr(testing, ':');
1596
+ if (!sep)
1597
+ die("GIT_TEST_MAINT_SCHEDULER unparseable: %s", testing);
1598
+ *sep = '\0';
1599
+ scheduler = testing;
1600
+ cmd = sep + 1;
1601
}
1593
- fclose(cron_list);
1602
1595
-cleanup:
1603
+ if (hold_lock_file_for_update(&lk, lock_path, LOCK_NO_DEREF) < 0)
1604
+ return error(_("another process is scheduling background maintenance"));
1605
+
1606
+ if (!strcmp(scheduler, "crontab"))
1607
+ result = crontab_update_schedule(enable, lk.tempfile->fd, cmd);
1608
+ else
1609
+ die("unknown background scheduler: %s", scheduler);
1610
+
1611
rollback_lock_file(&lk);
1612
+ free(testing);
1613
return result;
1614
}
1615
t/t7900-maintenance.sh
+4
-4
@@ -368,7 +368,7 @@ test_expect_success 'register and unregister' '
368
'
369
370
test_expect_success 'start from empty cron table' '
371
- GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
371
+ GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start &&
372
373
# start registers the repo
374
git config --get --global maintenance.repo "$(pwd)" &&
@@ -379,19 +379,19 @@ test_expect_success 'start from empty cron table' '
379
'
380
381
test_expect_success 'stop from existing schedule' '
382
- GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
382
+ GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance stop &&
383
384
# stop does not unregister the repo
385
git config --get --global maintenance.repo "$(pwd)" &&
386
387
# Operation is idempotent
388
- GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance stop &&
388
+ GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance stop &&
389
test_must_be_empty cron.txt
390
'
391
392
test_expect_success 'start preserves existing schedule' '
393
echo "Important information!" >cron.txt &&
394
- GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start &&
394
+ GIT_TEST_MAINT_SCHEDULER="crontab:test-tool crontab cron.txt" git maintenance start &&
395
grep "Important information!" cron.txt
396
'
397
t/test-lib.sh
+4
-3
@@ -1704,7 +1704,8 @@ test_lazy_prereq REBASE_P '
1704
'
1705
1706
# Ensure that no test accidentally triggers a Git command
1707
-# that runs 'crontab', affecting a user's cron schedule.
1708
-# Tests that verify the cron integration must set this locally
1707
+# that runs the actual maintenance scheduler, affecting a user's
1708
+# system permanently.
1709
+# Tests that verify the scheduler integration must set this locally
1710
# to avoid errors.
1710
-GIT_TEST_CRONTAB="exit 1"
1711
+GIT_TEST_MAINT_SCHEDULER="none:exit 1"