Hi Johannes
On 12/07/2024 00:11, Johannes Schindelin via GitGitGadget wrote:
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.
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
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.
Reported-by: Phillip Wood <phillip.wood123@xxxxxxxxx>
Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
## 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)
