Re: [PATCH v4 4/4] maintenance: use Windows scheduled tasks

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Nov 17, 2020 at 4:13 PM Derrick Stolee via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> [...]
> Since the GIT_TEST_MAINT_SCHEDULER environment variable allows us to
> specify 'schtasks' as the scheduler, we can test the Windows-specific
> logic on a macOS platform. Thus, add a check that the XML file written
> by Git is valid when xmllint exists on the system.

Nit: xmllint can be installed on Linux (and likely other platforms),
as well, so it's not clear why this calls out macOS specially. More
generally, it may not be important to call out xmllint at all in the
commit message; it's just _one_ thing being checked by a test which is
checking several other things not called out individually by the
commit message. Anyhow, this is minor; not worth a re-roll.

> diff --git a/builtin/gc.c b/builtin/gc.c
> @@ -1671,6 +1671,167 @@ static int launchctl_update_schedule(int run_maintenance, int fd, const char *cm
> +static int schtasks_schedule_task(const char *exec_path, enum schedule_priority schedule, const char *cmd)
> +{
> +       xmlpath =  xstrfmt("%s/schedule-%s.xml",
> +                          the_repository->objects->odb->path,
> +                          frequency);

I missed this in the earlier rounds since I wasn't paying close enough
attention, but placing this XML file within the object database
directory (.git/objects/) feels rather odd, even if it is just a
temporary file. Using the .git/ directory itself might be better,
perhaps like this:

    struct strbuf xmlpath = STRBUF_INIT;
    strbuf_git_common_path(&xmlpath, the_repository,
        "schtasks-%s.xml", frequency);
    ...
    strbuf_release(&xmlpath);