On Wed, Nov 4, 2020 at 3:06 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
> Git's background maintenance uses cron by default, but this is not
> available on Windows. Instead, integrate with Task Scheduler.
> [...]
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
> diff --git a/builtin/gc.c b/builtin/gc.c
> @@ -1698,6 +1698,187 @@ static int platform_update_schedule(int run_maintenance, int fd)
> +static int schedule_task(const char *exec_path, enum schedule_priority schedule)
> +{
> + xmlpath = xstrfmt("%s/schedule-%s.xml",
> + the_repository->objects->odb->path,
> + frequency);
Am I reading correctly that it is writing this throwaway XML file into
the Git object directory? Would writing to a temporary directory make
more sense? (Not worth a re-roll.)
> + xmlfp = fopen(xmlpath, "w");
> + if (!xmlfp)
> + die(_("failed to open '%s'"), xmlpath);
Could use xfopen() as mentioned previously.
> diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
> @@ -442,6 +442,36 @@ test_expect_success MACOS_MAINTENANCE 'start and stop macOS maintenance' '
> +test_expect_success MINGW 'start and stop Windows maintenance' '
> + for frequency in hourly daily weekly
> + do
> + printf "/create /tn Git Maintenance (%s) /f /xml .git/objects/schedule-%s.xml\n" \
> + $frequency $frequency
Nit: You lost the `|| return 1` which was present in the previous
version. True, it's very unlikely that `printf` could fail, but having
the `|| return 1` there makes it easier for the reader's eye to glide
over the code without having to worry about whether it is handling
error conditions correctly, thus reduces cognitive load.
> + done >expect &&
Rather than a loop, you could just use:
printf "/create ... (%s) /f /xml ...schedule-%s.xml\n" \
hourly hourly daily daily weekly weekly >expect &&
though it's subjective as to whether that is an improvement.
