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

> On Wed, Nov 18, 2020 at 1:30 PM Derrick Stolee <stolee@gmail.com> wrote:
>> On 11/18/2020 2:15 AM, Eric Sunshine wrote:
>>> On Tue, Nov 17, 2020 at 4:13 PM Derrick Stolee via GitGitGadget
>>> <gitgitgadget@gmail.com> wrote:
>>>> +       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:
>>
>> It does look odd, and in this case we could use the .git directory
>> instead. I specifically use the objects directory for the maintenance
>> lock in 'git maintenance run' to allow maintenance to run when
>> GIT_OBJECT_DIRECTORY points to an alternate, allowing us to maintain
>> object databases that don't have a full .git directory around them.
> 
> I guess I'm confused. Won't a Git "common" directory exist even for
> such a case when GIT_OBJECT_DIRECTORY is pointing elsewhere, whether
> the "common" directory is .git/ or a bare repository, or whatnot?

> Anyhow, this brings us back to my original suggestion of creating
> these temporary files in a genuine temporary directory (/tmp or
> $TMPDIR or $TEMP) instead of arbitrarily choosing a path within the
> repository itself. An important reason for using a genuine temporary
> directory for these temporary XML files is that it makes it less
> confusing for those who come along later and try to understand this
> code; they won't have to puzzle out why it is using a repository
> location for a file which is clearly temporary.
> 
> To make this really simple, you could use one of the
> x?mks_tempfile_t*() functions from tempfile.h which will automatically
> place the file in $TMPDIR, thus relieving this code from having to
> make the choice. Doing so would simplify this code even further since
> you would replace create_tempfile() with x?mks_tempfile_t*(), and
> wouldn't have to maintain (or free) `xmlpath` manually.
> 
> As for the test script, the `print-args` is already picking up the
> pathname of the temporary file specified by the /xml option, so it
> should be possible to make the rest of the test work with the
> generated temporary filename.