Re: [PATCH v4] gc: save log from daemonized gc --auto and print it next time

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 16, 2015, 16:00 UTC
Message-ID: <xmqqmvwms6b1.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <55F93646.9050709@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> I'm not sure what behavior you want. At one point you say "puts the file
> to a final place if it is non-empty" but later you say "leave it if
> non-empty". Should the file be written directly, or should it be written
> to a lockfile and renamed into place only when complete?

I do not think we care that deeply either way, as we do not want to run multiple auto-gc's at the same time in the first place. So either one of the following is perfectly fine:

 * We open the final destination directly, but with O_CREAT|O_EXCL,
   which as a side effect also detects the simultanous execution
   [*1*].  We do not do any "finalizing rename" if we go this route.
   When we are done, close it and remove it if we did not write
   anything, or leave it there if we did write something.

 * We open a lockfile the usual way.  When we are done, close it and
   and remove it if we did not write anything, or finalize it by
   renaming it if we did write something.

I think Duy's code wants to do the latter.

Show 8 quoted lines

> This doesn't seem like a common thing to want (as in, this might be the
> only caller), but it probably makes sense to build it into the
> tempfile/lockfile API nevertheless, because implementing it externally
> would require a lot of other code to be duplicated.
>
> Another possibility that might work (maybe without requiring changes to
> tempfile/lockfile): don't worry about deleting the log file if it is
> empty, but make observers treat an empty log file the same as an absent one.

Probably your "don't remove and check for emptiness" approach would be the simpler of the two, but I think we can go either way.

Thanks.

[Footnote]

*1* But one "gc" could be foreground gc that does not write to a log
    file and the other "gc" could be background that does write to a
    log file, so this cannot be the primary way to avoid double
    execution.