Change Native Windows shell

Change Native Windows shell

[Steven Penny]

The Git pager is set up by the function `setup_pager` [1]. `setup_pager` calls
`prepare_pager_args` [2]. `prepare_pager_args` sets `use_shell` [3]. Then
`setup_pager` calls `start_command` [4]. Then, since `use_shell` has been
defined, `prepare_shell_cmd` is called [5]. Finally, regardless of operating
system, `sh` is called [6]:

    #ifndef GIT_WINDOWS_NATIVE
                    argv_array_push(out, SHELL_PATH);
    #else
                    argv_array_push(out, "sh");
    #endif
                    argv_array_push(out, "-c");

The issue is, that it is possible to build a fully static native Windows
Git [7]. A Git like this can run on a stock Windows system, with no requirement
on MSYS2 or Cygwin. So it doesnt make sense for Git to be calling `sh`, when a
user may not have or need `sh` on their system. I think that on Native Windows
builds, that the Pager should be called directly, or if a shell must be used,
then make it `cmd.exe` or `powershell.exe`.

1. https://github.com/git/git/blob/20514004/pager.c#L106
2. https://github.com/git/git/blob/20514004/pager.c#L127
3. https://github.com/git/git/blob/20514004/pager.c#L101
4. https://github.com/git/git/blob/20514004/pager.c#L130
5. https://github.com/git/git/blob/20514004/run-command.c#L928
6. https://github.com/git/git/blob/20514004/run-command.c#L272-L277
7. https://github.com/nu8/gulf/blob/e9ea5c0b/chapter-2/program.sh

Re: Change Native Windows shell

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]

On 2020-06-04 at 23:33:16, Steven Penny wrote:
> The Git pager is set up by the function `setup_pager` [1]. `setup_pager` calls
> `prepare_pager_args` [2]. `prepare_pager_args` sets `use_shell` [3]. Then
> `setup_pager` calls `start_command` [4]. Then, since `use_shell` has been
> defined, `prepare_shell_cmd` is called [5]. Finally, regardless of operating
> system, `sh` is called [6]:
> 
>     #ifndef GIT_WINDOWS_NATIVE
>                     argv_array_push(out, SHELL_PATH);
>     #else
>                     argv_array_push(out, "sh");
>     #endif
>                     argv_array_push(out, "-c");
> 
> The issue is, that it is possible to build a fully static native Windows
> Git [7]. A Git like this can run on a stock Windows system, with no requirement
> on MSYS2 or Cygwin. So it doesnt make sense for Git to be calling `sh`, when a
> user may not have or need `sh` on their system. I think that on Native Windows
> builds, that the Pager should be called directly, or if a shell must be used,
> then make it `cmd.exe` or `powershell.exe`.

This makes it impossible to write a configuration that works across
platforms.  CMD, PowerShell, and sh have entirely different quoting
rules and functionality.  The user would not be able to specify a pager
or editor with arguments portably.

In addition, Git supports the EDITOR and VISUAL environment variables
for editors and these always, 100% of the time, must be passed to sh to
function correctly.  It would be a bug if, when I used Windows, these
variables were passed to CMD or PowerShell.  People also use GIT_EDITOR
or GIT_SEQUENCE_EDITOR for scripting changes to git rebase -i, and these
also need to be interpreted in a portable way across systems, or
programs will break.

Finally, Git needs sh for some commands, like git submodule, git bisect,
git filter-branch, and others.  While there's an effort to replace a lot
of these with C because they don't perform very well on Windows, some of
them are highly interactive and unlikely to be used for scripting, so
porting them doesn't make a lot of sense.

If you really need Git functionality that doesn't rely on sh, you can
look into libgit2 and its assorted language wrappers.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: Change Native Windows shell

[Steven Penny]

On Thu, Jun 4, 2020 at 7:01 PM brian m. carlson wrote:
> If you really need Git functionality that doesn't rely on sh, you can
> look into libgit2 and its assorted language wrappers.

Uh yeah, no. Im not reimplementing the entire Git program, when the fix is 7
lines. Here it is, if anyone is interested:

diff --git a/run-command.c b/run-command.c
index 9b3a57d..4945632 100644
--- a/run-command.c
+++ b/run-command.c
@@ -271,9 +271,6 @@ static const char **prepare_shell_cmd(struct
argv_array *out, const char **argv)
     if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
 #ifndef GIT_WINDOWS_NATIVE
         argv_array_push(out, SHELL_PATH);
-#else
-        argv_array_push(out, "sh");
-#endif
         argv_array_push(out, "-c");

         /*
@@ -284,6 +281,10 @@ static const char **prepare_shell_cmd(struct
argv_array *out, const char **argv)
             argv_array_push(out, argv[0]);
         else
             argv_array_pushf(out, "%s \"$@\"", argv[0]);
+#else
+        argv_array_push(out, "powershell");
+        argv_array_push(out, "-Command");
+#endif
     }

     argv_array_pushv(out, argv);

Re: Change Native Windows shell

[Jonathan Nieder]

Hi,

Steven Penny wrote:
> On Thu, Jun 4, 2020 at 7:01 PM brian m. carlson wrote:

>> If you really need Git functionality that doesn't rely on sh, you can
>> look into libgit2 and its assorted language wrappers.
>
> Uh yeah, no. Im not reimplementing the entire Git program, when the fix is 7
> lines. Here it is, if anyone is interested:

Understood: that's perfectly reasonable for your own use, and I can
hope that this is the first step toward becoming an active Git
contributor in the future. :)

However, for the reasons Brian said, it's not going into Git upstream.

Thanks and hope that helps,
Jonathan

Re: Change Native Windows shell

[Johannes Schindelin]

Hi Steven,

On Thu, 4 Jun 2020, Steven Penny wrote:

> On Thu, Jun 4, 2020 at 7:01 PM brian m. carlson wrote:
> > If you really need Git functionality that doesn't rely on sh, you can
> > look into libgit2 and its assorted language wrappers.
>
> Uh yeah, no. Im not reimplementing the entire Git program, when the fix is 7
> lines. Here it is, if anyone is interested:
>
> diff --git a/run-command.c b/run-command.c
> index 9b3a57d..4945632 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -271,9 +271,6 @@ static const char **prepare_shell_cmd(struct
> argv_array *out, const char **argv)
>      if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
>  #ifndef GIT_WINDOWS_NATIVE
>          argv_array_push(out, SHELL_PATH);
> -#else
> -        argv_array_push(out, "sh");
> -#endif
>          argv_array_push(out, "-c");
>
>          /*
> @@ -284,6 +281,10 @@ static const char **prepare_shell_cmd(struct
> argv_array *out, const char **argv)
>              argv_array_push(out, argv[0]);
>          else
>              argv_array_pushf(out, "%s \"$@\"", argv[0]);
> +#else
> +        argv_array_push(out, "powershell");
> +        argv_array_push(out, "-Command");
> +#endif

That assumes that PowerShell is installed, which is not guaranteed,
either.

Besides, quoting rules are most likely different with PowerShell than what
Git assumes, so you will have to take care of that, too.

Finally, there are plenty of tips out there in the internet that simply
expect a POSIX shell to execute those script snippets. Any user would be
completely (and unnnecessarily) puzzled if those snippets won't work with
their Git for Windows.

In short: it would be unwise for me to accept this change into Git for
Windows, at least as-is.

Ciao,
Johannes

>      }
>
>      argv_array_pushv(out, argv);
>

Re: Change Native Windows shell

[Steven Penny]

On Mon, Jun 8, 2020 at 5:46 PM Johannes Schindelin wrote:
> That assumes that PowerShell is installed, which is not guaranteed,
> either.
>
> Besides, quoting rules are most likely different with PowerShell than what
> Git assumes, so you will have to take care of that, too.
>
> Finally, there are plenty of tips out there in the internet that simply
> expect a POSIX shell to execute those script snippets. Any user would be
> completely (and unnnecessarily) puzzled if those snippets won't work with
> their Git for Windows.
>
> In short: it would be unwise for me to accept this change into Git for
> Windows, at least as-is.

I found a workaround. It seems as long as PAGER or similar is not get, then
Git just tries to call `less.exe`. I didnt realize when I first posted this,
but native Windows Less is available:

https://github.com/jftuga/less-Windows/releases

So I am just bundling that with my Windows native Git:

https://github.com/nu8/gulf/releases

Re: Change Native Windows shell

[Johannes Schindelin]

Hi Steven,

On Mon, 8 Jun 2020, Steven Penny wrote:

> On Mon, Jun 8, 2020 at 5:46 PM Johannes Schindelin wrote:
> > That assumes that PowerShell is installed, which is not guaranteed,
> > either.
> >
> > Besides, quoting rules are most likely different with PowerShell than what
> > Git assumes, so you will have to take care of that, too.
> >
> > Finally, there are plenty of tips out there in the internet that simply
> > expect a POSIX shell to execute those script snippets. Any user would be
> > completely (and unnnecessarily) puzzled if those snippets won't work with
> > their Git for Windows.
> >
> > In short: it would be unwise for me to accept this change into Git for
> > Windows, at least as-is.
>
> I found a workaround. It seems as long as PAGER or similar is not get, then
> Git just tries to call `less.exe`. I didnt realize when I first posted this,
> but native Windows Less is available:
>
> https://github.com/jftuga/less-Windows/releases
>
> So I am just bundling that with my Windows native Git:
>
> https://github.com/nu8/gulf/releases

What is your plan to address concerns of users trying to run popular
hooks? Those example hooks, and recommended hooks, that you can find e.g.
on StackOverflow are all POSIX shell scripts.

Ciao,
Johannes