Re: [PATCH] reftable: ignore file-in-use errors when unlink(3p) fails on Windows

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 27, 2025, 07:48 UTC
Message-ID: <Z5c6VlsdNXwYkUsv@pks.im>
In-Reply-To: <267314c0-3aba-4f69-9f41-89392391290f@kdbg.org>

On Sat, Jan 25, 2025 at 03:28:28PM +0100, Johannes Sixt wrote:

Show 15 quoted lines

> Am 25.01.25 um 09:32 schrieb Patrick Steinhardt:
> > The user report was explicitly about compatibility with JGit, which
> > still had these files open. We don't have control over third-party
> > clients and how exactly they open files, so it is expected that we may
> > still see failures with the deletion of in-use files.
> 
> Fair enough.
> 
> > I'd be happy to hear about alternative ideas that didn't came to my
> > mind.
> 
> Instead of calling _wunlink() in mingw_unlink, we could CreateFileW()
> with access mode DELETE and flag FILE_FLAG_DELETE_ON_CLOSE, then close
> the file right away. That would apply semantics that is similar, but not
> quite, POSIX at least among the files that we open ourselves.

Huh. And that works even when the file is still being held open by other processes? I don't know enough about Windows to be sure there and wouldn't quite feel comfortable with pushing a change like this into `unlink()` given that it is used in so many places by Git.

Show 5 quoted lines

> It would be even better that we do not depend on the POSIX behavior in
> the first place. As you said, the reftable library can live with failed
> deletes. And I don't think we depend on the POSIX behavior anywhere else
> because we would see the "try again?" question much more frequently than
> we do right now.

I have a feeling that there's a misunderstanding here, either on my side or on yours. It's the rest of Git that wants to have POSIX behaviour for `unlink()`, not the reftable library. In the reftable library we don't care at all whether or not the file got deleted -- we can live with it and know to retry at a later point. But because we use Git's `unlink()` implementation we get the "try again?" questions for free, even though we don't want to have it in the first place.

So the proposed fix is to _disable_ the POSIX emulation provided by Git in the reftable library. Which seems to be what you're proposing?

Let me know in case I misunderstood, I'm a bit confused right now :)

Patrick