On Tue, Nov 17, 2020 at 4:13 PM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> 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);