Support symbolic links on Windows
cover Support symbolic links on Windows
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
Signature · 1 line
gitgitgadget
PATCH 1/18 mingw: don't call `GetFileAttributes()` twice in `mingw_lstat()`
From: Karsten Blees <[email protected]>
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).
With the conversion to wide Unicode, we get the length of the path for free, and also have a (wide char) buffer that can be modified. This makes it easy to avoid that extraneous Win32 API call.
--- compat/mingw.c | 48 ++++++++++++------------------------------------ 1 file changed, 12 insertions(+), 36 deletions(-)
compat/mingw.c +12 -36
@@ -928,9 +928,18 @@ static int do_lstat(int follow, const char *file_name, struct stat *buf) {WIN32_FILE_ATTRIBUTE_DATA fdata;wchar_t wfilename[MAX_PATH ]; -if (xutftowcs_path (wfilename, file_name) <0 ) +int wlen =xutftowcs_path (wfilename, file_name); +if (wlen <0 )return -1 ; +/* strip trailing '/', or GetFileAttributes will fail */ +while (wlen &&is_dir_sep (wfilename[wlen -1 ])) + wfilename[--wlen] =0 ; +if (!wlen) { + errno =ENOENT ; +return -1 ; + } +if (GetFileAttributesExW (wfilename, GetFileExInfoStandard, &fdata)) { buf->st_ino =0 ; buf->st_gid =0 ;
@@ -990,39 +999,6 @@ static int do_lstat(int follow, const char *file_name, struct stat *buf)return -1 ; } -/* We provide our own lstat/fstat functions, since the provided -* lstat/fstat functions are so slow. These stat functions are -* tailored for Git's usage (read: fast), and are not meant to be -* complete. Note that Git stat()s are redirected to mingw_lstat() -* too, since Windows doesn't really handle symlinks that well. -*/ -static int do_stat_internal (int follow,const char *file_name,struct stat *buf) -{ -size_t namelen; -char alt_name[PATH_MAX ]; - -if (!do_lstat (follow, file_name, buf)) -return 0 ; - -/* if file_name ended in a '/', Windows returned ENOENT; -* try again without trailing slashes -*/ -if (errno !=ENOENT ) -return -1 ; - - namelen =strlen (file_name); -if (namelen && file_name[namelen-1 ] !='/' ) -return -1 ; -while (namelen && file_name[namelen-1 ] =='/' ) - --namelen; -if (!namelen || namelen >=PATH_MAX ) -return -1 ; - -memcpy (alt_name, file_name, namelen); - alt_name[namelen] =0 ; -return do_lstat (follow, alt_name, buf); -} -static int get_file_info_by_handle (HANDLE hnd,struct stat *buf) {BY_HANDLE_FILE_INFORMATION fdata;
@@ -1048,11 +1024,11 @@ static int get_file_info_by_handle(HANDLE hnd, struct stat *buf)int mingw_lstat (const char *file_name,struct stat *buf) { -return do_stat_internal (0 , file_name, buf); +return do_lstat (0 , file_name, buf); }int mingw_stat (const char *file_name,struct stat *buf) { -return do_stat_internal (1 , file_name, buf); +return do_lstat (1 , file_name, buf); }int mingw_fstat (int fd,struct stat *buf)
Signature · 1 line
gitgitgadget
PATCH 2/18 mingw: implement `stat()` with symlink support
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
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.
--- compat/mingw.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
compat/mingw.c +18 -1
@@ -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)
Signature · 1 line
gitgitgadget
PATCH 3/18 mingw: drop the separate `do_lstat()` function
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
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.
--- compat/mingw.c | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-)
compat/mingw.c +2 -20
@@ -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) {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 ];
Signature · 1 line
gitgitgadget
PATCH 4/18 mingw: let `mingw_lstat()` error early upon problems with reparse points
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
When obtaining lstat information for reparse points, we need to call FindFirstFile() in addition to GetFileInformationEx() to obtain the type of the reparse point (symlink, mount point etc.). However, currently there is no error handling whatsoever if FindFirstFile() fails.
Call FindFirstFile() before modifying the stat *buf output parameter and error out if the call fails.
Note: The FindFirstFile() return value includes all the data that we get from GetFileAttributesEx(), so we could replace GetFileAttributesEx() with FindFirstFile(). We don't do that because GetFileAttributesEx() is about twice as fast for single files. I.e. we only pay the extra cost of calling FindFirstFile() in the rare case that we encounter a reparse point.
Please also note that the indentation the remaining reparse point code changed, and hence the best way to look at this diff is with --color-moved -w. That code was _not_ moved because a subsequent commit will move it to an altogether different function, anyway.
--- compat/mingw.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)
compat/mingw.c +14 -10
@@ -920,6 +920,7 @@ static int has_valid_directory_prefix(wchar_t *wfilename)int mingw_lstat (const char *file_name,struct stat *buf) {WIN32_FILE_ATTRIBUTE_DATA fdata; +WIN32_FIND_DATAW findbuf = {0 };wchar_t wfilename[MAX_PATH ];int wlen =xutftowcs_path (wfilename, file_name);if (wlen <0 )
@@ -934,6 +935,13 @@ int mingw_lstat(const char *file_name, struct stat *buf) }if (GetFileAttributesExW (wfilename, GetFileExInfoStandard, &fdata)) { +/* for reparse points, use FindFirstFile to get the reparse tag */ +if (fdata.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) { +HANDLE handle =FindFirstFileW (wfilename, &findbuf); +if (handle ==INVALID_HANDLE_VALUE ) + goto error; +FindClose (handle); + } buf->st_ino =0 ; buf->st_gid =0 ; buf->st_uid =0 ;
@@ -946,20 +954,16 @@ int mingw_lstat(const char *file_name, struct stat *buf)filetime_to_timespec (&(fdata.ftLastWriteTime), &(buf->st_mtim));filetime_to_timespec (&(fdata.ftCreationTime), &(buf->st_ctim));if (fdata.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) { -WIN32_FIND_DATAW findbuf; -HANDLE handle =FindFirstFileW (wfilename, &findbuf); -if (handle !=INVALID_HANDLE_VALUE ) { -if ((findbuf.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) && - (findbuf.dwReserved0 ==IO_REPARSE_TAG_SYMLINK )) { - buf->st_mode =S_IFLNK |S_IREAD ; -if (!(findbuf.dwFileAttributes &FILE_ATTRIBUTE_READONLY )) - buf->st_mode |=S_IWRITE ; - } -FindClose (handle); +if ((findbuf.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) && + (findbuf.dwReserved0 ==IO_REPARSE_TAG_SYMLINK )) { + buf->st_mode =S_IFLNK |S_IREAD ; +if (!(findbuf.dwFileAttributes &FILE_ATTRIBUTE_READONLY )) + buf->st_mode |=S_IWRITE ; } }return 0 ; } +error:switch (GetLastError ()) {case ERROR_ACCESS_DENIED :case ERROR_SHARING_VIOLATION :
Signature · 1 line
gitgitgadget
PATCH 5/18 mingw: teach dirent about symlinks
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
Move the S_IFLNK detection to file_attr_to_st_mode().
Implement DT_LNK detection in dirent.c's readdir() function.
--- compat/mingw.c | 13 +++---------- compat/win32.h | 6 ++++-- compat/win32/dirent.c | 5 ++++- 3 files changed, 11 insertions(+), 13 deletions(-)
compat/mingw.c +3 -10
@@ -946,21 +946,14 @@ int mingw_lstat(const char *file_name, struct stat *buf) buf->st_gid =0 ; buf->st_uid =0 ; buf->st_nlink =1 ; - buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes); + buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes, + findbuf.dwReserved0); buf->st_size = fdata.nFileSizeLow | (((off_t )fdata.nFileSizeHigh)<<32 ); buf->st_dev = buf->st_rdev =0 ;/* not used by Git */ filetime_to_timespec (&(fdata.ftLastAccessTime), &(buf->st_atim));filetime_to_timespec (&(fdata.ftLastWriteTime), &(buf->st_mtim));filetime_to_timespec (&(fdata.ftCreationTime), &(buf->st_ctim)); -if (fdata.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) { -if ((findbuf.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) && - (findbuf.dwReserved0 ==IO_REPARSE_TAG_SYMLINK )) { - buf->st_mode =S_IFLNK |S_IREAD ; -if (!(findbuf.dwFileAttributes &FILE_ATTRIBUTE_READONLY )) - buf->st_mode |=S_IWRITE ; - } - }return 0 ; } error:
@@ -1003,7 +996,7 @@ static int get_file_info_by_handle(HANDLE hnd, struct stat *buf) buf->st_gid =0 ; buf->st_uid =0 ; buf->st_nlink =1 ; - buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes); + buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes,0 ); buf->st_size = fdata.nFileSizeLow | (((off_t )fdata.nFileSizeHigh)<<32 ); buf->st_dev = buf->st_rdev =0 ;/* not used by Git */
compat/win32.h +4 -2
@@ -6,10 +6,12 @@#include <windows.h> #endif -static inline int file_attr_to_st_mode (DWORD attr) +static inline int file_attr_to_st_mode (DWORD attr,DWORD tag) {int fMode =S_IREAD ; -if (attr &FILE_ATTRIBUTE_DIRECTORY ) +if ((attr &FILE_ATTRIBUTE_REPARSE_POINT ) && tag ==IO_REPARSE_TAG_SYMLINK ) + fMode |=S_IFLNK ; +else if (attr &FILE_ATTRIBUTE_DIRECTORY ) fMode |=S_IFDIR ;else fMode |=S_IFREG ;
compat/win32/dirent.c +4 -1
@@ -12,7 +12,10 @@ static inline void finddata2dirent(struct dirent *ent, WIN32_FIND_DATAW *fdata)xwcstoutf (ent->d_name, fdata->cFileName,sizeof (ent->d_name));/* Set file type, based on WIN32_FIND_DATA */ -if (fdata->dwFileAttributes &FILE_ATTRIBUTE_DIRECTORY ) +if ((fdata->dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) + && fdata->dwReserved0 ==IO_REPARSE_TAG_SYMLINK ) + ent->d_type =DT_LNK ; +else if (fdata->dwFileAttributes &FILE_ATTRIBUTE_DIRECTORY ) ent->d_type =DT_DIR ;else ent->d_type =DT_REG ;
Signature · 1 line
gitgitgadget
PATCH 6/18 mingw: compute the correct size for symlinks in `mingw_lstat()`
In reply to Johannes Schindelin via GitGitGadgetFrom: Bill Zissimopoulos <[email protected]>
POSIX specifies that upon successful return from lstat(): "the value of the st_size member shall be set to the length of the pathname contained in the symbolic link not including any terminating null byte".
Git typically doesn't trust the stat.st_size member of symlinks (e.g. see strbuf_readlink()). Therefore, it is tempting to save on the extra overhead of opening and reading the reparse point merely to calculate the exact size of the link target.
This is, in fact, what Git for Windows did, from May 2015 to May 2020. At least almost: some functions take shortcuts if st_size is 0 (e.g. diff_populate_filespec()), hence Git for Windows hard-coded the length of all symlinks to MAX_PATH.
This did cause problems, though, specifically in Git repositories that were also accessed by Git for Cygwin or Git for WSL. For example, doing git reset --hard using Git for Windows would update the size of symlinks in the index to be MAX_PATH; at a later time Git for Cygwin or Git for WSL would find that symlinks have changed size during git status and update the index. And then Git for Windows would think that the index needs to be updated. Even if the symlinks did not, in fact, change. To avoid that, the correct size must be determined.
--- compat/mingw.c | 114 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 104 insertions(+), 10 deletions(-)
compat/mingw.c +104 -10
@@ -21,6 +21,7 @@#define SECURITY_WIN32 #include <sspi.h> #include <wchar.h> +#include <winioctl.h> #include <winternl.h> #define STATUS_DELETE_PENDING ((NTSTATUS) 0xC0000056)
@@ -917,10 +918,102 @@ static int has_valid_directory_prefix(wchar_t *wfilename)return 1 ; } +#ifndef _WINNT_H +/* +* The REPARSE_DATA_BUFFER structure is defined in the Windows DDK (in +* ntifs.h) and in MSYS1's winnt.h (which defines _WINNT_H). So define +* it ourselves if we are on MSYS2 (whose winnt.h defines _WINNT_). +*/ +typedef struct _REPARSE_DATA_BUFFER { +DWORD ReparseTag; +WORD ReparseDataLength; +WORD Reserved; +#ifndef _MSC_VER + _ANONYMOUS_UNION +#endif +union { +struct { +WORD SubstituteNameOffset; +WORD SubstituteNameLength; +WORD PrintNameOffset; +WORD PrintNameLength; +ULONG Flags; +WCHAR PathBuffer[1 ]; + } SymbolicLinkReparseBuffer; +struct { +WORD SubstituteNameOffset; +WORD SubstituteNameLength; +WORD PrintNameOffset; +WORD PrintNameLength; +WCHAR PathBuffer[1 ]; + } MountPointReparseBuffer; +struct { +BYTE DataBuffer[1 ]; + } GenericReparseBuffer; + } DUMMYUNIONNAME; +}REPARSE_DATA_BUFFER , *PREPARSE_DATA_BUFFER ; +#endif + +static int read_reparse_point (const WCHAR *wpath,BOOL fail_on_unknown_tag, +char *tmpbuf,int *plen,DWORD *ptag) +{ +HANDLE handle; +WCHAR *wbuf; +REPARSE_DATA_BUFFER *b =alloca (MAXIMUM_REPARSE_DATA_BUFFER_SIZE ); +DWORD dummy; + +/* read reparse point data */ + handle =CreateFileW (wpath,0 , +FILE_SHARE_READ |FILE_SHARE_WRITE |FILE_SHARE_DELETE ,NULL , +OPEN_EXISTING , +FILE_FLAG_BACKUP_SEMANTICS |FILE_FLAG_OPEN_REPARSE_POINT ,NULL ); +if (handle ==INVALID_HANDLE_VALUE ) { + errno =err_win_to_posix (GetLastError ()); +return -1 ; + } +if (!DeviceIoControl (handle,FSCTL_GET_REPARSE_POINT ,NULL ,0 , b, +MAXIMUM_REPARSE_DATA_BUFFER_SIZE , &dummy,NULL )) { + errno =err_win_to_posix (GetLastError ()); +CloseHandle (handle); +return -1 ; + } +CloseHandle (handle); + +/* get target path for symlinks or mount points (aka 'junctions') */ +switch ((*ptag = b->ReparseTag)) { +case IO_REPARSE_TAG_SYMLINK : + wbuf = (WCHAR *) (((char *) b->SymbolicLinkReparseBuffer.PathBuffer) + + b->SymbolicLinkReparseBuffer.SubstituteNameOffset); + *(WCHAR *) (((char *) wbuf) + + b->SymbolicLinkReparseBuffer.SubstituteNameLength) =0 ; +break ; +case IO_REPARSE_TAG_MOUNT_POINT : + wbuf = (WCHAR *) (((char *) b->MountPointReparseBuffer.PathBuffer) + + b->MountPointReparseBuffer.SubstituteNameOffset); + *(WCHAR *) (((char *) wbuf) + + b->MountPointReparseBuffer.SubstituteNameLength) =0 ; +break ; +default : +if (fail_on_unknown_tag) { + errno =EINVAL ; +return -1 ; + }else { + *plen =MAX_PATH ; +return 0 ; + } + } + +if ((*plen = +xwcstoutf (tmpbuf,normalize_ntpath (wbuf),MAX_PATH )) <0 ) +return -1 ; +return 0 ; +} +int mingw_lstat (const char *file_name,struct stat *buf) {WIN32_FILE_ATTRIBUTE_DATA fdata; -WIN32_FIND_DATAW findbuf = {0 }; +DWORD reparse_tag =0 ; +int link_len =0 ;wchar_t wfilename[MAX_PATH ];int wlen =xutftowcs_path (wfilename, file_name);if (wlen <0 )
@@ -935,28 +1028,29 @@ int mingw_lstat(const char *file_name, struct stat *buf) }if (GetFileAttributesExW (wfilename, GetFileExInfoStandard, &fdata)) { -/* for reparse points, use FindFirstFile to get the reparse tag */ +/* for reparse points, get the link tag and length */ if (fdata.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) { -HANDLE handle =FindFirstFileW (wfilename, &findbuf); -if (handle ==INVALID_HANDLE_VALUE ) - goto error; -FindClose (handle); +char tmpbuf[MAX_PATH ]; + +if (read_reparse_point (wfilename, FALSE, tmpbuf, + &link_len, &reparse_tag) <0 ) +return -1 ; } buf->st_ino =0 ; buf->st_gid =0 ; buf->st_uid =0 ; buf->st_nlink =1 ; buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes, - findbuf.dwReserved0); - buf->st_size = fdata.nFileSizeLow | - (((off_t )fdata.nFileSizeHigh)<<32 ); + reparse_tag); + buf->st_size =S_ISLNK (buf->st_mode) ? link_len : + fdata.nFileSizeLow | (((off_t ) fdata.nFileSizeHigh) <<32 ); buf->st_dev = buf->st_rdev =0 ;/* not used by Git */ filetime_to_timespec (&(fdata.ftLastAccessTime), &(buf->st_atim));filetime_to_timespec (&(fdata.ftLastWriteTime), &(buf->st_mtim));filetime_to_timespec (&(fdata.ftCreationTime), &(buf->st_ctim));return 0 ; } -error: +switch (GetLastError ()) {case ERROR_ACCESS_DENIED :case ERROR_SHARING_VIOLATION :
Signature · 1 line
gitgitgadget
PATCH 7/18 mingw: factor out the retry logic
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
In several places, Git's Windows-specific code follows the pattern where it tries to perform an operation, and retries several times when that operation fails, sleeping an increasing amount of time, before finally giving up and asking the user whether to rety (after, say, closing an editor that held a handle to a file, preventing the operation from succeeding).
This logic is a bit hard to use, and inconsistent:mingw_unlink() and mingw_rmdir() duplicate the code to retry, and both of them do so incompletely. They also do not restore errno if the user answers 'no'.
Introduce a retry_ask_yes_no() helper function that handles retry with small delay, asking the user, and restoring errno.
Note that in mingw_unlink(), we include the _wchmod() call in the retry loop (which may fail if the file is locked exclusively).
In mingw_rmdir(), we include special error handling in the retry loop.
--- compat/mingw.c | 104 ++++++++++++++++++++++--------------------------- 1 file changed, 46 insertions(+), 58 deletions(-)
compat/mingw.c +46 -58
@@ -28,8 +28,6 @@#define HCAST (type, handle) ((type)(intptr_t)handle) -static const int delay[] = {0 ,1 ,10 ,20 ,40 }; -void open_in_gdb (void ) {static struct child_process cp =CHILD_PROCESS_INIT ;
@@ -205,15 +203,12 @@ static int read_yes_no_answer(void)return -1 ; } -static int ask_yes_no_if_possible (const char *format, ...) +static int ask_yes_no_if_possible (const char *format,va_list args) {char question[4096 ];const char *retry_hook; -va_list args; -va_start (args, format);vsnprintf (question,sizeof (question), format, args); -va_end (args); retry_hook =mingw_getenv ("GIT_ASK_YESNO" );if (retry_hook) {
@@ -238,6 +233,31 @@ static int ask_yes_no_if_possible(const char *format, ...) } } +static int retry_ask_yes_no (int *tries,const char *format, ...) +{ +static const int delay[] = {0 ,1 ,10 ,20 ,40 }; +va_list args; +int result, saved_errno = errno; + +if ((*tries) <ARRAY_SIZE (delay)) { +/* +* We assume that some other process had the file open at the wrong +* moment and retry. In order to give the other process a higher +* chance to complete its operation, we give up our time slice now. +* If we have to retry again, we do sleep a bit. +*/ +Sleep (delay[*tries]); + (*tries)++; +return 1 ; + } + +va_start (args, format); + result =ask_yes_no_if_possible (format, args); +va_end (args); + errno = saved_errno; +return result; +} +/* Windows only */ enum hide_dotfiles_type {HIDE_DOTFILES_FALSE =0 ,
@@ -298,7 +318,7 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)int mingw_unlink (const char *pathname,int handle_in_use_error) { -int ret, tries =0 ; +int tries =0 ;wchar_t wpathname[MAX_PATH ];if (xutftowcs_path (wpathname, pathname) <0 )return -1 ;
@@ -306,29 +326,19 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)if (DeleteFileW (wpathname))return 0 ; -/* read-only files cannot be removed */ -_wchmod (wpathname,0666 ); -while ((ret =_wunlink (wpathname)) ==-1 && tries <ARRAY_SIZE (delay)) { +do { +/* read-only files cannot be removed */ +_wchmod (wpathname,0666 ); +if (!_wunlink (wpathname)) +return 0 ;if (!is_file_in_use_error (GetLastError ()))break ;if (!handle_in_use_error) -return ret; +return -1 ; -/* -* We assume that some other process had the source or -* destination file open at the wrong moment and retry. -* In order to give the other process a higher chance to -* complete its operation, we give up our time slice now. -* If we have to retry again, we do sleep a bit. -*/ -Sleep (delay[tries]); - tries++; - } -while (ret ==-1 &&is_file_in_use_error (GetLastError ()) && -ask_yes_no_if_possible ("Unlink of file '%s' failed. " -"Should I try again?" , pathname)) - ret =_wunlink (wpathname); -return ret; + }while (retry_ask_yes_no (&tries,"Unlink of file '%s' failed. " +"Should I try again?" , pathname)); +return -1 ; }static int is_dir_empty (const wchar_t *wpath)
@@ -355,7 +365,7 @@ static int is_dir_empty(const wchar_t *wpath)int mingw_rmdir (const char *pathname) { -int ret, tries =0 ; +int tries =0 ;wchar_t wpathname[MAX_PATH ];struct stat st;
@@ -381,7 +391,11 @@ int mingw_rmdir(const char *pathname)if (xutftowcs_path (wpathname, pathname) <0 )return -1 ; -while ((ret =_wrmdir (wpathname)) ==-1 && tries <ARRAY_SIZE (delay)) { + do { + if (!_wrmdir(wpathname)) { + invalidate_lstat_cache(); + return 0; + } if (!is_file_in_use_error(GetLastError())) errno = err_win_to_posix(GetLastError()); if (errno != EACCES)
@@ -390,23 +404,9 @@ int mingw_rmdir(const char *pathname) errno =ENOTEMPTY ;break ; } -/* -* We assume that some other process had the source or -* destination file open at the wrong moment and retry. -* In order to give the other process a higher chance to -* complete its operation, we give up our time slice now. -* If we have to retry again, we do sleep a bit. -*/ -Sleep (delay[tries]); - tries++; - } -while (ret ==-1 && errno ==EACCES &&is_file_in_use_error (GetLastError ()) && -ask_yes_no_if_possible ("Deletion of directory '%s' failed. " -"Should I try again?" , pathname)) - ret =_wrmdir (wpathname); -if (!ret) -invalidate_lstat_cache (); -return ret; + }while (retry_ask_yes_no (&tries,"Deletion of directory '%s' failed. " +"Should I try again?" , pathname)); +return -1 ; }static inline int needs_hiding (const char *path)
@@ -2384,20 +2384,8 @@ repeat:SetFileAttributesW (wpnew, attrs); } } -if (tries <ARRAY_SIZE (delay) && gle ==ERROR_ACCESS_DENIED ) { -/* -* We assume that some other process had the source or -* destination file open at the wrong moment and retry. -* In order to give the other process a higher chance to -* complete its operation, we give up our time slice now. -* If we have to retry again, we do sleep a bit. -*/ -Sleep (delay[tries]); - tries++; - goto repeat; - }if (gle ==ERROR_ACCESS_DENIED && -ask_yes_no_if_possible ("Rename from '%s' to '%s' failed. " +retry_ask_yes_no (&tries,"Rename from '%s' to '%s' failed. " "Should I try again?" , pold, pnew)) goto repeat;
Signature · 1 line
gitgitgadget
PATCH 8/18 mingw: change default of `core.symlinks` to false
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
Symlinks on Windows don't work the same way as on Unix systems. For example, there are different types of symlinks for directories and files, and unless using a recent-ish Windows version in Developer Mode, creating symlinks requires administrative privileges.
By default, disable symlink support on Windows. That is, users explicitly have to enable it with git config [--system|--global] core.symlinks true; For convenience, git init (and git clone) will perform a test whether the current setup allows creating symlinks and will configure that setting in the repository config.
The test suite ignores system / global config files. Allow testing *with* symlink support by checking if native symlinks are enabled in MSYS2 (via setting the special environment variable MSYS=winsymlinks:nativestrict to ask the MSYS2 runtime to enable creating symlinks).
Note: This assumes that Git's test suite is run in MSYS2's Bash, which is true for the time being (an experiment to switch to BusyBox-w32 failed due to the experimental nature of BusyBox-w32).
--- compat/mingw.c | 9 +++++++++ 1 file changed, 9 insertions(+)
compat/mingw.c +9 -0
@@ -2862,6 +2862,15 @@ static void setup_windows_environment(void)if (!tmp && (tmp =getenv ("USERPROFILE" )))setenv ("HOME" , tmp,1 ); } + +/* +* Change 'core.symlinks' default to false, unless native symlinks are +* enabled in MSys2 (via 'MSYS=winsymlinks:nativestrict'). Thus we can +* run the test suite (which doesn't obey config files) with or without +* symlink support. +*/ +if (!(tmp =getenv ("MSYS" )) || !strstr (tmp,"winsymlinks:nativestrict" )) + has_symlinks =0 ; }static void get_current_user_sid (PSID *sid,HANDLE *linked_token)
Signature · 1 line
gitgitgadget
PATCH 9/18 mingw: add symlink-specific error codes
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
The Win32 API calls do not set errno; Instead, error codes for failed operations must be obtained via the GetLastError() function. Git would not know what to do with those error values, though, which is why Git's Windows compatibility layer translates them to errno values.
Let's handle a couple of symlink-related error codes that will become relevant with the upcoming support for symlinks on Windows.
--- compat/mingw.c | 5 +++++ 1 file changed, 5 insertions(+)
compat/mingw.c +5 -0
@@ -102,6 +102,7 @@ int err_win_to_posix(DWORD winerr)case ERROR_INVALID_PARAMETER : error =EINVAL ;break ;case ERROR_INVALID_PASSWORD : error =EPERM ;break ;case ERROR_INVALID_PRIMARY_GROUP : error =EINVAL ;break ; +case ERROR_INVALID_REPARSE_DATA : error =EINVAL ;break ;case ERROR_INVALID_SIGNAL_NUMBER : error =EINVAL ;break ;case ERROR_INVALID_TARGET_HANDLE : error =EIO ;break ;case ERROR_INVALID_WORKSTATION : error =EACCES ;break ;
@@ -116,6 +117,7 @@ int err_win_to_posix(DWORD winerr)case ERROR_NEGATIVE_SEEK : error =ESPIPE ;break ;case ERROR_NOACCESS : error =EFAULT ;break ;case ERROR_NONE_MAPPED : error =EINVAL ;break ; +case ERROR_NOT_A_REPARSE_POINT : error =EINVAL ;break ;case ERROR_NOT_ENOUGH_MEMORY : error =ENOMEM ;break ;case ERROR_NOT_READY : error =EAGAIN ;break ;case ERROR_NOT_SAME_DEVICE : error =EXDEV ;break ;
@@ -136,6 +138,9 @@ int err_win_to_posix(DWORD winerr)case ERROR_PIPE_NOT_CONNECTED : error =EPIPE ;break ;case ERROR_PRIVILEGE_NOT_HELD : error =EACCES ;break ;case ERROR_READ_FAULT : error =EIO ;break ; +case ERROR_REPARSE_ATTRIBUTE_CONFLICT : error =EINVAL ;break ; +case ERROR_REPARSE_TAG_INVALID : error =EINVAL ;break ; +case ERROR_REPARSE_TAG_MISMATCH : error =EINVAL ;break ;case ERROR_SEEK : error =EIO ;break ;case ERROR_SEEK_ON_DEVICE : error =ESPIPE ;break ;case ERROR_SHARING_BUFFER_EXCEEDED : error =ENFILE ;break ;
Signature · 1 line
gitgitgadget
PATCH 10/18 mingw: handle symlinks to directories in `mingw_unlink()`
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
The _wunlink() and DeleteFileW() functions refuse to delete symlinks to directories on Windows; The error code woutl be ERROR_ACCESS_DENIED in that case. Take that error code as an indicator that we need to try _wrmdir() as well. In the best case, it will remove a symlink. In the worst case, it will fail with the same error code again.
--- compat/mingw.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
compat/mingw.c +8 -1
@@ -338,9 +338,16 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)return 0 ;if (!is_file_in_use_error (GetLastError ()))break ; +/* +* _wunlink() / DeleteFileW() for directory symlinks fails with +* ERROR_ACCESS_DENIED (EACCES), so try _wrmdir() as well. This is the +* same error we get if a file is in use (already checked above). +*/ +if (!_wrmdir (wpathname)) +return 0 ; +if (!handle_in_use_error)return -1 ; - }while (retry_ask_yes_no (&tries,"Unlink of file '%s' failed. " "Should I try again?" , pathname));return -1 ;
Signature · 1 line
gitgitgadget
PATCH 11/18 mingw: support renaming symlinks
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
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.
--- compat/mingw.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-)
compat/mingw.c +16 -22
@@ -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;
Signature · 1 line
gitgitgadget
PATCH 12/18 mingw: allow `mingw_chdir()` to change to symlink-resolved directories
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
If symlinks are enabled, resolve all symlinks when changing directories, as required by POSIX.
Note: Git's real_path() function bases its link resolution algorithm on this property of chdir(). Unfortunately, the current directory on Windows is limited to only MAX_PATH (260) characters. Therefore using symlinks and long paths in combination may be problematic.
--- compat/mingw.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
compat/mingw.c +19 -1
@@ -866,9 +866,27 @@ int mingw_access(const char *filename, int mode)int mingw_chdir (const char *dirname) {wchar_t wdirname[MAX_PATH ]; +if (xutftowcs_path (wdirname, dirname) <0 )return -1 ; -return _wchdir (wdirname); + +if (has_symlinks) { +HANDLE hnd =CreateFileW (wdirname,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 ; + } +if (!GetFinalPathNameByHandleW (hnd, wdirname,ARRAY_SIZE (wdirname),0 )) { + errno =err_win_to_posix (GetLastError ()); +CloseHandle (hnd); +return -1 ; + } +CloseHandle (hnd); + } + +return _wchdir (normalize_ntpath (wdirname)); }int mingw_chmod (const char *filename,int mode)
Signature · 1 line
gitgitgadget
PATCH 13/18 mingw: implement `readlink()`
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
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.
--- compat/mingw-posix.h | 3 +-- compat/mingw.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-)
compat/mingw-posix.h +1 -2
@@ -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
compat/mingw.c +24 -0
@@ -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,
Signature · 1 line
gitgitgadget
PATCH 14/18 mingw: implement basic `symlink()` functionality (file symlinks only)
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
Implement symlink(). This implementation always creates _file_ symlinks (remember: Windows discerns between symlinks pointing to directories and those pointing to files). Support for directory symlinks will be added in a subseqeuent commit.
This implementation fails with ENOSYS if symlinks are disabled or unsupported.
--- compat/mingw-posix.h | 3 +-- compat/mingw.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-)
compat/mingw-posix.h +1 -2
@@ -121,8 +121,6 @@ struct utsname {* trivial stubs */ -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) { errno =ENOSYS ;return -1 ; }#ifndef __MINGW64_VERSION_MAJOR
@@ -195,6 +193,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 symlink (const char *target,const char *link);int readlink (const char *path,char *buf,size_t bufsiz);/*
compat/mingw.c +28 -0
@@ -2698,6 +2698,34 @@ int link(const char *oldpath, const char *newpath)return 0 ; } +int symlink (const char *target,const char *link) +{ +wchar_t wtarget[MAX_PATH ], wlink[MAX_PATH ]; +int len; + +/* fail if symlinks are disabled or API is not supported (WinXP) */ +if (!has_symlinks) { + errno =ENOSYS ; +return -1 ; + } + +if ((len =xutftowcs_path (wtarget, target)) <0 + ||xutftowcs_path (wlink, link) <0 ) +return -1 ; + +/* convert target dir separators to backslashes */ +while (len--) +if (wtarget[len] =='/' ) + wtarget[len] ='\\' ; + +/* create file symlink */ +if (!CreateSymbolicLinkW (wlink, wtarget,0 )) { + errno =err_win_to_posix (GetLastError ()); +return -1 ; + } +return 0 ; +} +int readlink (const char *path,char *buf,size_t bufsiz) {WCHAR wpath[MAX_PATH ];
Signature · 1 line
gitgitgadget
PATCH 15/18 mingw: add support for symlinks to directories
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
Symlinks on Windows have a flag that indicates whether the target is a file or a directory. Symlinks of wrong type simply don't work. This even affects core Win32 APIs (e.g. DeleteFile() refuses to delete directory symlinks).
However, CreateFile() with FILE_FLAG_BACKUP_SEMANTICS does work. Check the target type by first creating a tentative file symlink, opening it, and checking the type of the resulting handle. If it is a directory, recreate the symlink with the directory flag set.
It is possible to create symlinks before the target exists (or in case of symlinks to symlinks: before the target type is known). If this happens, create a tentative file symlink and postpone the directory
decision: keep a list of phantom symlinks to be processed whenever a new directory is created in mingw_mkdir().
Limitations: This algorithm may fail if a link target changes from file to directory or vice versa, or if the target directory is created in another process. It's the best Git can do, though.
--- compat/mingw.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+)
compat/mingw.c +164 -0
@@ -296,6 +296,131 @@ int mingw_core_config(const char *var, const char *value,return 0 ; } +static inline int is_wdir_sep (wchar_t wchar) +{ +return wchar ==L'/' || wchar ==L'\\' ; +} + +static const wchar_t *make_relative_to (const wchar_t *path, +const wchar_t *relative_to,wchar_t *out, +size_t size) +{ +size_t i =wcslen (relative_to), len; + +/* Is `path` already absolute? */ +if (is_wdir_sep (path[0 ]) || + (iswalpha (path[0 ]) && path[1 ] ==L':' &&is_wdir_sep (path[2 ]))) +return path; + +while (i >0 && !is_wdir_sep (relative_to[i -1 ])) + i--; + +/* Is `relative_to` in the current directory? */ +if (!i) +return path; + + len =wcslen (path); +if (i + len +1 > size) { +error ("Could not make '%ls' relative to '%ls' (too large)" , + path, relative_to); +return NULL ; + } + +memcpy (out, relative_to, i *sizeof (wchar_t)); +wcscpy (out + i, path); +return out; +} + +enum phantom_symlink_result { +PHANTOM_SYMLINK_RETRY , +PHANTOM_SYMLINK_DONE , +PHANTOM_SYMLINK_DIRECTORY +}; + +/* +* Changes a file symlink to a directory symlink if the target exists and is a +* directory. +*/ +static enum phantom_symlink_result +process_phantom_symlink (const wchar_t *wtarget,const wchar_t *wlink) +{ +HANDLE hnd; +BY_HANDLE_FILE_INFORMATION fdata; +wchar_t relative[MAX_PATH ]; +const wchar_t *rel; + +/* check that wlink is still a file symlink */ +if ((GetFileAttributesW (wlink) + & (FILE_ATTRIBUTE_REPARSE_POINT |FILE_ATTRIBUTE_DIRECTORY )) + !=FILE_ATTRIBUTE_REPARSE_POINT ) +return PHANTOM_SYMLINK_DONE ; + +/* make it relative, if necessary */ + rel =make_relative_to (wtarget, wlink, relative,ARRAY_SIZE (relative)); +if (!rel) +return PHANTOM_SYMLINK_DONE ; + +/* let Windows resolve the link by opening it */ + hnd =CreateFileW (rel,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 PHANTOM_SYMLINK_RETRY ; + } + +if (!GetFileInformationByHandle (hnd, &fdata)) { + errno =err_win_to_posix (GetLastError ()); +CloseHandle (hnd); +return PHANTOM_SYMLINK_RETRY ; + } +CloseHandle (hnd); + +/* if target exists and is a file, we're done */ +if (!(fdata.dwFileAttributes &FILE_ATTRIBUTE_DIRECTORY )) +return PHANTOM_SYMLINK_DONE ; + +/* otherwise recreate the symlink with directory flag */ +if (DeleteFileW (wlink) &&CreateSymbolicLinkW (wlink, wtarget,1 )) +return PHANTOM_SYMLINK_DIRECTORY ; + + errno =err_win_to_posix (GetLastError ()); +return PHANTOM_SYMLINK_RETRY ; +} + +/* keep track of newly created symlinks to non-existing targets */ +struct phantom_symlink_info { +struct phantom_symlink_info *next; +wchar_t *wlink; +wchar_t *wtarget; +}; + +static struct phantom_symlink_info *phantom_symlinks =NULL ; +static CRITICAL_SECTION phantom_symlinks_cs; + +static void process_phantom_symlinks (void ) +{ +struct phantom_symlink_info *current, **psi; +EnterCriticalSection (&phantom_symlinks_cs); +/* process phantom symlinks list */ + psi = &phantom_symlinks; +while ((current = *psi)) { +enum phantom_symlink_result result =process_phantom_symlink ( + current->wtarget, current->wlink); +if (result ==PHANTOM_SYMLINK_RETRY ) { + psi = ¤t->next; + }else { +/* symlink was processed, remove from list */ + *psi = current->next; +free (current); +/* if symlink was a directory, start over */ +if (result ==PHANTOM_SYMLINK_DIRECTORY ) + psi = &phantom_symlinks; + } + } +LeaveCriticalSection (&phantom_symlinks_cs); +} +/* Normalizes NT paths as returned by some low-level APIs. */ static wchar_t *normalize_ntpath (wchar_t *wbuf) {
@@ -479,6 +604,8 @@ int mingw_mkdir(const char *path, int mode UNUSED)if (xutftowcs_path (wpath, path) <0 )return -1 ; ret =_wmkdir (wpath); +if (!ret) +process_phantom_symlinks ();if (!ret &&needs_hiding (path))return set_hidden_flag (wpath,1 );return ret;
@@ -2723,6 +2850,42 @@ int symlink(const char *target, const char *link) errno =err_win_to_posix (GetLastError ());return -1 ; } + +/* convert to directory symlink if target exists */ +switch (process_phantom_symlink (wtarget, wlink)) { +case PHANTOM_SYMLINK_RETRY : { +/* if target doesn't exist, add to phantom symlinks list */ +wchar_t wfullpath[MAX_PATH ]; +struct phantom_symlink_info *psi; + +/* convert to absolute path to be independent of cwd */ + len =GetFullPathNameW (wlink,MAX_PATH , wfullpath,NULL ); +if (!len || len >=MAX_PATH ) { + errno =err_win_to_posix (GetLastError ()); +return -1 ; + } + +/* over-allocate and fill phantom_symlink_info structure */ + psi =xmalloc (sizeof (struct phantom_symlink_info ) + +sizeof (wchar_t) * (len +wcslen (wtarget) +2 )); + psi->wlink = (wchar_t *)(psi +1 ); +wcscpy (psi->wlink, wfullpath); + psi->wtarget = psi->wlink + len +1 ; +wcscpy (psi->wtarget, wtarget); + +EnterCriticalSection (&phantom_symlinks_cs); + psi->next = phantom_symlinks; + phantom_symlinks = psi; +LeaveCriticalSection (&phantom_symlinks_cs); +break ; + } +case PHANTOM_SYMLINK_DIRECTORY : +/* if we created a dir symlink, process other phantom symlinks */ +process_phantom_symlinks (); +break ; +default : +break ; + }return 0 ; }
@@ -3424,6 +3587,7 @@ int wmain(int argc, const wchar_t **wargv)/* initialize critical section for waitpid pinfo_t list */ InitializeCriticalSection (&pinfo_cs); +InitializeCriticalSection (&phantom_symlinks_cs);/* set up default file mode and file modes for stdin/out/err */ _fmode = _O_BINARY;
Signature · 1 line
gitgitgadget
PATCH 16/18 mingw: try to create symlinks without elevated permissions
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
As of Windows 10 Build 14972 in Developer Mode, a new flag is supported by CreateSymbolicLink() to create symbolic links even when running outside of an elevated session (which was previously required).
This new flag is called SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE and has the numeric value 0x02.
Previous Windows 10 versions will not understand that flag and return an ERROR_INVALID_PARAMETER, therefore we have to be careful to try passing that flag only when the build number indicates that it is supported.
For more information about the new flag, see this blog post:
https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/
--- compat/mingw.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
compat/mingw.c +24 -2
@@ -331,6 +331,8 @@ static const wchar_t *make_relative_to(const wchar_t *path,return out; } +static DWORD symlink_file_flags =0 , symlink_directory_flags =1 ; +enum phantom_symlink_result {PHANTOM_SYMLINK_RETRY ,PHANTOM_SYMLINK_DONE ,
@@ -381,7 +383,8 @@ process_phantom_symlink(const wchar_t *wtarget, const wchar_t *wlink)return PHANTOM_SYMLINK_DONE ;/* otherwise recreate the symlink with directory flag */ -if (DeleteFileW (wlink) &&CreateSymbolicLinkW (wlink, wtarget,1 )) +if (DeleteFileW (wlink) && +CreateSymbolicLinkW (wlink, wtarget, symlink_directory_flags))return PHANTOM_SYMLINK_DIRECTORY ; errno =err_win_to_posix (GetLastError ());
@@ -2846,7 +2849,7 @@ int symlink(const char *target, const char *link) wtarget[len] ='\\' ;/* create file symlink */ -if (!CreateSymbolicLinkW (wlink, wtarget,0 )) { +if (!CreateSymbolicLinkW (wlink, wtarget, symlink_file_flags)) { errno =err_win_to_posix (GetLastError ());return -1 ; }
@@ -3523,6 +3526,24 @@ static void maybe_redirect_std_handles(void) GENERIC_WRITE, FILE_FLAG_NO_BUFFERING); } +static void adjust_symlink_flags (void ) +{ +/* +* Starting with Windows 10 Build 14972, symbolic links can be created +* using CreateSymbolicLink() without elevation by passing the flag +* SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE (0x02) as last +* parameter, provided the Developer Mode has been enabled. Some +* earlier Windows versions complain about this flag with an +* ERROR_INVALID_PARAMETER, hence we have to test the build number +* specifically. +*/ +if (GetVersion () >=14972 <<16 ) { + symlink_file_flags |=2 ; + symlink_directory_flags |=2 ; + } + +} +#ifdef _MSC_VER#ifdef _DEBUG#include <crtdbg.h>
@@ -3558,6 +3579,7 @@ int wmain(int argc, const wchar_t **wargv)#endif maybe_redirect_std_handles (); +adjust_symlink_flags ();/* determine size of argv and environ conversion buffer */ maxlen =wcslen (wargv[0 ]);
Signature · 1 line
gitgitgadget
PATCH 17/18 mingw: emulate `stat()` a little more faithfully
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When creating directories via safe_create_leading_directories(), we might encounter an already-existing directory which is not readable by the current user. To handle that situation, Git's code calls stat() to determine whether we're looking at a directory.
In such a case, CreateFile() will fail, though, no matter what, and consequently mingw_stat() will fail, too. But POSIX semantics seem to still allow stat() to go forward.
So let's call mingw_lstat() to the rescue if we fail to get a file handle due to denied permission in mingw_stat(), and fill the stat info that way.
We need to be careful to not allow this to go forward in case that we're looking at a symbolic link: to resolve the link, we would still have to create a file handle, and we just found out that we cannot. Therefore, stat() still needs to fail with EACCES in that case.
This fixes https://github.com/git-for-windows/git/issues/2531.
--- compat/mingw.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
compat/mingw.c +13 -1
@@ -1273,7 +1273,19 @@ int mingw_stat(const char *file_name, struct stat *buf)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 ()); +DWORD err =GetLastError (); + +if (err ==ERROR_ACCESS_DENIED && + !mingw_lstat (file_name, buf) && + !S_ISLNK (buf->st_mode)) +/* +* POSIX semantics state to still try to fill +* information, even if permission is denied to create +* a file handle. +*/ +return 0 ; + + errno =err_win_to_posix (err);return -1 ; } result =get_file_info_by_handle (hnd, buf);
Signature · 1 line
gitgitgadget
PATCH 18/18 mingw: special-case index entries for symlinks with buggy size
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
In https://github.com/git-for-windows/git/pull/2637, we fixed a bug where symbolic links' target path sizes were recorded incorrectly in the index. The downside of this fix was that every user with tracked symbolic links in their checkouts would see them as modified in git status, but not in git diff, and only a git add <path> (or git add -u) would "fix" this.
Let's do better than that: we can detect that situation and simply pretend that a symbolic link with a known bad size (or a size that just happens to be that bad size, a _very_ unlikely scenario because it would overflow our buffers due to the trailing NUL byte) means that it needs to be re-checked as if we had just checked it out.
--- read-cache.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
read-cache.c +11 -0
@@ -470,6 +470,17 @@ int ie_modified(struct index_state *istate,* then we know it is. */ if ((changed &DATA_CHANGED ) && +#ifdef GIT_WINDOWS_NATIVE +/* +* Work around Git for Windows v2.27.0 fixing a bug where symlinks' +* target path lengths were not read at all, and instead recorded +* as 4096: now, all symlinks would appear as modified. +* +* So let's just special-case symlinks with a target path length +* (i.e. `sd_size`) of 4096 and force them to be re-checked. +*/ + (!S_ISLNK (st->st_mode) || ce->ce_stat_data.sd_size !=MAX_PATH ) && +#endif (S_ISGITLINK (ce->ce_mode) || ce->ce_stat_data.sd_size !=0 ))return changed;
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 3 lines · 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(-)
Re: [PATCH 10/18] mingw: handle symlinks to directories in `mingw_unlink()`
In reply to Karsten Blees via GitGitGadgetQuoted 7 lines · Karsten Blees via GitGitGadget
From: Karsten Blees <[email protected]>
The
_wunlink()andDeleteFileW()functions refuse to delete symlinks to directories on Windows; The error code woutl beERROR_ACCESS_DENIED
Casually reading; spotted “woutl.” Presumably would?
Re: [PATCH 01/18] mingw: don't call `GetFileAttributes()` twice in `mingw_lstat()`
In reply to Karsten Blees via GitGitGadgetAm 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
Quoted 8 lines
From: Karsten Blees <[email protected]>
The Win32 API function
GetFileAttributes()cannot handle paths with trailing dir separators. The currentmingw_stat()/mingw_lstat()implementation callsGetFileAttributes()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
Re: [PATCH 02/18] mingw: implement `stat()` with symlink support
In reply to Karsten Blees via GitGitGadgetAm 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
Quoted 6 lines
From: Karsten Blees <[email protected]>
With respect to symlinks, the current
mingw_stat()implementation is almost identical tomingw_lstat(): except for the file type (st_mode & S_IFMT), it returns information about the link rather than the target.
Quoted 6 lines
Implement
mingw_stat()by opening the file handle requesting minimal permissions, and then callingGetFileInformationByHandle()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 withELOOPif 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.
Quoted 29 lines
Signed-off-by Karsten Blees <[email protected]>Signed-off-by Johannes Schindelin <[email protected]>--- compat/mingw.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)compat/mingw.c +11
-1@@ -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 );
compat/mingw.c int mingw_lstat(const char *file_name, struct stat *buf) · line 1051
Quoted 10 lines
compat/mingw.c +7
-0+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
Re: [PATCH 03/18] mingw: drop the separate `do_lstat()` function
In reply to Karsten Blees via GitGitGadgetAm 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
Quoted 20 lines
From: Karsten Blees <[email protected]>
With the new
mingw_stat()implementation,do_lstat()is only called frommingw_lstat()(with the function parameterfollow == 0). Remove the extra function and the oldmingw_stat()-specific (follow == 1) logic.Signed-off-by Karsten Blees <[email protected]>Signed-off-by Johannes Schindelin <[email protected]>--- compat/mingw.c | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-)compat/mingw.c +0
-0@@ -917,14 +917,7 @@ static int has_valid_directory_prefix(wchar_t *wfilename)return 1 ; }
compat/mingw.c static int has_valid_directory_prefix(wchar_t *wfilename) · line 920
Quoted 10 lines
compat/mingw.c +1
-8-/* 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.
Quoted 20 lines
compat/mingw.c +1
-7{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 ;
compat/mingw.c static int get_file_info_by_handle(HANDLE hnd, struct stat *buf) · line 1014
Quoted 10 lines
compat/mingw.c +0
-5} -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
Re: [PATCH 11/18] mingw: support renaming symlinks
In reply to Karsten Blees via GitGitGadgetAm 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
Quoted 82 lines
From: Karsten Blees <[email protected]>
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 callMoveFileEx()with theMOVEFILE_COPY_ALLOWEDflag.Avoid the
_wrename()call, and go with directly callingMoveFileEx(), with proper error handling of course.Signed-off-by Karsten Blees <[email protected]>Signed-off-by Johannes Schindelin <[email protected]>--- compat/mingw.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-)compat/mingw.c +13
-19@@ -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);
compat/mingw.c repeat: · line 2397
Quoted 10 lines
compat/mingw.c +3
-3- } + 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
Re: [PATCH 13/18] mingw: implement `readlink()`
In reply to Karsten Blees via GitGitGadgetAm 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
Quoted 6 lines
From: Karsten Blees <[email protected]>
Implement
readlink()by reading NTFS reparse points via theread_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 withENOSYS.
This last sentence is obsolete, I think, because I cannot see how the patch achieves a failure with ENOSYS.
Quoted 54 lines
Signed-off-by Karsten Blees <[email protected]>Signed-off-by Johannes Schindelin <[email protected]>--- compat/mingw-posix.h | 3 +-- compat/mingw.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-)compat/mingw-posix.h +1
-2@@ -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 compat/mingw.c +17
-0@@ -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,
compat/mingw.c int link(const char *oldpath, const char *newpath) · line 2727
Quoted 10 lines
compat/mingw.c +7
-0+* 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
Am 17.12.25 um 15:08 schrieb Johannes Schindelin via GitGitGadget:
Quoted 37 lines
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 inmingw_lstat()Johannes Schindelin (3):
mingw: try to create symlinks without elevated permissions
mingw: emulatestat()a little more faithfully
mingw: special-case index entries for symlinks with buggy sizeKarsten Blees (14):
mingw: don't callGetFileAttributes()twice inmingw_lstat()
mingw: implementstat()with symlink support
mingw: drop the separatedo_lstat()function
mingw: letmingw_lstat()error early upon problems with reparse
points
mingw: teach dirent about symlinks
mingw: factor out the retry logic
mingw: change default ofcore.symlinksto false
mingw: add symlink-specific error codes
mingw: handle symlinks to directories inmingw_unlink()
mingw: support renaming symlinks
mingw: allowmingw_chdir()to change to symlink-resolved directories
mingw: implementreadlink()
mingw: implement basicsymlink()functionality (file symlinks only)
mingw: add support for symlinks to directoriescompat/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(-)
Quoted 4 lines
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
Am 18.12.2025 um 19:51 schrieb Johannes Sixt:
Quoted 5 lines
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
Re: [PATCH 02/18] mingw: implement `stat()` with symlink support
In reply to Johannes SixtHi Hannes,
Quoted 11 lines · Johannes Sixt
Am 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
Quoted 10 lines
From: Karsten Blees <[email protected]>
With respect to symlinks, the current
mingw_stat()implementation is almost identical tomingw_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 callingGetFileInformationByHandle()on it. This way, all links are resolved by the Windows file system layer.
Quoted 7 lines
Quoted 2 lines
If symlinks are disabled, use
mingw_lstat()as before, but fail withELOOPif 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?
You're right. Sharp eyes, by the way, I cannot count how often I glanced over this paragraph while pre-reviewing.
As to the reason why this paragraph is there: This comes from the initial version of this patch:
https://github.com/git-for-windows/git/commit/b908441ea594f022e862c04cefe8ac73bb8c0ab0
I can only try to reconstruct why I skipped the ELOOP logic in the rebased version at
https://github.com/git-for-windows/git/commit/0181eb0c78d04f5fb065cbe2f3346077b0f9930e
(my guess is that I realized that returning ELOOP when symlink support was disabled via core.symlinks = false was undesirable: In particular with Windows 7 semantics, where symlinks could be read and used, but required administrator permissions to create, that flag was meant to turn off symlink _creation_, but reading existing symlinks should work nevertheless).
I'll simply remove this paragraph from the commit message.
Ciao,
Johannes
Re: [PATCH 10/18] mingw: handle symlinks to directories in `mingw_unlink()`
In reply to Ben KnobleHi Ben,
Quoted 9 lines · Ben Knoble
Quoted 7 lines · Karsten Blees via GitGitGadget
From: Karsten Blees <[email protected]>
The
_wunlink()andDeleteFileW()functions refuse to delete symlinks to directories on Windows; The error code woutl beERROR_ACCESS_DENIEDCasually reading; spotted “woutl.” Presumably would?
Thank you for spotting! Fixed,
Johannes
Re: [PATCH 11/18] mingw: support renaming symlinks
In reply to Johannes SixtHi Hannes,
Quoted 91 lines · Johannes Sixt
Am 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
Quoted 90 lines
From: Karsten Blees <[email protected]>
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 callMoveFileEx()with theMOVEFILE_COPY_ALLOWEDflag.Avoid the
_wrename()call, and go with directly callingMoveFileEx(), with proper error handling of course.Signed-off-by Karsten Blees <[email protected]>Signed-off-by Johannes Schindelin <[email protected]>--- compat/mingw.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-)compat/mingw.c +16
-22@@ -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))
Quoted 8 lines
compat/mingw.c repeat: · line 2397
Quoted 2 lines
compat/mingw.c +0
-0goto 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.
I do agree that the logic is quite convoluted. Historically grown, like. But as you suspect: This has been battle-hardened, and I am loathe to introduce a regression by making it prettier at this point. This is tried and tested code, and that counts for something.
Ciao,
Johannes
Re: [PATCH 13/18] mingw: implement `readlink()`
In reply to Johannes SixtHi Hannes,
Quoted 10 lines · Johannes Sixt
Am 17.12.25 um 15:08 schrieb Karsten Blees via GitGitGadget:
Quoted 6 lines
From: Karsten Blees <[email protected]>
Implement
readlink()by reading NTFS reparse points via theread_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 withENOSYS.This last sentence is obsolete, I think, because I cannot see how the patch achieves a failure with ENOSYS.
Indeed, this is obsolete. Just like with the ELOOP commit message comment of 02/18, I must have dropped this because reading symlinks should work even if creating symlinks has been disabled via core.symlinks. Here is the range-diff between the last version of the patch that still had the ENOSYS logic and the first version that lacked it (Git for Windows-only commits):
1: 4f353d988de4 ! 1: 1d079621427c Win32: implement readlink()
@@ compat/mingw.c: int link(const char *oldpath, const char *newpath)
+ char tmpbuf[MAX_LONG_PATH];
+ int len;
+
-+ /* fail if symlinks are disabled */
-+ if (!has_symlinks) {
-+ errno = ENOSYS;
-+ return -1;
-+ }
-+
+ if (xutftowcs_long_path(wpath, path) < 0)
+ return -1;
+So: Unfortunately I have no record that I can readily produce that would motivate that change. Given that it happened during the same v2.19.2 timeframe as the ELOOP change, there must have been some broader discussion about this, but I could not find it, not even in the release notes of that version:
https://github.com/git-for-windows/git/releases/tag/v2.19.2.windows.1
All I can present is the reconstructed rationale that just because Git is not allowed (or able) to create symlinks does not mean that they cannot exist, and therefore Git should at least read and parse them as expected, independent of the value of core.symlinks.
So yes, this part of the commit message is just simply confusing at this point, so I'll drop it.
Ciao,
Johannes
v2 cover Support symbolic links on Windows
In reply to Johannes Schindelin via GitGitGadgetThis 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.
Changes since v1:
* Changed Karsten's email address to the current one.
* The changes to do_lstat() are now reflected by the comment preceding that function.
* The commit message mentioning some ELOOP logic when symlink support is disabled, which had no corresponding part in the commit's diff, was adjusted to no longer mention that long gone change.
* Fixed the typo "woutl".
* Dropped a misleading, stale comment about ENOSYS in the readlink() implementation (talking about a long-dropped part of the original patch).
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: 1887b3dd06823575e37ad19b5827d467e126c6ed
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2018%2Fdscho%2Fsymlinks-next-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2018/dscho/symlinks-next-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2018
Range-diff vs v1:
1: dae450dd0e ! 1: 6ec4ff7457 mingw: don't call GetFileAttributes() twice in mingw_lstat()
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: don't call GetFileAttributes() twice in mingw_lstat()
@@ Commit message
free, and also have a (wide char) buffer that can be modified. This makes it easy to avoid that extraneous Win32 API call.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
+@@ compat/mingw.c: static int has_valid_directory_prefix(wchar_t *wfilename)
+ }
+
+ /* 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.
++ * When a path ends with a slash, the call to GetFileAttributedExW()
++ * would fail. To prevent this, we strip any trailing slashes before that ++ * call.
+ *
+ * If follow is true then act like stat() and report on the link
+ * target. Otherwise report on the link itself.
@@ compat/mingw.c: static int do_lstat(int follow, const char *file_name, struct stat *buf)
{
WIN32_FILE_ATTRIBUTE_DATA fdata;
2: c36848eda7 ! 2: 5d83a8ab76 mingw: implement stat() with symlink support
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: implement stat() with symlink support
@@ Commit message
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.
-
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
3: aa0ca80bbb ! 3: d3953f1826 mingw: drop the separate do_lstat() function
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: drop the separate do_lstat() function
@@ Commit message
the extra function and the old mingw_stat()-specific (follow == 1) logic.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
@@ compat/mingw.c: static int has_valid_directory_prefix(wchar_t *wfilename)
}
-/* 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.
+- * When a path ends with a slash, the call to GetFileAttributedExW()
+- * would fail. To prevent this, we strip any trailing slashes before that
+- * call.
- *
- * If follow is true then act like stat() and report on the link
- * target. Otherwise report on the link itself.
4: 886044373b ! 4: a6355789c3 mingw: let mingw_lstat() error early upon problems with reparse points
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: let mingw_lstat() error early upon problems with reparse points
@@ Commit message
--color-moved -w. That code was _not_ moved because a subsequent commit will move it to an altogether different function, anyway.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
5: db1d156aa0 ! 5: 8b7f5a8fc7 mingw: teach dirent about symlinks
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: teach dirent about symlinks
@@ Commit message
Implement DT_LNK detection in dirent.c's readdir() function.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
6: 4c49a3d9bf = 6: 1159e86dc4 mingw: compute the correct size for symlinks in `mingw_lstat()` 7: ad74d540f2 ! 7: 4aeccd6656 mingw: factor out the retry logic
@@
## Metadata ##
-Author: Karsten Blees <[email protected]> +Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: factor out the retry logic
@@ Commit message
In mingw_rmdir(), we include special error handling in the retry loop.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
8: 25313cea76 ! 8: 86c0742748 mingw: change default of core.symlinks to false
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: change default of core.symlinks to false
@@ Commit message
is true for the time being (an experiment to switch to BusyBox-w32 failed due to the experimental nature of BusyBox-w32).
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
9: b698f4a002 ! 9: 9a0093d34b mingw: add symlink-specific error codes
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: add symlink-specific error codes
@@ Commit message
Let's handle a couple of symlink-related error codes that will become relevant with the upcoming support for symlinks on Windows.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
10: 282aba42e8 ! 10: 5dc90f9785 mingw: handle symlinks to directories in mingw_unlink()
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: handle symlinks to directories in mingw_unlink()
The _wunlink() and DeleteFileW() functions refuse to delete symlinks
- to directories on Windows; The error code woutl be ERROR_ACCESS_DENIED
+ to directories on Windows; The error code would be ERROR_ACCESS_DENIED
in that case. Take that error code as an indicator that we need to try _wrmdir() as well. In the best case, it will remove a symlink. In the worst case, it will fail with the same error code again.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
11: 5cb3b10500 ! 11: 3670a0a181 mingw: support renaming symlinks
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: support renaming symlinks
@@ Commit message
Avoid the _wrename() call, and go with directly calling MoveFileEx(), with proper error handling of course.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
12: 49920839bb ! 12: f79cf31dab mingw: allow mingw_chdir() to change to symlink-resolved directories
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: allow mingw_chdir() to change to symlink-resolved directories
@@ Commit message
Windows is limited to only MAX_PATH (260) characters. Therefore using symlinks and long paths in combination may be problematic.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
13: 8fef8220f4 ! 13: 1ae9c63a89 mingw: implement readlink()
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: implement readlink()
@@ Commit message
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.
+ junctions.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw-posix.h ##
14: 1dd5f9d6cd ! 14: 4e0ac43ef6 mingw: implement basic symlink() functionality (file symlinks only)
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: implement basic symlink() functionality (file symlinks only)
@@ Commit message
This implementation fails with ENOSYS if symlinks are disabled or unsupported.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw-posix.h ##
15: 7b6dbc73f7 ! 15: 3d479fd47e mingw: add support for symlinks to directories
@@
## Metadata ##
-Author: Karsten Blees <[email protected]>
+Author: Karsten Blees <[email protected]>
## Commit message ##
mingw: add support for symlinks to directories
@@ Commit message
to directory or vice versa, or if the target directory is created in another process. It's the best Git can do, though.
- Signed-off-by: Karsten Blees <[email protected]>
+ Signed-off-by: Karsten Blees <[email protected]>
## compat/mingw.c ##
16: d3b89c29b0 = 16: fb6aa461da mingw: try to create symlinks without elevated permissions 17: 2e73ab4221 = 17: 40c3f7f36e mingw: emulate `stat()` a little more faithfully 18: 817f488523 = 18: afcf2bbfcb mingw: special-case index entries for symlinks with buggy size
Signature · 1 line
gitgitgadget
PATCH v2 1/18 mingw: don't call `GetFileAttributes()` twice in `mingw_lstat()`
From: Karsten Blees <[email protected]>
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).
With the conversion to wide Unicode, we get the length of the path for free, and also have a (wide char) buffer that can be modified. This makes it easy to avoid that extraneous Win32 API call.
--- compat/mingw.c | 53 ++++++++++++++------------------------------------ 1 file changed, 15 insertions(+), 38 deletions(-)
compat/mingw.c +15 -38
@@ -918,8 +918,9 @@ static int has_valid_directory_prefix(wchar_t *wfilename) }/* 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. +* When a path ends with a slash, the call to `GetFileAttributedExW()` +* would fail. To prevent this, we strip any trailing slashes before that +* call. * * If follow is true then act like stat() and report on the link * target. Otherwise report on the link itself.
@@ -928,9 +929,18 @@ static int do_lstat(int follow, const char *file_name, struct stat *buf) {WIN32_FILE_ATTRIBUTE_DATA fdata;wchar_t wfilename[MAX_PATH ]; -if (xutftowcs_path (wfilename, file_name) <0 ) +int wlen =xutftowcs_path (wfilename, file_name); +if (wlen <0 )return -1 ; +/* strip trailing '/', or GetFileAttributes will fail */ +while (wlen &&is_dir_sep (wfilename[wlen -1 ])) + wfilename[--wlen] =0 ; +if (!wlen) { + errno =ENOENT ; +return -1 ; + } +if (GetFileAttributesExW (wfilename, GetFileExInfoStandard, &fdata)) { buf->st_ino =0 ; buf->st_gid =0 ;
@@ -990,39 +1000,6 @@ static int do_lstat(int follow, const char *file_name, struct stat *buf)return -1 ; } -/* We provide our own lstat/fstat functions, since the provided -* lstat/fstat functions are so slow. These stat functions are -* tailored for Git's usage (read: fast), and are not meant to be -* complete. Note that Git stat()s are redirected to mingw_lstat() -* too, since Windows doesn't really handle symlinks that well. -*/ -static int do_stat_internal (int follow,const char *file_name,struct stat *buf) -{ -size_t namelen; -char alt_name[PATH_MAX ]; - -if (!do_lstat (follow, file_name, buf)) -return 0 ; - -/* if file_name ended in a '/', Windows returned ENOENT; -* try again without trailing slashes -*/ -if (errno !=ENOENT ) -return -1 ; - - namelen =strlen (file_name); -if (namelen && file_name[namelen-1 ] !='/' ) -return -1 ; -while (namelen && file_name[namelen-1 ] =='/' ) - --namelen; -if (!namelen || namelen >=PATH_MAX ) -return -1 ; - -memcpy (alt_name, file_name, namelen); - alt_name[namelen] =0 ; -return do_lstat (follow, alt_name, buf); -} -static int get_file_info_by_handle (HANDLE hnd,struct stat *buf) {BY_HANDLE_FILE_INFORMATION fdata;
@@ -1048,11 +1025,11 @@ static int get_file_info_by_handle(HANDLE hnd, struct stat *buf)int mingw_lstat (const char *file_name,struct stat *buf) { -return do_stat_internal (0 , file_name, buf); +return do_lstat (0 , file_name, buf); }int mingw_stat (const char *file_name,struct stat *buf) { -return do_stat_internal (1 , file_name, buf); +return do_lstat (1 , file_name, buf); }int mingw_fstat (int fd,struct stat *buf)
Signature · 1 line
gitgitgadget
PATCH v2 2/18 mingw: implement `stat()` with symlink support
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
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.
--- compat/mingw.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
compat/mingw.c +18 -1
@@ -1027,9 +1027,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)
Signature · 1 line
gitgitgadget
PATCH v2 3/18 mingw: drop the separate `do_lstat()` function
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
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.
--- compat/mingw.c | 23 ++--------------------- 1 file changed, 2 insertions(+), 21 deletions(-)
compat/mingw.c +2 -21
@@ -917,15 +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 call to `GetFileAttributedExW()` -* would fail. To prevent this, we strip any trailing slashes before that -* call. -* -* 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) {WIN32_FILE_ATTRIBUTE_DATA fdata;wchar_t wfilename[MAX_PATH ];
@@ -959,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 ; }
@@ -1023,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 ];
Signature · 1 line
gitgitgadget
PATCH v2 4/18 mingw: let `mingw_lstat()` error early upon problems with reparse points
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
When obtaining lstat information for reparse points, we need to call FindFirstFile() in addition to GetFileInformationEx() to obtain the type of the reparse point (symlink, mount point etc.). However, currently there is no error handling whatsoever if FindFirstFile() fails.
Call FindFirstFile() before modifying the stat *buf output parameter and error out if the call fails.
Note: The FindFirstFile() return value includes all the data that we get from GetFileAttributesEx(), so we could replace GetFileAttributesEx() with FindFirstFile(). We don't do that because GetFileAttributesEx() is about twice as fast for single files. I.e. we only pay the extra cost of calling FindFirstFile() in the rare case that we encounter a reparse point.
Please also note that the indentation the remaining reparse point code changed, and hence the best way to look at this diff is with --color-moved -w. That code was _not_ moved because a subsequent commit will move it to an altogether different function, anyway.
--- compat/mingw.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-)
compat/mingw.c +14 -10
@@ -920,6 +920,7 @@ static int has_valid_directory_prefix(wchar_t *wfilename)int mingw_lstat (const char *file_name,struct stat *buf) {WIN32_FILE_ATTRIBUTE_DATA fdata; +WIN32_FIND_DATAW findbuf = {0 };wchar_t wfilename[MAX_PATH ];int wlen =xutftowcs_path (wfilename, file_name);if (wlen <0 )
@@ -934,6 +935,13 @@ int mingw_lstat(const char *file_name, struct stat *buf) }if (GetFileAttributesExW (wfilename, GetFileExInfoStandard, &fdata)) { +/* for reparse points, use FindFirstFile to get the reparse tag */ +if (fdata.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) { +HANDLE handle =FindFirstFileW (wfilename, &findbuf); +if (handle ==INVALID_HANDLE_VALUE ) + goto error; +FindClose (handle); + } buf->st_ino =0 ; buf->st_gid =0 ; buf->st_uid =0 ;
@@ -946,20 +954,16 @@ int mingw_lstat(const char *file_name, struct stat *buf)filetime_to_timespec (&(fdata.ftLastWriteTime), &(buf->st_mtim));filetime_to_timespec (&(fdata.ftCreationTime), &(buf->st_ctim));if (fdata.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) { -WIN32_FIND_DATAW findbuf; -HANDLE handle =FindFirstFileW (wfilename, &findbuf); -if (handle !=INVALID_HANDLE_VALUE ) { -if ((findbuf.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) && - (findbuf.dwReserved0 ==IO_REPARSE_TAG_SYMLINK )) { - buf->st_mode =S_IFLNK |S_IREAD ; -if (!(findbuf.dwFileAttributes &FILE_ATTRIBUTE_READONLY )) - buf->st_mode |=S_IWRITE ; - } -FindClose (handle); +if ((findbuf.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) && + (findbuf.dwReserved0 ==IO_REPARSE_TAG_SYMLINK )) { + buf->st_mode =S_IFLNK |S_IREAD ; +if (!(findbuf.dwFileAttributes &FILE_ATTRIBUTE_READONLY )) + buf->st_mode |=S_IWRITE ; } }return 0 ; } +error:switch (GetLastError ()) {case ERROR_ACCESS_DENIED :case ERROR_SHARING_VIOLATION :
Signature · 1 line
gitgitgadget
PATCH v2 5/18 mingw: teach dirent about symlinks
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
Move the S_IFLNK detection to file_attr_to_st_mode().
Implement DT_LNK detection in dirent.c's readdir() function.
--- compat/mingw.c | 13 +++---------- compat/win32.h | 6 ++++-- compat/win32/dirent.c | 5 ++++- 3 files changed, 11 insertions(+), 13 deletions(-)
compat/mingw.c +3 -10
@@ -946,21 +946,14 @@ int mingw_lstat(const char *file_name, struct stat *buf) buf->st_gid =0 ; buf->st_uid =0 ; buf->st_nlink =1 ; - buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes); + buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes, + findbuf.dwReserved0); buf->st_size = fdata.nFileSizeLow | (((off_t )fdata.nFileSizeHigh)<<32 ); buf->st_dev = buf->st_rdev =0 ;/* not used by Git */ filetime_to_timespec (&(fdata.ftLastAccessTime), &(buf->st_atim));filetime_to_timespec (&(fdata.ftLastWriteTime), &(buf->st_mtim));filetime_to_timespec (&(fdata.ftCreationTime), &(buf->st_ctim)); -if (fdata.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) { -if ((findbuf.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) && - (findbuf.dwReserved0 ==IO_REPARSE_TAG_SYMLINK )) { - buf->st_mode =S_IFLNK |S_IREAD ; -if (!(findbuf.dwFileAttributes &FILE_ATTRIBUTE_READONLY )) - buf->st_mode |=S_IWRITE ; - } - }return 0 ; } error:
@@ -1003,7 +996,7 @@ static int get_file_info_by_handle(HANDLE hnd, struct stat *buf) buf->st_gid =0 ; buf->st_uid =0 ; buf->st_nlink =1 ; - buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes); + buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes,0 ); buf->st_size = fdata.nFileSizeLow | (((off_t )fdata.nFileSizeHigh)<<32 ); buf->st_dev = buf->st_rdev =0 ;/* not used by Git */
compat/win32.h +4 -2
@@ -6,10 +6,12 @@#include <windows.h> #endif -static inline int file_attr_to_st_mode (DWORD attr) +static inline int file_attr_to_st_mode (DWORD attr,DWORD tag) {int fMode =S_IREAD ; -if (attr &FILE_ATTRIBUTE_DIRECTORY ) +if ((attr &FILE_ATTRIBUTE_REPARSE_POINT ) && tag ==IO_REPARSE_TAG_SYMLINK ) + fMode |=S_IFLNK ; +else if (attr &FILE_ATTRIBUTE_DIRECTORY ) fMode |=S_IFDIR ;else fMode |=S_IFREG ;
compat/win32/dirent.c +4 -1
@@ -12,7 +12,10 @@ static inline void finddata2dirent(struct dirent *ent, WIN32_FIND_DATAW *fdata)xwcstoutf (ent->d_name, fdata->cFileName,sizeof (ent->d_name));/* Set file type, based on WIN32_FIND_DATA */ -if (fdata->dwFileAttributes &FILE_ATTRIBUTE_DIRECTORY ) +if ((fdata->dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) + && fdata->dwReserved0 ==IO_REPARSE_TAG_SYMLINK ) + ent->d_type =DT_LNK ; +else if (fdata->dwFileAttributes &FILE_ATTRIBUTE_DIRECTORY ) ent->d_type =DT_DIR ;else ent->d_type =DT_REG ;
Signature · 1 line
gitgitgadget
PATCH v2 6/18 mingw: compute the correct size for symlinks in `mingw_lstat()`
In reply to Johannes Schindelin via GitGitGadgetFrom: Bill Zissimopoulos <[email protected]>
POSIX specifies that upon successful return from lstat(): "the value of the st_size member shall be set to the length of the pathname contained in the symbolic link not including any terminating null byte".
Git typically doesn't trust the stat.st_size member of symlinks (e.g. see strbuf_readlink()). Therefore, it is tempting to save on the extra overhead of opening and reading the reparse point merely to calculate the exact size of the link target.
This is, in fact, what Git for Windows did, from May 2015 to May 2020. At least almost: some functions take shortcuts if st_size is 0 (e.g. diff_populate_filespec()), hence Git for Windows hard-coded the length of all symlinks to MAX_PATH.
This did cause problems, though, specifically in Git repositories that were also accessed by Git for Cygwin or Git for WSL. For example, doing git reset --hard using Git for Windows would update the size of symlinks in the index to be MAX_PATH; at a later time Git for Cygwin or Git for WSL would find that symlinks have changed size during git status and update the index. And then Git for Windows would think that the index needs to be updated. Even if the symlinks did not, in fact, change. To avoid that, the correct size must be determined.
--- compat/mingw.c | 114 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 104 insertions(+), 10 deletions(-)
compat/mingw.c +104 -10
@@ -21,6 +21,7 @@#define SECURITY_WIN32 #include <sspi.h> #include <wchar.h> +#include <winioctl.h> #include <winternl.h> #define STATUS_DELETE_PENDING ((NTSTATUS) 0xC0000056)
@@ -917,10 +918,102 @@ static int has_valid_directory_prefix(wchar_t *wfilename)return 1 ; } +#ifndef _WINNT_H +/* +* The REPARSE_DATA_BUFFER structure is defined in the Windows DDK (in +* ntifs.h) and in MSYS1's winnt.h (which defines _WINNT_H). So define +* it ourselves if we are on MSYS2 (whose winnt.h defines _WINNT_). +*/ +typedef struct _REPARSE_DATA_BUFFER { +DWORD ReparseTag; +WORD ReparseDataLength; +WORD Reserved; +#ifndef _MSC_VER + _ANONYMOUS_UNION +#endif +union { +struct { +WORD SubstituteNameOffset; +WORD SubstituteNameLength; +WORD PrintNameOffset; +WORD PrintNameLength; +ULONG Flags; +WCHAR PathBuffer[1 ]; + } SymbolicLinkReparseBuffer; +struct { +WORD SubstituteNameOffset; +WORD SubstituteNameLength; +WORD PrintNameOffset; +WORD PrintNameLength; +WCHAR PathBuffer[1 ]; + } MountPointReparseBuffer; +struct { +BYTE DataBuffer[1 ]; + } GenericReparseBuffer; + } DUMMYUNIONNAME; +}REPARSE_DATA_BUFFER , *PREPARSE_DATA_BUFFER ; +#endif + +static int read_reparse_point (const WCHAR *wpath,BOOL fail_on_unknown_tag, +char *tmpbuf,int *plen,DWORD *ptag) +{ +HANDLE handle; +WCHAR *wbuf; +REPARSE_DATA_BUFFER *b =alloca (MAXIMUM_REPARSE_DATA_BUFFER_SIZE ); +DWORD dummy; + +/* read reparse point data */ + handle =CreateFileW (wpath,0 , +FILE_SHARE_READ |FILE_SHARE_WRITE |FILE_SHARE_DELETE ,NULL , +OPEN_EXISTING , +FILE_FLAG_BACKUP_SEMANTICS |FILE_FLAG_OPEN_REPARSE_POINT ,NULL ); +if (handle ==INVALID_HANDLE_VALUE ) { + errno =err_win_to_posix (GetLastError ()); +return -1 ; + } +if (!DeviceIoControl (handle,FSCTL_GET_REPARSE_POINT ,NULL ,0 , b, +MAXIMUM_REPARSE_DATA_BUFFER_SIZE , &dummy,NULL )) { + errno =err_win_to_posix (GetLastError ()); +CloseHandle (handle); +return -1 ; + } +CloseHandle (handle); + +/* get target path for symlinks or mount points (aka 'junctions') */ +switch ((*ptag = b->ReparseTag)) { +case IO_REPARSE_TAG_SYMLINK : + wbuf = (WCHAR *) (((char *) b->SymbolicLinkReparseBuffer.PathBuffer) + + b->SymbolicLinkReparseBuffer.SubstituteNameOffset); + *(WCHAR *) (((char *) wbuf) + + b->SymbolicLinkReparseBuffer.SubstituteNameLength) =0 ; +break ; +case IO_REPARSE_TAG_MOUNT_POINT : + wbuf = (WCHAR *) (((char *) b->MountPointReparseBuffer.PathBuffer) + + b->MountPointReparseBuffer.SubstituteNameOffset); + *(WCHAR *) (((char *) wbuf) + + b->MountPointReparseBuffer.SubstituteNameLength) =0 ; +break ; +default : +if (fail_on_unknown_tag) { + errno =EINVAL ; +return -1 ; + }else { + *plen =MAX_PATH ; +return 0 ; + } + } + +if ((*plen = +xwcstoutf (tmpbuf,normalize_ntpath (wbuf),MAX_PATH )) <0 ) +return -1 ; +return 0 ; +} +int mingw_lstat (const char *file_name,struct stat *buf) {WIN32_FILE_ATTRIBUTE_DATA fdata; -WIN32_FIND_DATAW findbuf = {0 }; +DWORD reparse_tag =0 ; +int link_len =0 ;wchar_t wfilename[MAX_PATH ];int wlen =xutftowcs_path (wfilename, file_name);if (wlen <0 )
@@ -935,28 +1028,29 @@ int mingw_lstat(const char *file_name, struct stat *buf) }if (GetFileAttributesExW (wfilename, GetFileExInfoStandard, &fdata)) { -/* for reparse points, use FindFirstFile to get the reparse tag */ +/* for reparse points, get the link tag and length */ if (fdata.dwFileAttributes &FILE_ATTRIBUTE_REPARSE_POINT ) { -HANDLE handle =FindFirstFileW (wfilename, &findbuf); -if (handle ==INVALID_HANDLE_VALUE ) - goto error; -FindClose (handle); +char tmpbuf[MAX_PATH ]; + +if (read_reparse_point (wfilename, FALSE, tmpbuf, + &link_len, &reparse_tag) <0 ) +return -1 ; } buf->st_ino =0 ; buf->st_gid =0 ; buf->st_uid =0 ; buf->st_nlink =1 ; buf->st_mode =file_attr_to_st_mode (fdata.dwFileAttributes, - findbuf.dwReserved0); - buf->st_size = fdata.nFileSizeLow | - (((off_t )fdata.nFileSizeHigh)<<32 ); + reparse_tag); + buf->st_size =S_ISLNK (buf->st_mode) ? link_len : + fdata.nFileSizeLow | (((off_t ) fdata.nFileSizeHigh) <<32 ); buf->st_dev = buf->st_rdev =0 ;/* not used by Git */ filetime_to_timespec (&(fdata.ftLastAccessTime), &(buf->st_atim));filetime_to_timespec (&(fdata.ftLastWriteTime), &(buf->st_mtim));filetime_to_timespec (&(fdata.ftCreationTime), &(buf->st_ctim));return 0 ; } -error: +switch (GetLastError ()) {case ERROR_ACCESS_DENIED :case ERROR_SHARING_VIOLATION :
Signature · 1 line
gitgitgadget
PATCH v2 7/18 mingw: factor out the retry logic
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
In several places, Git's Windows-specific code follows the pattern where it tries to perform an operation, and retries several times when that operation fails, sleeping an increasing amount of time, before finally giving up and asking the user whether to rety (after, say, closing an editor that held a handle to a file, preventing the operation from succeeding).
This logic is a bit hard to use, and inconsistent:mingw_unlink() and mingw_rmdir() duplicate the code to retry, and both of them do so incompletely. They also do not restore errno if the user answers 'no'.
Introduce a retry_ask_yes_no() helper function that handles retry with small delay, asking the user, and restoring errno.
Note that in mingw_unlink(), we include the _wchmod() call in the retry loop (which may fail if the file is locked exclusively).
In mingw_rmdir(), we include special error handling in the retry loop.
--- compat/mingw.c | 104 ++++++++++++++++++++++--------------------------- 1 file changed, 46 insertions(+), 58 deletions(-)
compat/mingw.c +46 -58
@@ -28,8 +28,6 @@#define HCAST (type, handle) ((type)(intptr_t)handle) -static const int delay[] = {0 ,1 ,10 ,20 ,40 }; -void open_in_gdb (void ) {static struct child_process cp =CHILD_PROCESS_INIT ;
@@ -205,15 +203,12 @@ static int read_yes_no_answer(void)return -1 ; } -static int ask_yes_no_if_possible (const char *format, ...) +static int ask_yes_no_if_possible (const char *format,va_list args) {char question[4096 ];const char *retry_hook; -va_list args; -va_start (args, format);vsnprintf (question,sizeof (question), format, args); -va_end (args); retry_hook =mingw_getenv ("GIT_ASK_YESNO" );if (retry_hook) {
@@ -238,6 +233,31 @@ static int ask_yes_no_if_possible(const char *format, ...) } } +static int retry_ask_yes_no (int *tries,const char *format, ...) +{ +static const int delay[] = {0 ,1 ,10 ,20 ,40 }; +va_list args; +int result, saved_errno = errno; + +if ((*tries) <ARRAY_SIZE (delay)) { +/* +* We assume that some other process had the file open at the wrong +* moment and retry. In order to give the other process a higher +* chance to complete its operation, we give up our time slice now. +* If we have to retry again, we do sleep a bit. +*/ +Sleep (delay[*tries]); + (*tries)++; +return 1 ; + } + +va_start (args, format); + result =ask_yes_no_if_possible (format, args); +va_end (args); + errno = saved_errno; +return result; +} +/* Windows only */ enum hide_dotfiles_type {HIDE_DOTFILES_FALSE =0 ,
@@ -298,7 +318,7 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)int mingw_unlink (const char *pathname,int handle_in_use_error) { -int ret, tries =0 ; +int tries =0 ;wchar_t wpathname[MAX_PATH ];if (xutftowcs_path (wpathname, pathname) <0 )return -1 ;
@@ -306,29 +326,19 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)if (DeleteFileW (wpathname))return 0 ; -/* read-only files cannot be removed */ -_wchmod (wpathname,0666 ); -while ((ret =_wunlink (wpathname)) ==-1 && tries <ARRAY_SIZE (delay)) { +do { +/* read-only files cannot be removed */ +_wchmod (wpathname,0666 ); +if (!_wunlink (wpathname)) +return 0 ;if (!is_file_in_use_error (GetLastError ()))break ;if (!handle_in_use_error) -return ret; +return -1 ; -/* -* We assume that some other process had the source or -* destination file open at the wrong moment and retry. -* In order to give the other process a higher chance to -* complete its operation, we give up our time slice now. -* If we have to retry again, we do sleep a bit. -*/ -Sleep (delay[tries]); - tries++; - } -while (ret ==-1 &&is_file_in_use_error (GetLastError ()) && -ask_yes_no_if_possible ("Unlink of file '%s' failed. " -"Should I try again?" , pathname)) - ret =_wunlink (wpathname); -return ret; + }while (retry_ask_yes_no (&tries,"Unlink of file '%s' failed. " +"Should I try again?" , pathname)); +return -1 ; }static int is_dir_empty (const wchar_t *wpath)
@@ -355,7 +365,7 @@ static int is_dir_empty(const wchar_t *wpath)int mingw_rmdir (const char *pathname) { -int ret, tries =0 ; +int tries =0 ;wchar_t wpathname[MAX_PATH ];struct stat st;
@@ -381,7 +391,11 @@ int mingw_rmdir(const char *pathname)if (xutftowcs_path (wpathname, pathname) <0 )return -1 ; -while ((ret =_wrmdir (wpathname)) ==-1 && tries <ARRAY_SIZE (delay)) { + do { + if (!_wrmdir(wpathname)) { + invalidate_lstat_cache(); + return 0; + } if (!is_file_in_use_error(GetLastError())) errno = err_win_to_posix(GetLastError()); if (errno != EACCES)
@@ -390,23 +404,9 @@ int mingw_rmdir(const char *pathname) errno =ENOTEMPTY ;break ; } -/* -* We assume that some other process had the source or -* destination file open at the wrong moment and retry. -* In order to give the other process a higher chance to -* complete its operation, we give up our time slice now. -* If we have to retry again, we do sleep a bit. -*/ -Sleep (delay[tries]); - tries++; - } -while (ret ==-1 && errno ==EACCES &&is_file_in_use_error (GetLastError ()) && -ask_yes_no_if_possible ("Deletion of directory '%s' failed. " -"Should I try again?" , pathname)) - ret =_wrmdir (wpathname); -if (!ret) -invalidate_lstat_cache (); -return ret; + }while (retry_ask_yes_no (&tries,"Deletion of directory '%s' failed. " +"Should I try again?" , pathname)); +return -1 ; }static inline int needs_hiding (const char *path)
@@ -2384,20 +2384,8 @@ repeat:SetFileAttributesW (wpnew, attrs); } } -if (tries <ARRAY_SIZE (delay) && gle ==ERROR_ACCESS_DENIED ) { -/* -* We assume that some other process had the source or -* destination file open at the wrong moment and retry. -* In order to give the other process a higher chance to -* complete its operation, we give up our time slice now. -* If we have to retry again, we do sleep a bit. -*/ -Sleep (delay[tries]); - tries++; - goto repeat; - }if (gle ==ERROR_ACCESS_DENIED && -ask_yes_no_if_possible ("Rename from '%s' to '%s' failed. " +retry_ask_yes_no (&tries,"Rename from '%s' to '%s' failed. " "Should I try again?" , pold, pnew)) goto repeat;
Signature · 1 line
gitgitgadget
PATCH v2 8/18 mingw: change default of `core.symlinks` to false
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
Symlinks on Windows don't work the same way as on Unix systems. For example, there are different types of symlinks for directories and files, and unless using a recent-ish Windows version in Developer Mode, creating symlinks requires administrative privileges.
By default, disable symlink support on Windows. That is, users explicitly have to enable it with git config [--system|--global] core.symlinks true; For convenience, git init (and git clone) will perform a test whether the current setup allows creating symlinks and will configure that setting in the repository config.
The test suite ignores system / global config files. Allow testing *with* symlink support by checking if native symlinks are enabled in MSYS2 (via setting the special environment variable MSYS=winsymlinks:nativestrict to ask the MSYS2 runtime to enable creating symlinks).
Note: This assumes that Git's test suite is run in MSYS2's Bash, which is true for the time being (an experiment to switch to BusyBox-w32 failed due to the experimental nature of BusyBox-w32).
--- compat/mingw.c | 9 +++++++++ 1 file changed, 9 insertions(+)
compat/mingw.c +9 -0
@@ -2862,6 +2862,15 @@ static void setup_windows_environment(void)if (!tmp && (tmp =getenv ("USERPROFILE" )))setenv ("HOME" , tmp,1 ); } + +/* +* Change 'core.symlinks' default to false, unless native symlinks are +* enabled in MSys2 (via 'MSYS=winsymlinks:nativestrict'). Thus we can +* run the test suite (which doesn't obey config files) with or without +* symlink support. +*/ +if (!(tmp =getenv ("MSYS" )) || !strstr (tmp,"winsymlinks:nativestrict" )) + has_symlinks =0 ; }static void get_current_user_sid (PSID *sid,HANDLE *linked_token)
Signature · 1 line
gitgitgadget
PATCH v2 9/18 mingw: add symlink-specific error codes
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
The Win32 API calls do not set errno; Instead, error codes for failed operations must be obtained via the GetLastError() function. Git would not know what to do with those error values, though, which is why Git's Windows compatibility layer translates them to errno values.
Let's handle a couple of symlink-related error codes that will become relevant with the upcoming support for symlinks on Windows.
--- compat/mingw.c | 5 +++++ 1 file changed, 5 insertions(+)
compat/mingw.c +5 -0
@@ -102,6 +102,7 @@ int err_win_to_posix(DWORD winerr)case ERROR_INVALID_PARAMETER : error =EINVAL ;break ;case ERROR_INVALID_PASSWORD : error =EPERM ;break ;case ERROR_INVALID_PRIMARY_GROUP : error =EINVAL ;break ; +case ERROR_INVALID_REPARSE_DATA : error =EINVAL ;break ;case ERROR_INVALID_SIGNAL_NUMBER : error =EINVAL ;break ;case ERROR_INVALID_TARGET_HANDLE : error =EIO ;break ;case ERROR_INVALID_WORKSTATION : error =EACCES ;break ;
@@ -116,6 +117,7 @@ int err_win_to_posix(DWORD winerr)case ERROR_NEGATIVE_SEEK : error =ESPIPE ;break ;case ERROR_NOACCESS : error =EFAULT ;break ;case ERROR_NONE_MAPPED : error =EINVAL ;break ; +case ERROR_NOT_A_REPARSE_POINT : error =EINVAL ;break ;case ERROR_NOT_ENOUGH_MEMORY : error =ENOMEM ;break ;case ERROR_NOT_READY : error =EAGAIN ;break ;case ERROR_NOT_SAME_DEVICE : error =EXDEV ;break ;
@@ -136,6 +138,9 @@ int err_win_to_posix(DWORD winerr)case ERROR_PIPE_NOT_CONNECTED : error =EPIPE ;break ;case ERROR_PRIVILEGE_NOT_HELD : error =EACCES ;break ;case ERROR_READ_FAULT : error =EIO ;break ; +case ERROR_REPARSE_ATTRIBUTE_CONFLICT : error =EINVAL ;break ; +case ERROR_REPARSE_TAG_INVALID : error =EINVAL ;break ; +case ERROR_REPARSE_TAG_MISMATCH : error =EINVAL ;break ;case ERROR_SEEK : error =EIO ;break ;case ERROR_SEEK_ON_DEVICE : error =ESPIPE ;break ;case ERROR_SHARING_BUFFER_EXCEEDED : error =ENFILE ;break ;
Signature · 1 line
gitgitgadget
PATCH v2 11/18 mingw: support renaming symlinks
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
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.
--- compat/mingw.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-)
compat/mingw.c +16 -22
@@ -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;
Signature · 1 line
gitgitgadget
PATCH v2 10/18 mingw: handle symlinks to directories in `mingw_unlink()`
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
The _wunlink() and DeleteFileW() functions refuse to delete symlinks to directories on Windows; The error code would be ERROR_ACCESS_DENIED in that case. Take that error code as an indicator that we need to try _wrmdir() as well. In the best case, it will remove a symlink. In the worst case, it will fail with the same error code again.
--- compat/mingw.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
compat/mingw.c +8 -1
@@ -338,9 +338,16 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)return 0 ;if (!is_file_in_use_error (GetLastError ()))break ; +/* +* _wunlink() / DeleteFileW() for directory symlinks fails with +* ERROR_ACCESS_DENIED (EACCES), so try _wrmdir() as well. This is the +* same error we get if a file is in use (already checked above). +*/ +if (!_wrmdir (wpathname)) +return 0 ; +if (!handle_in_use_error)return -1 ; - }while (retry_ask_yes_no (&tries,"Unlink of file '%s' failed. " "Should I try again?" , pathname));return -1 ;
Signature · 1 line
gitgitgadget
PATCH v2 12/18 mingw: allow `mingw_chdir()` to change to symlink-resolved directories
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
If symlinks are enabled, resolve all symlinks when changing directories, as required by POSIX.
Note: Git's real_path() function bases its link resolution algorithm on this property of chdir(). Unfortunately, the current directory on Windows is limited to only MAX_PATH (260) characters. Therefore using symlinks and long paths in combination may be problematic.
--- compat/mingw.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
compat/mingw.c +19 -1
@@ -866,9 +866,27 @@ int mingw_access(const char *filename, int mode)int mingw_chdir (const char *dirname) {wchar_t wdirname[MAX_PATH ]; +if (xutftowcs_path (wdirname, dirname) <0 )return -1 ; -return _wchdir (wdirname); + +if (has_symlinks) { +HANDLE hnd =CreateFileW (wdirname,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 ; + } +if (!GetFinalPathNameByHandleW (hnd, wdirname,ARRAY_SIZE (wdirname),0 )) { + errno =err_win_to_posix (GetLastError ()); +CloseHandle (hnd); +return -1 ; + } +CloseHandle (hnd); + } + +return _wchdir (normalize_ntpath (wdirname)); }int mingw_chmod (const char *filename,int mode)
Signature · 1 line
gitgitgadget
PATCH v2 13/18 mingw: implement `readlink()`
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
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.
--- compat/mingw-posix.h | 3 +-- compat/mingw.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-)
compat/mingw-posix.h +1 -2
@@ -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
compat/mingw.c +24 -0
@@ -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,
Signature · 1 line
gitgitgadget
PATCH v2 14/18 mingw: implement basic `symlink()` functionality (file symlinks only)
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
Implement symlink(). This implementation always creates _file_ symlinks (remember: Windows discerns between symlinks pointing to directories and those pointing to files). Support for directory symlinks will be added in a subseqeuent commit.
This implementation fails with ENOSYS if symlinks are disabled or unsupported.
--- compat/mingw-posix.h | 3 +-- compat/mingw.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-)
compat/mingw-posix.h +1 -2
@@ -121,8 +121,6 @@ struct utsname {* trivial stubs */ -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) { errno =ENOSYS ;return -1 ; }#ifndef __MINGW64_VERSION_MAJOR
@@ -195,6 +193,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 symlink (const char *target,const char *link);int readlink (const char *path,char *buf,size_t bufsiz);/*
compat/mingw.c +28 -0
@@ -2698,6 +2698,34 @@ int link(const char *oldpath, const char *newpath)return 0 ; } +int symlink (const char *target,const char *link) +{ +wchar_t wtarget[MAX_PATH ], wlink[MAX_PATH ]; +int len; + +/* fail if symlinks are disabled or API is not supported (WinXP) */ +if (!has_symlinks) { + errno =ENOSYS ; +return -1 ; + } + +if ((len =xutftowcs_path (wtarget, target)) <0 + ||xutftowcs_path (wlink, link) <0 ) +return -1 ; + +/* convert target dir separators to backslashes */ +while (len--) +if (wtarget[len] =='/' ) + wtarget[len] ='\\' ; + +/* create file symlink */ +if (!CreateSymbolicLinkW (wlink, wtarget,0 )) { + errno =err_win_to_posix (GetLastError ()); +return -1 ; + } +return 0 ; +} +int readlink (const char *path,char *buf,size_t bufsiz) {WCHAR wpath[MAX_PATH ];
Signature · 1 line
gitgitgadget
PATCH v2 15/18 mingw: add support for symlinks to directories
In reply to Johannes Schindelin via GitGitGadgetFrom: Karsten Blees <[email protected]>
Symlinks on Windows have a flag that indicates whether the target is a file or a directory. Symlinks of wrong type simply don't work. This even affects core Win32 APIs (e.g. DeleteFile() refuses to delete directory symlinks).
However, CreateFile() with FILE_FLAG_BACKUP_SEMANTICS does work. Check the target type by first creating a tentative file symlink, opening it, and checking the type of the resulting handle. If it is a directory, recreate the symlink with the directory flag set.
It is possible to create symlinks before the target exists (or in case of symlinks to symlinks: before the target type is known). If this happens, create a tentative file symlink and postpone the directory
decision: keep a list of phantom symlinks to be processed whenever a new directory is created in mingw_mkdir().
Limitations: This algorithm may fail if a link target changes from file to directory or vice versa, or if the target directory is created in another process. It's the best Git can do, though.
--- compat/mingw.c | 164 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 164 insertions(+)
compat/mingw.c +164 -0
@@ -296,6 +296,131 @@ int mingw_core_config(const char *var, const char *value,return 0 ; } +static inline int is_wdir_sep (wchar_t wchar) +{ +return wchar ==L'/' || wchar ==L'\\' ; +} + +static const wchar_t *make_relative_to (const wchar_t *path, +const wchar_t *relative_to,wchar_t *out, +size_t size) +{ +size_t i =wcslen (relative_to), len; + +/* Is `path` already absolute? */ +if (is_wdir_sep (path[0 ]) || + (iswalpha (path[0 ]) && path[1 ] ==L':' &&is_wdir_sep (path[2 ]))) +return path; + +while (i >0 && !is_wdir_sep (relative_to[i -1 ])) + i--; + +/* Is `relative_to` in the current directory? */ +if (!i) +return path; + + len =wcslen (path); +if (i + len +1 > size) { +error ("Could not make '%ls' relative to '%ls' (too large)" , + path, relative_to); +return NULL ; + } + +memcpy (out, relative_to, i *sizeof (wchar_t)); +wcscpy (out + i, path); +return out; +} + +enum phantom_symlink_result { +PHANTOM_SYMLINK_RETRY , +PHANTOM_SYMLINK_DONE , +PHANTOM_SYMLINK_DIRECTORY +}; + +/* +* Changes a file symlink to a directory symlink if the target exists and is a +* directory. +*/ +static enum phantom_symlink_result +process_phantom_symlink (const wchar_t *wtarget,const wchar_t *wlink) +{ +HANDLE hnd; +BY_HANDLE_FILE_INFORMATION fdata; +wchar_t relative[MAX_PATH ]; +const wchar_t *rel; + +/* check that wlink is still a file symlink */ +if ((GetFileAttributesW (wlink) + & (FILE_ATTRIBUTE_REPARSE_POINT |FILE_ATTRIBUTE_DIRECTORY )) + !=FILE_ATTRIBUTE_REPARSE_POINT ) +return PHANTOM_SYMLINK_DONE ; + +/* make it relative, if necessary */ + rel =make_relative_to (wtarget, wlink, relative,ARRAY_SIZE (relative)); +if (!rel) +return PHANTOM_SYMLINK_DONE ; + +/* let Windows resolve the link by opening it */ + hnd =CreateFileW (rel,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 PHANTOM_SYMLINK_RETRY ; + } + +if (!GetFileInformationByHandle (hnd, &fdata)) { + errno =err_win_to_posix (GetLastError ()); +CloseHandle (hnd); +return PHANTOM_SYMLINK_RETRY ; + } +CloseHandle (hnd); + +/* if target exists and is a file, we're done */ +if (!(fdata.dwFileAttributes &FILE_ATTRIBUTE_DIRECTORY )) +return PHANTOM_SYMLINK_DONE ; + +/* otherwise recreate the symlink with directory flag */ +if (DeleteFileW (wlink) &&CreateSymbolicLinkW (wlink, wtarget,1 )) +return PHANTOM_SYMLINK_DIRECTORY ; + + errno =err_win_to_posix (GetLastError ()); +return PHANTOM_SYMLINK_RETRY ; +} + +/* keep track of newly created symlinks to non-existing targets */ +struct phantom_symlink_info { +struct phantom_symlink_info *next; +wchar_t *wlink; +wchar_t *wtarget; +}; + +static struct phantom_symlink_info *phantom_symlinks =NULL ; +static CRITICAL_SECTION phantom_symlinks_cs; + +static void process_phantom_symlinks (void ) +{ +struct phantom_symlink_info *current, **psi; +EnterCriticalSection (&phantom_symlinks_cs); +/* process phantom symlinks list */ + psi = &phantom_symlinks; +while ((current = *psi)) { +enum phantom_symlink_result result =process_phantom_symlink ( + current->wtarget, current->wlink); +if (result ==PHANTOM_SYMLINK_RETRY ) { + psi = ¤t->next; + }else { +/* symlink was processed, remove from list */ + *psi = current->next; +free (current); +/* if symlink was a directory, start over */ +if (result ==PHANTOM_SYMLINK_DIRECTORY ) + psi = &phantom_symlinks; + } + } +LeaveCriticalSection (&phantom_symlinks_cs); +} +/* Normalizes NT paths as returned by some low-level APIs. */ static wchar_t *normalize_ntpath (wchar_t *wbuf) {
@@ -479,6 +604,8 @@ int mingw_mkdir(const char *path, int mode UNUSED)if (xutftowcs_path (wpath, path) <0 )return -1 ; ret =_wmkdir (wpath); +if (!ret) +process_phantom_symlinks ();if (!ret &&needs_hiding (path))return set_hidden_flag (wpath,1 );return ret;
@@ -2723,6 +2850,42 @@ int symlink(const char *target, const char *link) errno =err_win_to_posix (GetLastError ());return -1 ; } + +/* convert to directory symlink if target exists */ +switch (process_phantom_symlink (wtarget, wlink)) { +case PHANTOM_SYMLINK_RETRY : { +/* if target doesn't exist, add to phantom symlinks list */ +wchar_t wfullpath[MAX_PATH ]; +struct phantom_symlink_info *psi; + +/* convert to absolute path to be independent of cwd */ + len =GetFullPathNameW (wlink,MAX_PATH , wfullpath,NULL ); +if (!len || len >=MAX_PATH ) { + errno =err_win_to_posix (GetLastError ()); +return -1 ; + } + +/* over-allocate and fill phantom_symlink_info structure */ + psi =xmalloc (sizeof (struct phantom_symlink_info ) + +sizeof (wchar_t) * (len +wcslen (wtarget) +2 )); + psi->wlink = (wchar_t *)(psi +1 ); +wcscpy (psi->wlink, wfullpath); + psi->wtarget = psi->wlink + len +1 ; +wcscpy (psi->wtarget, wtarget); + +EnterCriticalSection (&phantom_symlinks_cs); + psi->next = phantom_symlinks; + phantom_symlinks = psi; +LeaveCriticalSection (&phantom_symlinks_cs); +break ; + } +case PHANTOM_SYMLINK_DIRECTORY : +/* if we created a dir symlink, process other phantom symlinks */ +process_phantom_symlinks (); +break ; +default : +break ; + }return 0 ; }
@@ -3424,6 +3587,7 @@ int wmain(int argc, const wchar_t **wargv)/* initialize critical section for waitpid pinfo_t list */ InitializeCriticalSection (&pinfo_cs); +InitializeCriticalSection (&phantom_symlinks_cs);/* set up default file mode and file modes for stdin/out/err */ _fmode = _O_BINARY;
Signature · 1 line
gitgitgadget
PATCH v2 16/18 mingw: try to create symlinks without elevated permissions
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
As of Windows 10 Build 14972 in Developer Mode, a new flag is supported by CreateSymbolicLink() to create symbolic links even when running outside of an elevated session (which was previously required).
This new flag is called SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE and has the numeric value 0x02.
Previous Windows 10 versions will not understand that flag and return an ERROR_INVALID_PARAMETER, therefore we have to be careful to try passing that flag only when the build number indicates that it is supported.
For more information about the new flag, see this blog post:
https://blogs.windows.com/buildingapps/2016/12/02/symlinks-windows-10/
--- compat/mingw.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)
compat/mingw.c +24 -2
@@ -331,6 +331,8 @@ static const wchar_t *make_relative_to(const wchar_t *path,return out; } +static DWORD symlink_file_flags =0 , symlink_directory_flags =1 ; +enum phantom_symlink_result {PHANTOM_SYMLINK_RETRY ,PHANTOM_SYMLINK_DONE ,
@@ -381,7 +383,8 @@ process_phantom_symlink(const wchar_t *wtarget, const wchar_t *wlink)return PHANTOM_SYMLINK_DONE ;/* otherwise recreate the symlink with directory flag */ -if (DeleteFileW (wlink) &&CreateSymbolicLinkW (wlink, wtarget,1 )) +if (DeleteFileW (wlink) && +CreateSymbolicLinkW (wlink, wtarget, symlink_directory_flags))return PHANTOM_SYMLINK_DIRECTORY ; errno =err_win_to_posix (GetLastError ());
@@ -2846,7 +2849,7 @@ int symlink(const char *target, const char *link) wtarget[len] ='\\' ;/* create file symlink */ -if (!CreateSymbolicLinkW (wlink, wtarget,0 )) { +if (!CreateSymbolicLinkW (wlink, wtarget, symlink_file_flags)) { errno =err_win_to_posix (GetLastError ());return -1 ; }
@@ -3523,6 +3526,24 @@ static void maybe_redirect_std_handles(void) GENERIC_WRITE, FILE_FLAG_NO_BUFFERING); } +static void adjust_symlink_flags (void ) +{ +/* +* Starting with Windows 10 Build 14972, symbolic links can be created +* using CreateSymbolicLink() without elevation by passing the flag +* SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE (0x02) as last +* parameter, provided the Developer Mode has been enabled. Some +* earlier Windows versions complain about this flag with an +* ERROR_INVALID_PARAMETER, hence we have to test the build number +* specifically. +*/ +if (GetVersion () >=14972 <<16 ) { + symlink_file_flags |=2 ; + symlink_directory_flags |=2 ; + } + +} +#ifdef _MSC_VER#ifdef _DEBUG#include <crtdbg.h>
@@ -3558,6 +3579,7 @@ int wmain(int argc, const wchar_t **wargv)#endif maybe_redirect_std_handles (); +adjust_symlink_flags ();/* determine size of argv and environ conversion buffer */ maxlen =wcslen (wargv[0 ]);
Signature · 1 line
gitgitgadget
PATCH v2 17/18 mingw: emulate `stat()` a little more faithfully
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When creating directories via safe_create_leading_directories(), we might encounter an already-existing directory which is not readable by the current user. To handle that situation, Git's code calls stat() to determine whether we're looking at a directory.
In such a case, CreateFile() will fail, though, no matter what, and consequently mingw_stat() will fail, too. But POSIX semantics seem to still allow stat() to go forward.
So let's call mingw_lstat() to the rescue if we fail to get a file handle due to denied permission in mingw_stat(), and fill the stat info that way.
We need to be careful to not allow this to go forward in case that we're looking at a symbolic link: to resolve the link, we would still have to create a file handle, and we just found out that we cannot. Therefore, stat() still needs to fail with EACCES in that case.
This fixes https://github.com/git-for-windows/git/issues/2531.
--- compat/mingw.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
compat/mingw.c +13 -1
@@ -1273,7 +1273,19 @@ int mingw_stat(const char *file_name, struct stat *buf)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 ()); +DWORD err =GetLastError (); + +if (err ==ERROR_ACCESS_DENIED && + !mingw_lstat (file_name, buf) && + !S_ISLNK (buf->st_mode)) +/* +* POSIX semantics state to still try to fill +* information, even if permission is denied to create +* a file handle. +*/ +return 0 ; + + errno =err_win_to_posix (err);return -1 ; } result =get_file_info_by_handle (hnd, buf);
Signature · 1 line
gitgitgadget
PATCH v2 18/18 mingw: special-case index entries for symlinks with buggy size
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
In https://github.com/git-for-windows/git/pull/2637, we fixed a bug where symbolic links' target path sizes were recorded incorrectly in the index. The downside of this fix was that every user with tracked symbolic links in their checkouts would see them as modified in git status, but not in git diff, and only a git add <path> (or git add -u) would "fix" this.
Let's do better than that: we can detect that situation and simply pretend that a symbolic link with a known bad size (or a size that just happens to be that bad size, a _very_ unlikely scenario because it would overflow our buffers due to the trailing NUL byte) means that it needs to be re-checked as if we had just checked it out.
--- read-cache.c | 11 +++++++++++ 1 file changed, 11 insertions(+)
read-cache.c +11 -0
@@ -470,6 +470,17 @@ int ie_modified(struct index_state *istate,* then we know it is. */ if ((changed &DATA_CHANGED ) && +#ifdef GIT_WINDOWS_NATIVE +/* +* Work around Git for Windows v2.27.0 fixing a bug where symlinks' +* target path lengths were not read at all, and instead recorded +* as 4096: now, all symlinks would appear as modified. +* +* So let's just special-case symlinks with a target path length +* (i.e. `sd_size`) of 4096 and force them to be re-checked. +*/ + (!S_ISLNK (st->st_mode) || ce->ce_stat_data.sd_size !=MAX_PATH ) && +#endif (S_ISGITLINK (ce->ce_mode) || ce->ce_stat_data.sd_size !=0 ))return changed;
Signature · 1 line
gitgitgadget