Re: [PATCH] checkout files in-place

[Clemens Buchacher <drizzd@gmx.net>]

+Cc: Orgad Shaneh

On Mon, Jun 11, 2018 at 08:35:41PM +0000, Edward Thomson wrote:
> On Sun, Jun 10, 2018 at 09:44:45PM +0200, Clemens Buchacher wrote:
> > 
> > It is safe to do this on Linux file systems, even if open file handles
> > still exist, because unlink only removes the directory reference to the
> > file. On Windows, however, a file cannot be deleted until all handles to
> > it are closed. If a file cannot be deleted, its name cannot be reused.
> 
> I'm nervous about this proposed change, since it feels like it's
> addressing an issue that only exists in QT Creator.
> 
> You've accurately described the default semantics in Win32.  A file
> cannot be deleted until all handles to it are closed, unless it was
> opened with `FILE_SHARE_DELETE` as their sharing mode.  This is not the
> default sharing mode in either Win32 or .NET.
> 
> However, for your patch to have an effect, all processes with a handle
> open must have specified `FILE_SHARE_WRITE`.  This is rather uncommon,
> since it's also not included in the default Win32 or .NET sharing mode.
> This is because it's uncommon that you would want other processes to
> change the data underneath you in between ReadFile() calls.
> 
> So your patch will benefit people who have processes that have
> `FILE_SHARE_WRITE` set but not `FILE_SHARE_DELETE` set, which I think is
> generally an uncommon scenario to want to support.
> 
> Generally if you're willing to accept files changing underneath you,
> then you probably want to allow them to be deleted, too.  So this feels
> like something that's very specific to QT Creator.  Or are there other
> IDEs or development tools that use these open semantics that I'm not
> aware of?

I am also not aware of other IDEs which have this issue.

Orgad, you also mentioned FILE_SHARE_DELETE here [*1*]. Does the Qt
Creator issue persist despite this flag? You also just commented on
Github that "Regarding Qt Creator, the issue should be mostly solved by
now in 4.7". So a fix in Git is no longer needed?

[*1*] https://github.com/git-for-windows/git/pull/1666