From f939773eeda6f4a02e731418f51abc6914fef0e5 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 ebe89850c1..bbae91907d 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -444,6 +444,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) { @@ -2871,48 +2919,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_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; + return create_phantom_symlink(wtarget, wlink); } #ifndef _WINNT_H From 0221500f083eecad371dce8d8a640e2ecf476693 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 +- builtin/init-db.c | 4 ++-- 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 +- 9 files changed, 20 insertions(+), 9 deletions(-) diff --git a/apply.c b/apply.c index 2b7cd930ef..f86a01b260 100644 --- a/apply.c +++ b/apply.c @@ -4366,7 +4366,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 4b10ad1a36..a133424fe3 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -513,7 +513,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/builtin/init-db.c b/builtin/init-db.c index f33d18e853..665d2da51b 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -78,7 +78,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); @@ -300,7 +300,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 */ diff --git a/compat/mingw.c b/compat/mingw.c index bbae91907d..9929ef0e14 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2899,7 +2899,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_LONG_PATH], wlink[MAX_LONG_PATH]; int len; diff --git a/compat/mingw.h b/compat/mingw.h index 4bd76ab741..56a3e8b9c1 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 87ff624a5e..c9a701db18 100644 --- a/entry.c +++ b/entry.c @@ -306,7 +306,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 a6a7d314c6..cbaf0a4276 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -558,6 +558,15 @@ 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, 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 4ddd3adea0..005b0cfd54 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -993,7 +993,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 e4009b3c42..2ed8f890da 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1880,7 +1880,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) From 6ad3d3db7372717de578088ce65f6262c37ec20c 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 4b36d51beb..85faf9c142 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -385,6 +385,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 9929ef0e14..42af051679 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -14,6 +14,7 @@ #define SECURITY_WIN32 #include #include "win32/fscache.h" +#include "../attr.h" #define HCAST(type, handle) ((type)(intptr_t)handle) @@ -2899,6 +2900,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_LONG_PATH], wlink[MAX_LONG_PATH]; @@ -2919,7 +2951,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 1e902dbf9a99e96e042510e2ce055b8a4c26dfc0 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 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