var(win32): do report the GIT_SHELL_PATH that is actually used
PATCH 1/1 var(win32): do report the GIT_SHELL_PATH that is actually used
From: Johannes Schindelin <[email protected]>
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. in Unix shell scripts' hash-bang lines. 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".
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, and also adjust the corresponding test case to verify that it actually finds a working executable.
---
var(win32): do report the GIT_SHELL_PATH that is actually used
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1760%2Fdscho%2Fgit-var-on-windows-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1760/dscho/git-var-on-windows-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1760
builtin/var.c | 6 ++++++ t/t0007-git-var.sh | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)
builtin/var.c +6 -0
@@ -51,7 +51,13 @@ static char *default_branch(int ident_flag UNUSED) static char *shell_path(int ident_flag UNUSED) { +#ifdef WIN32 +char *p =locate_in_PATH ("sh" ); +convert_slashes (p); +return p; +#else return xstrdup (SHELL_PATH ); +#endif } static char *git_attr_val_system(int ident_flag UNUSED)
t/t0007-git-var.sh +1 -1
@@ -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 '
base-commit: 06e570c0dfb2a2deb64d217db78e2ec21672f558
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 10 lines · writes:
On Windows, Unix-like paths like
/bin/shmake 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/shthat is recorded e.g. in Unix shell scripts' hash-bang lines. Instead, as of776297548e(Do not useSHELL_PATHfrom build system inprepare_shell_cmdon Windows, 2012-04-17), it re-interprets/bin/shas "look upshon thePATHand use the result instead".
This asks for a few naïve questions.
If there is a fixed path the "git" binary was compiled for, which can be referenced with a single variable GIT_SHELL_PATH, even though on non-POSIX systems it won't be like /bin/sh, wouldn't there be a path like "C:\Program Files\Git for Windows\bin\sh" (I do not do Windows, so you may be installing somewhere completely different) and wouldn't such a path work regardless of which drive is associated with the current directory?
I would actually understand that, with relocatable build where everything is relative to the installed directory, a single GIT_SHELL_PATH that is defined at the compile-time may not make much sense, and when you need to interpret a shell script, you may need to recompute the actual path, relative to the installation directory.
But I wonder why the replacement shell that is spawned is searched for in PATH, not where you installed it (which of course would be different from /bin/sh). In other words, when running script that calls for "#!/bin/sh", looking for "sh" on PATH might be a workable hack, and it might even yield the same result, especially if you prepend the path you installed git and bash as part of your installation package to the user's PATH, but wouldn't it make more sense to compute it just like how "git --exec-path" is recomputed with the relocatable build?
The "look on the %PATH%" strategy does not make any sense as an implementation for getting GIT_SHELL_PATH, which answers "what is the shell this instanciation of Git was built to work with?", at least to me. Maybe I am missing some knowledge on limitations on Windows and Git for Windows why it is done that way.
Thanks.
Quoted 10 lines · Junio C Hamano
This asks for a few naïve questions.
If there is a fixed path the "git" binary was compiled for, which can be referenced with a single variable
GIT_SHELL_PATH, even though on non-POSIX systems it won't be like/bin/sh, wouldn't there be a path like "C:\Program Files\Git for Windows\bin\sh" (I do not do Windows, so you may be installing somewhere completely different) and wouldn't such a path work regardless of which drive is associated with the current directory?
Quoted 6 lines
I would actually understand that, with relocatable build where everything is relative to the installed directory, a single
GIT_SHELL_PATHthat is defined at the compile-time may not make much sense, and when you need to interpret a shell script, you may need to recompute the actual path, relative to the installation directory.
I'll jump in here, and Dscho can correct me if I'm wrong, but I believe there's one build that's always relocatable (well, there's MinGit and the regular, but ignoring that). You can install to almost any drive and location, so it's not known at compile time.
Quoted 10 lines
But I wonder why the replacement shell that is spawned is searched for in PATH, not where you installed it (which of course would be different from
/bin/sh). In other words, when running script that calls for "#!/bin/sh", looking for "sh" on PATH might be a workable hack, and it might even yield the same result, especially if you prepend the path you installed git and bash as part of your installation package to the user's PATH, but wouldn't it make more sense to compute it just like how "git --exec-path" is recomputed with the relocatable build?
Quoted 5 lines
The "look on the %PATH%" strategy does not make any sense as an implementation for getting
GIT_SHELL_PATH, which answers "what is the shell this instanciation of Git was built to work with?", at least to me. Maybe I am missing some knowledge on limitations on Windows and Git for Windows why it is done that way.
Well, it may be that that's the approach that Git for Windows takes to look up the shell. (I don't know for certain.) If that _is_ what it does, then that's absolutely the value we want because we want to use whatever shell Git for Windows uses. I will say it's a risky approach because it could well also find a Cygwin or MINGW shell (or, if it were called bash, WSL), but we really want whatever Git for Windows does here.
That's because external users who rely on Git for Windows to furnish a POSIX shell will want to know the path, and this variable is the best way to do that (and the reason I added it).
Signature · 2 lines
brian m. carlson (they/them or he/him) Toronto, Ontario, CA
Quoted 6 lines · brian m. carlson
Quoted 5 lines
The "look on the %PATH%" strategy does not make any sense as an implementation for getting
GIT_SHELL_PATH, which answers "what is the shell this instanciation of Git was built to work with?", at least to me. Maybe I am missing some knowledge on limitations on Windows and Git for Windows why it is done that way.
Quoted 8 lines
Well, it may be that that's the approach that Git for Windows takes to look up the shell. (I don't know for certain.)
If that _is_ what it does, then that's absolutely the value we want because we want to use whatever shell Git for Windows uses. I will say it's a risky approach because it could well also find a Cygwin or MINGW shell (or, if it were called bash, WSL), but we really want whatever Git for Windows does here.
Yeah, absolutely it is risky unless it is doing the "we are relocatable, so where is the 'sh' _we_ installed?", which is what I would expect GIT_SHELL_PATH to be.
Quoted 3 lines
That's because external users who rely on Git for Windows to furnish a POSIX shell will want to know the path, and this variable is the best way to do that (and the reason I added it).
If Git for Windows furnishes programs other than POSIX shell, paths to which external users would also want to learn, what happens? GIT_PERL_PATH, GIT_WISH_PATH, etc.? Locate them on PATH themselves shouldn't be rocket science (and instead of locating, just spawning them themselves would be even easier, I would presume).
Thanks.
Quoted 11 lines · Junio C Hamano
Quoted 3 lines · brian m. carlson
That's because external users who rely on Git for Windows to furnish a POSIX shell will want to know the path, and this variable is the best way to do that (and the reason I added it).
If Git for Windows furnishes programs other than POSIX shell, paths to which external users would also want to learn, what happens?
GIT_PERL_PATH,GIT_WISH_PATH, etc.? Locate them on PATH themselves shouldn't be rocket science (and instead of locating, just spawning them themselves would be even easier, I would presume).
In my experience, they have not always been on PATH. It may be that they are now, which is fantastic, but I seem to remember that at some point bash.exe and git.exe were on PATH, but sh.exe and other commands were not. (There's also a different path under Git Bash than the regular Windows PATH.) You might say, "Well, why not use bash.exe?" and that works great until you realize that there's also a bash.exe that is part of WSL and will attempt to spawn WSL for you.
However, that doesn't always work, because sometimes WSL isn't installed or is disabled or broken, and so the operation fails with an error message. Also, users will typically have wanted Git for Windows's bash and not a Linux environment's bash, since the two environments will typically have different software available.
Why not add Perl or Wish or something else? Because once you have the Git for Windows sh, you can use a fixed Unix path to look them up and get a canonical Windows path with cygpath -w. Thus, this is just the minimal bootstrapping functionality to get that information.
Of course, on Unix, there can still be multiple Perl implementations, but you're typically either going to build against one in particular, which may or may not be what Git was built against, or you're going to use /usr/bin/env and choose whatever's first in PATH. In that case, it's very unlikely that anyone cares what version of Perl Git actually uses.
The only major benefit of using Git's shell on Unix is that you know it supports POSIX functionality (which /bin/sh does not on some proprietary Unices) and it also supports local, which may be convenient for scripting.
Signature · 2 lines
brian m. carlson (they/them or he/him) Toronto, Ontario, CA
Quoted 4 lines · brian m. carlson
Why not add Perl or Wish or something else? Because once you have the Git for Windows sh, you can use a fixed Unix path to look them up and get a canonical Windows path with cygpath -w. Thus, this is just the minimal bootstrapping functionality to get that information.
Besides, perl and wish are not part of Git. The same thing can be said that shell is not part of Git.
So stepping back and thinking why we have "git var GIT_SHELL_PATH", what should it mean to begin with? It is obviously not to tell where we installed the thing as a part of "Git" suite, even though Git for Windows installer may let users install the shell and other stuff (similar to how "apt" lets users install package on Debian derived systems).
Hence, I can accept that "git var GIT_SHELL_PATH" is not (and does not have to be) about where we installed what we shipped. It is where we use the shell from (i.e., when we need "sh", we use that shell).
The documentation for GIT_SHELL_PATH is already good. Sorry for my earlier confusion---I should have looked at it first. It says it is not about what we ship, but what we use when we need to shell out.
I am OK with GIT_SHELL_PATH computed differently depending on the platform, as different platform apparently use different mechanism to decide which shell to spawn. On POSIX systems, it is the one we compiled to use, while on Windows it is the one that happens to be on the end-user's PATH.
But then the comment I made in my earlier review still stands that such a platform dependent logic should be implemented a bit more cleanly than the posted patch.
"Which shell do we use at runtime" should influence a lot of what the things in run-command.c do, so perhaps
- we remove builtin/var.c:shell_path()
- We create run-command.c:git_shell_path() immediately above run-command.c:prepare_shell_cmd(). We will add conditional compilation there (i.e. #ifdef GIT_WINDOWS_NATIVE). The default implementation is to return SHELL_PATH, and on Windows it looks for "sh" on %PATH%.
- The entry for GIT_SHELL_PATH in builtin/var.c:git_vars[] should be updated to point at git_shell_path() in run-command.c
- Near the beginning of run-command.c:prepare_shell_cmd(), there is a conditional compilation. If we can remove it by using git_shell_path(), that would be a nice bonus.
would give us a good approach for implementation?
Thanks.
Quoted 11 lines · brian m. carlson
Quoted 5 lines · Junio C Hamano
The "look on the %PATH%" strategy does not make any sense as an implementation for getting
GIT_SHELL_PATH, which answers "what is the shell this instanciation of Git was built to work with?", at least to me. Maybe I am missing some knowledge on limitations on Windows and Git for Windows why it is done that way.Well, it may be that that's the approach that Git for Windows takes to look up the shell. (I don't know for certain.) If that _is_ what it does, then that's absolutely the value we want because we want to use whatever shell Git for Windows uses.
As I understand it this is the approach Git for Windows takes
Quoted 4 lines
I will say it's a risky approach
because it could well also find a Cygwin or MINGW shell (or, if it were called bash, WSL), but we really want whatever Git for Windows does here.
Git for Windows prepends the MINGW system directories to $PATH via some extra downstream code in setup_windows_environment() so looking up the shell in $PATH will find the correct executable.
Best Wishes
Phillip
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 6 lines · writes:
builtin/var.c +0
-0@@ -51,7 +51,13 @@ static char *default_branch(int ident_flag UNUSED)
builtin/var.c static char *default_branch(int ident_flag UNUSED) · line 61
Quoted 10 lines
builtin/var.c +6
-0static char *shell_path(int ident_flag UNUSED) { +#ifdef WIN32 +char *p =locate_in_PATH ("sh" ); +convert_slashes (p); +return p; +#else return xstrdup (SHELL_PATH ); +#endif }
We have done well without any ugly conditional compilation in this file so far. If we needed to report dynamic path on certain platforms, we'd need something a bit cleaner, like
* rename "shell_path()" to something that is unlikely to conflict with others (e.g., "git_shell_path()") and make it extern
* allow it to be overridden from compat/
which is often how we hide such an implementation quirk from the general code paths.
Quoted 16 lines
builtin/var.c +0
-0static char *git_attr_val_system(int ident_flag UNUSED)t/t0007-git-var.sh +1
-1@@ -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 'base-commit:
06e570c0dfb2a2deb64d217db78e2ec21672f558
Hi Johannes
Quoted 1 line · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
Thanks for putting a patch together so quickly
builtin/var.c static char *default_branch(int ident_flag UNUSED) · line 55
Quoted 4 lines
static char *shell_path(int ident_flag UNUSED) {builtin/var.c +2
-0+#ifdef WIN32 +char *p =locate_in_PATH ("sh" );
If I'm reading is_busybox_applet() (which only exists in git-for-windows) correctly then this will return "busybox.exe" under mingit-busybox rather than ash.exe, so the calling program would have to know to set argv[0] (which is likely not possible unless the calling program is written in C) or pass "sh" as the first argument. As the code to support busybox does not exist upstream I guess that's best handled downstream.
Best Wishes
Phillip
Quoted 23 lines
builtin/var.c +4
-0+convert_slashes (p); +return p; +#else return xstrdup (SHELL_PATH ); +#endif } static char *git_attr_val_system(int ident_flag UNUSED)t/t0007-git-var.sh +1
-1@@ -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 'base-commit:
06e570c0dfb2a2deb64d217db78e2ec21672f558
Hi Phillip,
Quoted 5 lines · Phillip Wood
Quoted 1 line · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
Thanks for putting a patch together so quickly
Quoted 11 lines
builtin/var.c static char *default_branch(int ident_flag UNUSED) · line 55
Quoted 4 lines
static char *shell_path(int ident_flag UNUSED) {builtin/var.c +2
-0+#ifdef WIN32 +char *p =locate_in_PATH ("sh" );If I'm reading
is_busybox_applet()(which only exists in git-for-windows) correctly then this will return "busybox.exe" under mingit-busybox rather than ash.exe, so the calling program would have to know to set argv[0] (which is likely not possible unless the calling program is written in C) or pass "sh" as the first argument. As the code to support busybox does not exist upstream I guess that's best handled downstream.
BusyBox-w32 is unfortunately displaying strange performance patterns. It is partially (and expectedly) faster than the MSYS2 Bash, but in other scenarios it is substantially slower (which is totally unexpected).
Some time ago, I tried to make this all work and investigate the unexpected performance issues (and hoped to fix them, too), but ran out of time [*1*]. That was almost two years ago, and I am unsure whether I will ever be able to elevate the BusyBox flavor of MinGit to a non-experimental state.
My original plan was to eventually no longer include busybox.exe in the mingit-busybox packages, but instead a copy of that executable with the name sh.exe and thereby have it work without that hack in the Git code to call the busybox.exe with the sh argument inserted before the regular command-line arguments.
In the context of the patch (or now: patch series) at hand, I don't think we need to let BusyBox play any role.
Ciao,
Johannes
Footnote *1*: Interested parties can find the latest state here:
https://github.com/git-for-windows/git/compare/main...dscho:git:busybox
Hi Johannes
Quoted 31 lines · Johannes Schindelin
Hi Phillip,
Quoted 5 lines · Phillip Wood
Quoted 1 line · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
Thanks for putting a patch together so quickly
Quoted 11 lines
builtin/var.c static char *default_branch(int ident_flag UNUSED) · line 55
Quoted 4 lines
static char *shell_path(int ident_flag UNUSED) {builtin/var.c +2
-0+#ifdef WIN32 +char *p =locate_in_PATH ("sh" );If I'm reading
is_busybox_applet()(which only exists in git-for-windows) correctly then this will return "busybox.exe" under mingit-busybox rather than ash.exe, so the calling program would have to know to set argv[0] (which is likely not possible unless the calling program is written in C) or pass "sh" as the first argument. As the code to support busybox does not exist upstream I guess that's best handled downstream.BusyBox-w32 is unfortunately displaying strange performance patterns. It is partially (and expectedly) faster than the MSYS2 Bash, but in other scenarios it is substantially slower (which is totally unexpected).
Some time ago, I tried to make this all work and investigate the unexpected performance issues (and hoped to fix them, too), but ran out of time [*1*]. That was almost two years ago, and I am unsure whether I will ever be able to elevate the BusyBox flavor of MinGit to a non-experimental state.
Quoted 5 lines
My original plan was to eventually no longer include
busybox.exein the mingit-busybox packages, but instead a copy of that executable with the namesh.exeand thereby have it work without that hack in the Git code to call thebusybox.exewith theshargument inserted before the regular command-line arguments.
Thanks for the information.
Quoted 2 lines
In the context of the patch (or now: patch series) at hand, I don't think we need to let BusyBox play any role.
Agreed
Best Wishes
Phillip
Quoted 5 lines
Ciao,
JohannesFootnote *1*: Interested parties can find the latest state here:
https://github.com/git-for-windows/git/compare/main...dscho:git:busybox
v2 cover var(win32): do report the GIT_SHELL_PATH that is actually used
In reply to Johannes Schindelin via GitGitGadgetChanges 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 | 25 +++++++++++++++++++++++++ compat/win32/path-utils.h | 2 ++ dir.c | 2 +- dir.h | 2 +- git-compat-util.h | 5 +++++ run-command.c | 17 ++++++++++++----- run-command.h | 5 +++++ strvec.c | 2 +- strvec.h | 3 +++ t/t0007-git-var.sh | 2 +- 12 files changed, 59 insertions(+), 11 deletions(-)
base-commit: 06e570c0dfb2a2deb64d217db78e2ec21672f558
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1760%2Fdscho%2Fgit-var-on-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1760/dscho/git-var-on-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1760
Range-diff vs v1:
-: ----------- > 1: e0970381042 run-command: refactor getting the Unix shell path into its own function -: ----------- > 2: 91ebccbc39f strvec: declare the `strvec_push_nodup()` function globally -: ----------- > 3: a718183bb3b win32: override `fspathcmp()` with a directory separator-aware version -: ----------- > 4: f04cfd91bd9 mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too -: ----------- > 5: 707daf246bd run-command(win32): resolve the path to the Unix shell early -: ----------- > 6: a74a7b4bb11 run-command: declare the `git_shell_path()` function globally 1: ef62c3fc122 ! 7: 8bfd23cfa00 var(win32): do report the GIT_SHELL_PATH that is actually used
@@ Commit message
associated with the current directory.
To that end, Git does not actually use the path /bin/sh that is
- recorded e.g. in Unix shell scripts' hash-bang lines. 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".
+ 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, and also adjust the corresponding test case to verify
- that it actually finds a working executable.
+ 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.
## builtin/var.c ##
+@@
+ #include "refs.h"
+ #include "path.h"
+ #include "strbuf.h"
++#include "run-command.h"
+
+ static const char var_usage[] = "git var (-l | <variable>)"; +
@@ builtin/var.c: static char *default_branch(int ident_flag UNUSED)
static char *shell_path(int ident_flag UNUSED) {
-+#ifdef WIN32
-+ char *p = locate_in_PATH("sh");
-+ convert_slashes(p);
-+ return p;
-+#else
- return xstrdup(SHELL_PATH);
-+#endif
+- return xstrdup(SHELL_PATH);
++ return git_shell_path();
}
static char *git_attr_val_system(int ident_flag UNUSED)
Signature · 1 line
gitgitgadget
PATCH v2 1/7 run-command: refactor getting the Unix shell path into its own function
From: Johannes Schindelin <[email protected]>
This encapsulates the platform-specific logic better.
--- run-command.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
run-command.c +10 -5
@@ -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" );/*
Signature · 1 line
gitgitgadget
PATCH v2 2/7 strvec: declare the `strvec_push_nodup()` function globally
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- strvec.c | 2 +- strvec.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)
strvec.c +1 -1
@@ -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 ;
strvec.h +3 -0
@@ -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`.
Signature · 1 line
gitgitgadget
PATCH v2 3/7 win32: override `fspathcmp()` with a directory separator-aware version
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- compat/win32/path-utils.c | 25 +++++++++++++++++++++++++ compat/win32/path-utils.h | 2 ++ dir.c | 2 +- dir.h | 2 +- git-compat-util.h | 5 +++++ 5 files changed, 34 insertions(+), 2 deletions(-)
compat/win32/path-utils.c +25 -0
@@ -50,3 +50,28 @@ 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 diff; + +for (;;) { +if (!*a) +return !*b ?0 :-1 ; +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 = (unsigned char )tolower (*(a++)) - (unsigned char )tolower (*(b++)); +if (diff) +return diff; + } +}
compat/win32/path-utils.h +2 -0
@@ -29,5 +29,7 @@ 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#endif
dir.c +1 -1
@@ -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); }
dir.h +1 -1
@@ -541,7 +541,7 @@ 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);unsigned int fspathhash (const char *str);
git-compat-util.h +5 -0
@@ -506,6 +506,11 @@ 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 is_valid_path#define is_valid_path (path) 1#endif
Signature · 1 line
gitgitgadget
Re: [PATCH v2 3/7] win32: override `fspathcmp()` with a directory separator-aware version
Hi Johannes
Quoted 7 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
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/banda\bare equivalent, andfspathcmp()needs to be made aware of that fact.
How does fspathncmp() behave? It would be good for the two to match. I've left a couple of thoughts below but I'm not sure they're worth re-rolling for
Quoted 15 lines
Signed-off-by Johannes Schindelin <[email protected]>--- compat/win32/path-utils.c | 25 +++++++++++++++++++++++++ compat/win32/path-utils.h | 2 ++ dir.c | 2 +- dir.h | 2 +- git-compat-util.h | 5 +++++ 5 files changed, 34 insertions(+), 2 deletions(-)compat/win32/path-utils.c +0
-0@@ -50,3 +50,28 @@ int win32_offset_1st_component(const char *path)
compat/win32/path-utils.c int win32_offset_1st_component(const char *path) · line 60
Quoted 10 lines
compat/win32/path-utils.c +8
-0return pos +is_dir_sep (*pos) - path; } + +int win32_fspathcmp (const char *a,const char *b) +{ +int diff; + +for (;;) { +if (!*a) +return !*b ?0 :-1 ;
Personally I'd find this easier to read as
return *b ? -1 : 0;
compat/win32/path-utils.c int win32_offset_1st_component(const char *path) · line 73
Quoted 13 lines
compat/win32/path-utils.c +13
-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 = (unsigned char )tolower (*(a++)) - (unsigned char )tolower (*(b++));
There is a subtle difference between this and strcasecmp() because the latter is locale dependent but our tolower() is not because we override the standard library's version. As they're both useless on multibyte locales it probably doesn't make much difference in practice.
Thanks
Phillip
Quoted 58 lines
compat/win32/path-utils.c +4
-0+if (diff) +return diff; + } +}compat/win32/path-utils.h +2
-0@@ -29,5 +29,7 @@ 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#endif dir.c +1
-1@@ -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); }dir.h +1
-1@@ -541,7 +541,7 @@ 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);unsigned int fspathhash (const char *str);git-compat-util.h +5
-0@@ -506,6 +506,11 @@ 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 is_valid_path#define is_valid_path (path) 1#endif
PATCH v2 4/7 mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- compat/mingw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
compat/mingw.c +1 -1
@@ -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)
Signature · 1 line
gitgitgadget
Re: [PATCH v2 4/7] mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too
Quoted 7 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
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.
Quoted 5 lines
In preparation for
prepare_shell_cmd()to use the full path tosh.exe(with forward slashes for consistency), let's teach theis_msys2_sh()function about this; Otherwise 5580.4 'clone with backslashed path' would fail onceprepare_shell_cmd()uses the full path instead of merelysh.
Makes sense
Quoted 2 lines
This patch relies on the just-introduced fix where
fspathcmp()handles backslashes and forward slashes as equivalent on Windows.
That dependency isn't obvious from the patch but there is a call to fspathcmp() a couple of lines below the last context line.
Thanks
Phillip
Quoted 18 lines
Signed-off-by Johannes Schindelin <[email protected]>--- compat/mingw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)compat/mingw.c +1
-1@@ -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)
PATCH v2 5/7 run-command(win32): resolve the path to the Unix shell early
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- run-command.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
run-command.c +6 -4
@@ -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" );/*
Signature · 1 line
gitgitgadget
PATCH v2 6/7 run-command: declare the `git_shell_path()` function globally
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The intention is to use it in git var GIT_SHELL_PATH, therefore we need this function to stop being file-local only.
--- run-command.c | 2 +- run-command.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)
run-command.c +1 -1
@@ -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 );
run-command.h +5 -0
@@ -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).
Signature · 1 line
gitgitgadget
PATCH v2 7/7 var(win32): do report the GIT_SHELL_PATH that is actually used
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- builtin/var.c | 3 ++- t/t0007-git-var.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
builtin/var.c +2 -1
@@ -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 | <variable>)" ;
@@ -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)
t/t0007-git-var.sh +1 -1
@@ -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 '
Signature · 1 line
gitgitgadget
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 7 lines · writes:
From: Johannes Schindelin <[email protected]>
On Windows, Unix-like paths like
/bin/shmake 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, ...
This does not quite explain why a hardcoded full path does not make sense, though. If you do not want "relative to the current drive", you can even hardcode the path including the drive letter and now it no longer falls into that "worst case" because there is no way to misinterpret it as such. I think the real reason is that the port's "relocatable" nature does not mix well with the "let's make sure we know what we use exactly by deciding the paths to various tools at compile time" mentality. So if I were rerolling this I would say
Git ported to Windows expect to be installed at user-chosen places, which means hardcoded paths to tools decided at compile time are bad fit.
or something.
But it does not matter if the stated reason is to use the "find 'sh' that is on PATH" approach was taken tells the whole story or not. The important thing is that we do ...
Quoted 6 lines
Git does not actually use the path
/bin/shthat is recorded e.g. whenrun_command()is called with a Unix shell command-line. Instead, as of776297548e(Do not useSHELL_PATHfrom build system inprepare_shell_cmdon Windows, 2012-04-17), it re-interprets/bin/shas "look upshon thePATHand use the result instead".
... this. And with the design constraints that it has to be installable anywhere and cannot rely on compile-time determined hardcoded paths, the above design decision is a very valid one.
Quoted 2 lines
This is the logic users expect to be followed when running
git var GIT_SHELL_PATH.
And matching git var GIT_SHELL_PATH to what the code path does make sense.
Quoted 13 lines
builtin/var.c +1
-0@@ -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 | <variable>)" ;
builtin/var.c static char *default_branch(int ident_flag UNUSED) · line 56
Quoted 6 lines
builtin/var.c +1
-1@@ -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 (); }
Simple and clear. Nicely concluded.
Thanks. Will queue.
Hi Johannes
Quoted 5 lines · Johannes Schindelin via GitGitGadget
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 varGIT_SHELL_PATH.
This was a pleasant read, each step was well described and easy to follow. I've left a couple of comments but I think this is good as it is.
Thanks
Phillip
Quoted 96 lines
Johannes Schindelin (7):
run-command: refactor getting the Unix shell path into its own
function
strvec: declare thestrvec_push_nodup()function globally
win32: overridefspathcmp()with a directory separator-aware versionmingw(is_msys2_sh): handle forward slashes in thesh.exepath, too run-command(win32): resolve the path to the Unix shell early
run-command: declare thegit_shell_path()function globally var(win32): do report theGIT_SHELL_PATHthat is actually usedbuiltin/var.c | 3 ++- compat/mingw.c | 2 +- compat/win32/path-utils.c | 25 +++++++++++++++++++++++++ compat/win32/path-utils.h | 2 ++ dir.c | 2 +- dir.h | 2 +- git-compat-util.h | 5 +++++ run-command.c | 17 ++++++++++++----- run-command.h | 5 +++++ strvec.c | 2 +- strvec.h | 3 +++ t/t0007-git-var.sh | 2 +- 12 files changed, 59 insertions(+), 11 deletions(-)base-commit:
06e570c0dfb2a2deb64d217db78e2ec21672f558
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1760%2Fdscho%2Fgit-var-on-windows-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1760/dscho/git-var-on-windows-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1760Range-diff vs v1:
-: ----------- > 1: e0970381042 run-command: refactor getting the Unix shell path into its own function -: ----------- > 2: 91ebccbc39f strvec: declare the `strvec_push_nodup()` function globally -: ----------- > 3: a718183bb3b win32: override `fspathcmp()` with a directory separator-aware version -: ----------- > 4: f04cfd91bd9 mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too -: ----------- > 5: 707daf246bd run-command(win32): resolve the path to the Unix shell early -: ----------- > 6: a74a7b4bb11 run-command: declare the `git_shell_path()` function globally 1: ef62c3fc122 ! 7: 8bfd23cfa00 var(win32): do report the GIT_SHELL_PATH that is actually used@@ Commit message
associated with the current directory.To that end, Git does not actually use the path
/bin/shthat is
- recorded e.g. in Unix shell scripts' hash-bang lines. Instead, as of
-776297548e(Do not useSHELL_PATHfrom build system inprepare_shell_cmd
- on Windows, 2012-04-17), it re-interprets/bin/shas "look upshon
- thePATHand use the result instead".
+ recorded e.g. whenrun_command()is called with a Unix shell
+ command-line. Instead, as of776297548e(Do not useSHELL_PATHfrom
+ build system inprepare_shell_cmdon Windows, 2012-04-17), it
+ re-interprets/bin/shas "look upshon thePATHand 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 forgit var GIT_SHELL_PATH, Windows was not special-cased as above, which is why it outputs/bin/sheven though that disagrees with what Git actually uses.- Let's fix this, and also adjust the corresponding test case to verify
- that it actually finds a working executable.
+ Let's fix this by using the exact same logic asprepare_shell_cmd(),
+ adjusting the Windows-specificgit var GIT_SHELL_PATHtest case to
+ verify that it actually finds a working executable.Reported-by Phillip Wood <[email protected]>Signed-off-by Johannes Schindelin <[email protected]>##
builtin/var.c##
+@@
+ #include "refs.h"
+ #include "path.h"
+ #include "strbuf.h"
++#include "run-command.h"
+
+ static const char var_usage[] = "git var (-l | <variable>)"; +
@@builtin/var.c: static char *default_branch(intident_flagUNUSED)static char *shell_path(int
ident_flagUNUSED) {
-+#ifdef WIN32
-+ char *p =locate_in_PATH("sh");
-+convert_slashes(p);
-+ return p;
-+#else
- returnxstrdup(SHELL_PATH);
-+#endif
+- returnxstrdup(SHELL_PATH);
++ returngit_shell_path();
}static char *git_attr_val_system(int
ident_flagUNUSED)
Quoted 11 lines · Phillip Wood
Hi Johannes
Quoted 5 lines · Johannes Schindelin via GitGitGadget
Changes since v1:
* This patch series now shares the logic that determines the path
of the
Unix shell that Git uses betweenprepare_shell_cmd()and git varGIT_SHELL_PATH.This was a pleasant read, each step was well described and easy to follow.
Ditto.
Quoted 4 lines
I've left a couple of comments but I think this is good as it is.
Thanks
Thanks, both.
v3 cover var(win32): do report the GIT_SHELL_PATH that is actually used
In reply to Johannes Schindelin via GitGitGadgetChanges since v2:
* Now fspathncmp() is overridden on Windows just like fspathcmp().
* The win32_fspath*cmp() functions now respect core.ignoreCase.
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 <[email protected]>
## 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 usedSignature · 1 line
gitgitgadget
PATCH v3 1/7 run-command: refactor getting the Unix shell path into its own function
From: Johannes Schindelin <[email protected]>
This encapsulates the platform-specific logic better.
--- run-command.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)
run-command.c +10 -5
@@ -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" );/*
Signature · 1 line
gitgitgadget
PATCH v3 2/7 strvec: declare the `strvec_push_nodup()` function globally
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- strvec.c | 2 +- strvec.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-)
strvec.c +1 -1
@@ -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 ;
strvec.h +3 -0
@@ -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`.
Signature · 1 line
gitgitgadget
PATCH v3 3/7 win32: override `fspathcmp()` with a directory separator-aware version
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- 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(-)
compat/win32/path-utils.c +37 -0
@@ -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 ); +}
compat/win32/path-utils.h +4 -0
@@ -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
dir.c +2 -2
@@ -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); }
dir.h +2 -2
@@ -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);/*
git-compat-util.h +8 -0
@@ -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
Signature · 1 line
gitgitgadget
PATCH v3 4/7 mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, too
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- compat/mingw.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
compat/mingw.c +1 -1
@@ -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)
Signature · 1 line
gitgitgadget
PATCH v3 5/7 run-command(win32): resolve the path to the Unix shell early
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- run-command.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
run-command.c +6 -4
@@ -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" );/*
Signature · 1 line
gitgitgadget
PATCH v3 6/7 run-command: declare the `git_shell_path()` function globally
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The intention is to use it in git var GIT_SHELL_PATH, therefore we need this function to stop being file-local only.
--- run-command.c | 2 +- run-command.h | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-)
run-command.c +1 -1
@@ -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 );
run-command.h +5 -0
@@ -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).
Signature · 1 line
gitgitgadget
PATCH v3 7/7 var(win32): do report the GIT_SHELL_PATH that is actually used
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
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.
--- builtin/var.c | 3 ++- t/t0007-git-var.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)
builtin/var.c +2 -1
@@ -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 | <variable>)" ;
@@ -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)
t/t0007-git-var.sh +1 -1
@@ -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 '
Signature · 1 line
gitgitgadget
Hi Johannes
Quoted 4 lines · Johannes Schindelin via GitGitGadget
Changes since v2:
* Now
fspathncmp()is overridden on Windows just likefspathcmp().
* The win32_fspath*cmp() functions now respect core.ignoreCase.
These changes in patch 3 look good. Thanks for fixing this
Phillip
Quoted 160 lines
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 varGIT_SHELL_PATH.Johannes Schindelin (7):
run-command: refactor getting the Unix shell path into its own
function
strvec: declare thestrvec_push_nodup()function globally
win32: overridefspathcmp()with a directory separator-aware versionmingw(is_msys2_sh): handle forward slashes in thesh.exepath, too run-command(win32): resolve the path to the Unix shell early
run-command: declare thegit_shell_path()function globally var(win32): do report theGIT_SHELL_PATHthat is actually usedbuiltin/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/1760Range-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 <[email protected]> ## 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
Quoted 11 lines · Johannes Schindelin via GitGitGadget
Changes since v2:
* Now
fspathncmp()is overridden on Windows just likefspathcmp().
* The win32_fspath*cmp() functions now respect core.ignoreCase.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 varGIT_SHELL_PATH.
Quoted 9 lines
Johannes Schindelin (7):
run-command: refactor getting the Unix shell path into its own
function
strvec: declare thestrvec_push_nodup()function globally
win32: overridefspathcmp()with a directory separator-aware versionmingw(is_msys2_sh): handle forward slashes in thesh.exepath, too run-command(win32): resolve the path to the Unix shell early
run-command: declare thegit_shell_path()function globally var(win32): do report theGIT_SHELL_PATHthat is actually used
This series seems reasonable to me as well. I of course can't speak to whether the approach for finding sh is the right one, since I'm not a Windows developer, but I have confidence you know the answer and have thought through it fully.
Signature · 2 lines
brian m. carlson (they/them or he/him) Toronto, Ontario, CA
Quoted 10 lines · brian m. carlson
Quoted 9 lines
Johannes Schindelin (7):
run-command: refactor getting the Unix shell path into its own
function
strvec: declare thestrvec_push_nodup()function globally
win32: overridefspathcmp()with a directory separator-aware versionmingw(is_msys2_sh): handle forward slashes in thesh.exepath, too run-command(win32): resolve the path to the Unix shell early
run-command: declare thegit_shell_path()function globally var(win32): do report theGIT_SHELL_PATHthat is actually used
Quoted 4 lines
This series seems reasonable to me as well. I of course can't speak to whether the approach for finding sh is the right one, since I'm not a Windows developer, but I have confidence you know the answer and have thought through it fully.
... and the series will be in 2.46-rc1
Thanks.