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

dscho · 2024-07-04T22:50:50Z

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 #4996.

/cc @chrisd8088

dscho · 2024-07-04T23:33:34Z

This is how the error message looks now:

compat/win32/path-utils.c

chrisd8088

Thank you so much for tackling this challenge! I really appreciate the effort here, and I've learned a ton from doing a review about both the Windows and MS-DOS executable file formats and Go's own linker and file format internals.

I had a few suggestions only; out of them all, the main thing I wondered about was whether the get_go_version() function could accidentally read past the end of the data in its p while trying to parse the various pieces of the Go header and version string, etc., if the input data was bad or malicious or whatever.

So I tried to write up and test some code which might check for that condition and a few others; I ran a variety of experiments with bad input data, but I won't claim I did a completely exhaustive set of checks.

I have also set up a Windows 7 SP1 VM, and see the problem with the current Git for Windows distribution this PR aims to work around, but 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. I hope to get to that soon.

Thanks again very much for all the investigation and work to try to help out our Git LFS users on old Windows systems! 🙇

compat/win32/path-utils.c

dscho · 2024-07-08T21:15:13Z

@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 · 2024-07-08T21:29:22Z

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 · 2024-07-09T10:46:13Z

@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?

chrisd8088

Glad to help! It's been fun doing some C coding again; I know it's a language with flaws, but I miss reading and writing it.

Speaking of the flaws of C, though, I think the current version of this code may still read past the end of p in one specific case, which I tried to outline in my latest comments.

compat/win32/path-utils.c

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: git-for-windows#4996 (comment) 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 git-for-windows#4996. Signed-off-by: Johannes Schindelin <[email protected]>

dscho · 2024-07-10T09:49:18Z

/add relnote bug Git LFS v3.5.x and newer no longer support Windows 7. Instead of a helpful error message, it now simply crashes on that Windows version, leaving the user with the error message "panic before malloc heap initialized". This has been addressed: In addition to the unhelpful error message, Git is now saying what is going on and how to get out of the situation.

The workflow run was started

Git LFS v3.5.x and newer no longer support Windows 7. Instead of a helpful error message, it now simply [crashes](git-for-windows/git#4996) on that Windows version, leaving the user with the error message "panic before malloc heap initialized". This has been [addressed](git-for-windows/git#5042): In addition to the unhelpful error message, Git is now saying what is going on and how to get out of the situation. Signed-off-by: gitforwindowshelper[bot] <[email protected]>

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

@chrisd8088

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](#4996 (comment)). 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 #4996. /cc @chrisd8088

In commit git-for-windows/git@46d14a6 of PR git-for-windows#5042 the wait_or_whine() function in run-command.c was revised to call a new function in the case where an executable fails to run, to check whether this was caused by a Git LFS binary compiled with a version of the Go language that no longer supports Windows 7. This change was made to address the issue reported in git-for-windows#4996. The new function, win32_warn_about_git_lfs_on_windows7(), performs several initial checks to test whether the failed executable returned the exit code 0x0b00 and is named "git-lfs", and whether we are running Windows 7 or not. Only if all these conditions are met does it then proceed to try to extract the Go language version from the binary file and check whether it is 1.21.0 or higher. However, these initial checks may not cover all possible use and failure cases. First, when running in Git Bash, the exit code seen from a recent Git LFS executable was 0x02, so we also want to check for that value as well as 0x0b00. Second, the name of the executable may not always be entirely lowercase, since it is possible to invoke Git LFS through Git by running, for example, "git LFS ls-files" (at least, on Windows, and with a case-insensitive filesystem). We therefore need to ignore case when checking the executable's name. And third, the name of the executable may not have a trailing space character, so we also need to check for the case where the name in argv0 is simply "git-lfs". With these changes, we can more reliably detect a failure of the Git LFS executable in a wider range of circumstances. Signed-off-by: Chris Darroch <[email protected]>

This commit refines the Git LFS check on Windows 7. In commit git-for-windows/git@46d14a6 of PR git-for-windows#5042 the wait_or_whine() function in run-command.c was revised to call a new function in the case where an executable fails to run, to check whether this was caused by a Git LFS binary compiled with a version of the Go language that no longer supports Windows 7. This change was made to address the issue reported in git-for-windows#4996. The new function, win32_warn_about_git_lfs_on_windows7(), performs several initial checks to test whether the failed executable returned the exit code 0x0b00 and is named "git-lfs", and whether we are running Windows 7 or not. Only if all these conditions are met does it then proceed to try to extract the Go language version from the binary file and check whether it is 1.21.0 or higher. However, these initial checks may not cover all possible use and failure cases. First, when running in Git Bash, the exit code seen from a recent Git LFS executable was 0x02. It would therefore appear that the exact exit code value is not reliable, so we want to check for a non-zero exit code instead. Second, the name of the executable may not always be entirely lowercase, since it is possible to invoke Git LFS through Git by running, for example, "git LFS ls-files" (at least, on Windows, and with a case-insensitive filesystem). We therefore need to ignore case when checking the executable's name. And third, the name of the executable may not have a trailing space character, so we also need to check for the case where the name in argv0 is simply "git-lfs". With these changes, we can more reliably detect a failure of the Git LFS executable in a wider range of circumstances. Signed-off-by: Chris Darroch <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>

In commit 46d14a6 of PR #5042 the `wait_or_whine()` function in `run-command.c` was revised to [call](https://github.com/git-for-windows/git/blob/b105301ef9107c8c2980a24eca89a2992a1c074b/run-command.c#L571) a new function in the case where an executable fails to run, to check whether this was caused by a Git LFS binary compiled with a version of the Go language that no longer supports Windows 7. This change was made to address the issue reported in #4996. The new function, `win32_warn_about_git_lfs_on_windows7()`, [performs](https://github.com/git-for-windows/git/blob/b105301ef9107c8c2980a24eca89a2992a1c074b/compat/win32/path-utils.c#L226-L232) several initial checks to test whether the failed executable returned the exit code `0x0b00` and is named `git-lfs`, and whether we are running Windows 7 or not. Only if all these conditions are met does it then proceed to try to extract the Go language version from the binary file and check whether it is 1.21.0 or higher. However, these initial checks may not cover all possible use and failure cases. First, when running in Git Bash (in a Windows 7 SP1 virtual machine), the exit code seen from a recent Git LFS executable was `0x02`, so we also want to check for that value as well as `0x0b00`. Second, the name of the executable may not always be entirely lowercase, since it is possible to invoke Git LFS through Git by running, for example, `git LFS ls-files` (at least, on Windows, and with a case-insensitive filesystem). We therefore need to ignore case when checking the executable's name. And third, the name of the executable may not have a trailing space character, so we also need to check for the case where the name in `argv0` is simply `git-lfs`. With these changes, we can more reliably detect a failure of the Git LFS executable in a wider range of circumstances.

dscho self-assigned this Jul 4, 2024

dscho mentioned this pull request Jul 4, 2024

Get Exception when using command after update to 2.45.1 #4996

Closed

1 task

rimrul reviewed Jul 5, 2024

View reviewed changes

compat/win32/path-utils.c Show resolved Hide resolved

compat/win32/path-utils.c Show resolved Hide resolved

chrisd8088 reviewed Jul 8, 2024

View reviewed changes

dscho changed the title ~~run-command: be helpful with Git LFS fails on Windows 7~~ run-command: be helpful when Git LFS fails on Windows 7 Jul 9, 2024

dscho force-pushed the warn-about-git-lfs-on-win7 branch from 1fe2fa9 to c99be51 Compare July 9, 2024 10:42

chrisd8088 mentioned this pull request Jul 9, 2024

Doesn't work in Windows 7 git-lfs/git-lfs#5821

Closed

chrisd8088 reviewed Jul 9, 2024

View reviewed changes

compat/win32/path-utils.c Show resolved Hide resolved

compat/win32/path-utils.c Show resolved Hide resolved

dscho force-pushed the warn-about-git-lfs-on-win7 branch from c99be51 to 3324d3b Compare July 10, 2024 08:51

dscho added this to the Next release milestone Jul 10, 2024

dscho merged commit dbc64e8 into git-for-windows:main Jul 10, 2024
44 checks passed

dscho deleted the warn-about-git-lfs-on-win7 branch July 10, 2024 09:49

chrisd8088 mentioned this pull request Jul 15, 2024

run-command: refine Git LFS check on Windows 7 #5059

Merged

dscho linked an issue Jul 15, 2024 that may be closed by this pull request

Get Exception when using command after update to 2.45.1 #4996

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

dscho commented Jul 4, 2024

dscho commented Jul 4, 2024

chrisd8088 left a comment •

edited

Loading

dscho commented Jul 8, 2024

chrisd8088 commented Jul 8, 2024 •

edited

Loading

dscho commented Jul 9, 2024

chrisd8088 left a comment

dscho commented Jul 10, 2024 •

edited by gitforwindowshelper bot

Loading

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

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

Conversation

dscho commented Jul 4, 2024

dscho commented Jul 4, 2024

chrisd8088 left a comment • edited Loading

Choose a reason for hiding this comment

dscho commented Jul 8, 2024

chrisd8088 commented Jul 8, 2024 • edited Loading

dscho commented Jul 9, 2024

chrisd8088 left a comment

Choose a reason for hiding this comment

dscho commented Jul 10, 2024 • edited by gitforwindowshelper bot Loading

chrisd8088 left a comment •

edited

Loading

chrisd8088 commented Jul 8, 2024 •

edited

Loading

dscho commented Jul 10, 2024 •

edited by gitforwindowshelper bot

Loading