Allow running Git directly from C:\Program Files\Git\mingw64\bin\git.exe

[dscho]

We still recommend running git.exe via the Git wrapper in /cmd/, just to make sure that the environment variables are set appropriately, such as adding the /usr/bin/ directory to the PATH.

This is relevant e.g. for running the hooks via the Bash.

With this PR, we no longer require that Git be run via the /cmd/git.exe wrapper: it can be run directly from /mingw64/bin/git.exe, configuring the environment variables appropriately itself (using the absence of the MSYSTEM variable as indicator that it should do so).

This addresses #2283

[jeffhostetler]

Is there a test that we could add to demonstrate the change? Such as: does the output of git --exec-path (no argument) change?

[jeffhostetler]

Should we prepend rather than append them to the PATH ??

[jeffhostetler]

Maybe I'm over analyzing this. But if we're appending $HOME/bin and /usr/bin and /mingXX/ to the PATH, aren't we introducing ambiguity? If I have an official release installed in Program Files and a private version installed in my $HOME, which version of a lib-exec (or maybe just non-builtin) exe gets run when I run a command?

I think a series of tests with before and after output would be helpful here.

[dscho]

Should we prepend rather than append them to the PATH ??

The function name threw you off, right? It appends to the buffer, not to the PATH. The value of getenv("PATH") (if any) is actually appended to the buffer after appending the system bin directories. The idea is that the buffer is initialized with <HOME>/bin/; (HOME is set, which at that point should be the case), then append the system bin directories /mingw64/bin/ and /usr/bin/, and then append the value of getenv("PATH") (if it was set to begin with).

[dscho]

Maybe I'm over analyzing this. But if we're appending $HOME/bin and /usr/bin and /mingXX/ to the PATH, aren't we introducing ambiguity?

We're actually prepending it, by virtue of starting with a completely empty buffer. And then we add ~/bin/, /mingw64/bin/ and /usr/bin/, in this order. After that, we append the original value of PATH.

If I have an official release installed in Program Files and a private version installed in my $HOME, which version of a lib-exec (or maybe just non-builtin) exe gets run when I run a command?

Note that none of this has anything to do with the /mingw64/libexec/git-core/ directory. That is prepended at a totally different point in time, long after this here code.

But your concern is valid: we really need to prepend the components to the PATH (and we do, actually).

I think a series of tests with before and after output would be helpful here.

The biggest problem with adding a test here is that the functionality added in this PR depends highly on the RUNTIME_PREFIX feature: git.exe will figure out its absolute path, then determine where in the MSYS pseudo root it is, depending on its location. For example, if run as C:/Program Files/Git/mingw64/bin/git.exe, it will figure out that it lives in the /mingw64/bin/ directory inside the MSYS pseudo root, and correctly add that, and then <MSYS>/usr/bin to the PATH.

If run as C:/Program Files/Git/mingw64/libexec/git-core/git.exe (which is also a valid scenario), it will detect that it lives in the /mingw64/libexec/git-core/ subdirectory, and again use the correct MSYS pseudo root to figure out where /usr/bin/ is.

If run as D:/a/1/sgit.exe (as would be the case in our CI builds), or as C:/Users/me/Desktop/git/git.exe (a relatively common scenario with contributors), there is zero chance to figure out where the MSYS pseudo root is. That information would have to come from the test suite, via an environment variable, but we do not have any such facility to override the RUNTIME_PREFIX (at least not yet).

What we can do (and what I will do) is to add a simple test that verifies that MSYSTEM has been set.

[jeffhostetler]

Um, perhaps I'm just being dense here, but I find all this startup magic way more confusing than I think it needs to be, but I realize there are historical issues here.

Yes, append in the name of the function did throw me, but also the different meanings of the variable "path" in the caller and callee didn't help.

As for testing, would it be helpful to create a tXXXX.sh script that does essentially:

cd $TRASH_DIRECTORY
mkdir foo
mkdir foo/Git/mingw64/bin
mkdir foo/Git/mingw64/libexec/git-core
... and so on ...
cp git.exe foo/Git/mingw64/bin
echo >foo/Git/mingw64/libexec/git-core/git-echo-path <<EOF
    #!/bin/sh
    echo $PATH
EOF
export HOME=foo/home
mkdir foo/home/bin

./foo/Git/mingw64/bin/git echo-path >OUTPUT

And then confirm that the child script was invoked correctly (meaning git.exe found the correct associated git-core directory and was able find the "gitDASHecho-path" ) and that the child script inherited the expected PATH.

Perhaps that's overkill. Perhaps not.

[jeffhostetler]

I'm going to be away from my keyboard for a while today, so I'm going to go ahead and approve this. I'd like to see some tests and maybe a little more comments about what we're doing, but I don't want to block this for that.

[dscho]

Um, perhaps I'm just being dense here, but I find all this startup magic way more confusing than I think it needs to be, but I realize there are historical issues here.

Yes, you're right, hysterical raisins at play. And the fact that Git is not really portable enough to support Windows, we go out of our way to pretend that we're just implementing POSIX shims in compat/mingw.c.

As for testing, would it be helpful to create a tXXXX.sh script that does essentially:

cd $TRASH_DIRECTORY
mkdir foo
mkdir foo/Git/mingw64/bin
mkdir foo/Git/mingw64/libexec/git-core
... and so on ...
cp git.exe foo/Git/mingw64/bin
echo >foo/Git/mingw64/libexec/git-core/git-echo-path <<EOF
    #!/bin/sh
    echo $PATH
EOF
export HOME=foo/home
mkdir foo/home/bin

./foo/Git/mingw64/bin/git echo-path >OUTPUT

That's a good idea! Originally, I was worried about .dll files: if I only copy git.exe and not, say, zlib1.dll, then Windows looks for that DLL in C:\Windows\system32 directory before it looks at the PATH. Only a DLL in the same directory as the .exe takes precedence. But in practice, that will probably not matter. So I should be able to copy $GIT_EXEC_PATH/git.exe as you indicated.

[jeffhostetler]

yeah, i forgot about the DLLs, but maybe also copy them into that bin directory too.

(I'm thinking about a MSVC=1 build that will complain differently....)

[dscho]

yeah, i forgot about the DLLs, but maybe also copy them into that bin directory too.

The biggest problem with that will be that I don't know which DLLs are required in that test...

But $GIT_EXEC_PATH should be in the $PATH (and if not, I can make it so, in the scope of this test).

(I'm thinking about a MSVC=1 build that will complain differently....)

It should not complain differently, because as part of the build, the DLLs are copied into what will become $GIT_EXEC_PATH in the test script.

[dscho]

@jeffhostetler just to make sure: do you want me to add any more information to the commit message?

[jeffhostetler]

very nice. thanks for the extra info. jeff Jeff

…

________________________________ From: Johannes Schindelin <notifications@github.com> Sent: Wednesday, February 5, 2020 3:07:01 PM To: git-for-windows/git <git@noreply.github.com> Cc: Jeff Hostetler <Jeff.Hostetler@microsoft.com>; Mention <mention@noreply.github.com> Subject: Re: [git-for-windows/git] Allow running Git directly from `C:\Program Files\Git\mingw64\bin\git.exe` (#2506) @jeffhostetler<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fjeffhostetler&data=02%7C01%7CJeff.Hostetler%40microsoft.com%7Ccdfb4d3c54bc49e7f40808d7aa76f6f9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637165300236421291&sdata=O83T5SVfX4TJNBcJzKoUR66qfzEkx%2FjX5pBvDG5uAO4%3D&reserved=0> just to make sure: do you want me to add any more information to the commit message? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgit-for-windows%2Fgit%2Fpull%2F2506%3Femail_source%3Dnotifications%26email_token%3DABZFRSDDFVCXYFI5LNL45CTRBML6LA5CNFSM4KO4DNIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK4ZOKA%23issuecomment-582588200&data=02%7C01%7CJeff.Hostetler%40microsoft.com%7Ccdfb4d3c54bc49e7f40808d7aa76f6f9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637165300236431285&sdata=o6eQBJ56EwxiO6xpuebWRJ65MIdgqr8RmxItROLEtMU%3D&reserved=0>, or unsubscribe<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FABZFRSFUEPZHSDGD7QU4G63RBML6LANCNFSM4KO4DNIA&data=02%7C01%7CJeff.Hostetler%40microsoft.com%7Ccdfb4d3c54bc49e7f40808d7aa76f6f9%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637165300236436287&sdata=%2BVy2qWmTkcFm1iL6vL0MArgix3EuX5VjDltVjTbUQec%3D&reserved=0>.