From 0593c1ea30737cf2d61f7c191d3687b37badf3be Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 Jul 2024 21:08:18 +0000 Subject: [PATCH 1/7] run-command: refactor getting the Unix shell path into its own function This encapsulates the platform-specific logic better. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- run-command.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/run-command.c b/run-command.c index d9f80fabe6..59e433bf91 100644 --- a/run-command.c +++ b/run-command.c @@ -274,17 +274,22 @@ int sane_execvp(const char *file, char * const argv[]) return -1; } +static const char *git_shell_path(void) +{ +#ifndef GIT_WINDOWS_NATIVE + return SHELL_PATH; +#else + return "sh"; +#endif +} + static const char **prepare_shell_cmd(struct strvec *out, const char **argv) { if (!argv[0]) BUG("shell command is empty"); if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) { -#ifndef GIT_WINDOWS_NATIVE - strvec_push(out, SHELL_PATH); -#else - strvec_push(out, "sh"); -#endif + strvec_push(out, git_shell_path()); strvec_push(out, "-c"); /* From ce68178a0ae222b71fe5f89f05dc7113267c079b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 Jul 2024 21:08:19 +0000 Subject: [PATCH 2/7] strvec: declare the `strvec_push_nodup()` function globally This function differs from `strvec_push()` in that it takes ownership of the allocated string that is passed as second argument. This is useful when appending elements to the string array that have been freshly allocated and serve no further other purpose after that. Without declaring this function globally, call sites would allocate the memory, only to have `strvec_push()` duplicate the string, and then the first copy would need to be released. Having this function globally avoids that kind of unnecessary work. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- strvec.c | 2 +- strvec.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/strvec.c b/strvec.c index d4073ec9fa..f712070f57 100644 --- a/strvec.c +++ b/strvec.c @@ -10,7 +10,7 @@ void strvec_init(struct strvec *array) memcpy(array, &blank, sizeof(*array)); } -static void strvec_push_nodup(struct strvec *array, const char *value) +void strvec_push_nodup(struct strvec *array, char *value) { if (array->v == empty_strvec) array->v = NULL; diff --git a/strvec.h b/strvec.h index 6c7e8b7d50..4b73c1f092 100644 --- a/strvec.h +++ b/strvec.h @@ -46,6 +46,9 @@ void strvec_init(struct strvec *); /* Push a copy of a string onto the end of the array. */ const char *strvec_push(struct strvec *, const char *); +/* Push an allocated string onto the end of the array, taking ownership. */ +void strvec_push_nodup(struct strvec *array, char *value); + /** * Format a string and push it onto the end of the array. This is a * convenience wrapper combining `strbuf_addf` and `strvec_push`. From 193eda7507d0ccb2fe6fd42b403c56ecc205e546 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 Jul 2024 21:08:20 +0000 Subject: [PATCH 3/7] win32: override `fspathcmp()` with a directory separator-aware version On Windows, the backslash is the directory separator, even if the forward slash can be used, too, at least since Windows NT. This means that the paths `a/b` and `a\b` are equivalent, and `fspathcmp()` needs to be made aware of that fact. Note that we have to override both `fspathcmp()` and `fspathncmp()`, and the former cannot be a mere pre-processor constant that transforms calls to `fspathcmp(a, b)` into `fspathncmp(a, b, (size_t)-1)` because the function `report_collided_checkout()` in `unpack-trees.c` wants to assign `list.cmp = fspathcmp`. Also note that `fspatheq()` does _not_ need to be overridden because it calls `fspathcmp()` internally. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- compat/win32/path-utils.c | 37 +++++++++++++++++++++++++++++++++++++ compat/win32/path-utils.h | 4 ++++ dir.c | 4 ++-- dir.h | 4 ++-- git-compat-util.h | 8 ++++++++ 5 files changed, 53 insertions(+), 4 deletions(-) diff --git a/compat/win32/path-utils.c b/compat/win32/path-utils.c index ebf2f12eb6..b658ca3f81 100644 --- a/compat/win32/path-utils.c +++ b/compat/win32/path-utils.c @@ -1,4 +1,5 @@ #include "../../git-compat-util.h" +#include "../../environment.h" int win32_has_dos_drive_prefix(const char *path) { @@ -50,3 +51,39 @@ int win32_offset_1st_component(const char *path) return pos + is_dir_sep(*pos) - path; } + +int win32_fspathncmp(const char *a, const char *b, size_t count) +{ + int diff; + + for (;;) { + if (!count--) + return 0; + if (!*a) + return *b ? -1 : 0; + if (!*b) + return +1; + + if (is_dir_sep(*a)) { + if (!is_dir_sep(*b)) + return -1; + a++; + b++; + continue; + } else if (is_dir_sep(*b)) + return +1; + + diff = ignore_case ? + (unsigned char)tolower(*a) - (int)(unsigned char)tolower(*b) : + (unsigned char)*a - (int)(unsigned char)*b; + if (diff) + return diff; + a++; + b++; + } +} + +int win32_fspathcmp(const char *a, const char *b) +{ + return win32_fspathncmp(a, b, (size_t)-1); +} diff --git a/compat/win32/path-utils.h b/compat/win32/path-utils.h index 65fa3b9263..a561c700e7 100644 --- a/compat/win32/path-utils.h +++ b/compat/win32/path-utils.h @@ -29,5 +29,9 @@ static inline int win32_has_dir_sep(const char *path) #define has_dir_sep(path) win32_has_dir_sep(path) int win32_offset_1st_component(const char *path); #define offset_1st_component win32_offset_1st_component +int win32_fspathcmp(const char *a, const char *b); +#define fspathcmp win32_fspathcmp +int win32_fspathncmp(const char *a, const char *b, size_t count); +#define fspathncmp win32_fspathncmp #endif diff --git a/dir.c b/dir.c index b7a6625ebd..5a23376bda 100644 --- a/dir.c +++ b/dir.c @@ -95,7 +95,7 @@ int count_slashes(const char *s) return cnt; } -int fspathcmp(const char *a, const char *b) +int git_fspathcmp(const char *a, const char *b) { return ignore_case ? strcasecmp(a, b) : strcmp(a, b); } @@ -105,7 +105,7 @@ int fspatheq(const char *a, const char *b) return !fspathcmp(a, b); } -int fspathncmp(const char *a, const char *b, size_t count) +int git_fspathncmp(const char *a, const char *b, size_t count) { return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count); } diff --git a/dir.h b/dir.h index 69a76d8bdd..a3a2f00f5d 100644 --- a/dir.h +++ b/dir.h @@ -541,9 +541,9 @@ int remove_dir_recursively(struct strbuf *path, int flag); */ int remove_path(const char *path); -int fspathcmp(const char *a, const char *b); +int git_fspathcmp(const char *a, const char *b); int fspatheq(const char *a, const char *b); -int fspathncmp(const char *a, const char *b, size_t count); +int git_fspathncmp(const char *a, const char *b, size_t count); unsigned int fspathhash(const char *str); /* diff --git a/git-compat-util.h b/git-compat-util.h index ca7678a379..71b4d23f03 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -506,6 +506,14 @@ static inline int git_offset_1st_component(const char *path) #define offset_1st_component git_offset_1st_component #endif +#ifndef fspathcmp +#define fspathcmp git_fspathcmp +#endif + +#ifndef fspathncmp +#define fspathncmp git_fspathncmp +#endif + #ifndef is_valid_path #define is_valid_path(path) 1 #endif From f1ed769a3bc045d9f79396fb6074bfb33b83e09d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 Jul 2024 21:08:21 +0000 Subject: [PATCH 4/7] mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too Whether the full path to the MSYS2 Bash is specified using backslashes or forward slashes, in either case the command-line arguments need to be quoted in the MSYS2-specific manner instead of using regular Win32 command-line quoting rules. In preparation for `prepare_shell_cmd()` to use the full path to `sh.exe` (with forward slashes for consistency), let's teach the `is_msys2_sh()` function about this; Otherwise 5580.4 'clone with backslashed path' would fail once `prepare_shell_cmd()` uses the full path instead of merely `sh`. This patch relies on the just-introduced fix where `fspathcmp()` handles backslashes and forward slashes as equivalent on Windows. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- compat/mingw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compat/mingw.c b/compat/mingw.c index 6097b8f9e6..29d3f09768 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -1546,7 +1546,7 @@ static int is_msys2_sh(const char *cmd) return ret; } - if (ends_with(cmd, "\\sh.exe")) { + if (ends_with(cmd, "\\sh.exe") || ends_with(cmd, "/sh.exe")) { static char *sh; if (!sh) From 92fe7c7d42cc941ed70d6fce988d6b7936a9765a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 Jul 2024 21:08:22 +0000 Subject: [PATCH 5/7] run-command(win32): resolve the path to the Unix shell early In 776297548e (Do not use SHELL_PATH from build system in prepare_shell_cmd on Windows, 2012-04-17), the hard-coded path to the Unix shell was replaced by passing `sh` instead when executing Unix shell scripts in Git. This was done because the hard-coded path to the Unix shell is incorrect on Windows because it not only is a Unix-style absolute path instead of a Windows one, but Git uses the runtime prefix feature on Windows, i.e. the correct path cannot be hard-coded. Naturally, the `sh` argument will be resolved to the full path of said executable eventually. To help fixing the bug where `git var GIT_SHELL_PATH` currently does not reflect that logic, but shows that incorrect hard-coded Unix-style absolute path, let's resolve the full path to the `sh` executable early in the `git_shell_path()` function so that we can use it in `git var`, too, and be sure that the output is equivalent to what `run_command()` does when it is asked to execute a command-line using a Unix shell. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- run-command.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/run-command.c b/run-command.c index 59e433bf91..60a79db8f0 100644 --- a/run-command.c +++ b/run-command.c @@ -274,12 +274,14 @@ int sane_execvp(const char *file, char * const argv[]) return -1; } -static const char *git_shell_path(void) +static char *git_shell_path(void) { #ifndef GIT_WINDOWS_NATIVE - return SHELL_PATH; + return xstrdup(SHELL_PATH); #else - return "sh"; + char *p = locate_in_PATH("sh"); + convert_slashes(p); + return p; #endif } @@ -289,7 +291,7 @@ static const char **prepare_shell_cmd(struct strvec *out, const char **argv) BUG("shell command is empty"); if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) { - strvec_push(out, git_shell_path()); + strvec_push_nodup(out, git_shell_path()); strvec_push(out, "-c"); /* From 877da5e208dfd747750e16f34a0275f3e598d8d2 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 Jul 2024 21:08:23 +0000 Subject: [PATCH 6/7] run-command: declare the `git_shell_path()` function globally The intention is to use it in `git var GIT_SHELL_PATH`, therefore we need this function to stop being file-local only. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- run-command.c | 2 +- run-command.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/run-command.c b/run-command.c index 60a79db8f0..45ba544932 100644 --- a/run-command.c +++ b/run-command.c @@ -274,7 +274,7 @@ int sane_execvp(const char *file, char * const argv[]) return -1; } -static char *git_shell_path(void) +char *git_shell_path(void) { #ifndef GIT_WINDOWS_NATIVE return xstrdup(SHELL_PATH); diff --git a/run-command.h b/run-command.h index 55f6631a2a..03e7222d8b 100644 --- a/run-command.h +++ b/run-command.h @@ -195,6 +195,11 @@ int is_executable(const char *name); */ int exists_in_PATH(const char *command); +/** + * Return the path that is used to execute Unix shell command-lines. + */ +char *git_shell_path(void); + /** * Start a sub-process. Takes a pointer to a `struct child_process` * that specifies the details and returns pipe FDs (if requested). From 9ed143ee33c03880f4e7fd1d41df7b1bf8b244da Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 13 Jul 2024 21:08:24 +0000 Subject: [PATCH 7/7] var(win32): do report the GIT_SHELL_PATH that is actually used On Windows, Unix-like paths like `/bin/sh` make very little sense. In the best case, they simply don't work, in the worst case they are misinterpreted as absolute paths that are relative to the drive associated with the current directory. To that end, Git does not actually use the path `/bin/sh` that is recorded e.g. when `run_command()` is called with a Unix shell command-line. Instead, as of 776297548e (Do not use SHELL_PATH from build system in prepare_shell_cmd on Windows, 2012-04-17), it re-interprets `/bin/sh` as "look up `sh` on the `PATH` and use the result instead". This is the logic users expect to be followed when running `git var GIT_SHELL_PATH`. However, when 1e65721227 (var: add support for listing the shell, 2023-06-27) introduced support for `git var GIT_SHELL_PATH`, Windows was not special-cased as above, which is why it outputs `/bin/sh` even though that disagrees with what Git actually uses. Let's fix this by using the exact same logic as `prepare_shell_cmd()`, adjusting the Windows-specific `git var GIT_SHELL_PATH` test case to verify that it actually finds a working executable. Reported-by: Phillip Wood Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/var.c | 3 ++- t/t0007-git-var.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/builtin/var.c b/builtin/var.c index 5dc384810c..e30ff45be1 100644 --- a/builtin/var.c +++ b/builtin/var.c @@ -12,6 +12,7 @@ #include "refs.h" #include "path.h" #include "strbuf.h" +#include "run-command.h" static const char var_usage[] = "git var (-l | )"; @@ -51,7 +52,7 @@ static char *default_branch(int ident_flag UNUSED) static char *shell_path(int ident_flag UNUSED) { - return xstrdup(SHELL_PATH); + return git_shell_path(); } static char *git_attr_val_system(int ident_flag UNUSED) diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh index ff4fd9348c..9fc5882387 100755 --- a/t/t0007-git-var.sh +++ b/t/t0007-git-var.sh @@ -157,7 +157,7 @@ test_expect_success POSIXPERM 'GIT_SHELL_PATH points to a valid executable' ' test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' ' shellpath=$(git var GIT_SHELL_PATH) && case "$shellpath" in - *sh) ;; + [A-Z]:/*/sh.exe) test -f "$shellpath";; *) return 1;; esac '