Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to Use difftool on More than 8 Files #2007

Closed
Jeff-G opened this issue Jan 3, 2019 · 12 comments · Fixed by #2026
Closed

Unable to Use difftool on More than 8 Files #2007

Jeff-G opened this issue Jan 3, 2019 · 12 comments · Fixed by #2026
Milestone

Comments

@Jeff-G
Copy link

Jeff-G commented Jan 3, 2019

I have a branch with >8 modified files. When I run the command git difftool ., the command crashes when trying to diff the 9th file. I have tested this with both vimdiff and Beyond Compare configured on my system, and they both fail. The error message I receive is something like:

error: cannot spawn [two seemingly random ASCII symbols]: No such file or directory
fatal: external diff died, stopping at [relative path to 9th file]

Given the random ASCII symbols (e.g., running the same command twice generates different ASCII symbols), I'm guessing this is due to memory corruption or buffer overflow.

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?

64-bit, tested in both 2.20.0 and 2.20.1. For 2.20.1, the version reported by Git is:

git version 2.20.1.windows.1
cpu: x86_64
built from commit 7c9fbc0...19b64f
sizeof-long: 4
sizeof-size_t: 8

  • Which version of Windows are you running? Vista, 7, 8, 10? Is it 32-bit or 64-bit?

Windows 10 Pro 64-bit
Version 1703 (OS Build 15063.1029)

  • What options did you set as part of the installation? Or did you choose the
    defaults?

Editor Option: Notepad++
Custom Editor Path:
Path Option: Cmd
SSH Option: OpenSSH
CURL Option: OpenSSL
CRLF Option: CRLFCommitAsIs
Bash Terminal Option: MinTTY
Performance Tweaks FSCache: Enabled
Use Credential Manager: Enabled
Enable Symlinks: Enabled

  • Any other interesting things about your environment that might be related
    to the issue you're seeing?

No

Details

  • Which terminal/shell are you running Git from? e.g Bash/CMD/PowerShell/other

I have tried in both Windows cmd.exe and Git Bash, with the same results.

git difftool .
  • What did you expect to occur after running these commands?

Each of my files would open in my selected difference tool, in succession. This was working in Git 2.19.

  • What actually happened instead?

The first eight (8) files opened in my configured difference tool. The ninth (9) file crashed the git difftool command with the error:

error: cannot spawn [two seemingly random ASCII symbols]: No such file or directory
fatal: external diff died, stopping at [relative path to 9th file]
  • If the problem was occurring with a specific repository, can you provide the
    URL to that repository to help us with testing?

The affected Git repositories are offline, but it was tested in multiple repositories (all hosted in the same offline environment).

@scooterchris
Copy link

scooterchris commented Jan 4, 2019

This is a bug that was introduced in Git for Windows version 2.19.2 64-bit. Version 2.19.1 64-bit and earlier work correctly when git difftool launches 9 or more comparisons.

Until the issue is resolved, uninstall Git for Windows, then install version 2.19.1.

Git for Windows 2.19.1 download.

This issue was also reported in the Scooter Software (Beyond Compare) forum thread, "Error: cannot spawn ??: No such file or directory...... fatal: external diff died."

@Jeff-G
Copy link
Author

Jeff-G commented Jan 6, 2019

Is there any way to confirm that this issue was reported to the Git team? I can't figure out how to search for open issues, and it would be a shame if this doesn't get fixed in the next version. I'm starting to get frustrated with the number of consecutively broken versions of Git for Windows (I haven't had a fully-functional version since 2.18).

@PhilipOakley
Copy link

Hi @Jeff-G
While the issue is noted here on the G-f-W site, I don't think it's been reported to the upstream.

However, the main issue is that the ratio of contributors to users of the G-f-W code is measure in parts per million. I.e we do need help, lots of it.

If some one can at least go through the release note for upstream git and the git.git upstream repo to find the merged branch on which the problem may have started, then that would help the limited eyes go looking in the right area.

The other helpful think would be to see if you can create a really compact MVCE that doesn't need a special difftool and creates a very simple test repo and scenario to allow a git bisect. The simpler the better.

Thanks

@PhilipOakley
Copy link

This #2007 appears to be the same issue as #1982 and #1974 so the information between the three can be correlated.

Those reports are different from #2001 which was an MYS2 error message when the ssh connection had been 'lost' when a terminal was closed without closing its underlying connection. It maybe that the error report is ultimately from the same place in the code, but the reasons for reaching there sound different.

Though thinking out loud it maybe that we have somehow reach a connection limit to the difftool invocation(s) - maybe there is some other library update that has had an impact.

@kgybels
Copy link

kgybels commented Jan 9, 2019

I've sent a patch to the git mailing list:

diff: ensure correct lifetime of external_diff_cmd

According to getenv(3)'s notes:

    The implementation of getenv() is not required to be reentrant.  The
    string pointed to by the return value of getenv() may be statically
    allocated, and can be modified by a subsequent call to getenv(),
    putenv(3), setenv(3), or unsetenv(3).

Since strings returned by getenv() are allowed to change on subsequent
calls to getenv(), make sure to duplicate when caching external_diff_cmd
from environment.

This problem becomes apparent on Git for Windows since fe21c6b285df
(mingw: reencode environment variables on the fly (UTF-16 <-> UTF-8)),
when the getenv() implementation provided in compat/mingw.c was changed
to keep a certain amount of alloc'ed strings and freeing them on
subsequent calls.

This fixes https://github.com/git-for-windows/git/issues/2007:

    $ yes n | git -c difftool.prompt=yes difftool fe21c6b285df fe21c6b285df~100

    Viewing (1/404): '.gitignore'
    Launch 'bc3' [Y/n]?
    Viewing (2/404): 'Documentation/.gitignore'
    Launch 'bc3' [Y/n]?
    Viewing (3/404): 'Documentation/Makefile'
    Launch 'bc3' [Y/n]?
    Viewing (4/404): 'Documentation/RelNotes/2.14.5.txt'
    Launch 'bc3' [Y/n]?
    Viewing (5/404): 'Documentation/RelNotes/2.15.3.txt'
    Launch 'bc3' [Y/n]?
    Viewing (6/404): 'Documentation/RelNotes/2.16.5.txt'
    Launch 'bc3' [Y/n]?
    Viewing (7/404): 'Documentation/RelNotes/2.17.2.txt'
    Launch 'bc3' [Y/n]?
    Viewing (8/404): 'Documentation/RelNotes/2.18.1.txt'
    Launch 'bc3' [Y/n]?
    Viewing (9/404): 'Documentation/RelNotes/2.19.0.txt'
    Launch 'bc3' [Y/n]? error: cannot spawn ¦?: No such file or directory
    fatal: external diff died, stopping at Documentation/RelNotes/2.19.1.txt

Signed-off-by: Kim Gybels <[email protected]>
---
 diff.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/diff.c b/diff.c
index dc9965e836..f69687e288 100644
--- a/diff.c
+++ b/diff.c
@@ -492,6 +492,9 @@ static const char *external_diff(void)
        external_diff_cmd = getenv("GIT_EXTERNAL_DIFF");
        if (!external_diff_cmd)
                external_diff_cmd = external_diff_cmd_cfg;
+       else
+               external_diff_cmd = xstrdup(external_diff_cmd);
+
        done_preparing = 1;
        return external_diff_cmd;
 }
--
2.20.1.windows.1

@PhilipOakley
Copy link

@kgybels That looks like some good investigative work!

Did you happened to also look at #2001 to see if it had a similar root cause ('No' is perfectly acceptable ;-)

@kgybels
Copy link

kgybels commented Jan 10, 2019

@PhilipOakley Did you happened to also look at #2001 to see if it had a similar root cause ('No' is perfectly acceptable ;-)

No :)

From a quick look at the bug reports I don't think #2001 is related, but #2011 might.

@PhilipOakley
Copy link

Looks like dscho is on the case for the other instances of getenv... #2019

@aggieNick02
Copy link

Looks like this is all wrapped up and just waiting for the patch to merged; thank you @kgybels . Stumbled here after encountering the issue with the old and venerable CompareIt!

gitster pushed a commit to git/git that referenced this issue Jan 14, 2019
According to getenv(3)'s notes:

    The implementation of getenv() is not required to be reentrant.  The
    string pointed to by the return value of getenv() may be statically
    allocated, and can be modified by a subsequent call to getenv(),
    putenv(3), setenv(3), or unsetenv(3).

Since strings returned by getenv() are allowed to change on subsequent
calls to getenv(), make sure to duplicate when caching external_diff_cmd
from environment.

This problem becomes apparent on Git for Windows since fe21c6b
(mingw: reencode environment variables on the fly (UTF-16 <-> UTF-8)),
when the getenv() implementation provided in compat/mingw.c was changed
to keep a certain amount of alloc'ed strings and freeing them on
subsequent calls.

This fixes git-for-windows#2007:

    $ yes n | git -c difftool.prompt=yes difftool fe21c6b fe21c6b~100

    Viewing (1/404): '.gitignore'
    Launch 'bc3' [Y/n]?
    Viewing (2/404): 'Documentation/.gitignore'
    Launch 'bc3' [Y/n]?
    Viewing (3/404): 'Documentation/Makefile'
    Launch 'bc3' [Y/n]?
    Viewing (4/404): 'Documentation/RelNotes/2.14.5.txt'
    Launch 'bc3' [Y/n]?
    Viewing (5/404): 'Documentation/RelNotes/2.15.3.txt'
    Launch 'bc3' [Y/n]?
    Viewing (6/404): 'Documentation/RelNotes/2.16.5.txt'
    Launch 'bc3' [Y/n]?
    Viewing (7/404): 'Documentation/RelNotes/2.17.2.txt'
    Launch 'bc3' [Y/n]?
    Viewing (8/404): 'Documentation/RelNotes/2.18.1.txt'
    Launch 'bc3' [Y/n]?
    Viewing (9/404): 'Documentation/RelNotes/2.19.0.txt'
    Launch 'bc3' [Y/n]? error: cannot spawn ¦?: No such file or directory
    fatal: external diff died, stopping at Documentation/RelNotes/2.19.1.txt

Signed-off-by: Kim Gybels <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
@dscho
Copy link
Member

dscho commented Jan 16, 2019

Once https://git-for-windows.visualstudio.com/git/_build/results?buildId=28848 completes successfully, there will be a new snapshot at https://wingit.blob.core.windows.net/files/index.html. @Jeff-G it would be good if you could give that snapshot a whirl (you can use the portable version if you do not want to install it over your existing Git for Windows).

@mikeschuld
Copy link

I am having this exact problem. Just tried the latest snapshot and the issue goes away for me. I tried the 2.20.1 release first from the main download page and that didn't fix the problem. I'll be using the snapshot until a full release includes this as it has totally killed my code review flow for weeks.

@dscho
Copy link
Member

dscho commented Jan 31, 2019

Thanks for testing, and yes, I think using the snapshot for now is a good workaround (the snapshot contains code-signed executables just like official versions, so there should not even be a visible difference even with respect to anti-malware software running in the background).

dscho pushed a commit to dscho/git that referenced this issue Feb 7, 2019
According to getenv(3)'s notes:

    The implementation of getenv() is not required to be reentrant.  The
    string pointed to by the return value of getenv() may be statically
    allocated, and can be modified by a subsequent call to getenv(),
    putenv(3), setenv(3), or unsetenv(3).

Since strings returned by getenv() are allowed to change on subsequent
calls to getenv(), make sure to duplicate when caching external_diff_cmd
from environment.

This problem becomes apparent on Git for Windows since fe21c6b
(mingw: reencode environment variables on the fly (UTF-16 <-> UTF-8)),
when the getenv() implementation provided in compat/mingw.c was changed
to keep a certain amount of alloc'ed strings and freeing them on
subsequent calls.

This fixes git-for-windows#2007:

    $ yes n | git -c difftool.prompt=yes difftool fe21c6b fe21c6b~100

    Viewing (1/404): '.gitignore'
    Launch 'bc3' [Y/n]?
    Viewing (2/404): 'Documentation/.gitignore'
    Launch 'bc3' [Y/n]?
    Viewing (3/404): 'Documentation/Makefile'
    Launch 'bc3' [Y/n]?
    Viewing (4/404): 'Documentation/RelNotes/2.14.5.txt'
    Launch 'bc3' [Y/n]?
    Viewing (5/404): 'Documentation/RelNotes/2.15.3.txt'
    Launch 'bc3' [Y/n]?
    Viewing (6/404): 'Documentation/RelNotes/2.16.5.txt'
    Launch 'bc3' [Y/n]?
    Viewing (7/404): 'Documentation/RelNotes/2.17.2.txt'
    Launch 'bc3' [Y/n]?
    Viewing (8/404): 'Documentation/RelNotes/2.18.1.txt'
    Launch 'bc3' [Y/n]?
    Viewing (9/404): 'Documentation/RelNotes/2.19.0.txt'
    Launch 'bc3' [Y/n]? error: cannot spawn ¦?: No such file or directory
    fatal: external diff died, stopping at Documentation/RelNotes/2.19.1.txt

Signed-off-by: Kim Gybels <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants