search

git-for-windows / git

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

run-command: be helpful when Git LFS fails on Windows 7 #5042

Open dscho opened 5 days ago

dscho commented 5 days ago

Git LFS is now built with Go 1.21 which no longer supports Windows 7. However, Git for Windows still wants to support Windows 7.

Ideally, Git LFS would re-introduce Windows 7 support until Git for Windows drops support for Windows 7, but that's not going to happen.

The next best thing we can do is to let the users know what is happening, and how to get out of their fix, at least.

This is not quite as easy as it would first seem because programs compiled with Go 1.21 or newer will simply throw an exception and fail with an Access Violation on Windows 7.

The only way I found to address this is to replicate the logic from Go's very own version command (which can determine the Go version with which a given executable was built) to detect the situation, and in that case offer a helpful error message.

This addresses https://github.com/git-for-windows/git/issues/4996.

/cc @chrisd8088

dscho commented 5 days ago

This is how the error message looks now: image

dscho commented 1 day ago

@chrisd8088 thank you for your review! I will reply to your concerns in the threads, except for this one:

I haven't yet tried compiling enough of Git to get this PR's changes (with or without my suggestions) running in the VM and testable.

FWIW you could simply download the PR build artifacts and overwrite git.exe (that should be enough to replicate the work-around, at least it was here).

chrisd8088 commented 1 day ago

FWIW you could simply download the PR build artifacts and overwrite git.exe (that should be enough to replicate the work-around, at least it was here).

Ah, thanks -- I've done that before for Git LFS build artifacts. (There's a bit of work getting them onto the VM, but it's doable.)

dscho commented 17 hours ago

@chrisd8088 wow, what a thorough review! I do not see a lot of such high-quality reviews on the Git mailing list, and I am delighted. Thank you so, so much.

I force-pushed an update that addresses your comments as well as the bug where 32-bit git-lfs.exe was not parsed correctly. I have a couple of Portable Git instances lying around on my machine, both 32-bit and 64-bit flavors, and verified that the Go version was extracted correctly with the updated code:

3.2.0 go1.18.2
3.3.0 go1.19.3
3.4.0 go1.20.6
3.4.1 go1.20.11
3.5.1 go1.21.7

Could you have a final look over the diff before I merge the PR?