run-command: be helpful when Git LFS fails on Windows 7 by dscho · Pull Request #5042 · git-for-windows/git

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:

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! 🙇

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.

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

1edeb9a (Win32: warn if the console font doesn't support Unicode, 2014-06-10) introduced both code to detect the current console font on Windows Vista and newer and a fallback for older systems to detect the default console font and issue a warning if that font doesn't support unicode. Since we haven't supported any Windows older than Vista in almost a decade, we don't need to keep the workaround. This more or less fell out of #6108, but didn't quite fit into that PR. There are also some other version specific hacks and workarounds I considered dropping, but decided against: * 492f709 * I'm unsure if this regression has ever been fixed or just become the new normal. * #5042 * So far this hasn't been an issue on Windows 8.1, but officially Go 1.21 and newer only support Windows 10 and newer. So this might become a problem at any point.

git-for-windows@1edeb9a (Win32: warn if the console font doesn't support Unicode, 2014-06-10) introduced both code to detect the current console font on Windows Vista and newer and a fallback for older systems to detect the default console font and issue a warning if that font doesn't support unicode. Since we haven't supported any Windows older than Vista in almost a decade, we don't need to keep the workaround. This more or less fell out of git-for-windows#6108, but didn't quite fit into that PR. There are also some other version specific hacks and workarounds I considered dropping, but decided against: * git-for-windows@492f709 * I'm unsure if this regression has ever been fixed or just become the new normal. * git-for-windows#5042 * So far this hasn't been an issue on Windows 8.1, but officially Go 1.21 and newer only support Windows 10 and newer. So this might become a problem at any point.

1edeb9a (Win32: warn if the console font doesn't support Unicode, 2014-06-10) introduced both code to detect the current console font on Windows Vista and newer and a fallback for older systems to detect the default console font and issue a warning if that font doesn't support unicode. Since we haven't supported any Windows older than Vista in almost a decade, we don't need to keep the workaround. This more or less fell out of #6108, but didn't quite fit into that PR. There are also some other version specific hacks and workarounds I considered dropping, but decided against: * 492f709 * I'm unsure if this regression has ever been fixed or just become the new normal. * #5042 * So far this hasn't been an issue on Windows 8.1, but officially Go 1.21 and newer only support Windows 10 and newer. So this might become a problem at any point.

git-for-windows@1edeb9a (Win32: warn if the console font doesn't support Unicode, 2014-06-10) introduced both code to detect the current console font on Windows Vista and newer and a fallback for older systems to detect the default console font and issue a warning if that font doesn't support unicode. Since we haven't supported any Windows older than Vista in almost a decade, we don't need to keep the workaround. This more or less fell out of git-for-windows#6108, but didn't quite fit into that PR. There are also some other version specific hacks and workarounds I considered dropping, but decided against: * git-for-windows@492f709 * I'm unsure if this regression has ever been fixed or just become the new normal. * git-for-windows#5042 * So far this hasn't been an issue on Windows 8.1, but officially Go 1.21 and newer only support Windows 10 and newer. So this might become a problem at any point.

Git for Windows dropped Windows 7 support a long time ago. There is a patch (introduced in #5042 and in #5059) to detect when Git LFS failed to start because of lack of Windows 7 support (which was lost somewhat surprisingly, via a Go upgrade that slipped that change in), and to provid a helpful message to the user in that instance. Since Git for Windows itself does not support Windows 7 anymore, that patch is no longer necessary, either.

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

Comment thread compat/win32/path-utils.c

Comment thread compat/win32/path-utils.c

chrisd8088 reviewed Jul 8, 2024

View reviewed changes

Comment thread compat/win32/path-utils.c Outdated

Comment thread compat/win32/path-utils.c

Comment thread compat/win32/path-utils.c Outdated

Comment thread compat/win32/path-utils.c Outdated

Comment thread compat/win32/path-utils.c Outdated

Comment thread compat/win32/path-utils.c Outdated

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

Comment thread compat/win32/path-utils.c

Comment thread compat/win32/path-utils.c

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

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

dscho mentioned this pull request Jul 14, 2026

Drop Windows 7 support #6325

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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 merged 1 commit into
git-for-windows:mainfrom
dscho:warn-about-git-lfs-on-win7

dscho commented Jul 4, 2024

Uh oh!

dscho commented Jul 4, 2024

Uh oh!

Uh oh!

Uh oh!

chrisd8088 left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dscho commented Jul 8, 2024

Uh oh!

chrisd8088 commented Jul 8, 2024 •

edited

Loading

Uh oh!

dscho commented Jul 9, 2024

Uh oh!

chrisd8088 left a comment

Uh oh!

Uh oh!

Uh oh!

dscho commented Jul 10, 2024 •

edited by gitforwindowshelper Bot

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dscho commented Jul 4, 2024

Uh oh!

dscho commented Jul 4, 2024

Uh oh!

Uh oh!

Uh oh!

chrisd8088 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dscho commented Jul 8, 2024

Uh oh!

chrisd8088 commented Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dscho commented Jul 9, 2024

Uh oh!

chrisd8088 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dscho commented Jul 10, 2024 • edited by gitforwindowshelper Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chrisd8088 left a comment •

edited

Loading

chrisd8088 commented Jul 8, 2024 •

edited

Loading

dscho commented Jul 10, 2024 •

edited by gitforwindowshelper Bot

Loading