run-command: refine Git LFS check on Windows 7

[chrisd8088]

In commit 46d14a6 of PR #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 #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 (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]

Maybe the exit code is completely bogus? If that is the case, testing for a non-zero exit code might be desirable.

[chrisd8088]

Maybe the exit code is completely bogus? If that is the case, testing for a non-zero exit code might be desirable.

I don't know enough about Windows exit codes (or how Git might modify them) but that seems possible to me, and if you think it's better to just check for a non-zero code, I'd be OK with that instead.

[dscho]

Maybe the exit code is completely bogus? If that is the case, testing for a non-zero exit code might be desirable.

I don't know enough about Windows exit codes (or how Git might modify them) but that seems possible to me, and if you think it's better to just check for a non-zero code, I'd be OK with that instead.

@chrisd8088 yes, I think the non-zero code is the better option here; Otherwise we might find out that other users' Windows 7 produce yet other exit codes that we haven't handled. The other conditions are probably enough to not go overboard and perform the .exe inspection unnecessarily.

I took the liberty of amending the commit and force-pushing. Here is the range-diff:

• 1: 5d6ce4b ! 1: ca7b9f1 run-command: refine Git LFS check on Windows 7

  @@ Metadata
   Author: Chris Darroch <[email protected]>
   
    ## Commit message ##
  -    run-command: refine Git LFS check on Windows 7
  +    fixup! run-command: be helpful with Git LFS fails on Windows 7
  +
  +    This commit refines the Git LFS check on Windows 7.
   
       In commit git-for-windows/git@46d14a6f7189a604a0812b143dc5c51488a01895 of
       PR git-for-windows/git#5042 the wait_or_whine() function in run-command.c
  @@ Commit message
       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.
  +    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
  @@ Commit message
       executable in a wider range of circumstances.
   
       Signed-off-by: Chris Darroch <[email protected]>
  +    Signed-off-by: Johannes Schindelin <[email protected]>
   
    ## compat/win32/path-utils.c ##
   @@ compat/win32/path-utils.c: void win32_warn_about_git_lfs_on_windows7(int exit_code, const char *argv0)
  + 	 * Git LFS v3.5.1 fails with an Access Violation on Windows 7; That
    	 * would usually show up as an exit code 0xc0000005. For some reason
    	 * (probably because at this point, we no longer have the _original_
  - 	 * HANDLE that was returned by `CreateProcess()`) we get 0xb00 instead.
  -+	 * When running in Git Bash we get exit code 2, so we check for that
  -+	 * as well.
  +-	 * HANDLE that was returned by `CreateProcess()`) we get 0xb00 instead.
  ++	 * HANDLE that was returned by `CreateProcess()`) we observe other
  ++	 * values like 0xb00 and 0x2 instead. Since the exact exit code
  ++	 * seems to be inconsistent, we check for a non-zero exit status.
    	 */
   -	if (exit_code != 0x0b00)
  -+	if (exit_code != 0x0002 && exit_code != 0x0b00)
  ++	if (exit_code == 0)
    		return;
    	if (GetVersion() >> 16 > 7601)
    		return; /* Warn only on Windows 7 or older */

[chrisd8088]

I took the liberty of amending the commit and force-pushing.

Great, thank you! It's nice to wake up and find there's nothing else for me to do (on this issue, at least!) 🙇