Skip to content

Native paths with backslashes do not work in $env:EDITOR #3822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rkitover opened this issue Apr 26, 2022 · 23 comments
Closed

Native paths with backslashes do not work in $env:EDITOR #3822

rkitover opened this issue Apr 26, 2022 · 23 comments
Labels

Comments

@rkitover
Copy link

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
$ git --version --build-options
git version 2.36.0.windows.1.6.ga8150f0d2f.20220422215518
cpu: x86_64
built from commit: a8150f0d2fd0a2229b25dac6b0285215d5aa0801
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon
  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?
$ cmd.exe /c ver
Microsoft Windows [Version 10.0.22598.200]
  • What options did you set as part of the installation? Or did you choose the
    defaults?
Editor Option: VIM
Custom Editor Path:
Default Branch Option: master
Path Option: Cmd
SSH Option: ExternalOpenSSH
Tortoise Option: false
CURL Option: WinSSL
CRLF Option: CRLFCommitAsIs
Bash Terminal Option: ConHost
Git Pull Behavior Option: Rebase
Use Credential Manager: Disabled
Performance Tweaks FSCache: Enabled
Enable Symlinks: Enabled
Enable Pseudo Console Support: Enabled
Enable FSMonitor: Enabled
  • Any other interesting things about your environment that might be related
    to the issue you're seeing?

Using PowerShell core and Windows Terminal as environment.

Details

  • Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

Using Windows Terminal with PowerShell core 7.2.2.

> $PSVersionTable

Name                           Value
----                           -----
PSVersion                      7.2.2
PSEdition                      Core
GitCommitId                    7.2.2
OS                             Microsoft Windows 10.0.22598
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

As discussed in #3629, for which this is a followup:

$env:EDITOR = 'C:\tools\neovim\Neovim\bin\nvim.exe'
git commit -a --verbose --signoff -S
  • What did you expect to occur after running these commands?

Git to start editor for editing commit message.

  • What actually happened instead?
hint: Waiting for your editor to close the file... C:\tools\neovim\Neovim\bin\nvim.exe: C:toolsneovimNeovimbinnvim.exe: command not found
error: There was a problem with the editor 'C:\tools\neovim\Neovim\bin\nvim.exe'.
Please supply the message using either -m or -F option.

Workaround for now:

$env:EDITOR = '''C:\tools\neovim\Neovim\bin\nvim.exe'''

this encloses the path in single quotes, which works fine in Git bash.

@dscho
Copy link
Member

dscho commented Apr 27, 2022

$env:EDITOR = 'C:\tools\neovim\Neovim\bin\nvim.exe'

I am afraid that this is incorrect. The backslash character is used as escape character in shell invocations, and like so many other programs, git executes $EDITOR as a shell snippet.

Workaround for now:

$env:EDITOR = '''C:\tools\neovim\Neovim\bin\nvim.exe'''

I can think of two better workarounds:

  1. Add C:\tools\neovim\Neovim\bin to your PATH and use EDITOR=nvim.
  2. Use forward slashes instead of backslashes.

I strongly prefer the first of these two. If you want a third workaround: Create the file $HOME/bin/nvim with the following contents:

#!/bin/sh

exec /c/tools/neovim/Neovim/bin/nvim.exe "$@"

and use EDITOR=nvim. This works because Git will add ~/bin to your PATH, and this strategy can be used if you really do not want to modify your PATH as suggested above.

But as I said, EDITOR is commonly assumed to be a shell script snippet (you can even set it to code -w to call Visual Studio Code with the -w option that ensures that the command only returns once the file has been closed), and therefore what you reported is Git behaving as designed. Therefore I will close this ticket.

@dscho dscho closed this as completed Apr 27, 2022
@dscho dscho added the question label Apr 27, 2022
@rkitover
Copy link
Author

It's assumed to be a shell command on unix, why would there be an assumption about a unix shell on Windows, that makes no sense.

@dscho
Copy link
Member

dscho commented Apr 27, 2022

@rkitover many parts of Git assume that a Unix shell is present. It's just the reality I live in, I do not claim that it makes things easy.

@rkitover
Copy link
Author

There is no standard as of yet for $env:EDITOR in the Windows ecosystem, but this situation seems far from ideal. I would propose something like checking if the initial token is a valid Windows path, or perhaps a global setting for which shell should interpret env vars and such.

@dscho
Copy link
Member

dscho commented Apr 27, 2022

There is no standard as of yet for $env:EDITOR in the Windows ecosystem

There is a standard, however, in the Unix ecosystem, from where Git hails. And if you want to introduce anything in the Windows world that is incompatible with the Unix version thereof, I am not interested 😁.

@rkitover
Copy link
Author

I think I'd be able to come up with a fix for this that wouldn't introduce much incompatibility, it just needs to check if the first part is a path to a windows executable.

@dscho
Copy link
Member

dscho commented Apr 27, 2022

No, that would introduce an inconsistency. Please do keep in mind that EDITOR has well-known semantics, many users, and many projects operating using said semantics.

What you're arguing for is to introduce an incompatibility.

@rkitover
Copy link
Author

@rimrul @phil-blain @PhilipOakley Any thoughts on this?

@phil-blain
Copy link

I don't have a strong opinion as I'm not really using Git on Windows and I'm really new to Windows.

What I do think is that in general there could be more documentation around how Git treats paths to programs on Windows. This here issue is about EDITOR but there are a lot of executables that can be used by Git: fs-monitor, external diff drivers, SSH, etc. Some of them have corresponding environment variables, some just have settings in the Git config. I'm not sure if they are all treated the same....

A note somewhere in the documentation (in git(1) or git-config(1)) about what kind of paths to use under different shells would be a nice addition, I would think. I found https://github.com/git-for-windows/git/wiki/File-names,-Branch-names,-Path-quotation,-Executable-bit-and-file-modes,-core.FileMode#path-quotation but there's not a lot there, plus it seems to relate to paths of tracked files...

@PhilipOakley
Copy link

It's assumed to be a shell command on unix, why would there be an assumption about a unix shell on Windows, that makes no sense.

Ultimately, this 'problem' (as I see it), is that Git (project) doesn't declare that it's raison d'etre is to be the SCM for Linux, which is what makes the assumption about using the unix shell syntax, and dir/path style, the 'sensible' option.

The success of Git in eating the world does mean that Windows compatibility can be awkward at times, but unfortunately Git isn't going to stop being Linux (unix) focussed any time soon.

I don't think we have a good plan/approach to documenting compatibility solutions so that all the good advice above can/could be collated into the [Git-for-Windows] version of the documentation.

It would be nice if there was a way of shimming in a e.g. 'Compatibility' section in various man pages that would allow different OS's (Mac, Windows, Non-Stop) to provide guidance (such as above) so it become integrated into the project slightly more than the current wiki and other notes.

[Aside: In a similar vein I'd also like the option of adding 'tutorial examples' to man pages to give newer users a help up, given that the man pages are in 'reference' style that presumes existing understanding of significant concepts]

@rkitover
Copy link
Author

rkitover commented May 1, 2022

Is Git for Windows officially a project affiliated with Git itself?

The focus here seems to be on your MSYS2 "git bash" stuff, which a lot of people for some reason like better than regular MSYS2 (for one thing the MSYS2 git port is not great.)

But all of that stuff is horrible and a dead end.

You said you don't want to maintain too much code for VS support, so you are admitting that this is a low priority for you.

As we discussed in the other PR, the VS port currently does not even use the manifest, so VS binaries will be subtly wrong in their behavior whenever they do an OS build number check. I'll try to send a PR for that later.

I'd like to see a more native focused fork, built with MSVC of course, and using native tools and shell etc. most of which are already available in choco. Rather than bringing along a very crappy port of unix. A good native port doesn't port the whole os along with the program, but aims to target native idioms and users.

I probably don't have time to do something like that myself, but if some people are interested.

It doesn't look like you're interested in this being a separate track for this project anyway.

Ultimately, this 'problem' (as I see it), is that Git (project) doesn't declare that it's raison d'etre is to be the SCM for Linux, which is what makes the assumption about using the unix shell syntax, and dir/path style, the 'sensible' option.

I don't see how that makes things like this sensible, sorry.

@PhilipOakley
Copy link

Is Git for Windows officially a project affiliated with Git itself?

It's probably better to say that Git for Windows is closely affiliated with Git. Being 'Open Source' means there is more latitude in the arrangements.

The Git for Windows project seeks to provide the Git capabilities, on Windows, which does put it in a leader-follower position. The Git for Windows compatibility shims and patches are continually rebased on top of the latest Git release, rather than GfW choosing which parts of the Git release to pull in.

The focus here seems to be on your MSYS2 "git bash" stuff, which a lot of people for some reason like better than regular MSYS2

MSYS2 is the basic shim to fit Git onto Windows. Some may have chosen CYGWIN in the past, but from one perspective that's just abdicating from the underlying platform. The MSYS2 (+ MinGW) is the half way house (worst of both world from some perspectives ;-). If folk want to start with MSYS2, and then add apps, such as Git, then it's their choice. But it doesn't put Git (i.e. distributing source code storage and versioning to users) first.

Aside: I came to Git because I recognised it would solve my problem of personal source code versioning when writing exploratory Matlab code. The corporate offerings were too locked down.

One feature of the Git setup is its expectation of posix shell scripting (aka Torvalds), as that is still used in some of the Git commands and in the test suite. So that's needed somehow, which results in using/choosing bash (other shells possible).

A good native port doesn't port the whole os along with the program ...
I probably don't have time to do something like that myself, but if some people ...

The problem is a) Fast Moving Target b) lack of resources (devs) c) false focus.
There is a lot of progress and churn in Git driven by individual (usually Linux) contributors - keeping up is hard.
The Git core 'team' is actually quite small and looking for more contributors, so forks are unlikely to do better (unless they are Microsoft, but they have bought into the value of Git. "Insert MSFT concerns here").
Revision management is about ensuring security of the code, which is all about having a single core standard.

There is only so much that can be done within Git for Windows without active support, and an appreciation of the long term maintenance load. Steering that course isn't easy. I'm grateful for what @dscho has done, often stuck between a rock and a hard place.

@dscho
Copy link
Member

dscho commented May 5, 2022

the MSYS2 git port is not great

Every time you judge someone, you reveal a part of yourself that needs healing.

You said you don't want to maintain too much code for VS support, so you are admitting that this is a low priority for you.

It's not that it is a low priority. It's more that I do not want to maintain two essentially distinct products that do not need to be distinct in the first place.

I'd like to see a more native focused fork, built with MSVC

There are performance issues with MSVC's malloc() implementation. See #3427 (comment) for more details. Those issues would have to be addressed first.

In general, replacing a known component or toolchain with another one should be not only assessed looking for benefits, but also for risks. Ripping out the GCC toolchain and replacing it with an MSVC one bears a rather huge risk of introducing unwanted and unexpected problems. And I am not only talking about malloc() being slow, I also talk about problems like dependency hell, where we would have to rebuild all of the 164 distinct packages that are used to build Git for Windows ourselves, using the new toolchain, and then maintaining that ourselves, too, rather than tapping into the MSYS2 project for mutually-beneficial collaboration.

Rather than bringing along a very crappy port of unix.

See the comment above regarding judging someone.

One feature of the Git setup is its expectation of posix shell scripting

Indeed. One promise of Git (and hence, of Git for Windows) is that hooks and aliases are executed via a POSIX shell.

You can, of course, try to break that promise. It's up to the Git for Windows maintainer, however, to pay the price for that. And since I am the Git for Windows maintainer, that means I would have to pay the price for a goal that I do not even find particularly convincing ("avoid porting unix"; We are not actually porting Unix at all).

As a consequence, out of sheer self-preservation I will dismiss every attempt to remove the requirement to run hooks and aliases (and commands like git mergetool and git bisect) via a POSIX shell as a Chesterton's Fence.

@rkitover
Copy link
Author

rkitover commented May 7, 2022

the MSYS2 git port is not great

Every time you judge someone, you reveal a part of yourself that needs healing.

I meant the git package in MSYS2 not this project, I was not insulting your work.

As a consequence, out of sheer self-preservation I will dismiss every attempt to remove the requirement to run hooks and aliases (and commands like git mergetool and git bisect) via a POSIX shell as a Chesterton's Fence.

Hooks and aliases are not even part of a git repo. There's also a native port of busybox for windows that will handle basic POSIX scripting where this is required for whatever reason. Things like simple bisect commands would even work in PowerShell as they are.

Where is this requirement for a POSIX shell documented anyway?

we would have to rebuild all of the 164 distinct packages that are used to build Git for Windows ourselves, using the new toolchain, and then maintaining that ourselves, too, rather than tapping into the MSYS2 project for mutually-beneficial collaboration.

conan has just about everything, as binary packages too.

@dscho
Copy link
Member

dscho commented Jun 1, 2022

There's also a native port of busybox for windows that will handle basic POSIX scripting where this is required for whatever reason.

Yes, there is. There is also #1439.

Things like simple bisect commands would even work in PowerShell as they are.

Careful with such claims. https://github.com/git-for-windows/git/blob/v2.36.1.windows.1/git-bisect.sh is a shell script that would not work in PowerShell unless you launch a POSIX shell (which is arguably not "working in PowerShell").

Where is this requirement for a POSIX shell documented anyway?

It's an implicit requirement:

  • There are still a few Git commands implemented as POSIX shells scripts. git bisect is one, git submodule is another one.

  • In the documentation of the git config command, it says this about aliases:

    If the alias expansion is prefixed with an exclamation point, it will be treated as a shell command.

    Of course, it does not say that it is a POSIX shell command, but that is what's meant.

  • All over the internet, there is documentation how to script Git. The vast majority of those scripts are POSIX shell scripts. Most of Git's users learn from such resources instead of from Git's documentation, and it would be quite frustrating if all those examples stopped working because they assume POSIX shell semantics, iff Git for Windows were to nilly-willy remove that support. As long as I am maintainer of Git for Windows, that won't happen.

we would have to rebuild all of the 164 distinct packages that are used to build Git for Windows ourselves, using the new toolchain, and then maintaining that ourselves, too, rather than tapping into the MSYS2 project for mutually-beneficial collaboration.

conan has just about everything, as binary packages too.

Nothing and nobody is stopping you from trying to move Git for Windows over to conan. From experience, I'd expect that to take about 4-5 months. That's how long it took me in 2015 to move Git for Windows from MSys to MSYS2.

@rkitover
Copy link
Author

rkitover commented Jun 1, 2022

If the alias expansion is prefixed with an exclamation point, it will be treated as a shell command.
Of course, it does not say that it is a POSIX shell command, but that is what's meant.

I would assume a user would want the shell that they actually use, and are aware of that fact.

All over the internet, there is documentation how to script Git. The vast majority of those scripts are POSIX shell scripts.

Most of which have nothing to do with Git itself and are obviously for bash which a user can run in MSYS2/WSL/Cygwin/etc..

and it would be quite frustrating if all those examples stopped working because they assume POSIX shell semantics

If a user is not aware of the shell that they are currently using, they deserve all the frustration that they get.

ff Git for Windows were to nilly-willy remove that support. As long as I am maintainer of Git for Windows, that won't happen.

I was not suggesting any such thing. The original issue was $env:EDITOR not interpreting the OS path separator as an escape character in an OS shell such as powershell, which is insane. There are numerous possible solutions to this problem but you are obviously not interested.

Nothing and nobody is stopping you from trying to move Git for Windows over to conan. From experience, I'd expect that to take about 4-5 months. That's how long it took me in 2015 to move Git for Windows from MSys to MSYS2.

My interest is in the git command working properly in powershell, a proper windows port would be nice, but I have enough to do without taking that on. This is what my original issue was about, and at least that is fixed.

On this subject, I spent considerable effort making some improvements to the cmake code, and you said you don't want to "maintain a 100 line function" which is by far one of the most insulting code reviews I've ever seen. Nevertheless I'll try to finish that PR and look at the manifest issue.

Relying on a manifest for code compatibility instead of making it run on the current OS is another thing I wouldn't do.

@dscho
Copy link
Member

dscho commented Jun 3, 2022

If the alias expansion is prefixed with an exclamation point, it will be treated as a shell command.
Of course, it does not say that it is a POSIX shell command, but that is what's meant.

I would assume a user would want the shell that they actually use, and are aware of that fact.

Neither CMD nor PowerShell are a shell.

Besides, your suggestion to change this now would literally break existing setups. Like thousands. Maybe even hundreds of thousands. Nobody can sincerely expect me to do that.

The original issue was $env:EDITOR not interpreting the OS path separator as an escape character in an OS shell such as powershell, which is insane.

I dislike this language, as I perceive it as presumptuous and unnecessarily incendiary, and would like to ask you to word your responses more carefully and more kindly.

Apart from that, I guess I will repeat this point again: EDITOR is a concept originating from Unix, where it is a well-established concept. In Git for Windows, we inherited that concept by virtue of Git respecting that variable. Hence we inherited the Unix-style semantics, too.

This is so well-established, for well over a decade, that I am puzzled that there is any uncertainty about how fixed in stone this is.

I spent considerable effort making some improvements to the cmake code, and you said you don't want to "maintain a 100 line function"

That is not what I said, and given that I maintain many, many functions that are well over 100 lines long, it should be obvious that even if had I said it, it is not what I would have meant.

I maintain code, even complex code, when there is a good justification for the complexity.

In the CMake case, I just did not see the need to interpret environment variables in Git's CMake definition when regular CMake cache entries (think: -D <var>=<value>) are plenty sufficient to reach the same goal.

I might be wrong about that conclusion. There might be a compelling reason why a developer who wishes to build Git via the CMake definition should be unable to set or modify those CMake cache entries, and that in such an instance there would somehow exist a convenient way to set environment variables scoped to that very same build process instead.

It does not sound very likely to me that such a compelling reason exists, and if it does, I am certainly not yet privy to it. If it exists, I definitely would love to learn about it.

@rkitover
Copy link
Author

rkitover commented Jun 7, 2022

If the alias expansion is prefixed with an exclamation point, it will be treated as a shell command.
Of course, it does not say that it is a POSIX shell command, but that is what's meant.

I would assume a user would want the shell that they actually use, and are aware of that fact.

Neither CMD nor PowerShell are a shell.

They are scripting interpreters that make it convenient to run external binaries/scripts. In fact, PowerShell runs very well on Linux, and can perform all the functions of other Linux shells such as being your login shell, etc. etc..

A cursory look at the git sources indicates that it expects SHELL_PATH to be a POSIX shell, and commands are first tried as-is and then interpreted with it.

Of course, that doesn't stop someone from using non-POSIX bash syntax in an alias for example, so you aren't guaranteeing universal compatibility.

In any case, support for other shells for aliases would have to be handled upstream, if anyone has any interest in that.

Besides, your suggestion to change this now would literally break existing setups. Like thousands. Maybe even hundreds of thousands. Nobody can sincerely expect me to do that.

I never suggested this, and it would not break anything presuming the alias shell is configurable. Some people might have an interest in using other shells for their aliases, such as zsh, tcsh or nushell or whatever, bash is not the only shell that exists. But this looks like an upstream project for anyone who has an interest in it.

The original issue was $env:EDITOR not interpreting the OS path separator as an escape character in an OS shell such as powershell, which is insane.

I dislike this language, as I perceive it as presumptuous and unnecessarily incendiary, and would like to ask you to word your responses more carefully and more kindly.

Apart from that, I guess I will repeat this point again: EDITOR is a concept originating from Unix, where it is a well-established concept. In Git for Windows, we inherited that concept by virtue of Git respecting that variable. Hence we inherited the Unix-style semantics, too.

This is so well-established, for well over a decade, that I am puzzled that there is any uncertainty about how fixed in stone this is.

It would be reasonable to expect a concept to match the OS, regardless of where it is inherited from, like the OS representation of paths.

You might want to do something like this in your PowerShell profile:

$env:EDITOR = (get-command nvim).source

, but this doesn't work with Git, so you have to do something like:

$env:EDITOR = (get-command nvim).source -replace '\\','/'

, which is still a valid Windows path fortunately, but should not be necessary.

Under the UNIX semantics for Git, start_command() is supposed to try to run the command with execve() before interpreting it as a shell command, if that was the case in Git for Windows, my original example would work fine.

Your example of e.g.

$env:EDITOR = "C:\code\code -someparam"

, would also work fine. In the case of simple commands such as this, shell interpretation should not even be necessary, regardless of choice of shell.

So I would argue that this is a bug from the perspective of preserving the UNIX semantics in Git for Windows as well.

PowerShell also natively supports $env:PAGER, and some other Windows programs do as well, without any special path conversion.

It does not sound very likely to me that such a compelling reason exists, and if it does, I am certainly not yet privy to it. If it exists, I definitely would love to learn about it.

Let's continue this in the relevant thread.

@dscho
Copy link
Member

dscho commented Jun 8, 2022

Neither CMD nor PowerShell are a shell.

They are scripting interpreters that make it convenient to run external binaries/scripts.

Python is also a scripting interpreter. So is node.js.

Besides, your suggestion to change this now would literally break existing setups. Like thousands. Maybe even hundreds of thousands. Nobody can sincerely expect me to do that.

I never suggested this, and it would not break anything presuming the alias shell is configurable.

You suggested precisely that, by suggesting to let something else than a POSIX-compliant shell handle the ! expansion of Git's aliases. Or the expansion/interpretation of the EDITOR variable.

Quite honestly, it defies my sense of logic why we are still discussing this.

@rkitover
Copy link
Author

rkitover commented Jun 8, 2022

You have not responded to my other points, namely that the first attempt to expand an ! alias is NOT done by a POSIX shell, but by execve(), these are the semantics on UNIX and Git for Windows does not match them.

@rkitover
Copy link
Author

rkitover commented Jun 8, 2022

See around line 830 in run-command.c.

@dscho
Copy link
Member

dscho commented Jun 8, 2022

You have not responded to my other points

The idea is that ! aliases and EDITOR and quite a few other things are handled via a POSIX-compliant shell. If we take shortcuts in that handling when we realize that we do not need to bother with a POSIX shell because we get a trivial command-line, then that does not change anything about the POSIX shell requirement in the non-trivial cases.

@rkitover
Copy link
Author

rkitover commented Jun 8, 2022

The cases in this issue are in fact trivial cases and would work fine via direct execution, but DO NOT work with a POSIX shell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants