Support symbolic links on Windows

[dscho]

This finally upstreams Git for Windows' support for Windows' branch of symbolic links, which has been maturing since 2015. It is based off of js/prep-symlink-windows.

cc: Ben Knoble ben.knoble@gmail.com
cc: Johannes Sixt j6t@kdbg.org
cc: Karsten Blees karsten.blees@gmail.com

[gitgitgadget]

There are issues in commit ca10b27:
mingw: Allow mingw_chdir() to change to symlink-resolved directories
Prefixed commit message must be in lower case

[dscho]

/submit

[gitgitgadget]

Submitted as pull.2018.git.1765980535.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2018/dscho/symlinks-next-v1

To fetch this version to local tag pr-2018/dscho/symlinks-next-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2018/dscho/symlinks-next-v1

[gitgitgadget]

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

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> This finally upstreams Git for Windows' support for Windows' branch of
> symbolic links, which has been maturing since 2015. It is based off of
> js/prep-symlink-windows.

The three topics taken together touch 19 paths in total, about a
half of which are t/ test files.

I've read the changes to generic parts (i.e., outside compat/) and
saw nothing questionable.  Very nicely done.

Thanks.

 apply.c                             |   2 +-
 compat/mingw-posix.h                |   6 +-
 compat/mingw.c                      | 667 +++++++++++++++++++++++++++---------
 compat/win32.h                      |   6 +-
 compat/win32/dirent.c               |   5 +-
 environment.c                       |   4 +-
 environment.h                       |   2 +
 lockfile.c                          |   4 +-
 read-cache.c                        |  11 +
 setup.c                             |   2 +-
 strbuf.c                            |  10 +-
 t/t0001-init.sh                     |   6 +-
 t/t0301-credential-cache.sh         |   3 +-
 t/t0600-reffiles-backend.sh         |   2 +-
 t/t1006-cat-file.sh                 |  24 +-
 t/t1305-config-include.sh           |   4 +-
 t/t6423-merge-rename-directories.sh |   9 +-
 t/t7800-difftool.sh                 |   8 +-
 t/t9700/test.pl                     |   9 +-
 19 files changed, 585 insertions(+), 199 deletions(-)

[gitgitgadget]

On the Git mailing list, Ben Knoble wrote (reply to this):

> Le 17 déc. 2025 à 09:17, Karsten Blees via GitGitGadget <gitgitgadget@gmail.com> a écrit :
> 
> From: Karsten Blees <blees@dcon.de>
> 
> The `_wunlink()` and `DeleteFileW()` functions refuse to delete symlinks
> to directories on Windows; The error code woutl be `ERROR_ACCESS_DENIED`

Casually reading; spotted “woutl.” Presumably would?

[gitgitgadget]

User Ben Knoble <ben.knoble@gmail.com> has been added to the cc: list.

[gitgitgadget]

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

Am 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
> From: Karsten Blees <blees@dcon.de>
> 
> The Win32 API function `GetFileAttributes()` cannot handle paths with
> trailing dir separators. The current `mingw_stat()`/`mingw_lstat()`
> implementation calls `GetFileAttributes()` twice if the path has
> trailing slashes (first with the original path that was passed as
> function parameter, and and a second time with a path copy with trailing
> '/' removed).
A comment above do_lstat() mentions this procedure. This patch doesn't
change the comment, but it should.

-- Hannes

[gitgitgadget]

User Johannes Sixt <j6t@kdbg.org> has been added to the cc: list.

[gitgitgadget]

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

Am 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
> From: Karsten Blees <blees@dcon.de>
> 
> With respect to symlinks, the current `mingw_stat()` implementation is
> almost identical to `mingw_lstat()`: except for the file type (`st_mode
> & S_IFMT`), it returns information about the link rather than the target.
> 
> Implement `mingw_stat()` by opening the file handle requesting minimal
> permissions, and then calling `GetFileInformationByHandle()` on it. This
> way, all links are resolved by the Windows file system layer.
> 
> If symlinks are disabled, use `mingw_lstat()` as before, but fail with
> `ELOOP` if a symlink would have to be resolved.

This last paragraph is disconnected from the patch text. I can't find a
use of ELOOP anywhere in the code that has something to do with the goal
of this patch. Is this a remnant from early times where symbolic links
were optional?

The patch text looks good.

> 
> Signed-off-by: Karsten Blees <blees@dcon.de>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/mingw.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index f5a0fe3325..59afd69686 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1026,9 +1026,26 @@ int mingw_lstat(const char *file_name, struct stat *buf)
>  {
>  	return do_lstat(0, file_name, buf);
>  }
> +
>  int mingw_stat(const char *file_name, struct stat *buf)
>  {
> -	return do_lstat(1, file_name, buf);
> +	wchar_t wfile_name[MAX_PATH];
> +	HANDLE hnd;
> +	int result;
> +
> +	/* open the file and let Windows resolve the links */
> +	if (xutftowcs_path(wfile_name, file_name) < 0)
> +		return -1;
> +	hnd = CreateFileW(wfile_name, 0,
> +			FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
> +			OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
> +	if (hnd == INVALID_HANDLE_VALUE) {
> +		errno = err_win_to_posix(GetLastError());
> +		return -1;
> +	}
> +	result = get_file_info_by_handle(hnd, buf);
> +	CloseHandle(hnd);
> +	return result;
>  }
>  
>  int mingw_fstat(int fd, struct stat *buf)

-- Hannes

[gitgitgadget]

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

Am 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
> From: Karsten Blees <blees@dcon.de>
> 
> With the new `mingw_stat()` implementation, `do_lstat()` is only called
> from `mingw_lstat()` (with the function parameter `follow == 0`). Remove
> the extra function and the old `mingw_stat()`-specific (`follow == 1`)
> logic.
> 
> Signed-off-by: Karsten Blees <blees@dcon.de>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/mingw.c | 22 ++--------------------
>  1 file changed, 2 insertions(+), 20 deletions(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 59afd69686..ec6c2801d3 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -917,14 +917,7 @@ static int has_valid_directory_prefix(wchar_t *wfilename)
>  	return 1;
>  }
>  
> -/* We keep the do_lstat code in a separate function to avoid recursion.
> - * When a path ends with a slash, the stat will fail with ENOENT. In
> - * this case, we strip the trailing slashes and stat again.
> - *
> - * If follow is true then act like stat() and report on the link
> - * target. Otherwise report on the link itself.
> - */
> -static int do_lstat(int follow, const char *file_name, struct stat *buf)
> +int mingw_lstat(const char *file_name, struct stat *buf)

Oh, here goes the entire function including the comment. Fine, then.
Disregard my comment on 01/18.

>  {
>  	WIN32_FILE_ATTRIBUTE_DATA fdata;
>  	wchar_t wfilename[MAX_PATH];
> @@ -958,13 +951,7 @@ static int do_lstat(int follow, const char *file_name, struct stat *buf)
>  			if (handle != INVALID_HANDLE_VALUE) {
>  				if ((findbuf.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) &&
>  						(findbuf.dwReserved0 == IO_REPARSE_TAG_SYMLINK)) {
> -					if (follow) {
> -						char buffer[MAXIMUM_REPARSE_DATA_BUFFER_SIZE];
> -						buf->st_size = readlink(file_name, buffer, MAXIMUM_REPARSE_DATA_BUFFER_SIZE);
> -					} else {
> -						buf->st_mode = S_IFLNK;
> -					}
> -					buf->st_mode |= S_IREAD;
> +					buf->st_mode = S_IFLNK | S_IREAD;
>  					if (!(findbuf.dwFileAttributes & FILE_ATTRIBUTE_READONLY))
>  						buf->st_mode |= S_IWRITE;
>  				}
> @@ -1022,11 +1009,6 @@ static int get_file_info_by_handle(HANDLE hnd, struct stat *buf)
>  	return 0;
>  }
>  
> -int mingw_lstat(const char *file_name, struct stat *buf)
> -{
> -	return do_lstat(0, file_name, buf);
> -}
> -
>  int mingw_stat(const char *file_name, struct stat *buf)
>  {
>  	wchar_t wfile_name[MAX_PATH];

An obviously correct rewrite.

-- Hannes

[gitgitgadget]

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

Am 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
> From: Karsten Blees <blees@dcon.de>
> 
> Older MSVCRT's `_wrename()` function cannot rename symlinks over
> existing files: it returns success without doing anything. Newer
> MSVCR*.dll versions probably do not share this problem: according to CRT
> sources, they just call `MoveFileEx()` with the `MOVEFILE_COPY_ALLOWED`
> flag.
> 
> Avoid the `_wrename()` call, and go with directly calling
> `MoveFileEx()`, with proper error handling of course.
> 
> Signed-off-by: Karsten Blees <blees@dcon.de>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/mingw.c | 38 ++++++++++++++++----------------------
>  1 file changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/compat/mingw.c b/compat/mingw.c
> index b1cc30d0f1..55f0bb478e 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -2275,7 +2275,7 @@ int mingw_accept(int sockfd1, struct sockaddr *sa, socklen_t *sz)
>  int mingw_rename(const char *pold, const char *pnew)
>  {
>  	static int supports_file_rename_info_ex = 1;
> -	DWORD attrs, gle;
> +	DWORD attrs = INVALID_FILE_ATTRIBUTES, gle;
>  	int tries = 0;
>  	wchar_t wpold[MAX_PATH], wpnew[MAX_PATH];
>  	int wpnew_len;
> @@ -2286,15 +2286,6 @@ int mingw_rename(const char *pold, const char *pnew)
>  	if (wpnew_len < 0)
>  		return -1;
>  
> -	/*
> -	 * Try native rename() first to get errno right.
> -	 * It is based on MoveFile(), which cannot overwrite existing files.
> -	 */
> -	if (!_wrename(wpold, wpnew))
> -		return 0;
> -	if (errno != EEXIST)
> -		return -1;
> -
>  repeat:
>  	if (supports_file_rename_info_ex) {
>  		/*
> @@ -2370,13 +2361,22 @@ repeat:
>  		 * to retry.
>  		 */
>  	} else {
> -		if (MoveFileExW(wpold, wpnew, MOVEFILE_REPLACE_EXISTING))
> +		if (MoveFileExW(wpold, wpnew,
> +				MOVEFILE_REPLACE_EXISTING | MOVEFILE_COPY_ALLOWED))
>  			return 0;
>  		gle = GetLastError();
>  	}
>  
> -	/* TODO: translate more errors */
> -	if (gle == ERROR_ACCESS_DENIED &&
> +	/* revert file attributes on failure */
> +	if (attrs != INVALID_FILE_ATTRIBUTES)
> +		SetFileAttributesW(wpnew, attrs);
> +
> +	if (!is_file_in_use_error(gle)) {
> +		errno = err_win_to_posix(gle);
> +		return -1;
> +	}
> +
> +	if (attrs == INVALID_FILE_ATTRIBUTES &&
>  	    (attrs = GetFileAttributesW(wpnew)) != INVALID_FILE_ATTRIBUTES) {
>  		if (attrs & FILE_ATTRIBUTE_DIRECTORY) {
>  			DWORD attrsold = GetFileAttributesW(wpold);
> @@ -2388,16 +2388,10 @@ repeat:
>  			return -1;
>  		}
>  		if ((attrs & FILE_ATTRIBUTE_READONLY) &&
> -		    SetFileAttributesW(wpnew, attrs & ~FILE_ATTRIBUTE_READONLY)) {
> -			if (MoveFileExW(wpold, wpnew, MOVEFILE_REPLACE_EXISTING))
> -				return 0;
> -			gle = GetLastError();
> -			/* revert file attributes on failure */
> -			SetFileAttributesW(wpnew, attrs);
> -		}
> +		    SetFileAttributesW(wpnew, attrs & ~FILE_ATTRIBUTE_READONLY))
> +			goto repeat;
>  	}
> -	if (gle == ERROR_ACCESS_DENIED &&
> -	       retry_ask_yes_no(&tries, "Rename from '%s' to '%s' failed. "
> +	if (retry_ask_yes_no(&tries, "Rename from '%s' to '%s' failed. "
>  		       "Should I try again?", pold, pnew))
>  		goto repeat;
>  

The logic in this function is incredibly convoluted. It does look
somewhat reasonable, at least on the non-error path, but whether the
variable attr is changed and reset as needed after 'goto repeat' and the
various failure modes, I cannot tell. I give up and trust that this code
has been battle-tested during the past decade and works as desired.

-- Hannes

[gitgitgadget]

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

Am 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
> From: Karsten Blees <blees@dcon.de>
> 
> Implement `readlink()` by reading NTFS reparse points via the
> `read_reparse_point()` function that was introduced earlier to determine
> the length of symlink targets. Works for symlinks and directory
> junctions. If symlinks are disabled, fail with `ENOSYS`.

This last sentence is obsolete, I think, because I cannot see how the
patch achieves a failure with ENOSYS.

> 
> Signed-off-by: Karsten Blees <blees@dcon.de>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  compat/mingw-posix.h |  3 +--
>  compat/mingw.c       | 24 ++++++++++++++++++++++++
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/compat/mingw-posix.h b/compat/mingw-posix.h
> index 0939feff27..896aa976b1 100644
> --- a/compat/mingw-posix.h
> +++ b/compat/mingw-posix.h
> @@ -121,8 +121,6 @@ struct utsname {
>   * trivial stubs
>   */
>  
> -static inline int readlink(const char *path UNUSED, char *buf UNUSED, size_t bufsiz UNUSED)
> -{ errno = ENOSYS; return -1; }
>  static inline int symlink(const char *oldpath UNUSED, const char *newpath UNUSED)
>  { errno = ENOSYS; return -1; }
>  static inline int fchmod(int fildes UNUSED, mode_t mode UNUSED)
> @@ -197,6 +195,7 @@ int setitimer(int type, struct itimerval *in, struct itimerval *out);
>  int sigaction(int sig, struct sigaction *in, struct sigaction *out);
>  int link(const char *oldpath, const char *newpath);
>  int uname(struct utsname *buf);
> +int readlink(const char *path, char *buf, size_t bufsiz);
>  
>  /*
>   * replacements of existing functions
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 5d2a8c247c..b407a2ac07 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -2698,6 +2698,30 @@ int link(const char *oldpath, const char *newpath)
>  	return 0;
>  }
>  
> +int readlink(const char *path, char *buf, size_t bufsiz)
> +{
> +	WCHAR wpath[MAX_PATH];
> +	char tmpbuf[MAX_PATH];
> +	int len;
> +	DWORD tag;
> +
> +	if (xutftowcs_path(wpath, path) < 0)
> +		return -1;
> +
> +	if (read_reparse_point(wpath, TRUE, tmpbuf, &len, &tag) < 0)
> +		return -1;
> +
> +	/*
> +	 * Adapt to strange readlink() API: Copy up to bufsiz *bytes*, potentially
> +	 * cutting off a UTF-8 sequence. Insufficient bufsize is *not* a failure
> +	 * condition. There is no conversion function that produces invalid UTF-8,
> +	 * so convert to a (hopefully large enough) temporary buffer, then memcpy
> +	 * the requested number of bytes (including '\0' for robustness).
> +	 */
> +	memcpy(buf, tmpbuf, min(bufsiz, len + 1));
> +	return min(bufsiz, len);
> +}
> +
>  pid_t waitpid(pid_t pid, int *status, int options)
>  {
>  	HANDLE h = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,

-- Hannes

[gitgitgadget]

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

Am 17.12.25 um 15:08 schrieb Johannes Schindelin via GitGitGadget:
> This finally upstreams Git for Windows' support for Windows' branch of
> symbolic links, which has been maturing since 2015. It is based off of
> js/prep-symlink-windows.
> 
> Bill Zissimopoulos (1):
>   mingw: compute the correct size for symlinks in `mingw_lstat()`
> 
> Johannes Schindelin (3):
>   mingw: try to create symlinks without elevated permissions
>   mingw: emulate `stat()` a little more faithfully
>   mingw: special-case index entries for symlinks with buggy size
> 
> Karsten Blees (14):
>   mingw: don't call `GetFileAttributes()` twice in `mingw_lstat()`
>   mingw: implement `stat()` with symlink support
>   mingw: drop the separate `do_lstat()` function
>   mingw: let `mingw_lstat()` error early upon problems with reparse
>     points
>   mingw: teach dirent about symlinks
>   mingw: factor out the retry logic
>   mingw: change default of `core.symlinks` to false
>   mingw: add symlink-specific error codes
>   mingw: handle symlinks to directories in `mingw_unlink()`
>   mingw: support renaming symlinks
>   mingw: allow `mingw_chdir()` to change to symlink-resolved directories
>   mingw: implement `readlink()`
>   mingw: implement basic `symlink()` functionality (file symlinks only)
>   mingw: add support for symlinks to directories
> 
>  compat/mingw-posix.h  |   6 +-
>  compat/mingw.c        | 635 ++++++++++++++++++++++++++++++++----------
>  compat/win32.h        |   6 +-
>  compat/win32/dirent.c |   5 +-
>  read-cache.c          |  11 +
>  5 files changed, 507 insertions(+), 156 deletions(-)
> 
> 
> base-commit: 6f6fe02f5fe587ec9788f8a5a34281949d7b2ca1
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2018%2Fdscho%2Fsymlinks-next-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2018/dscho/symlinks-next-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2018

I've reviewed this series and had a few comments on some of them.

All others look good, with one caveat though: symbolic links on Windows
aren't exactly an itch of mine, and I'm unfamiliar with the
corresponding API. That said, I didn't spot anything unusual at a
superficial level.

I notice that Karsten's emails bounce. Would it be appropriate to
redirect authorship and sign-off to the other email that is registered
in .mailmap?

-- Hannes

[gitgitgadget]

On the Git mailing list, Karsten Blees wrote (reply to this):

Am 18.12.2025 um 19:51 schrieb Johannes Sixt:
> I notice that Karsten's emails bounce. Would it be appropriate to
> redirect authorship and sign-off to the other email that is registered
> in .mailmap?
>
> -- Hannes

Hi,

indeed, the @dcon.de address that I used to sign my patches no longer works, as I'm no longer working for that company. Feel free to change to my current address.

Cheers,

Karsten

[gitgitgadget]

User Karsten Blees <karsten.blees@gmail.com> has been added to the cc: list.