Re: [PATCH v3 0/7] var(win32): do report the GIT_SHELL_PATH that is actually used

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Johannes

On 13/07/2024 22:08, Johannes Schindelin via GitGitGadget wrote:
> Changes since v2:
> 
>   * Now fspathncmp() is overridden on Windows just like fspathcmp().
>   * The win32_fspath*cmp() functions now respect core.ignoreCase.

These changes in patch 3 look good. Thanks for fixing this

Phillip

> Changes since v1:
> 
>   * This patch series now shares the logic that determines the path of the
>     Unix shell that Git uses between prepare_shell_cmd() and git var
>     GIT_SHELL_PATH.
> 
> Johannes Schindelin (7):
>    run-command: refactor getting the Unix shell path into its own
>      function
>    strvec: declare the `strvec_push_nodup()` function globally
>    win32: override `fspathcmp()` with a directory separator-aware version
>    mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too
>    run-command(win32): resolve the path to the Unix shell early
>    run-command: declare the `git_shell_path()` function globally
>    var(win32): do report the GIT_SHELL_PATH that is actually used
> 
>   builtin/var.c             |  3 ++-
>   compat/mingw.c            |  2 +-
>   compat/win32/path-utils.c | 37 +++++++++++++++++++++++++++++++++++++
>   compat/win32/path-utils.h |  4 ++++
>   dir.c                     |  4 ++--
>   dir.h                     |  4 ++--
>   git-compat-util.h         |  8 ++++++++
>   run-command.c             | 17 ++++++++++++-----
>   run-command.h             |  5 +++++
>   strvec.c                  |  2 +-
>   strvec.h                  |  3 +++
>   t/t0007-git-var.sh        |  2 +-
>   12 files changed, 78 insertions(+), 13 deletions(-)
> 
> 
> base-commit: 06e570c0dfb2a2deb64d217db78e2ec21672f558
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1760%2Fdscho%2Fgit-var-on-windows-v3
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1760/dscho/git-var-on-windows-v3
> Pull-Request: https://github.com/gitgitgadget/git/pull/1760
> 
> Range-diff vs v2:
> 
>   1:  e0970381042 = 1:  e0970381042 run-command: refactor getting the Unix shell path into its own function
>   2:  91ebccbc39f = 2:  91ebccbc39f strvec: declare the `strvec_push_nodup()` function globally
>   3:  a718183bb3b ! 3:  f46315ac0b2 win32: override `fspathcmp()` with a directory separator-aware version
>       @@ Commit message
>            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 <johannes.schindelin@gmx.de>
>        
>         ## compat/win32/path-utils.c ##
>       +@@
>       + #include "../../git-compat-util.h"
>       ++#include "../../environment.h"
>       +
>       + int win32_has_dos_drive_prefix(const char *path)
>       + {
>        @@ compat/win32/path-utils.c: int win32_offset_1st_component(const char *path)
>         
>         	return pos + is_dir_sep(*pos) - path;
>         }
>        +
>       -+int win32_fspathcmp(const char *a, const char *b)
>       ++int win32_fspathncmp(const char *a, const char *b, size_t count)
>        +{
>        +	int diff;
>        +
>        +	for (;;) {
>       ++		if (!count--)
>       ++			return 0;
>        +		if (!*a)
>       -+			return !*b ? 0 : -1;
>       ++			return *b ? -1 : 0;
>        +		if (!*b)
>        +			return +1;
>        +
>       @@ compat/win32/path-utils.c: int win32_offset_1st_component(const char *path)
>        +		} else if (is_dir_sep(*b))
>        +			return +1;
>        +
>       -+		diff = (unsigned char)tolower(*(a++)) - (unsigned char)tolower(*(b++));
>       ++		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);
>        +}
>        
>         ## compat/win32/path-utils.h ##
>       @@ compat/win32/path-utils.h: static inline int win32_has_dir_sep(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
>        
>       @@ dir.c: int count_slashes(const char *s)
>         {
>         	return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
>         }
>       +@@ dir.c: 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);
>       + }
>        
>         ## dir.h ##
>        @@ dir.h: int remove_dir_recursively(struct strbuf *path, int flag);
>       @@ dir.h: int remove_dir_recursively(struct strbuf *path, int flag);
>        -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 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);
>       +
>       + /*
>        
>         ## git-compat-util.h ##
>        @@ git-compat-util.h: 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
>   4:  f04cfd91bd9 = 4:  60fde81d35c mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too
>   5:  707daf246bd = 5:  797cf9094ea run-command(win32): resolve the path to the Unix shell early
>   6:  a74a7b4bb11 = 6:  7c53a4f4707 run-command: declare the `git_shell_path()` function globally
>   7:  8bfd23cfa00 = 7:  7c8eba5bfd8 var(win32): do report the GIT_SHELL_PATH that is actually used
>