search

git-for-windows / git

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

Get the BusyBox-based MinGit production-ready #1439

Open dscho opened 6 years ago

dscho commented 6 years ago

Cygwin (and hence MSYS2, which is a derivative) tries to emulate POSIX functionality on top of the Win32 API. When spawning child processes, this means that fork() needs to be emulated, which is hard, and requires the MSYS2 runtime's address range to be pinned. This leads to many a problem on many, many setups sometimes even only after unrelated software is upgraded!

Git for Windows uses the MSYS2 runtime essentially for two things: SSH and Unix shell/Perl scripting.

In MinGit, we already do not include any Perl scripts. But plenty of Unix shell scripts. Ideally, those would be converted into "builtins", i.e. pure, portable, performant C, which would make everything quite a bit more robust, not to mention fast. Sadly, this is no priority of core Git's developers/maintainer and it is a lot of work.

To side-step this, we put in quite an effort last year to ship a "BusyBox-based" variant of MinGit. BusyBox is an executable that offers minimal versions of many Unix tools, such as a Unix shell, sed, awk, etc, in a single binary (much like git.exe includes many subcommands as "builtins"), and there exists a pure Win32 version of BusyBox that we helped along until it could run Git's Unix shell scripts and test suite.

It is time to get this BusyBox-based MinGit to a point where it is robust enough to be the default MinGit.

shiftkey commented 6 years ago

@dscho I'm interested in this, especially to reduce/avoid the dependency on MSYS2/Cygwin. How can others help?

dscho commented 6 years ago

How can others help?

Mostly by testing in as close to production as you dare ;-)

I vaguely remember that there have been some inexplicable hangs when I tried to run the test suite (I used GIT_TEST_ARGS=--quiet GIT_TEST_INSTALLED=/path/to/mingw64/bin busybox xargs -P15 ./test-[0-9]*.sh after copying the test artifacts (which are now bundled as Pacman package, too) into a clone of git-for-windows/git). That's the main part I want to address in the near future.

shiftkey commented 6 years ago

Mostly by testing in as close to production as you dare ;-)

dscho commented 6 years ago

Hey everyone it been hard to get anything out of this

I am sorry to hear that. But I also have to say that I have a hard time understanding what exactly the problem is. Care to explain in any sort of detail?

mingwandroid commented 6 years ago

The Anaconda Distribution has been trying to use busybox Git for Windows and ran into a problem with git submodule hanging:

C:\gfw\cmd\git.exe submodule update --init --recursive
10:53:00.229338 git.c:576               trace: exec: git-submodule update --init --recursive
10:53:00.229338 run-command.c:640       trace: run_command: git-submodule update --init --recursive

I added set -x and a few echo's to C:\gfw\mingw64\libexec\git-core\git-submodule to see what I could see and I saw two problems:

  1. It uses basename and that doesn't seem to be backslash happy:
+ basename 'C:\gfw\mingw64/libexec/git-core\git-submodule'
+ echo 'git-core\git-submodule'
git-core\git-submodule
+ basename 'C:\gfw\mingw64/libexec/git-core\git-submodule'
  1. It hangs at the call to sed. It seems that another busybox got spawned ok and is pegging one of my CPU threads but I guess they're not chatting to each other?
+ sed -e 's/-/ /'
mingwandroid commented 6 years ago

I see a commandline of sh --forkshell 00000000000002CC for the last spawned, busy busybox.

mingwandroid commented 6 years ago

Switching the busybox.exe in the latest release to this one allows this to work (or at least to not hang).

dscho commented 6 years ago

Sorry for the late reply. I am still struggling to find any time for any serious work on BusyBox. This comment makes me believe that the hang is caused by the patches I introduced on top of BusyBox-w32, and that's what most likely also causes the so far unexplained hangs in the test suite I observed.

shiftkey commented 6 years ago

@dscho is there a particular build we should be helping with testing if this has accidentally resolved itself, or should we hold off for the moment?

mingwandroid commented 6 years ago

No problem at all @dscho there's only so many hours in a day.

dscho commented 6 years ago

is there a particular build we should be helping with testing if this has accidentally resolved itself, or should we hold off for the moment?

@shiftkey Sadly, there is no chance of this being resolved accidentally...

@mingwandroid thanks for understanding!

leonyu commented 6 years ago

I actually found busybox mingit to be more usable by end-user than regular mingit, as you can simply run busybox sh to get a working shell, while bash from regular mingit is basically unusable.

shiftkey commented 6 years ago

while bash from regular mingit is basically unusable.

I don't believe bash.exe ships in the vanilla MinGit environment. sh.exe does, and if you've seen issues with that I'd love to hear more.

leonyu commented 6 years ago

it has bash as sh. At least that's what --version tells me.

However, it is not usable as an interactive shell, cause there is no readline, and and the PATH is not automatically set to include /usr/bin, so it doesn't resolve those unix commands by default, like busybox does. (Or am i missing something)

Update, path is set if I call sh.exe --login. However readline is still pretty broken in.

In addition, mingit busybox has broken Unicode support (probably expected given it took a few versions to get unicode support in MSYS).

dscho commented 6 years ago

MinGit is not intended to be used interactively, skipping a ton of parts in the quest to minimize the footprint for applications which want to ship with Git. Any interactive functionality you use in MinGit (BusyBox variant or not) might go away at any stage, without prior warning (apart from this here stern one).

drizzd commented 5 years ago

I am increasingly annoyed by the fact that Git-bash behavior is just so special that commands which are intended to work both in cmd.exe and in a real POSIX shell, but they always need special hanling in Git-bash. For example, I frequently use winpty and MSYS2_ARG_CONV_EXCL='*' to run simple commands like winpty docker run -it ubuntu ls //bin. And when I manage to do this some commands crash anyway or produce strange output because somehow the pseudo terminal emulation works differently.

As far as I am concerned, I don't need the full power of bash on Windows, or the path conversion, or the terminal emulation. I only want to native Windows applications in a native Windows terminal. On the other hand, I just can't get used to cmd.exe or PowerShell because I am so addicted to basic command line editing shortkeys like ctrl-p, ctrl-n, ctrl-a, ctrl-e, alt-b, alt-f, alt-w, alt-d, and maybe also ctrl-z and ctrl-r.

I think it would be huge if we can rid Git-for-Windows of the complexity induced by the MSYS2/Cygwin emulation layer (both runtime and terminal emulation) and replace it with Windows native code. Why do we consider BusyBox only for MinGit, but not as a replacement for MSYS2/Cygwin/Git-bash? Ignoring backwards compatibility for now, what would be the minimum requirements for such a replacement?

Do we need more than the following:

dscho commented 5 years ago

Note that BusyBox comes with its own less and its own readline lookalike. Having said that, we are currently very reliant on MSYS2. Off the top of my head:

Also, please note that BusyBox supports Ctrl+R, but not Ctrl+Z.

Further, BusyBox-w32 has troublesome performance issues, at least currently. In theory it should be a lot faster to execute shell scripts using BusyBox' ash than using MSYS2's Bash. In practice, it seems to be the opposite. My guess is that the way the forkshell emulation is implemented is suboptimal, and leaves a lot of room for improvement.

Finally, let's not forget that Git advertises scripting as the way to make things work. Hooks are strongly expected to be shell scripts. And those shell scripts are definitely outside of the control of Git's own source code, so we would quite likely break power users' scripts by simply switching to BusyBox, as most of its commands/options are noticeably limited compared to the full commands.

So I think that the best we can do is to offer an opt-in to BusyBox. After making it work. Robustly so.

drizzd commented 4 years ago

FWIW, I am now happy with Clink, which effectively adds readline capabilities to cmd.exe.

ur4t commented 3 years ago

There must be a compatible /usr/bin/sh.exe for ssh to work correctly. Details: #3285.

ur4t commented 3 years ago

Diffutils shipped with mingit-busybox $GIT_INSTALL_ROOT/usr/bin/{cmp,diff,diff3}.exe needs libiconv msys-iconv-2.dll (1000KB) and libintl msys-intl-8.dll(43KB) to work properly. Nothing misfunctions in my use cases, and busybox provides cmp and diff. I think these three executable can be excluded.

ur4t commented 3 years ago

Some helper scripts like git-difftool--helper works with mingit-busybox and mingit, which is important for git difftool to work properly.

dscho commented 3 years ago

There must be a compatible /usr/bin/sh.exe for ssh to work correctly. Details: #3285.

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.

Diffutils shipped with mingit-busybox $GIT_INSTALL_ROOT/usr/bin/{cmp,diff,diff3}.exe needs libiconv msys-iconv-2.dll (1000KB) and libintl msys-intl-8.dll(43KB) to work properly. Nothing misfunctions in my use cases, and busybox provides cmp and diff. I think these three executable can be excluded.

Right!

We do have a script to check for missing .dll files, and an Azure Pipeline that runs it regularly to verify that Git for Windows and MinGit do not ship with .exe files with broken links to .dll files. I think the two tasks here are:

Porting the Pipeline should be relatively easy: just use the setup-git-for-windows-sdk Action and then execute this shell script snippet:

  printf 'Checking full file list\n\n' >&2
  ./check-for-missing-dlls.sh | tee full.txt
  printf '\n\nChecking MinGit file list\n\n' >&2
  MINIMAL_GIT=1 ./check-for-missing-dlls.sh | tee min.txt
  ! grep ' is missing ' full.txt min.txt
  ! grep -i 'unused ' min.txt

Some helper scripts like git-difftool--helper works with mingit-busybox and mingit, which is important for git difftool to work properly.

I am not that sure about git difftool... MinGit is intended to be used by applications, not directly by humans. And difftool strikes me as a particularly human-oriented Git command, i.e. it is not low-level enough to truly fit into MinGit's mission.

ur4t commented 3 years ago

I've find two large executable with the same content. Can one of them be a wrapper to another?

~/mingit-busybox/mingw64/bin $ 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
~/mingit-busybox/mingw64/bin $ ls -al git-remote-http*
-rwxrwxr-x    1 ur4t     ur4t       2092048 Jun 07 20:27 git-remote-http.exe
-rwxrwxr-x    1 ur4t     ur4t       2092048 Jun 07 20:27 git-remote-https.exe
~/mingit-busybox/mingw64/bin $ sha512sum git-remote-http*
7eaf2ad99a1cc9f0d95286ebedea11bd700249b5618529468bf8d65dfeeccde0  git-remote-http.exe
7eaf2ad99a1cc9f0d95286ebedea11bd700249b5618529468bf8d65dfeeccde0  git-remote-https.exe
dscho commented 3 years ago

I've find two large executable with the same content. Can one of them be a wrapper to another?

We would have to teach the git-wrapper.c that trick (most likely to redirect from git-remote-http.exe to git-remote-https.exe, so as to not punish the more common case, https://), but yes it would be totally doable, and it would save on .zip size. Good find!

ur4t commented 3 years ago

In mingit-busybox, msys executable inside /usr/bin/ belongs to these packages: diffutils, libfido2, msys2-runtime, openssh, tcl, rebase.

So here comes a crazy idea: completely remove msys part, provide Win32-Openssh or even ask users to download it themselves (sinve 1809, Windows10 bundles it).

The only keypoint is that how many software depend on msys tcl.

dscho commented 3 years ago

The only keypoint is that how many software depend on msys tcl.

That looks like an oversight on my part, too. There are parts of Git that depend on Tcl, but they also depend on Tk because they are gitk and Git GUI, both of which are inappropriate for bundling inside MinGit.

diffutils is completely broken, which can be removed.

Good point.

So here comes a crazy idea: completely remove msys part, provide Win32-Openssh or even ask users to download it themselves (sinve 1809, Windows10 bundles it).

That might be an option if we did not support Windows Vista, still.

But I give you this: it is tempting an idea. And I have thought about it a lot. But for that, we definitely need to work a lot harder on BusyBox-w32. We do not have any current build of mingw-w64-busybox, for starters, let alone any proper CI testing that runs Git's test suite.

dscho commented 3 years ago

I believe that I addressed the tcl, diffutils and git-remote-http.exe parts.

dscho commented 2 years ago

After a looong hiatus, I upgraded to the latest BusyBox-w32 tag, integrated a CI build, and the Pipeline to build the mingw-w64-busybox package is currently running.

ur4t commented 1 year ago

So here comes a crazy idea: completely remove msys part, provide Win32-Openssh or even ask users to download it themselves (sinve 1809, Windows10 bundles it).

That might be an option if we did not support Windows Vista, still.

As announced in https://github.com/git-for-windows/git/releases/tag/v2.36.1.windows.1, we can make it true now.

Git for Windows will also stop supporting Windows Vista soon after Git for Windows 2.36.0 is released. Around the beginning of 2023, Git for Windows will drop support for Windows 7 and for Windows 8, following Cygwin's and MSYS2's lead (Git for Windows relies on MSYS2 for components such as Bash and Perl).

dscho commented 1 year ago

So here comes a crazy idea: completely remove msys part, provide Win32-Openssh or even ask users to download it themselves (sinve 1809, Windows10 bundles it).

That might be an option if we did not support Windows Vista, still.

As announced in https://github.com/git-for-windows/git/releases/tag/v2.36.1.windows.1, we can make it true now.

I am not so sure about that.

dscho commented 1 month ago

It would seem that I never have enough time to push this through, and there are still substantial hurdles:

Nevertheless, I think it would be valuable to enhance the current state by:

In short: There is a lot to do, and the more hands help, the quicker it will get done.