git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jul 12, 2024, 13:51 UTC
Message-ID
<3b687707-7aa7-4f4f-8043-0e31fb0c09ce@gmail.com>
In-Reply-To
<pull.1760.v2.git.1720739496.gitgitgadget@gmail.com>
Hi Johannes
On 12/07/2024 00:11, Johannes Schindelin via GitGitGadget wrote:
Show 5 quoted 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 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
Show 96 quoted lines
> 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@gmail.com>
>            Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
>        
>         ## 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)
> 
Previous: Junio C HamanoNext: Junio C Hamano
Message 23 of 35 in “var(win32): do report the GIT_SHELL_PATH that is actually used”
  1. var(win32): do report the GIT_SHELL_PATH that is actually usedJohannes Schindelin via GitGitGadget, Jul 8, 2024
  2. Junio C HamanoJul 8, 2024
  3. brian m. carlsonJul 8, 2024
  4. Junio C HamanoJul 8, 2024
  5. brian m. carlsonJul 9, 2024
  6. Junio C HamanoJul 9, 2024
  7. Phillip WoodJul 9, 2024
  8. Junio C HamanoJul 8, 2024
  9. Phillip WoodJul 9, 2024
  10. Johannes SchindelinJul 11, 2024
  11. phillip.wood123@gmail.comJul 17, 2024
  12. 0/7 var(win32): do report the GIT_SHELL_PATH that is actually usedJohannes Schindelin via GitGitGadget, Jul 11, 2024
  13. 1/7 run-command: refactor getting the Unix shell path into its own functionJohannes Schindelin via GitGitGadget, Jul 11, 2024
  14. 2/7 strvec: declare the `strvec_push_nodup()` function globallyJohannes Schindelin via GitGitGadget, Jul 11, 2024
  15. 3/7 win32: override `fspathcmp()` with a directory separator-aware versionJohannes Schindelin via GitGitGadget, Jul 11, 2024
  16. Phillip WoodJul 12, 2024
  17. 4/7 mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, tooJohannes Schindelin via GitGitGadget, Jul 11, 2024
  18. Phillip WoodJul 12, 2024
  19. 5/7 run-command(win32): resolve the path to the Unix shell earlyJohannes Schindelin via GitGitGadget, Jul 11, 2024
  20. 6/7 run-command: declare the `git_shell_path()` function globallyJohannes Schindelin via GitGitGadget, Jul 11, 2024
  21. 7/7 var(win32): do report the GIT_SHELL_PATH that is actually usedJohannes Schindelin via GitGitGadget, Jul 11, 2024
  22. Junio C HamanoJul 12, 2024
  23. Phillip WoodJul 12, 2024
  24. Junio C HamanoJul 12, 2024
  25. 0/7 var(win32): do report the GIT_SHELL_PATH that is actually usedJohannes Schindelin via GitGitGadget, Jul 13, 2024
  26. 1/7 run-command: refactor getting the Unix shell path into its own functionJohannes Schindelin via GitGitGadget, Jul 13, 2024
  27. 2/7 strvec: declare the `strvec_push_nodup()` function globallyJohannes Schindelin via GitGitGadget, Jul 13, 2024
  28. 3/7 win32: override `fspathcmp()` with a directory separator-aware versionJohannes Schindelin via GitGitGadget, Jul 13, 2024
  29. 4/7 mingw(is_msys2_sh): handle forward slashes in the `sh.exe` path, tooJohannes Schindelin via GitGitGadget, Jul 13, 2024
  30. 5/7 run-command(win32): resolve the path to the Unix shell earlyJohannes Schindelin via GitGitGadget, Jul 13, 2024
  31. 6/7 run-command: declare the `git_shell_path()` function globallyJohannes Schindelin via GitGitGadget, Jul 13, 2024
  32. 7/7 var(win32): do report the GIT_SHELL_PATH that is actually usedJohannes Schindelin via GitGitGadget, Jul 13, 2024
  33. Phillip WoodJul 17, 2024
  34. brian m. carlsonJul 17, 2024
  35. Junio C HamanoJul 17, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.