diff --git a/builtin/gc.c b/builtin/gc.c index f3942188a614c9..0a515ecb8e33f2 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1708,6 +1708,15 @@ static int get_schedule_cmd(const char **cmd, int *is_available) return 1; } +static int get_random_minute(void) +{ + /* Use a static value when under tests. */ + if (!getenv("GIT_TEST_MAINTENANCE_SCHEDULER")) + return 13; + + return git_rand() % 60; +} + static int is_launchctl_available(void) { const char *cmd = "launchctl"; @@ -1820,6 +1829,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit struct strbuf plist = STRBUF_INIT, plist2 = STRBUF_INIT; struct stat st; const char *cmd = "launchctl"; + int minute = get_random_minute(); get_schedule_cmd(&cmd, NULL); preamble = "\n" @@ -1845,29 +1855,30 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit case SCHEDULE_HOURLY: repeat = "\n" "Hour%d\n" - "Minute0\n" + "Minute%d\n" "\n"; for (i = 1; i <= 23; i++) - strbuf_addf(&plist, repeat, i); + strbuf_addf(&plist, repeat, i, minute); break; case SCHEDULE_DAILY: repeat = "\n" "Day%d\n" "Hour0\n" - "Minute0\n" + "Minute%d\n" "\n"; for (i = 1; i <= 6; i++) - strbuf_addf(&plist, repeat, i); + strbuf_addf(&plist, repeat, i, minute); break; case SCHEDULE_WEEKLY: - strbuf_addstr(&plist, - "\n" - "Day0\n" - "Hour0\n" - "Minute0\n" - "\n"); + strbuf_addf(&plist, + "\n" + "Day0\n" + "Hour0\n" + "Minute%d\n" + "\n", + minute); break; default: @@ -1984,6 +1995,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority const char *frequency = get_frequency(schedule); char *name = schtasks_task_name(frequency); struct strbuf tfilename = STRBUF_INIT; + int minute = get_random_minute(); get_schedule_cmd(&cmd, NULL); @@ -2004,7 +2016,7 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority switch (schedule) { case SCHEDULE_HOURLY: fprintf(tfile->fp, - "2020-01-01T01:00:00\n" + "2020-01-01T01:%02d:00\n" "true\n" "\n" "1\n" @@ -2013,12 +2025,13 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority "PT1H\n" "PT23H\n" "false\n" - "\n"); + "\n", + minute); break; case SCHEDULE_DAILY: fprintf(tfile->fp, - "2020-01-01T00:00:00\n" + "2020-01-01T00:%02d:00\n" "true\n" "\n" "\n" @@ -2030,19 +2043,21 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority "\n" "\n" "1\n" - "\n"); + "\n", + minute); break; case SCHEDULE_WEEKLY: fprintf(tfile->fp, - "2020-01-01T00:00:00\n" + "2020-01-01T00:%02d:00\n" "true\n" "\n" "\n" "\n" "\n" "1\n" - "\n"); + "\n", + minute); break; default: @@ -2159,6 +2174,7 @@ static int crontab_update_schedule(int run_maintenance, int fd) FILE *cron_list, *cron_in; struct strbuf line = STRBUF_INIT; struct tempfile *tmpedit = NULL; + int minute = get_random_minute(); get_schedule_cmd(&cmd, NULL); strvec_split(&crontab_list.args, cmd); @@ -2213,11 +2229,11 @@ static int crontab_update_schedule(int run_maintenance, int fd) "# replaced in the future by a Git command.\n\n"); strbuf_addf(&line_format, - "%%s %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\n", + "%%d %%s * * %%s \"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%s\n", exec_path, exec_path); - fprintf(cron_in, line_format.buf, "0", "1-23", "*", "hourly"); - fprintf(cron_in, line_format.buf, "0", "0", "1-6", "daily"); - fprintf(cron_in, line_format.buf, "0", "0", "0", "weekly"); + fprintf(cron_in, line_format.buf, minute, "1-23", "*", "hourly"); + fprintf(cron_in, line_format.buf, minute, "0", "1-6", "daily"); + fprintf(cron_in, line_format.buf, minute, "0", "0", "weekly"); strbuf_release(&line_format); fprintf(cron_in, "\n%s\n", END_LINE); @@ -2276,77 +2292,22 @@ static char *xdg_config_home_systemd(const char *filename) return xdg_config_home_for("systemd/user", filename); } -static int systemd_timer_enable_unit(int enable, - enum schedule_priority schedule) -{ - const char *cmd = "systemctl"; - struct child_process child = CHILD_PROCESS_INIT; - const char *frequency = get_frequency(schedule); - - /* - * Disabling the systemd unit while it is already disabled makes - * systemctl print an error. - * Let's ignore it since it means we already are in the expected state: - * the unit is disabled. - * - * On the other hand, enabling a systemd unit which is already enabled - * produces no error. - */ - if (!enable) - child.no_stderr = 1; - - get_schedule_cmd(&cmd, NULL); - strvec_split(&child.args, cmd); - strvec_pushl(&child.args, "--user", enable ? "enable" : "disable", - "--now", NULL); - strvec_pushf(&child.args, "git-maintenance@%s.timer", frequency); - - if (start_command(&child)) - return error(_("failed to start systemctl")); - if (finish_command(&child)) - /* - * Disabling an already disabled systemd unit makes - * systemctl fail. - * Let's ignore this failure. - * - * Enabling an enabled systemd unit doesn't fail. - */ - if (enable) - return error(_("failed to run systemctl")); - return 0; -} - -static int systemd_timer_delete_unit_templates(void) -{ - int ret = 0; - char *filename = xdg_config_home_systemd("git-maintenance@.timer"); - if (unlink(filename) && !is_missing_file_error(errno)) - ret = error_errno(_("failed to delete '%s'"), filename); - FREE_AND_NULL(filename); - - filename = xdg_config_home_systemd("git-maintenance@.service"); - if (unlink(filename) && !is_missing_file_error(errno)) - ret = error_errno(_("failed to delete '%s'"), filename); +#define SYSTEMD_UNIT_FORMAT "git-maintenance@%s.%s" - free(filename); - return ret; -} - -static int systemd_timer_delete_units(void) -{ - return systemd_timer_enable_unit(0, SCHEDULE_HOURLY) || - systemd_timer_enable_unit(0, SCHEDULE_DAILY) || - systemd_timer_enable_unit(0, SCHEDULE_WEEKLY) || - systemd_timer_delete_unit_templates(); -} - -static int systemd_timer_write_unit_templates(const char *exec_path) +static int systemd_timer_write_unit_file(enum schedule_priority schedule, + const char *exec_path, + int minute) { char *filename; FILE *file; const char *unit; + char *schedule_pattern = NULL; + const char *frequency = get_frequency(schedule); + char *local_timer_name = xstrfmt(SYSTEMD_UNIT_FORMAT, frequency, "timer"); + char *local_service_name = xstrfmt(SYSTEMD_UNIT_FORMAT, frequency, "service"); + + filename = xdg_config_home_systemd(local_timer_name); - filename = xdg_config_home_systemd("git-maintenance@.timer"); if (safe_create_leading_directories(filename)) { error(_("failed to create directories for '%s'"), filename); goto error; @@ -2355,6 +2316,23 @@ static int systemd_timer_write_unit_templates(const char *exec_path) if (!file) goto error; + switch (schedule) { + case SCHEDULE_HOURLY: + schedule_pattern = xstrfmt("*-*-* 1..23:%02d:00", minute); + break; + + case SCHEDULE_DAILY: + schedule_pattern = xstrfmt("Tue..Sun *-*-* 0:%02d:00", minute); + break; + + case SCHEDULE_WEEKLY: + schedule_pattern = xstrfmt("Mon 0:%02d:00", minute); + break; + + default: + BUG("Unhandled schedule_priority"); + } + unit = "# This file was created and is maintained by Git.\n" "# Any edits made in this file might be replaced in the future\n" "# by a Git command.\n" @@ -2363,12 +2341,12 @@ static int systemd_timer_write_unit_templates(const char *exec_path) "Description=Optimize Git repositories data\n" "\n" "[Timer]\n" - "OnCalendar=%i\n" + "OnCalendar=%s\n" "Persistent=true\n" "\n" "[Install]\n" "WantedBy=timers.target\n"; - if (fputs(unit, file) == EOF) { + if (fprintf(file, unit, schedule_pattern) < 0) { error(_("failed to write to '%s'"), filename); fclose(file); goto error; @@ -2379,7 +2357,7 @@ static int systemd_timer_write_unit_templates(const char *exec_path) } free(filename); - filename = xdg_config_home_systemd("git-maintenance@.service"); + filename = xdg_config_home_systemd(local_service_name); file = fopen_or_warn(filename, "w"); if (!file) goto error; @@ -2393,7 +2371,7 @@ static int systemd_timer_write_unit_templates(const char *exec_path) "\n" "[Service]\n" "Type=oneshot\n" - "ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%%i\n" + "ExecStart=\"%s/git\" --exec-path=\"%s\" for-each-repo --config=maintenance.repo maintenance run --schedule=%s\n" "LockPersonality=yes\n" "MemoryDenyWriteExecute=yes\n" "NoNewPrivileges=yes\n" @@ -2403,7 +2381,7 @@ static int systemd_timer_write_unit_templates(const char *exec_path) "RestrictSUIDSGID=yes\n" "SystemCallArchitectures=native\n" "SystemCallFilter=@system-service\n"; - if (fprintf(file, unit, exec_path, exec_path) < 0) { + if (fprintf(file, unit, exec_path, exec_path, frequency) < 0) { error(_("failed to write to '%s'"), filename); fclose(file); goto error; @@ -2416,19 +2394,104 @@ static int systemd_timer_write_unit_templates(const char *exec_path) return 0; error: + free(schedule_pattern); + free(local_timer_name); free(filename); - systemd_timer_delete_unit_templates(); return -1; } +static int systemd_timer_enable_unit(int enable, + enum schedule_priority schedule, + const char *exec_path, + int minute) +{ + const char *cmd = "systemctl"; + struct child_process child = CHILD_PROCESS_INIT; + const char *frequency = get_frequency(schedule); + + /* + * Disabling the systemd unit while it is already disabled makes + * systemctl print an error. + * Let's ignore it since it means we already are in the expected state: + * the unit is disabled. + * + * On the other hand, enabling a systemd unit which is already enabled + * produces no error. + */ + if (!enable) + child.no_stderr = 1; + else if (systemd_timer_write_unit_file(schedule, exec_path, minute)) + return -1; + + get_schedule_cmd(&cmd, NULL); + strvec_split(&child.args, cmd); + strvec_pushl(&child.args, "--user", enable ? "enable" : "disable", + "--now", NULL); + strvec_pushf(&child.args, SYSTEMD_UNIT_FORMAT, frequency, "timer"); + + if (start_command(&child)) + return error(_("failed to start systemctl")); + if (finish_command(&child)) + /* + * Disabling an already disabled systemd unit makes + * systemctl fail. + * Let's ignore this failure. + * + * Enabling an enabled systemd unit doesn't fail. + */ + if (enable) + return error(_("failed to run systemctl")); + return 0; +} + +static int systemd_timer_delete_unit_file(enum schedule_priority priority) +{ + const char *frequency = get_frequency(priority); + char *local_timer_name = xstrfmt(SYSTEMD_UNIT_FORMAT, frequency, "timer"); + char *local_service_name = xstrfmt(SYSTEMD_UNIT_FORMAT, frequency, "service"); + int ret = 0; + char *filename = xdg_config_home_systemd(local_timer_name); + if (unlink(filename) && !is_missing_file_error(errno)) + ret = error_errno(_("failed to delete '%s'"), filename); + FREE_AND_NULL(filename); + + filename = xdg_config_home_systemd(local_service_name); + if (unlink(filename) && !is_missing_file_error(errno)) + ret = error_errno(_("failed to delete '%s'"), filename); + + free(filename); + free(local_timer_name); + free(local_service_name); + return ret; +} + +static int systemd_timer_delete_unit_files(void) +{ + /* Purposefully not short-circuited to make sure all are called. */ + return systemd_timer_delete_unit_file(SCHEDULE_HOURLY) | + systemd_timer_delete_unit_file(SCHEDULE_DAILY) | + systemd_timer_delete_unit_file(SCHEDULE_WEEKLY); +} + +static int systemd_timer_delete_units(void) +{ + int minute = get_random_minute(); + const char *exec_path = git_exec_path(); + /* Purposefully not short-circuited to make sure all are called. */ + return systemd_timer_enable_unit(0, SCHEDULE_HOURLY, exec_path, minute) | + systemd_timer_enable_unit(0, SCHEDULE_DAILY, exec_path, minute) | + systemd_timer_enable_unit(0, SCHEDULE_WEEKLY, exec_path, minute) | + systemd_timer_delete_unit_files(); +} + static int systemd_timer_setup_units(void) { + int minute = get_random_minute(); const char *exec_path = git_exec_path(); - int ret = systemd_timer_write_unit_templates(exec_path) || - systemd_timer_enable_unit(1, SCHEDULE_HOURLY) || - systemd_timer_enable_unit(1, SCHEDULE_DAILY) || - systemd_timer_enable_unit(1, SCHEDULE_WEEKLY); + int ret = systemd_timer_enable_unit(1, SCHEDULE_HOURLY, exec_path, minute) || + systemd_timer_enable_unit(1, SCHEDULE_DAILY, exec_path, minute) || + systemd_timer_enable_unit(1, SCHEDULE_WEEKLY, exec_path, minute); if (ret) systemd_timer_delete_units(); return ret; diff --git a/git-compat-util.h b/git-compat-util.h index 5b2b99c17c564c..62ce1fb5d64c0d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1673,4 +1673,15 @@ void sleep_millisec(int millisec); */ int csprng_bytes(void *buf, size_t len); +/* + * Returns a random int from rand() (0 to MAX_RAND inclusive). Converted + * to a uint32_t to emphasize that it will never be negative, though the + * number is from an isolated subset of the possible unsigned range. + * + * git_rand() centralizes the initializer of the pseudo-random number + * generator so we do not accidentally initialize it twice and reuse + * numbers from the sequence created after initialization. + */ +uint32_t git_rand(void); + #endif diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh index 487e326b3fac12..a327973990e069 100755 --- a/t/t7900-maintenance.sh +++ b/t/t7900-maintenance.sh @@ -744,7 +744,9 @@ test_expect_success 'start and stop Linux/systemd maintenance' ' # start registers the repo git config --get --global --fixed-value maintenance.repo "$(pwd)" && - test_systemd_analyze_verify "systemd/user/git-maintenance@.service" && + test_systemd_analyze_verify "systemd/user/git-maintenance@hourly.service" && + test_systemd_analyze_verify "systemd/user/git-maintenance@daily.service" && + test_systemd_analyze_verify "systemd/user/git-maintenance@weekly.service" && printf -- "--user enable --now git-maintenance@%s.timer\n" hourly daily weekly >expect && test_cmp expect args && @@ -755,8 +757,11 @@ test_expect_success 'start and stop Linux/systemd maintenance' ' # stop does not unregister the repo git config --get --global --fixed-value maintenance.repo "$(pwd)" && - test_path_is_missing "systemd/user/git-maintenance@.timer" && - test_path_is_missing "systemd/user/git-maintenance@.service" && + for schedule in hourly daily weekly + do + test_path_is_missing "systemd/user/git-maintenance@$schedule.timer" && + test_path_is_missing "systemd/user/git-maintenance@$schedule.service" || return 1 + done && printf -- "--user disable --now git-maintenance@%s.timer\n" hourly daily weekly >expect && test_cmp expect args diff --git a/wrapper.c b/wrapper.c index 67f5f5dbe199f8..41aa7d64c58357 100644 --- a/wrapper.c +++ b/wrapper.c @@ -835,3 +835,15 @@ int csprng_bytes(void *buf, size_t len) return 0; #endif } + +uint32_t git_rand(void) +{ + static int random_initialized = 0; + + if (!random_initialized) { + srand((unsigned int)getpid()); + random_initialized = 1; + } + + return rand(); +} \ No newline at end of file