Use env as interpreter for all exe and scripts. #3869
Conversation
Git requires a MSYS2 perl. However, there are many distributions of perl. If another perl is in the PATH in front of the MSYS2 one, git will mistakenly find it. The shebang is right #!/usr/bin/perl, but actually git only recognizes perl and finds the first one in the PATH. This pull request removes the naïve parser of shebang, and just finds `env.exe` as the interpreter. It should be in the `PATH` in MSYS2. Signed-off-by: Yuyi Wang <Strawberry_Str@hotmail.com>
|
Hmm. While I understand the motivation, the performance penalty of adding yet another instance of the MSYS2 runtime will add to the slow performance we unfortunately already experience on Windows. Granted, this is only for scripts, but hooks are also run as scripts and running them through Besides, it is only an implementation detail that MinGit includes a working My favored solution to the problem would be to fix |
|
Yes, I agree that it is a performance regression. I've also considered a solution that, only pass to I don't know why I don't know well about perl and the scripts. Maybe it is practical to make them work with MinGW perl, but I don't know how. It seems to me that this pull request makes all things work successfully with a little change. It is a little radical, but I can add a fallback if you want. However, if you insist to modify the scripts rather than forwarding to |
Why do we perform a I feel like we should recognize that as an absolute path and that the appropriate executable exists in that path, before trying to look for alternatives in |
Because I've just come across another solution: grap some part of MSYS2 code related to UNIX path handling and merge into (or static/dynamic link to) |
Historical reasons, as Msys had its
Letting all Git commands exit with code 123 would also be a little change. Just because some PR introduces a little change does not make it correct. The performance regression this PR introduces makes it incorrect.
Maybe we can start by showing the symptoms how MINGW perl fails to execute the command? |
Well, I agree it is a performance regression in some cases, but I don't agree that it is incorrect. It doesn't produce incorrect results, on the contrary, it finds correct interpreter and makes subcommands work correctly. Anyway, I'm now using
I need to have another check then. You can try yourself with I'd like to remind that not only the perl script may fail when using different interpreter, but maybe the python script etc. as well. You cannot deal with all cases, and it will be a lot of patches. Some third-party scripts may also face problems and you cannot fix all of them. If git can find the MinGW perl instead of MSYS2 one, it may also find the cygwin one, or the one in the The calling of |
Neither can
No. The MinGW one here is special in that It'll be on the
All of those except the stub are equally fine to us.
By extension of that logic, Git on Windows should just do the proper Windows thing and treat only file extensions in
At least currently it can't. It could potentially in the future with MinGit, but that would mean MinGit is basically broken if you don't have Msys2 or Cygwin installed. So that's probably not a good Idea. |
I agree that
Other
I'm not rather sure about that, because the scripts may not deal with paths and OS specific things well.
Many subcommands are scripts and the filename doesn't end with an extension. Windows cannot know which interpreter is correct. If you add an extension, the subcommands will become
Yes, not executing
No, I'm not suggesting to include cygwin into MinGit instead of MSYS2. It's just talking about possible cases. I understand why there's MSYS2 in MinGit. |
Well, I actually didn't understand this extension. Of course a Windows native executable cannot handle POSIX path well. Let's forward things to professionals. Windows affair on Win32, and POSIX affair inside POSIX-compatible environment (MSYS2, or cygwin, or even WSL). |
Oh, yes. The current code would just use
No, we already prevent that.
Sure, but we have no way to tell what a user script expects in that regard. All of them are equally as likely to be the correct python for this specific script.
I'm fully aware. I wasn't suggesting to do that. I was exaggerating this "native Windows applications shouldn't handle POSIX things" thought. The whole concept that shebangs have meanings (and files with a shebang need some kind of special treatment) is as foreign to Windows as Unix paths.
No, what I meant was, that we'll always find the Msys2 |
Well, I've never noticed this tool...and never used it before. It seems a wrapper shipped with MinGit.
Yes, with Well, you convinced me that this PR is not a must for git-for-windows distributions, and even not a bug fix. However, I'm not using the specific distribution, but MSYS2 with a MinGW git. As I commented at first, I installed a MinGW perl in the same path as git. |
That's not a good measure of the performance impact. I doubt you're working on a project the size of, say, Microsoft Office, let alone with dozens of custom scripted commands or hooks. "Works on my computer" is rightfully a frowned-upon reaction in our industry.
The Git wrapper is installed into But I fear that we're spending a lot of time on arguing about an approach that will not make it into Git for Windows, and that time may very well be better spent on diagnosing how the Git commands that are implemented in Perl fail, actually. You may find that #1604 may have addressed at least some of these issues already, but in the end the progress on that PR stalled. Maybe you can revive it because you have a vested interest in getting this to work? |
I'd like to see more professional benchmarks, too.
I'm sorry about that. It would be nice to make
Yes, maybe. I'm not good at perl though. I'll try looking into it, but may not succeed. |
I don't have any current ones, and then only ones whose output I am not at liberty to share verbatim, just the conclusion. We did have reports in the past (but I could not find them in a brief search) that
Nobody is. But together we can address your issue, I am certain of it. |
Git uses a lot scripts which requires bash, perl, and python. The interpreter is specified by shebang. However, Windows don't know about shebang, and
CreateProcesscould not spawn a script file.Therefore, git chose to parse the shebang and find the interpreter itself. The interpreter is found in
PATH. It is reasonable because git is a Windows native program, and doesn't know anything about POSIX path. However, the method is not that perfect.Git requires a MSYS2 perl. However, MSYS2 also provides a MinGW perl. If both perl are installed (for example,
texliverequires a MinGW perl), git will mistakenly find the MinGW perl, and scripts require perl (e.g.git-send-email) will raise errors. The shebang is right#!/usr/bin/perl, but actually git only recognizesperland finds the first one in thePATH.Luckily there's a program in MSYS2 called
env. It is commonly used in *NIX systems to find the right environment. The MSYS2envcould not only parse the shebang, but also recognize all kinds of path: POSIX path, Windows path, and even mixed path (C:/msys64/usr/bin\less.exe). Technically, it could be the interpreter for all executables and scripts.This pull request removes the naïve parser of shebang, and just finds
env.exeas the interpreter. It should be in thePATHin MSYS2 environment. I don't open this one to the upstreamgit/gitbecause it assumes MSYS2 env.