Subject: Re: Making split commit graphs pick up new options (namely --changed-paths)

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 15 Jun 2021 16:35:47 +0200

On Tue, Jun 15 2021, Derrick Stolee wrote:

> On 6/11/2021 3:01 PM, Taylor Blau wrote:
>> On Fri, Jun 11, 2021 at 01:47:28PM -0400, Derrick Stolee wrote:
>> (FWIW, I had to re-read 8d84097f96 (commit-graph: expire commit-graph
>> files, 2019-06-18) which mentions that a configuration variable would be
>> introduced to change the expiration window, but we don't have any such
>> configuration option. It also doesn't make any mention of handling this
>> problem on Windows, which made me think that the unlink() calls weren't
>> checking their return values by accident when in fact it was probably on
>> purpose.)
>
> That config option never appeared, probably because ignoring the
> unlink() return was sufficient to get around this problem. Thanks
> for digging in and making sure I remembered this correctly.

Isn't the whole ignoring the return value of unlink() Windows-specific
code? There's no issue with unlinking a file someone else has open on
POSIX systems, indeed unlinking a file you just created (but hold a FD
to) is a common pattern for getting a temporary file that you don't need
to unlink on atexit(). It's just not used in e.g. Git's codebase because
of portability concerns.

So not a big deal at all, but I wonder if there should be a warning
there on !Windows, if you can't unlink a file on a POSIX system that
suggests e.g. a persistent permission problem that won't be going away
if you ignore it.