From 34522d3979caf8b72bf10ea74c3d8ac34421932b Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 26 Oct 2018 11:13:45 +0200 Subject: [PATCH 1/4] Win32: symlink: move phantom symlink creation to a separate function Signed-off-by: Bert Belder --- compat/mingw.c | 91 +++++++++++++++++++++++++++----------------------- 1 file changed, 49 insertions(+), 42 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 1874985f7e8889..ff00e463cbae1f 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -427,6 +427,54 @@ static void process_phantom_symlinks(void) LeaveCriticalSection(&phantom_symlinks_cs); } +static int create_phantom_symlink(wchar_t *wtarget, wchar_t *wlink) +{ + int len; + + /* create file symlink */ + if (!CreateSymbolicLinkW(wlink, wtarget, symlink_file_flags)) { + 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; +} + /* Normalizes NT paths as returned by some low-level APIs. */ static wchar_t *normalize_ntpath(wchar_t *wbuf) { @@ -2631,48 +2679,7 @@ int symlink(const char *target, const char *link) if (wtarget[len] == '/') wtarget[len] = '\\'; - /* create file symlink */ - if (!CreateSymbolicLinkW(wlink, wtarget, symlink_file_flags)) { - 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; + return create_phantom_symlink(wtarget, wlink); } #ifndef _WINNT_H From 48af1ac033d7bf31365294c7be0eb7678fb6f175 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 11 Feb 2019 14:19:18 +0100 Subject: [PATCH 2/4] Introduce helper to create symlinks that knows about index_state On Windows, symbolic links actually have a type depending on the target: it can be a file or a directory. In certain circumstances, this poses problems, e.g. when a symbolic link is supposed to point into a submodule that is not checked out, so there is no way for Git to auto-detect the type. To help with that, we will add support over the course of the next commits to specify that symlink type via the Git attributes. This requires an index_state, though, something that Git for Windows' `symlink()` replacement cannot know about because the function signature is defined by the POSIX standard and not ours to change. So let's introduce a helper function to create symbolic links that *does* know about the index_state. Signed-off-by: Johannes Schindelin --- apply.c | 2 +- builtin/difftool.c | 2 +- compat/mingw.c | 2 +- compat/mingw.h | 4 +++- entry.c | 2 +- git-compat-util.h | 9 +++++++++ merge-recursive.c | 2 +- refs/files-backend.c | 2 +- setup.c | 4 ++-- 9 files changed, 20 insertions(+), 9 deletions(-) diff --git a/apply.c b/apply.c index 6e1060a952c317..d2a34abafe480e 100644 --- a/apply.c +++ b/apply.c @@ -4440,7 +4440,7 @@ static int try_create_file(struct apply_state *state, const char *path, /* Although buf:size is counted string, it also is NUL * terminated. */ - return !!symlink(buf, path); + return !!create_symlink(state && state->repo ? state->repo->index : NULL, buf, path); fd = open(path, O_CREAT | O_EXCL | O_WRONLY, (mode & 0100) ? 0777 : 0666); if (fd < 0) diff --git a/builtin/difftool.c b/builtin/difftool.c index dcc68e190c25b4..5cca20276a06c2 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -523,7 +523,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, } add_path(&wtdir, wtdir_len, dst_path); if (symlinks) { - if (symlink(wtdir.buf, rdir.buf)) { + if (create_symlink(lstate.istate, wtdir.buf, rdir.buf)) { ret = error_errno("could not symlink '%s' to '%s'", wtdir.buf, rdir.buf); goto finish; } diff --git a/compat/mingw.c b/compat/mingw.c index ff00e463cbae1f..1e4ef595337dd3 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2659,7 +2659,7 @@ int link(const char *oldpath, const char *newpath) return 0; } -int symlink(const char *target, const char *link) +int mingw_create_symlink(struct index_state *index, const char *target, const char *link) { wchar_t wtarget[MAX_PATH], wlink[MAX_PATH]; int len; diff --git a/compat/mingw.h b/compat/mingw.h index ef0a2c0a25c9cf..89d2452f9b1b26 100644 --- a/compat/mingw.h +++ b/compat/mingw.h @@ -215,8 +215,10 @@ 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); +struct index_state; +int mingw_create_symlink(struct index_state *index, const char *target, const char *link); +#define create_symlink mingw_create_symlink /* * replacements of existing functions diff --git a/entry.c b/entry.c index 3143b9996bf491..d61f509ad6c0dc 100644 --- a/entry.c +++ b/entry.c @@ -322,7 +322,7 @@ static int write_entry(struct cache_entry *ce, char *path, struct conv_attrs *ca if (!has_symlinks || to_tempfile) goto write_file_entry; - ret = symlink(new_blob, path); + ret = create_symlink(state->istate, new_blob, path); free(new_blob); if (ret) return error_errno("unable to create symlink %s", path); diff --git a/git-compat-util.h b/git-compat-util.h index 71b4d23f0386b2..2b6c5457a6e35e 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -603,6 +603,15 @@ static inline int git_has_dir_sep(const char *path) #define has_dir_sep(path) git_has_dir_sep(path) #endif +#ifndef create_symlink +struct index_state; +static inline int git_create_symlink(struct index_state *index, const char *target, const char *link) +{ + return symlink(target, link); +} +#define create_symlink git_create_symlink +#endif + #ifndef query_user_email #define query_user_email() NULL #endif diff --git a/merge-recursive.c b/merge-recursive.c index ed64a4c537ca89..46be3b03286b9f 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1004,7 +1004,7 @@ static int update_file_flags(struct merge_options *opt, char *lnk = xmemdupz(buf, size); safe_create_leading_directories_const(path); unlink(path); - if (symlink(lnk, path)) + if (create_symlink(&opt->priv->orig_index, lnk, path)) ret = err(opt, _("failed to symlink '%s': %s"), path, strerror(errno)); free(lnk); diff --git a/refs/files-backend.c b/refs/files-backend.c index 1cff65f6ae548b..0d4562d616e89c 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1952,7 +1952,7 @@ static int create_ref_symlink(struct ref_lock *lock, const char *target) #ifndef NO_SYMLINK_HEAD char *ref_path = get_locked_file_path(&lock->lk); unlink(ref_path); - ret = symlink(target, ref_path); + ret = create_symlink(NULL, target, ref_path); free(ref_path); if (ret) diff --git a/setup.c b/setup.c index f790289685e950..cd914dcbdcc363 100644 --- a/setup.c +++ b/setup.c @@ -2012,7 +2012,7 @@ static void copy_templates_1(struct strbuf *path, struct strbuf *template_path, if (strbuf_readlink(&lnk, template_path->buf, st_template.st_size) < 0) die_errno(_("cannot readlink '%s'"), template_path->buf); - if (symlink(lnk.buf, path->buf)) + if (create_symlink(NULL, lnk.buf, path->buf)) die_errno(_("cannot symlink '%s' '%s'"), lnk.buf, path->buf); strbuf_release(&lnk); @@ -2259,7 +2259,7 @@ static int create_default_files(const char *template_path, path = git_path_buf(&buf, "tXXXXXX"); if (!close(xmkstemp(path)) && !unlink(path) && - !symlink("testing", path) && + !create_symlink(NULL, "testing", path) && !lstat(path, &st1) && S_ISLNK(st1.st_mode)) unlink(path); /* good */ From 79f09c95c8a373b057aa7a67d7007360aafed854 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 26 Oct 2018 11:51:51 +0200 Subject: [PATCH 3/4] mingw: allow to specify the symlink type in .gitattributes On Windows, symbolic links have a type: a "file symlink" must point at a file, and a "directory symlink" must point at a directory. If the type of symlink does not match its target, it doesn't work. Git does not record the type of symlink in the index or in a tree. On checkout it'll guess the type, which only works if the target exists at the time the symlink is created. This may often not be the case, for example when the link points at a directory inside a submodule. By specifying `symlink=file` or `symlink=dir` the user can specify what type of symlink Git should create, so Git doesn't have to rely on unreliable heuristics. Signed-off-by: Bert Belder Signed-off-by: Johannes Schindelin --- Documentation/gitattributes.txt | 30 +++++++++++++++++ compat/mingw.c | 58 ++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 1 deletion(-) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index e6150595af8210..f2c07fa1b72be7 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -403,6 +403,36 @@ sign `$` upon checkout. Any byte sequence that begins with with `$Id$` upon check-in. +`symlink` +^^^^^^^^^ + +On Windows, symbolic links have a type: a "file symlink" must point at +a file, and a "directory symlink" must point at a directory. If the +type of symlink does not match its target, it doesn't work. + +Git does not record the type of symlink in the index or in a tree. On +checkout it'll guess the type, which only works if the target exists +at the time the symlink is created. This may often not be the case, +for example when the link points at a directory inside a submodule. + +The `symlink` attribute allows you to explicitly set the type of symlink +to `file` or `dir`, so Git doesn't have to guess. If you have a set of +symlinks that point at other files, you can do: + +------------------------ +*.gif symlink=file +------------------------ + +To tell Git that a symlink points at a directory, use: + +------------------------ +tools_folder symlink=dir +------------------------ + +The `symlink` attribute is ignored on platforms other than Windows, +since they don't distinguish between different types of symlinks. + + `filter` ^^^^^^^^ diff --git a/compat/mingw.c b/compat/mingw.c index 1e4ef595337dd3..7e7a88cc45463f 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -19,6 +19,7 @@ #include "gettext.h" #define SECURITY_WIN32 #include +#include "../attr.h" #define HCAST(type, handle) ((type)(intptr_t)handle) @@ -2659,6 +2660,37 @@ int link(const char *oldpath, const char *newpath) return 0; } +enum symlink_type { + SYMLINK_TYPE_UNSPECIFIED = 0, + SYMLINK_TYPE_FILE, + SYMLINK_TYPE_DIRECTORY, +}; + +static enum symlink_type check_symlink_attr(struct index_state *index, const char *link) +{ + static struct attr_check *check; + const char *value; + + if (!index) + return SYMLINK_TYPE_UNSPECIFIED; + + if (!check) + check = attr_check_initl("symlink", NULL); + + git_check_attr(index, link, check); + + value = check->items[0].value; + if (ATTR_UNSET(value)) + return SYMLINK_TYPE_UNSPECIFIED; + if (!strcmp(value, "file")) + return SYMLINK_TYPE_FILE; + if (!strcmp(value, "dir") || !strcmp(value, "directory")) + return SYMLINK_TYPE_DIRECTORY; + + warning(_("ignoring invalid symlink type '%s' for '%s'"), value, link); + return SYMLINK_TYPE_UNSPECIFIED; +} + int mingw_create_symlink(struct index_state *index, const char *target, const char *link) { wchar_t wtarget[MAX_PATH], wlink[MAX_PATH]; @@ -2679,7 +2711,31 @@ int mingw_create_symlink(struct index_state *index, const char *target, const ch if (wtarget[len] == '/') wtarget[len] = '\\'; - return create_phantom_symlink(wtarget, wlink); + switch (check_symlink_attr(index, link)) { + case SYMLINK_TYPE_UNSPECIFIED: + /* Create a phantom symlink: it is initially created as a file + * symlink, but may change to a directory symlink later if/when + * the target exists. */ + return create_phantom_symlink(wtarget, wlink); + case SYMLINK_TYPE_FILE: + if (!CreateSymbolicLinkW(wlink, wtarget, symlink_file_flags)) + break; + return 0; + case SYMLINK_TYPE_DIRECTORY: + if (!CreateSymbolicLinkW(wlink, wtarget, + symlink_directory_flags)) + break; + /* There may be dangling phantom symlinks that point at this + * one, which should now morph into directory symlinks. */ + process_phantom_symlinks(); + return 0; + default: + BUG("unhandled symlink type"); + } + + /* CreateSymbolicLinkW failed. */ + errno = err_win_to_posix(GetLastError()); + return -1; } #ifndef _WINNT_H From c97d837c48f1e172f4fa35b1fd55b2ab44ad3565 Mon Sep 17 00:00:00 2001 From: Bert Belder Date: Fri, 26 Oct 2018 23:42:09 +0200 Subject: [PATCH 4/4] Win32: symlink: add test for `symlink` attribute To verify that the symlink is resolved correctly, we use the fact that `git.exe` is a native Win32 program, and that `git.exe config -f ` therefore uses the native symlink resolution. Signed-off-by: Bert Belder Signed-off-by: Johannes Schindelin --- t/t2040-checkout-symlink-attr.sh | 46 ++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100755 t/t2040-checkout-symlink-attr.sh diff --git a/t/t2040-checkout-symlink-attr.sh b/t/t2040-checkout-symlink-attr.sh new file mode 100755 index 00000000000000..e00c31d096ce88 --- /dev/null +++ b/t/t2040-checkout-symlink-attr.sh @@ -0,0 +1,46 @@ +#!/bin/sh + +test_description='checkout symlinks with `symlink` attribute on Windows + +Ensures that Git for Windows creates symlinks of the right type, +as specified by the `symlink` attribute in `.gitattributes`.' + +# Tell MSYS to create native symlinks. Without this flag test-lib's +# prerequisite detection for SYMLINKS doesn't detect the right thing. +MSYS=winsymlinks:nativestrict && export MSYS + +. ./test-lib.sh + +if ! test_have_prereq MINGW,SYMLINKS +then + skip_all='skipping $0: MinGW-only test, which requires symlink support.' + test_done +fi + +# Adds a symlink to the index without clobbering the work tree. +cache_symlink () { + sha=$(printf '%s' "$1" | git hash-object --stdin -w) && + git update-index --add --cacheinfo 120000,$sha,"$2" +} + +test_expect_success 'checkout symlinks with attr' ' + cache_symlink file1 file-link && + cache_symlink dir dir-link && + + printf "file-link symlink=file\ndir-link symlink=dir\n" >.gitattributes && + git add .gitattributes && + + git checkout . && + + mkdir dir && + echo "[a]b=c" >file1 && + echo "[x]y=z" >dir/file2 && + + # MSYS2 is very forgiving, it will resolve symlinks even if the + # symlink type is incorrect. To make this test meaningful, try + # them with a native, non-MSYS executable, such as `git config`. + test "$(git config -f file-link a.b)" = "c" && + test "$(git config -f dir-link/file2 x.y)" = "z" +' + +test_done