Skip to content

Prepare Git's test suite for symbolic link support on Windows #2009

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Prepare Git's test suite for symbolic link support on Windows #2009

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2d32983
t9700: accommodate for Windows paths
dscho Nov 17, 2025
b97afa9
apply: symbolic links lack a "trustable executable bit"
dscho Jun 18, 2025
96e279f
mingw: special-case `open(symlink, O_CREAT | O_EXCL)`
dscho Jun 18, 2025
9639e04
t0001: handle `diff --no-index` gracefully
dscho Nov 17, 2025
3db0599
t0301: another fix for Windows compatibility
dscho Nov 17, 2025
b8a357f
t0600: fix incomplete prerequisite for a test case
dscho Nov 17, 2025
b6dd95a
t1006: accommodate for symlink support in MSYS2
dscho Nov 17, 2025
95efb6b
t1305: skip symlink tests that do not apply to Windows
dscho Jun 18, 2025
aed0eb1
t6423: introduce Windows-specific handling for symlinking to /dev/null
dscho Nov 17, 2025
e04cf02
t7800: work around the MSYS path conversion on Windows
dscho Nov 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apply.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3779,7 +3779,7 @@ static int check_preimage(struct apply_state *state,
if (*ce && !(*ce)->ce_mode)
BUG("ce_mode == 0 for path '%s'", old_name);

if (trust_executable_bit)
if (trust_executable_bit || !S_ISREG(st->st_mode))
st_mode = ce_mode_from_stat(*ce, st->st_mode);
else if (*ce)
st_mode = (*ce)->ce_mode;
Expand Down
14 changes: 14 additions & 0 deletions compat/mingw.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,7 @@ int mingw_open (const char *filename, int oflags, ...)
int fd, create = (oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL);
Copy link

@gitgitgadget gitgitgadget bot Nov 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

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

> 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);
>  
>  	/*

Copy link

@gitgitgadget gitgitgadget bot Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Junio,

On Sat, 29 Nov 2025, Junio C Hamano wrote:

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

> > 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);
> >  
> >  	/*
>

wchar_t wfilename[MAX_PATH];
open_fn_t open_fn;
WIN32_FILE_ATTRIBUTE_DATA fdata;

DECLARE_PROC_ADDR(ntdll.dll, NTSTATUS, NTAPI, RtlGetLastNtStatus, void);

Expand All @@ -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.
*/
if (create &&
GetFileAttributesExW(wfilename, GetFileExInfoStandard, &fdata) &&
(fdata.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) {
errno = EEXIST;
return -1;
}

fd = open_fn(wfilename, oflags, mode);

/*
Expand Down
9 changes: 7 additions & 2 deletions t/t9700/test.pl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,12 @@ sub adjust_dirsep {
unlink $tmpfile;

# paths
is($r->repo_path, $abs_repo_dir . "/.git", "repo_path");
my $abs_git_dir = $abs_repo_dir . "/.git";
if ($^O eq 'msys' or $^O eq 'cygwin') {
$abs_git_dir = `cygpath -am "$abs_repo_dir/.git"`;
$abs_git_dir =~ s/\r?\n?$//;
}
is($r->repo_path, $abs_git_dir, "repo_path");
is($r->wc_path, $abs_repo_dir . "/", "wc_path");
is($r->wc_subdir, "", "wc_subdir initial");
$r->wc_chdir("directory1");
Expand All @@ -127,7 +132,7 @@ sub adjust_dirsep {
# Object generation in sub directory
chdir("directory2");
my $r2 = Git->repository();
is($r2->repo_path, $abs_repo_dir . "/.git", "repo_path (2)");
is($r2->repo_path, $abs_git_dir, "repo_path (2)");
is($r2->wc_path, $abs_repo_dir . "/", "wc_path (2)");
is($r2->wc_subdir, "directory2/", "wc_subdir initial (2)");

Expand Down