Re: [PATCH v4 4/4] maintenance: use Windows scheduled tasks
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Nov 18, 2020, 20:54 UTC
- Message-ID
- <CAPig+cQt+RFpikr-8cXfLtsXapS4tK4gufKZa-U1yN-ynJbiow@mail.gmail.com>
- In-Reply-To
- <e8922076-1ed4-b540-f1c9-9b4a8fa7a085@gmail.com>
On Wed, Nov 18, 2020 at 1:30 PM Derrick Stolee <stolee@gmail.com> wrote:
Show 18 quoted lines
> 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.