From ef3c8ad25962344afe896686ccc35a761d54f13f 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 1b10236541..83175dcfe5 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -477,6 +477,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_LONG_PATH]; + struct phantom_symlink_info *psi; + + /* convert to absolute path to be independent of cwd */ + len = GetFullPathNameW(wlink, MAX_LONG_PATH, wfullpath, NULL); + if (!len || len >= MAX_LONG_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) { @@ -3289,48 +3337,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); } int readlink(const char *path, char *buf, size_t bufsiz) From 392cdcf6ea10c6db7e0849f3b56b5b1288c55c97 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-posix.h | 4 +++- compat/mingw.c | 2 +- entry.c | 2 +- git-compat-util.h | 10 ++++++++++ refs/files-backend.c | 2 +- setup.c | 4 ++-- 8 files changed, 20 insertions(+), 8 deletions(-) diff --git a/apply.c b/apply.c index 249248d4f2..749b515ffc 100644 --- a/apply.c +++ b/apply.c @@ -4515,7 +4515,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 2a21005f2e..35e363c05f 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -544,7 +544,7 @@ static int run_dir_diff(struct repository *repo, } add_path(&wtdir, wtdir_len, dst_path); if (dt_options->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-posix.h b/compat/mingw-posix.h index 1a917890b1..9158f89d89 100644 --- a/compat/mingw-posix.h +++ b/compat/mingw-posix.h @@ -193,8 +193,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/compat/mingw.c b/compat/mingw.c index 83175dcfe5..faaec43314 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -3317,7 +3317,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 UNUSED, const char *target, const char *link) { wchar_t wtarget[MAX_LONG_PATH], wlink[MAX_LONG_PATH]; int len; diff --git a/entry.c b/entry.c index 33c0aacb45..196fb6acda 100644 --- a/entry.c +++ b/entry.c @@ -324,7 +324,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 a7cc548b2a..0fb51b936b 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -359,6 +359,16 @@ static inline int git_has_dir_sep(const char *path) #define is_mount_point is_mount_point_via_stat #endif +#ifndef create_symlink +struct index_state; +static inline int git_create_symlink(struct index_state *index UNUSED, + 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/refs/files-backend.c b/refs/files-backend.c index a4c7858787..be15e3ad41 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2099,7 +2099,7 @@ static int create_ref_symlink(struct ref_lock *lock, const char *target) 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 52d33c1583..399f3cdbcd 100644 --- a/setup.c +++ b/setup.c @@ -2343,7 +2343,7 @@ static void copy_templates_1(struct repository *repo, 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); @@ -2625,7 +2625,7 @@ static int create_default_files(struct repository *repo, repo_git_path_replace(repo, &path, "tXXXXXX"); if (!close(xmkstemp(path.buf)) && !unlink(path.buf) && - !symlink("testing", path.buf) && + !create_symlink(NULL, "testing", path.buf) && !lstat(path.buf, &st1) && S_ISLNK(st1.st_mode)) unlink(path.buf); /* good */ From e0ba10f9606170d00982b8b5234cb85af7f87c4e 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.adoc | 30 ++++++++++++++++ compat/mingw.c | 60 ++++++++++++++++++++++++++++++-- 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/Documentation/gitattributes.adoc b/Documentation/gitattributes.adoc index bd76167a45..579d9940f6 100644 --- a/Documentation/gitattributes.adoc +++ b/Documentation/gitattributes.adoc @@ -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 faaec43314..4bbb9f24f1 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -4,6 +4,7 @@ #include "git-compat-util.h" #include "abspath.h" #include "alloc.h" +#include "attr.h" #include "config.h" #include "dir.h" #include "environment.h" @@ -3317,7 +3318,38 @@ int link(const char *oldpath, const char *newpath) return 0; } -int mingw_create_symlink(struct index_state *index UNUSED, const char *target, const char *link) +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_LONG_PATH], wlink[MAX_LONG_PATH]; int len; @@ -3337,7 +3369,31 @@ int mingw_create_symlink(struct index_state *index UNUSED, const char *target, c 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; } int readlink(const char *path, char *buf, size_t bufsiz) From b4a0182c88e0684040935bc21e0ea3557b2e19b1 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/meson.build | 1 + t/t2040-checkout-symlink-attr.sh | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100755 t/t2040-checkout-symlink-attr.sh diff --git a/t/meson.build b/t/meson.build index 2358338bc5..4a49c20e7d 100644 --- a/t/meson.build +++ b/t/meson.build @@ -277,6 +277,7 @@ integration_tests = [ 't2027-checkout-track.sh', 't2030-unresolve-info.sh', 't2031-checkout-long-paths.sh', + 't2040-checkout-symlink-attr.sh', 't2050-git-dir-relative.sh', 't2060-switch.sh', 't2070-restore.sh', diff --git a/t/t2040-checkout-symlink-attr.sh b/t/t2040-checkout-symlink-attr.sh new file mode 100755 index 0000000000..e00c31d096 --- /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