search

git-for-windows / git

A fork of Git containing Windows-specific patches.
http://gitforwindows.org/
Other
8.32k stars 2.53k forks source link

Mingit-busybox unable to clone from a proxyed ssh server. #3285

Open ur4t opened 3 years ago

ur4t commented 3 years ago

Workarounds

Setup

$ git --version --build-options
git version 2.32.0.windows.1
cpu: x86_64
built from commit: 4c204998d0e156d13d81abe1d1963051b1418fc0
sizeof-long: 4
sizeof-size_t: 8
shell-path: /bin/sh
feature: fsmonitor--daemon

Actually previous versions act the same way.

$ cmd.exe /c ver
Microsoft Windows [Version 10.0.19043.1052]
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

No /etc/install-options file found.

Details

$ cat ~/.ssh/config
Host behind.proxy.me
  ProxyCommand ssh gate.proxy.me -W %h:%p
$ git clone git@behind.proxy.me:path/to/repo
/bin/sh: No such file or directory
banner exchange: Connection to UNKNOWN port 65535: Broken pipe
fatal: Could not read from remote repository.

Please make sure you have the correct access rights

Workaround

Simply copy /mingw64/bin/busybox.exe to /usr/bin/sh.exe.

dscho commented 3 years ago

Simply copy /mingw64/bin/busybox.exe to /usr/bin/sh.exe.

Does it also work if you copy it to /mingw64/bin/sh.exe? That might be more appropriate, although I wish there was a better way.

ur4t commented 3 years ago

Simply copy /mingw64/bin/busybox.exe to /usr/bin/sh.exe.

Does it also work if you copy it to /mingw64/bin/sh.exe? That might be more appropriate, although I wish there was a better way.

No. And my first try is /bin/sh.exe, which does not work neither.

ur4t commented 3 years ago

Can a busybox named /usr/bin/sh.exe provide the same functionality as named /mingw64/bin/busybox.exe?

I can not find how mingit-busybox are built and have no idea how it utilizes busybox.exe.

ur4t commented 3 years ago

It is necessary to keep /usr/bin/sh.exe a compatiable shell (which was provided by bash in build environment) in order to provide ssh proxy command in mingit-busybox, and personally i prefer use busybox as /usr/bin/sh.exe (much better to use and smaller in size than /usr/bin/sh.exe provided by bash shipped with mingit).

After investigating make-file-list.sh and release.sh, the only approach I can provide is this dirty hack.

diff --git i/mingit/release.sh w/mingit/release.sh
index af65e2b..300e67a 100755
--- i/mingit/release.sh
+++ w/mingit/release.sh
@@ -146,6 +146,18 @@ die "Could not copy libexec/git-core/*.exe"

 test ! -f "$TARGET" || rm "$TARGET" || die "Could not remove $TARGET"

+
+if test -n "$MINIMAL_GIT_WITH_BUSYBOX"
+then
+       mkdir -pv "$SCRIPT_PATH"/root/usr/bin/
+       cp -r /mingw64/bin/busybox.exe "$SCRIPT_PATH"/root/usr/bin/sh.exe
+fi
+
 echo "Creating .zip archive" &&
 (cd / && 7za a -mx9 "$TARGET" $LIST "$SCRIPT_PATH"/root/*) &&
 echo "Success! You will find the new MinGit at \"$TARGET\"."
+
+if test -n "$MINIMAL_GIT_WITH_BUSYBOX"
+then
+       rm -rf "$SCRIPT_PATH"/root/usr/
+fi
ur4t commented 3 years ago

Calling $GIT_INSTALL_ROOT/usr/bin/sh.exe ==> / is $GIT_INSTALL_ROOT/usr. Calling $GIT_INSTALL_ROOT/usr/bin/busybox.exe sh ==> / is $GIT_INSTALL_ROOT. I think that is why we must place a compatible shell as /usr/bin/sh.exe because shell-path is /bin/sh.

dscho commented 3 years ago

Can a busybox named /usr/bin/sh.exe provide the same functionality as named /mingw64/bin/busybox.exe?

No. /usr/ is reserved for MSYS programs, i.e. programs that understand the Unix-style paths like /usr/bin/.

The MINGW programs, in contrast, live in /mingw64/bin/ (and they cannot interpret that path correctly, they would interpret it as an absolute path relative to the current directory's drive, e.g. if you are in D:\Users\ur4t, MINGW programs would interpret /mingw64/bin as D:\mingw64\bin).

I can not find how mingit-busybox are built and have no idea how it utilizes busybox.exe.

Part of it is still a really ugly hack: 1dbbcfbfd2386a8ae29ff7ed8ce1bda9bf09ffaa

Another part is setting the environment variable MINIMAL_GIT_WITH_BUSYBOX while building MinGit: https://github.com/git-for-windows/build-extra/blob/1702020599af804bddb4e77fbcafa6fb5b44f303/mingit/release.sh#L22-L23

It is necessary to keep /usr/bin/sh.exe a compatiable shell

What does "compatible" mean? If it means compatible with the MSYS version of OpenSSH's ideas what constitutes a path (see the /mingw64 example above), then BusyBox' ash definitely is not such a shell.

If it means POSIX-compliant (without funny ideas that POSIX requires Unix-style paths, which it definitely does not, Exhibit A: VMS), then BusyBox should be good enough.

After investigating make-file-list.sh and release.sh, the only approach I can provide is this dirty hack.

diff --git i/mingit/release.sh w/mingit/release.sh
index af65e2b..300e67a 100755
--- i/mingit/release.sh
+++ w/mingit/release.sh
@@ -146,6 +146,18 @@ die "Could not copy libexec/git-core/*.exe"

 test ! -f "$TARGET" || rm "$TARGET" || die "Could not remove $TARGET"

+
+if test -n "$MINIMAL_GIT_WITH_BUSYBOX"
+then
+       mkdir -pv "$SCRIPT_PATH"/root/usr/bin/
+       cp -r /mingw64/bin/busybox.exe "$SCRIPT_PATH"/root/usr/bin/sh.exe
+fi
+
 echo "Creating .zip archive" &&
 (cd / && 7za a -mx9 "$TARGET" $LIST "$SCRIPT_PATH"/root/*) &&
 echo "Success! You will find the new MinGit at \"$TARGET\"."
+
+if test -n "$MINIMAL_GIT_WITH_BUSYBOX"
+then
+       rm -rf "$SCRIPT_PATH"/root/usr/
+fi

Yep, that's a dirty hack all right ;-) I am not sure about the rm -rf ... part, though, we do not clean up after any other copied file either, so why start with BusyBox.

Calling $GIT_INSTALL_ROOT/usr/bin/sh.exe ==> / is $GIT_INSTALL_ROOT/usr. Calling $GIT_INSTALL_ROOT/usr/bin/busybox.exe sh ==> / is $GIT_INSTALL_ROOT. I think that is why we must place a compatible shell as /usr/bin/sh.exe because shell-path is /bin/sh.

That could potentially be a bug in our fork of BusyBox-w32.

But I am more worried about the requirements to that local shell by OpenSSH.

In any case, I would much rather have BusyBox' shell be executed as busybox sh. Looking at OpenSSH's source code, that is not possible, though: it expects to be able to call the shell via a single argv[0]: https://github.com/openssh/openssh-portable/blob/a023138957ea2becf1c7f93fcc42b0aaac6f2b03/sshconnect.c#L158

So my less favorite solution is to copy busybox.exe to /mingw64/libexec/busybox/sh.exe (because we cannot ship it hard-linked: .zip file does not have a robust standard to represent hard links), but then, the BusyBox executable weighs in with 1.1 megabyte so that would bloat the .zip rather unhealthily.

BTW we can easily override the shell to be used via the environment variable SHELL: https://github.com/openssh/openssh-portable/blob/a023138957ea2becf1c7f93fcc42b0aaac6f2b03/sshconnect.c#L122-L123

Now that I thought about it for a moment, there is yet another solution. We already have the problem of hard-linked files: Git's own "built-ins" are hard linked copies of git.exe. To avoid bloating, we already have what we call the "Git wrapper", a small executable that simply hands off to the correct executable. We could teach the wrapper to interpret the base name sh.exe to mean that it should call busybox.exe (located in the same directory as sh.exe with sh shifted in as argv[1] and then the rest of the command-line arguments. Then, we could copy the Git wrapper (which is small, therefore duplicating it is not all that painful) and use it.

ur4t commented 3 years ago

BTW we can easily override the shell to be used via the environment variable SHELL.

In any case, I would much rather have BusyBox' shell be executed as busybox sh. Looking at OpenSSH's source code, that is not possible.

I hope that we will get a chance to resolve this more elegantly, e.g. by some magic /etc/ssh/config setting that hopefully exists to that end. https://github.com/git-for-windows/git/issues/1439#issuecomment-865886799

It is impossible to seperately sett SHELL for ssh in ssh_config, at least I didn't find any approach.

What does "compatible" mean?

If it means compatible with the MSYS version of OpenSSH's ideas what constitutes a path (see the /mingw64 example above), then BusyBox' ash definitely is not such a shell.

If it means POSIX-compliant (without funny ideas that POSIX requires Unix-style paths, which it definitely does not, Exhibit A: VMS), then BusyBox should be good enough.

According to man pages of ssh_config, ssh utilizes user's shell to run all commands with exec in configuraion files, including ProxyCommand. In this case, I think POSIX-compliant is enough. And as for Win32-openssh, it can even be not POSIX-compliant.

The MINGW programs, in contrast, live in /mingw64/bin/ (and they cannot interpret that path correctly, they would interpret it as an absolute path relative to the current directory's drive, e.g. if you are in D:\Users\ur4t, MINGW programs would interpret /mingw64/bin as D:\mingw64\bin).

That could potentially be a bug in our fork of BusyBox-w32.

Position of busybox.exe (or in form of sh.exe) /
$GIT_INSTALL_ROOT/mingw64/bin/ $GIT_INSTALL_ROOT
$GIT_INSTALL_ROOT/usr/bin/ $GIT_INSTALL_ROOT/usr
ANYWHERE_ELSE ANYWHERE_ELSE

It seems that out hacked busybox-w32 tries to emulate msys path. Is msys busybox more suitable?

To avoid bloating, we already have what we call the "Git wrapper".

Great technique! There is a good target, /usr/bin/dash.exe (109KB), for both wrapping or even simply renaming. On Debian, /bin/sh is just a symlink of dash.

dscho commented 3 years ago

I hope that we will get a chance to resolve this more elegantly, e.g. by some magic /etc/ssh/config setting that hopefully exists to that end. #1439 (comment)

It is impossible to seperately sett SHELL for ssh in ssh_config, at least I didn't find any approach.

True. But we can set SHELL in /cmd/git.exe and in /bin/sh.exe (which are copies of the Git wrapper). Or we can set it in /mingw64/bin/git.exe when we determine that we're configured to run BusyBox.

Speaking of which: the current method of falling back to using BusyBox if a given command was not found on the PATH is not my favorite thing ever. For a long time I am already thinking that we should introduce a new config setting, say, core.useBusyBox and use that to determine whether we should run sh through BusyBox.

Another idea I had was to not copy busybox.exe verbatim into MinGit, but under the name /mingw64/bin/sh.exe. It would be the duty of BusyBox to set the SHELL accordingly (unless already set). There are two disadvantages through this approach:

It seems that out hacked busybox-w32 tries to emulate msys path.

Right, if you use forward slashes, it tries to emulate that an "absolute" path is relative to the installation root, via mingw_pathconv(). And that installation root is determined via looking at the executable path and stripping off /bin or even /mingw[0-9][0-9]/bin.

Is msys busybox more suitable?

That would take away almost the complete speed benefit of BusyBox-w32's not using that POSIX emulation layer, the MSYS2 runtime.

ur4t commented 3 years ago

Another idea I had was to not copy busybox.exe verbatim into MinGit, but under the name /mingw64/bin/sh.exe. It would be the duty of BusyBox to set the SHELL accordingly (unless already set).

This goes too far for solving this issue.

I provided some workarounds in the first comment for searching for solution.

dscho commented 3 years ago

Another idea I had was to not copy busybox.exe verbatim into MinGit, but under the name /mingw64/bin/sh.exe. It would be the duty of BusyBox to set the SHELL accordingly (unless already set).

This goes too far for solving this issue.

Would it really?

ur4t commented 3 years ago

Another idea I had was to not copy busybox.exe verbatim into MinGit, but under the name /mingw64/bin/sh.exe. It would be the duty of BusyBox to set the SHELL accordingly (unless already set).

This goes too far for solving this issue.

Would it really?

My fault. Sorry.

I thought now that this issue thread is mixed up with #1439, we can continue out discussion there.

ur4t commented 3 years ago

As we must keep msys part, I think use /usr/bin/dash.exe as shell is a good solution. Not only mingit-busybox, mingit will also benefit from this change, which means the bigger bash package can be stripped out.

dscho commented 3 years ago

As we must keep msys part, I think use /usr/bin/dash.exe as shell is a good solution. Not only mingit-busybox, mingit will also benefit from this change, which means the bigger bash package can be stripped out.

What about users who think that their hooks that use Bash-isms should keep working? MinGit is used e.g. inside Visual Studio, and there are so many Visual Studio users out there that even if only 0.01% of them use hook scripts with Bash-isms, we're talking about thousands of users.

ur4t commented 3 years ago

What about users who think that their hooks that use Bash-isms should keep working? MinGit is used e.g. inside Visual Studio, and there are so many Visual Studio users out there that even if only 0.01% of them use hook scripts with Bash-isms, we're talking about thousands of users.

If we want mingit-busybox to be able to replace mingit completely in the future, we will face this problem too. Or we should keep sh.exe (1964KB) which is a hardlink (on Windows it means a copy) of bash.exe.

dscho commented 3 years ago

Hardlinks are incompatible with .zip files. That's why we had to jump through that hoop with the Git wrapper in the first place.

dscho commented 3 years ago

@ur4t could I ask you to test this with the just-generated artifacts from https://dev.azure.com/git-for-windows/git/_build/results?buildId=80233&view=artifacts&pathAsName=false&type=publishedArtifacts? Like, copy git-cmd.exe to mingw64/bin/ash.exe and point SHELL to that file's absolute path, then try to access your remote SSH server via that proxy?

ur4t commented 3 years ago

Sorry for this belated response. In the new release (2.32.0.windows.2) this ash wrapper works well as expected.

ur4t commented 1 year ago

usr/bin/sh is bunbled since v2.36.1 (https://github.com/git-for-windows/git/issues/3825), thus mingw64/bin/ash.exe wrapper is no longer needed.

dscho commented 1 year ago

usr/bin/sh is bunbled since v2.36.1 (#3825), thus mingw64/bin/ash.exe wrapper is no longer needed.

/usr/bin/sh does not refer to BusyBox at all.

ur4t commented 1 year ago

usr/bin/sh is bunbled since v2.36.1 (#3825), thus mingw64/bin/ash.exe wrapper is no longer needed.

/usr/bin/sh does not refer to BusyBox at all.

Yes.

For more information as contexts, mingw64/bin/ash.exe wrapper was added to help solve the problem in this issue, which was caused by missed usr/bin/sh. Now that usr/bin/sh is present, this problem is solved.

mingw64/bin/ash.exe wrapper is redundant, but innocuous.

After whether to keep providing mingw64/bin/ash.exe wrapper is determined, this issue can be closed.

dscho commented 1 year ago

@ur4t I fear there is a misunderstanding there. /usr/bin/sh.exe is not BusyBox at all. It is a regular MSYS2 Bash, which is a bug: MinGit(BusyBox) should not even contain that file.

ur4t commented 1 year ago

/usr/bin/sh.exe is not BusyBox at all. It is a regular MSYS2 Bash, which is a bug: MinGit(BusyBox) should not even contain that file.

Yes I know that is not busybox at all. I was thinking that:

Though not perfect, current solution (provide a busybox ash wrapper and ask users to set SHELL to execute proxy commands) is acceptable (at least for me).

Could we close this issue and record todos and blockers in https://github.com/git-for-windows/git/issues/1439?