Open seanmars opened 4 weeks ago
dscho
commented
4 weeks ago insert your response here
That's unfortunately quite a lot of placeholders, and not much information to go on.
The fact that your report shows assembly code (which Git for Windows does not do) suggests that in particular the "Any other interesting things about your environment that might be related to the issue you're seeing?" section would probably benefit from a careful revisit.
rimrul
commented
4 weeks ago Could this be git lfs? Because Git for Windows 2.45.1 contains git lfs 3.5.1
which is built with go 1.21, which requires Windows 10 and seems to cause a very similar error message on older Windows versions.
dscho
commented
4 weeks ago Could this be
git lfs? Because Git for Windows 2.45.1 containsgit lfs3.5.1 which is built withgo1.21, which requires Windows 10 and seems to cause a very similar error message on older Windows versions.
Good finding, adding more evidence to the fact that providing more information in bug reports is better than omitting it.
mjcheetham
commented
4 weeks ago Git for Windows 2.45.1 contains git lfs 3.5.1 which is https://github.com/git-lfs/git-lfs/pull/5668, which requires https://github.com/golang/go/issues/64622#issuecomment-1847475161
Sorry for a tangent, but just curious about what this means for the ability of something like GCM, that is also available bundled with Git for Windows like LFS, for it to drop Windows 7/8.x support?
rimrul
commented
3 weeks ago Sorry for a tangent, but just curious about what this means for the ability of something like GCM, that is also available bundled with Git for Windows like LFS, for it to drop Windows 7/8.x support?
Dropping support for Windows 7 and 8.0 is completely fine from our perspective as we're doing the same in the next feature release. As for Windows 8.1, we'd prefer if it stayed supported, but we have previously discussed redirecting Windows 8.1 users to the last supported standalone GCM installer and only providing bundled GCM for users on Windows 10 and newer.
The only remaining concern is whatever agreement exists with the Visual studio team.
mjcheetham
commented
3 weeks ago Right, I was just referencing that Git LFS 3.5.1 no longer working on Windows 8.1 (instead requiring 10+) is also a conflict in supportable components of GfW
rimrul
commented
3 weeks ago What's more of a problem for us is that we didn't notice it upfront and Git for Windows 2.45.2 is probably the last release supporting Windows 7 and 8 and shipped with that Git LFS version.
dscho
commented
3 weeks ago What's more of a problem for us is that we didn't notice it upfront and Git for Windows 2.45.2 is probably the last release supporting Windows 7 and 8 and shipped with that Git LFS version.
Indeed. If I had realized from the release notes that it dropped support for even some Windows 10 versions, I would not have integrated it into Git for Windows. But the release notes are really mum about this change, which leads me to believe that the Git LFS maintainers were unaware that the Go version upgrade would cause this.
rimrul
commented
3 weeks ago What's more of a problem for us is that we didn't notice it upfront and Git for Windows 2.45.2 is probably the last release supporting Windows 7 and 8 and shipped with that Git LFS version.
I'd like to not leave it like that. How do we want to resolve this? The simplest solution would be rolling back to 3.4.1, but that would lock users of newer Windows versions out of new features. One approach might be to keep a mingw-w64-git-lfs-3.4 package as an alternate for affected windows versions, kind of similar to our curl alternates. Another approach might be to try and build git-lfs from source with an older version of go, but we've avoided even building git-lfs from source with a supported vesion of go so far, so this might be a path we want to keep avoiding. Or we could point users of affected Windows versions to a standalone download of Git LFS.
Indeed. If I had realized from the release notes that it dropped support for even some Windows 10 versions,
Do you have a link for this? I can't seem to find any information about this.
seanmars
commented
3 weeks ago insert your response here
That's unfortunately quite a lot of placeholders, and not much information to go on.
The fact that your report shows assembly code (which Git for Windows does not do) suggests that in particular the "Any other interesting things about your environment that might be related to the issue you're seeing?" section would probably benefit from a careful revisit.
I'm sorry for not providing more information that I found useful, because the entire system only had problems with updating GfW, so I didn't think of other possible causes.
But now it seems that it may be because git-lfs updates the golang version, because I found that many people have raised errors related to golang that are very similar to my error message.
dscho
commented
3 weeks ago What's more of a problem for us is that we didn't notice it upfront and Git for Windows 2.45.2 is probably the last release supporting Windows 7 and 8 and shipped with that Git LFS version.
I'd like to not leave it like that.
@bk2204 @chrisd8088 any feedback from you kind Git LFS people? The issue is that Git LFS v3.5.1 dropped support for Windows versions before Windows 10. Any guidance how the Git LFS project wants to navigate this (so that Git for Windows may follow suite)?
chrisd8088
commented
3 weeks ago @bk2204 @chrisd8088 any feedback from you kind Git LFS people? The issue is that Git LFS v3.5.1 dropped support for Windows versions before Windows 10. Any guidance how the Git LFS project wants to navigate this (so that Git for Windows may follow suite)?
Hmm -- thanks for bringing this to our attention. I don't know that either of us was aware of this change in Windows support from the Go language upgrade.
Our policy has been to try to stay current with the latest Go version because their policy is to only provide support for the two most recent major versions. So if it's possible to align with the Windows 10+ requirement in Git for Windows more broadly, that would obviously be convenient for us.
If not, though, I suppose we could investigate building a custom release on an older Go version for a fixed period of time to help Git for Windows, but I think we'd want to know that time period was going to be as brief as possible.
dscho
commented
3 weeks ago @chrisd8088 Git for Windows carefully announced the pending end of support for Windows 7/8 for quite a while, but will have to support Windows 8.1 for now.
I only see a couple of inconvenient options going forward:
Currently, my thinking is that the first option is the least bad.
chrisd8088
commented
2 weeks ago @dscho -- You should make whatever decision is best for the Git for Windows project, of course. If you want to build Git LFS with an older Go version, we could certainly collaborate to try to make that possible.
One other idea occurred to me: would you consider it a viable path forward if the installer process (either the Git for Windows installer, or the Git LFS installer) checked the Windows version and made a decision as to whether to install Git LFS based on that?
bk2204
commented
2 weeks ago My concern about building with an older Go version is that we'll have Git LFS versions without Go security updates, since versions of Go before 1.21 aren't receiving any. (We know users very much want those because they complain frequently when the version of Go isn't the latest.) However, I'm not opposed in principle to others (such as Git for Windows) building such binaries and us providing some tacit support for that case at least for a little while, as long as complaints about that fact don't come to us. I don't think I'd want to build such binaries myself, but I'm also stepping down from the project shortly, so I'll defer to @chrisd8088 as to the best approach here.
I think Chris's suggestion of simply not allowing Git LFS to be installed on Windows 8.1 (which appears to be really, truly dead upstream in terms of security updates) might be the best approach, though.
dscho
commented
2 weeks ago The big problem with "simply not allowing Git LFS to be installed on Windows 8.1" is that this suggestion misses the fact that not all Git for Windows instances are installed using Git for Windows' installer. A lot of instances are MinGit or Portable Git. It's really an unideal situation, first quietly dropping support for Windows 8.1 and then not even having any good option to reinstate it.
chrisd8088
commented
1 week ago I agree it's not an ideal situation, and unfortunately there's probably not a one-size-fits-all solution.
We've aimed to build Git LFS releases using current versions of Go for multiple reasons. I think it's a good practice overall, since we'd otherwise be using unsupported versions of Go, and we benefit from any fixes and performance improvements in Go as a matter of course. For example, Go 1.21 includes support for SHA-256 hashing using native AMD64 CPU instructions, when available, which is a definite boost for Git LFS (see also golang/go#50543):
SHA-224 and SHA-256 operations now use native instructions when available when
GOARCH=amd64, providing a performance improvement on the order of 3-4x.
Also, if we stick to an older version, users get alerts from security vulnerability scanners and open issues to raise those concerns, as @bk2204 noted (see, for instance, git-lfs/git-lfs#4825, git-lfs/git-lfs#4888, git-lfs/git-lfs#5289, git-lfs/git-lfs#5348, git-lfs/git-lfs#5496, etc.) These issues are not always applicable to Git LFS specifically, but they cause consternation and the expenditure of effort both by our users and by us as Git LFS maintainers.
That said, I offered before that we could, for a limited time, try to provide a special version built on Go 1.20, if you want to include that in the Git for Windows package until you also drop support for Windows 8.1 (which appears to have reached the end of official support from Microsoft). Or we can help you build such a version for inclusion in the Git for Windows project.
But in general, I don't think we want to offer this in the long term, or provide support for it ourselves, given how constrained our own time is for this kind of work.
What is the timeline for Git for Windows dropping support for Windows 8.1, by the way?
rimrul
commented
1 week ago What is the timeline for Git for Windows dropping support for Windows 8.1, by the way?
There is no timeline, so far.
To give you a rough ballpark idea: XP was supported until Git for Windows 2.10 in 2016 (2 years after end of support). Vista was supported until 2.37 in 2022 (5 years after end of support). 7 is supported until 2.45 in 2024 (4 years after end of support). 8 is supported until 2.45 in 2024 (8 years after end of support).
We'd probably want to announce the end of Windows 8.1 support at least 2 Git feature releases in advance, so definitely no earlier than 2.48 (probably around late January 2025).
dscho
commented
1 week ago My current thinking is to keep distributing the official Git LFS with Git for Windows and patch both git.exe (in finish_command()) as well as the Git wrapper (which is used as /cmd/git-lfs.exe) to provide a more helpful message (detecting the situation e.g. by looking at the exit code and the Windows version).
Thoughts?
chrisd8088
commented
1 week ago @dscho --
My current thinking is to keep distributing the official Git LFS with Git for Windows and patch both git.exe (in finish_command()) as well as the Git wrapper (which is used as /cmd/git-lfs.exe) to provide a more helpful message (detecting the situation e.g. by looking at the exit code and the Windows version).
If that's a viable approach, I'm certainly in favour of it! Let me know if you need any assistance developing the patch or testing it.
Thanks very much for the idea!
dscho
commented
1 week ago Things are unfortunately not as easy as I thought; Git's complicated (and overly exit()-happy) architecture strikes once again. The way git-lfs filter-process involves these steps:
apply_multi_file_filter() function starts the git-lfs process.start_multi_file_filter_fn() function calls subprocess_handshake() to negotiate the mode (clean, smudge or delay).packet_read_line().packet_read() function (calling the latter function with the option PACKET_READ_CHOMP_NEWLINE), which itself is a thin wrapper around the packet_read_with_status() function.packet_get_data(), which die()s because the options passed through did not include PACKET_READ_GENTLE_ON_READ_ERROR.subprocess_exit_handler() function just gets a chance to wrap up the git-lfs child process, completely ignoring the exit status (which, funnily enough, is not 0xc0000005 here, but 0xb00, something I cannot quite explain).remove_junk() atexit handler gets a chance to say that the checkout failed, but is totally unhelpful by omitting any useful information (because that information simply has been lost in the meantime) as to why.Now, the most natural location to handle an Access Violation in git-lfs.exe would be the waitpid() function in compat/mingw.c, where the exit code is retrieved. But as I stated above, the exit code is not 0xc0000005, unexpectedly, but 0xb00 instead (my current theory is that the problem is that we no longer have a HANDLE to the process anymore but have to retrieve it via OpenProcess() via the process ID, and that at the process has been already been reaped at that point, at least partially).
Besides, the information that this had been a git-lfs.exe process is not available at that layer, it is only available in finish_command(), or even better in the wait_or_whine() function which is called by finish_command() and which does get the argv0 parameter.
I guess there is no good way to handle this.
dscho
commented
6 days ago After realizing that go version git-lfs.exe reports the Go version used to build git-lfs.exe and after spending waaaaay too much time scouring the corresponding Go source code, I came up with this hack to determine the Go version used to build a given .exe file:
#define _GNU_SOURCE
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <unistd.h>
#include <stdlib.h>
static int read_in_full(int fd, char *buffer, size_t size)
{
while (size > 0) {
int count = read(fd, buffer, size);
if (count < 0) {
fprintf(stderr, "read error: %s\n", strerror(errno));
return -1;
}
if (count == 0) {
fprintf(stderr, "short read: %d remaining\n", (int)size);
return -1;
}
buffer += count;
size -= count;
}
return 0;
}
static int read_at(int fd, char *buffer, size_t offset, size_t size)
{
if (lseek(fd, offset, SEEK_SET) < 0) {
fprintf(stderr, "could not seek to 0x%x\n", (unsigned int)offset);
return -1;
}
return read_in_full(fd, buffer, size);
}
static size_t le16(const char *buffer)
{
unsigned char *u = (unsigned char *)buffer;
return u[0] | (u[1] << 8);
}
static size_t le32(const char *buffer)
{
return le16(buffer) | (le16(buffer + 2) << 16);
}
int main(int argc, char **argv)
{
const char *path;
int fd;
char buffer[1024];
off_t offset;
size_t num_sections, opt_header_size, i;
char *p = NULL, *q;
if (argc != 2) {
fprintf(stderr, "Need a path!\n");
return 1;
}
path = argv[1];
fd = open(path, O_RDONLY);
if (fd < 0) {
fprintf(stderr, "Could not open '%s': %s\n", path, strerror(errno));
return 1;
}
if (read_in_full(fd, buffer, 2) < 0) {
fprintf(stderr, "Error reading MZ\n");
return 1;
}
if (buffer[0] != 'M' || buffer[1] != 'Z') {
fprintf(stderr, "missing MZ\n");
fail:
free(p);
close(fd);
return 1;
}
if (read_at(fd, buffer, 0x3c, 4) < 0) {
fprintf(stderr, "could not read pointer to PE\n");
goto fail;
}
offset = le32(buffer);
if (read_at(fd, buffer, offset, 24) < 0) {
fprintf(stderr, "could not read PE\n");
goto fail;
}
if (buffer[0] != 'P' || buffer[1] != 'E' || buffer[2] != '\0' || buffer[3] != '\0') {
fprintf(stderr, "missing PE\n");
goto fail;
}
num_sections = le16(buffer + 6);
opt_header_size = le16(buffer + 20);
offset += 24; /* skip file header */
if (read_at(fd, buffer, offset, 2) < 0) {
fprintf(stderr, "could not read optional header magic\n");
goto fail;
}
offset += opt_header_size;
for (i = 0; i < num_sections; i++) {
if (read_at(fd, buffer, offset + i * 40, 40) < 0) {
fprintf(stderr, "could not read section #%d\n", (int)i + 1);
goto fail;
}
if ((le32(buffer + 36) /* characteristics */ & ~0x600000) /* IMAGE_SCN_ALIGN_32BYTES */ ==
(/* IMAGE_SCN_CNT_INITIALIZED_DATA */ 0x00000040 |
/* IMAGE_SCN_MEM_READ */ 0x40000000 |
/* IMAGE_SCN_MEM_WRITE */ 0x80000000)) {
size_t size = le32(buffer + 16);
p = malloc(size);
if (!p || read_at(fd, p, le32(buffer + 20), size) < 0) {
fprintf(stderr, "could not read section\n");
goto fail;
}
q = memmem(p, size, "\xff Go buildinf:", 14);
if (!q) {
fprintf(stderr, "could not find Go version magic\n");
goto fail;
}
if (q[14] == 8 && q[15] == 2) {
if (q[32] & 0x80) {
fprintf(stderr, "insanely large Go version ignored\n");
} else {
write(1, q + 33, q[32]);
}
}
break;
}
}
close(fd);
return 0;
}Is it pretty? No. Does it work? Probably well enough, at least.
Setup
Microsoft Windows [Version 6.1.7601] 64-bit
One of the following:
insert your machine's response here
git status
Exception 0xc0000005 0x8 0x0 0x0 PC=0x0
runtime.asmstdcall() runtime/sys_windows_amd64.s:65 +0x75 fp=0x22fca0 sp=0x22fc80 pc=0x46cd35 rax 0x0 rbx 0xe64b20 rcx 0xeb9140 rdi 0x7fffffdd000 rsi 0x22fea0 rbp 0x22fde0 rsp 0x22fc78 r8 0x0 r9 0x22fee0 r10 0xe8aab8 r11 0x21 r12 0x22fec0 r13 0x1 r14 0xe64280 r15 0x0 rip 0x0 rflags 0x10293 cs 0x33 fs 0x53 gs 0x2b fatal: the remote end hung up unexpectedly