Allow to use CreateFile with FILE_SHARE_DELETE for O_RDONLY

[csware]

This prevents FILE_SHARED_VIOLATIONS when used in tools such as TortoiseGit TGitCache, because the file is not locked any more.

Without this commit there could be errors while operating in explorer when a file is checked (e.g. using git_repository_hashfile) in background.

[ethomson]

I'm sympathetic to wanting FILE_SHARE_DELETE, but I'm a bit concerned about the potential implications. Do we really want to do this always or is it something that people should opt into?

What files are you seeing problems here? Working directory files or files in the git repository or both?

Anyway. I'm very open to this, but I think that a branch that does CreateFile and a branch that does _wopen is a bummer. Let's move all opens to do a CreateFile that emulates _wopen (_wopen is, IIRC, just a wrapper around CreateFile) and change the flags as we see fit. Does that make sense?

[csware]

I was experiencing the issue mainly with working tree files (when calling git_repository_hashfile) - at first I added the CreateFile handling there but thought "Why not do it for every read-only open case".

I focussed on the read-only part because I saw no issue with mixing _wopen and CreateFile as _wopen is just a wrapper.

I can prepare a new patch which replaces all _wopen calls with CreateFile.

[ethomson]

I was experiencing the issue mainly with working tree files (when calling git_repository_hashfile) - at first I added the CreateFile handling there but thought "Why not do it for every read-only open case".

That was my guess - I think I would still like to see an option that you can set to enable it instead of making this the new default; it seems like it could be a very breaking change for people who expect the locking.

I focussed on the read-only part because I saw no issue with mixing _wopen and CreateFile as _wopen is just a wrapper.

I can prepare a new patch which replaces all _wopen calls with CreateFile.

I don't think there's a technical problem in intermixing them, I would just like to reduce the cyclomatic complexity.

Thanks for tackling this!

[csware]

How would you tackle the "opt-in"?

[ethomson]

I think that I would like to see git_libgit2_opts expanded to learn how to set this...

There's a couple of options here. This could be as direct as:

git_libgit2_opts(GIT_OPT_SET_WINDOWS_SHAREMODE, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE)

And then passing those given flags directly to CreateFile...

Is that satisfactory for TortoiseGit?

[csware]

Is that satisfactory for TortoiseGit?

I see no issues so far.

[csware]

The setting is now applied for all files. This is ok for me, but my initial attempt was to implement this only for files which are read only.

[csware]

@ethomson Updated.

[ethomson]

This was merged in #4192 - thanks again, and sorry for the delay in getting this integrated!