git.exe pull error: cannot spawn git: Invalid argument
I rolled back to the version 2.16.0, the problem has disappeared. Git for Windows 2.16.0(2)
Update:
Git for Windows 2.16.1(4) is out and that should fix this issue: https://github.com/git-for-windows/git/releases
Old answer:
This is a known issue in Git for Windows 2.16.1(2) and 2.16.1(3): https://github.com/git-for-windows/git/issues/1481
Only workaround is to use Git for Windows 2.16.1 (Download) (or older; 2.16.0 has other critical issues: TortoiseGit revert failed - unable to revert local changes).
Just for the notes, bugreport in TortoiseGit: https://gitlab.com/tortoisegit/tortoisegit/issues/3156
PS: For Git for Windows >= 2.16 you need at least TortoiseGit 2.5.7 (cf. https://stackoverflow.com/a/48457419/3906760).
git-for-windows/git
issue 1481 mentioned in 2018 for Git 2.16 is formally fixed with Git 2.25 (Q1 2020).
A Work around has been introduced for an issue where a FD is left open when spawning a child process and is kept open in the child can interfere with the operation in the parent process on Windows.
See commit 3ba3720 (02 Dec 2019), commit 4d0375c (30 Nov 2019), and commit ac33519, commit 9a780a3, commit c5a03b1, commit eea4a7f (22 Nov 2019) by Johannes Schindelin (dscho
).
(Merged by Junio C Hamano -- gitster
-- in commit 55d607d, 10 Dec 2019)
mingw
: work around incorrect standard handlesSigned-off-by: Johannes Schindelin
For some reason, when being called via TortoiseGit the standard handles, or at least what is returned by
_get_osfhandle(
0) for standard input, can take on the value (HANDLE)-2 (which is not a legal value, according to the documentation).Even if this value is not documented anywhere,
CreateProcess()
seems to work fine without complaints ifhStdInput
set to this value.In contrast, the upcoming code to restrict which file handles get inherited by spawned processes would result in
ERROR_INVALID_PARAMETER
when including such handle values in the list.
Note:
mingw
: restrict file handle inheritance only on Windows 7 and laterSigned-off-by: Johannes Schindelin
Turns out that it don't work so well on Vista, see
git-for-windows/git
issue 1742 for details.According to Old New Thing, it should work on Windows Vista and later.
But apparently there are issues on Windows Vista when pipes are involved. Given that Windows Vista is past its end of life (official support ended on April 11th, 2017), let's not spend too much time on this issue and just disable the file handle inheritance restriction on any Windows version earlier than Windows 7. To help this, special-case the value (HANDLE)-2 returned by
_get_osfhandle()
and replace it withINVALID_HANDLE_VALUE,
which will hopefully let the handle inheritance restriction work even when called from TortoiseGit.
That is why there is a new configuration setting:
core.restrictinheritedhandles:
Windows-only: override whether spawned processes inherit only standard file handles (
stdin
,stdout
andstderr
) or all handles.
Can beauto
,true
orfalse
. Defaults toauto
, which meanstrue
on Windows 7 and later, andfalse
on older Windows versions.
mingw
: spawned processes need to inherit only standard handlesSigned-off-by: Johannes Schindelin
By default,
CreateProcess()
does not inherit any open file handles, unless thebInheritHandles
parameter is set toTRUE
. Which we do need to set because we need to pass in stdin/stdout/stderr to talk to the child processes.
Sadly, this means that all file handles (unless marked viaO_NOINHERIT
) are inherited.This lead to problems in VFS for Git (the Virtual File System for Git), where a long-running read-object hook is used to hydrate missing objects, and depending on the circumstances, might only be called after Git opened a file handle.
Ideally, we would not open files without
O_NOINHERIT
unless really necessary (i.e. when we want to pass the opened file handle as standard handle into a child process), but apparently it is all-too-easy to introduce incorrectopen()
calls: this happened, and prevented updating a file after the read-object hook was started because the hook still held a handle on said file.Happily, there is a solution: as described in the "Old New Thing" there is a way, starting with Windows Vista, that lets us define precisely which handles should be inherited by the child process.
And since we bumped the minimum Windows version for use with Git for Windows to Vista with v2.10.1 (i.e. a long time ago), we can use this method. So let's do exactly that.
We need to make sure that the list of handles to inherit does not contain duplicates; Otherwise
CreateProcessW()
would fail withERROR_INVALID_ARGUMENT
.While at it, stop setting
errno
toENOENT
unless it really is the correct value.Also, fall back to not limiting handle inheritance under certain error conditions (e.g. on Windows 7, which is a lot stricter in what handles you can specify to limit to).
And:
mingw
: do seterrno
correctly when trying to restrict handle inheritanceReported-by: Johannes Sixt
Signed-off-by: Johannes Schindelin
Acked-by: Johannes SixtIn 9a780a384de ("
mingw
: spawned processes need to inherit only standard handles", 2019-11-22, Git v2.25.0 -- merge listed in batch #5), we taught the Windows-specific part to restrict which file handles are passed on to the spawned processes.Since this logic seemed to be a bit fragile across Windows versions (we still support Windows Vista in Git for Windows, for example), a fall-back was added to try spawning the process again, this time without restricting which file handles are to be inherited by the spawned process.
In the common case (i.e. when the process could not be spawned for reasons other than the file handle inheritance), the fall-back attempt would still fail, of course.
Crucially, one thing we missed in that code path was to set
errno
appropriately.Let's fix that by making sure that
errno
is set correctly. It even appears thaterrno
was set in the wrong case previously:CreateProcessW()
returns non-zero upon success, buterrno
was set only in the non-zero case.However, when
mingw_spawnvpe()
wants to see whether the file in question is a script, it callsparse_interpreter()
, which in turn tries toopen()
the file. Obviously, this call fails, and setserrno
toENOENT
, deep inside the call chain started from that test helper.Instead, we force re-set
errno
at the beginning of the functionmingw_spawnve_fd()
, which should be safe given that callers of that function will want to look aterrno
if -1 was returned. And if thaterrno
is 0 ("No error"), regression tests like t0061.2 will kick in.