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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jun 15, 2021, 14:35 UTC
Message-ID: <87fsxjyz01.fsf@evledraar.gmail.com>
In-Reply-To: <eb5e2df7-25d8-a878-78ba-cca771265978@gmail.com>

On Tue, Jun 15 2021, Derrick Stolee wrote:

Show 13 quoted lines

> 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.