Re: [PATCH 03/10] mingw: special-case `open(symlink, O_CREAT | O_EXCL)`
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Dec 1, 2025, 13:18 UTC
- Message-ID
- <4cf1c638-3b0b-a36a-ce35-41d55e9fae12@gmx.de>
- In-Reply-To
- <xmqqecpgc8wd.fsf@gitster.g>
Hi Junio,
On Sat, 29 Nov 2025, Junio C Hamano wrote:
Show 14 quoted lines
> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> > writes: > > > From: Johannes Schindelin <johannes.schindelin@gmx.de> > > > > The `_wopen()` function would gladly follow a symbolic link to a > > non-existent file and create it when given above-mentioned flags. > > > > Git expects the `open()` call to fail, though. So let's add yet another > > work-around to pretend that Windows behaves like Linux. > > "like Linux" -> "as POSIX expects"? > > cf. https://pubs.opengroup.org/onlinepubs/007904875/functions/open.html#:~:text=If%20O_CREAT%20and%20O_EXCL%20are,set%2C%20the%20result%20is%20undefined.
You are both correct and incorrect. The behavior I described indeed is not limited to Linux, insofar you are correct. The behavior I wanted to imitate is Linux', though, not POSIX.
I noticed that there was a recent shift, mostly by one particular contributor on this list, who pushes for POSIX compliance to be the gold standard Git lives by. However, that does not match my understanding of what the Git project agreed upon, as documented in https://gitlab.com/git-scm/git/-/blob/v2.52.0/Documentation/CodingGuidelines?ref_type=tags#L4-6 (and there was no attempt to change this).
As such, I still deem it more appropriate to keep the commit message as I wrote it, as it matches my intent.
You're of course free to edit the commit message to your liking, as you have done in the past. It just would not match my intent anymore.
Ciao, Johannes
Show 63 quoted lines
> > This is required to let t4115.8(--reject removes .rej symlink if it
> > exists) pass on Windows when enabling the MSYS2 runtime's symbolic link
> > support.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> > ---
> > compat/mingw.c | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/compat/mingw.c b/compat/mingw.c
> > index 736a07a028..9fbf12a3d3 100644
> > --- a/compat/mingw.c
> > +++ b/compat/mingw.c
> > @@ -627,6 +627,7 @@ int mingw_open (const char *filename, int oflags, ...)
> > int fd, create = (oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL);
> > wchar_t wfilename[MAX_PATH];
> > open_fn_t open_fn;
> > + WIN32_FILE_ATTRIBUTE_DATA fdata;
> >
> > DECLARE_PROC_ADDR(ntdll.dll, NTSTATUS, NTAPI, RtlGetLastNtStatus, void);
> >
> > @@ -651,6 +652,19 @@ int mingw_open (const char *filename, int oflags, ...)
> > else if (xutftowcs_path(wfilename, filename) < 0)
> > return -1;
> >
> > + /*
> > + * When `symlink` exists and is a symbolic link pointing to a
> > + * non-existing file, `_wopen(symlink, O_CREAT | O_EXCL)` would
> > + * create that file. Not what we want: Linux would say `EEXIST`
> > + * in that instance, which is therefore what Git expects.
> > + */
>
> "Linux" -> "open() on POSIX-compliant systems".
>
> IOW, _wopen() does not have to behave like POSIX open() and the compat/
> layer is how the emulation goes.
>
> FWIW, this is not limited to symbolic links but anything that exists
> at the path specified should cause the same EEXIST failure. The
> O_CREAT|O_EXCL combination asks the system to atomically create the
> thing anew (or fail).
>
> O_EXCL
> If O_CREAT and O_EXCL are set, open() shall fail if the file
> exists. The check for the existence of the file and the creation of
> the file if it does not exist shall be atomic with respect to other
> threads executing open() naming the same filename in the same
> directory with O_EXCL and O_CREAT set. If O_EXCL and O_CREAT are
> set, and path names a symbolic link, open() shall fail and set errno
> to [EEXIST], regardless of the contents of the symbolic link. If
> O_EXCL is set and O_CREAT is not set, the result is undefined.
>
> > + if (create &&
> > + GetFileAttributesExW(wfilename, GetFileExInfoStandard, &fdata) &&
> > + (fdata.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) {
> > + errno = EEXIST;
> > + return -1;
> > + }
> > +
> > fd = open_fn(wfilename, oflags, mode);
> >
> > /*
>