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

Git for Windows crash when I try to use git difftool -d on a file added with -N option #2677

Closed
1 task
sachinrk opened this issue Jun 9, 2020 · 11 comments · Fixed by #2733
Closed
1 task
Milestone

Comments

@sachinrk
Copy link

sachinrk commented Jun 9, 2020

  • I was not able to find an open or closed issue matching what I'm seeing

Setup

  • Which version of Git for Windows are you using? Is it 32-bit or 64-bit?
    git version 2.27.0.windows.1
    64 bit.

I recently upgraded the git version, but the issue was reproducible on older versions as well.

$ git --version --build-options

git version 2.27.0.windows.1
cpu: x86_64
built from commit: 907ab1011dce9112700498e034b974ba60f8b407
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?
$ cmd.exe /c ver

Microsoft Windows [Version 10.0.18363.836
  • What options did you set as part of the installation? Or did you choose the
    defaults?
# One of the following:
> type "C:\Program Files\Git\etc\install-options.txt"
> type "C:\Program Files (x86)\Git\etc\install-options.txt"
> type "%USERPROFILE%\AppData\Local\Programs\Git\etc\install-options.txt"
$ cat /etc/install-options.txt

type "C:\Program Files\Git\etc\install-options.txt"

Editor Option: VIM
Custom Editor Path:
Path Option: Cmd
SSH Option: OpenSSH
Tortoise Option: false
CURL Option: OpenSSL
CRLF Option: CRLFAlways
Bash Terminal Option: MinTTY
Git Pull Behavior Option: Merge
Performance Tweaks FSCache: Enabled
Use Credential Manager: Enabled
Enable Symlinks: Disabled
Enable Pseudo Console Support: Disabled
  • Any other interesting things about your environment that might be related
    to the issue you're seeing?

Nothing interesting.

Details

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

CMD

In a new branch, simply 
notepad++ a.txt
git add -N a.txt
git difftool -d

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

No crash, and that git would open my difftool with the diff of the file

  • What actually happened instead?
d:\RoutingPlane\sources\dev\CAFE\NanoProxy>git difftool -d
error: unable to read sha1 file of C:\Users\sakul\AppData\Local\Temp\git-difftool.a32148/right/build/configuration/public/xml/a.txt (4b825dc642cb6eb9a060e54bf8d69288fbee4904)
error: could not write 'build/configuration/public/xml/a.txt'

Crash dialog saying Git for Windows has stopped working

  • If the problem was occurring with a specific repository, can you provide the
    URL to that repository to help us with testing?

Not specific to the repository

@dscho
Copy link
Member

dscho commented Jun 9, 2020

I can reproduce, even on Linux (read: this problem is not Windows-specific):

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index e66af3682f8..731ed73f421 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -720,6 +720,14 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
 	test_cmp expect actual
 '
 
+test_expect_success 'add -N and difftool -d' '
+	test_when_finished git reset --hard &&
+
+	test_write_lines A B C >intent-to-add &&
+	git add -N intent-to-add &&
+	git difftool --dir-diff --extcmd ls
+'
+
 test_expect_success 'outside worktree' '
 	echo 1 >1 &&
 	echo 2 >2 &&

And this seems to fix it:

diff --git a/diff.c b/diff.c
index d1ad6a3c4ad..3503dcdf0db 100644
--- a/diff.c
+++ b/diff.c
@@ -5684,6 +5684,9 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
 	int line_termination = opt->line_termination;
 	int inter_name_termination = line_termination ? '\t' : '\0';
 
+	diff_fill_oid_info(p->one, opt->repo->index);
+	diff_fill_oid_info(p->two, opt->repo->index);
+
 	fprintf(opt->file, "%s", diff_line_prefix(opt));
 	if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) {
 		fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode,

@sachinrk
Copy link
Author

sachinrk commented Jun 9, 2020

I can reproduce, even on Linux (read: this problem is not Windows-specific):

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index e66af3682f8..731ed73f421 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -720,6 +720,14 @@ test_expect_success SYMLINKS 'difftool --dir-diff handles modified symlinks' '
 	test_cmp expect actual
 '
 
+test_expect_success 'add -N and difftool -d' '
+	test_when_finished git reset --hard &&
+
+	test_write_lines A B C >intent-to-add &&
+	git add -N intent-to-add &&
+	git difftool --dir-diff --extcmd ls
+'
+
 test_expect_success 'outside worktree' '
 	echo 1 >1 &&
 	echo 2 >2 &&

And this seems to fix it:

diff --git a/diff.c b/diff.c
index d1ad6a3c4ad..3503dcdf0db 100644
--- a/diff.c
+++ b/diff.c
@@ -5684,6 +5684,9 @@ static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
 	int line_termination = opt->line_termination;
 	int inter_name_termination = line_termination ? '\t' : '\0';
 
+	diff_fill_oid_info(p->one, opt->repo->index);
+	diff_fill_oid_info(p->two, opt->repo->index);
+
 	fprintf(opt->file, "%s", diff_line_prefix(opt));
 	if (!(opt->output_format & DIFF_FORMAT_NAME_STATUS)) {
 		fprintf(opt->file, ":%06o %06o %s ", p->one->mode, p->two->mode,

Thanks for the quick fix! :)

@dscho
Copy link
Member

dscho commented Jun 10, 2020

It's not actually a fix because it fails the test suite, pointing out a rather big conceptual problem (I did not figure out how to distinguish between a cache_entry that is --intent-to-add vs one that is simply in need of refreshing).

dscho added a commit to dscho/git that referenced this issue Jun 10, 2020
In `run_diff_files()`, files that have been staged with the intention to
add are queued without a valid OID in the `diff_filepair`.

When the output mode is, say, `DIFF_FORMAT_PATCH`, the
`diff_fill_oid_info()` function, called from `run_diff()`, will remedy
that situation by reading the file contents from disk.

However, when the output mode is `DIFF_FORMAT_RAW`, that does not hold
true, and the output will contain a bogus OID (and the flag `M` for
"modified" instead of the correct `A` for "added").

As a consequence, `git difftool -d` (which relies on `git diff-files
--raw`'s output) does not work correctly.

Let's fix this specifically by imitating `diff_fill_oid_info()`.

Note: we can only do that for diff formats that do not actually need the
file contents, such as `DIFF_FORMAT_PATCH`: `run_diff()` would try to
read the blob contents, but that blob might not have been written to
Git's object database.

This fixes git-for-windows#2677

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git that referenced this issue Jun 10, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed.

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git that referenced this issue Jun 23, 2020
In `run_diff_files()`, files that have been staged with the intention to
add are queued without a valid OID in the `diff_filepair`.

When the output mode is, say, `DIFF_FORMAT_PATCH`, the
`diff_fill_oid_info()` function, called from `run_diff()`, will remedy
that situation by reading the file contents from disk.

However, when the output mode is `DIFF_FORMAT_RAW`, that does not hold
true, and the output will contain a bogus OID (and the flag `M` for
"modified" instead of the correct `A` for "added").

As a consequence, `git difftool -d` (which relies on `git diff-files
--raw`'s output) does not work correctly.

Let's fix this specifically by imitating `diff_fill_oid_info()`.

Note: we can only do that for diff formats that do not actually need the
file contents, such as `DIFF_FORMAT_PATCH`: `run_diff()` would try to
read the blob contents, but that blob might not have been written to
Git's object database.

This fixes git-for-windows#2677

This patch _also_ fixes the expectations set by the regression test
introduced in feea694 (diff-files: treat "i-t-a" files as
"not-in-index", 2020-06-20).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git that referenced this issue Jun 23, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed.

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
gitster pushed a commit to git/git that referenced this issue Jun 24, 2020
In `run_diff_files()`, files that have been staged with the intention to
add are queued without a valid OID in the `diff_filepair`.

When the output mode is, say, `DIFF_FORMAT_PATCH`, the
`diff_fill_oid_info()` function, called from `run_diff()`, will remedy
that situation by reading the file contents from disk.

However, when the output mode is `DIFF_FORMAT_RAW`, that does not hold
true, and the output will contain a bogus OID (and the flag `M` for
"modified" instead of the correct `A` for "added").

As a consequence, `git difftool -d` (which relies on `git diff-files
--raw`'s output) does not work correctly.

Let's fix this specifically by imitating `diff_fill_oid_info()`.

Note: we can only do that for diff formats that do not actually need the
file contents, such as `DIFF_FORMAT_PATCH`: `run_diff()` would try to
read the blob contents, but that blob might not have been written to
Git's object database.

This fixes git-for-windows#2677

This patch _also_ fixes the expectations set by the regression test
introduced in feea694 (diff-files: treat "i-t-a" files as
"not-in-index", 2020-06-20).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
gitster pushed a commit to git/git that referenced this issue Jun 24, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed.

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
dscho added a commit to dscho/git that referenced this issue Jun 24, 2020
In `run_diff_files()`, files that have been staged with the intention to
add are queued without a valid OID in the `diff_filepair`.

When the output mode is, say, `DIFF_FORMAT_PATCH`, the
`diff_fill_oid_info()` function, called from `run_diff()`, will remedy
that situation by reading the file contents from disk.

However, when the output mode is `DIFF_FORMAT_RAW`, that does not hold
true, and the output will contain a bogus OID (and the flag `M` for
"modified" instead of the correct `A` for "added").

As a consequence, `git difftool -d` (which relies on `git diff-files
--raw`'s output) does not work correctly.

Let's fix this specifically by imitating `diff_fill_oid_info()`.

Note: we can only do that for diff formats that do not actually need the
file contents, such as `DIFF_FORMAT_PATCH`: `run_diff()` would try to
read the blob contents, but that blob might not have been written to
Git's object database.

This fixes git-for-windows#2677

This patch _also_ fixes the expectations set by the regression test
introduced in feea694 (diff-files: treat "i-t-a" files as
"not-in-index", 2020-06-20).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git that referenced this issue Jun 24, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed.

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
dscho added a commit to dscho/git that referenced this issue Jun 25, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed.

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
gitster pushed a commit to git/git that referenced this issue Jun 25, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed.

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
dscho added a commit to dscho/git that referenced this issue Jul 1, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed: it reported intent-to-add files
with the empty _tree_ as the post-image OID, when we need to show
an all-zero (or, "null") OID instead, to indicate to the caller that
they have to look at the worktree file.

The symptom of that problem shown by `git difftool` was this:

	error: unable to read sha1 file of <path> (<empty-tree-OID>)
	error: could not write '<filename>'

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
gitster pushed a commit to git/git that referenced this issue Jul 2, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed: it reported intent-to-add files
with the empty _tree_ as the post-image OID, when we need to show
an all-zero (or, "null") OID instead, to indicate to the caller that
they have to look at the worktree file.

The symptom of that problem shown by `git difftool` was this:

	error: unable to read sha1 file of <path> (<empty-tree-OID>)
	error: could not write '<filename>'

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@dscho
Copy link
Member

dscho commented Jul 2, 2020

Just a quick update that gitgitgadget#654 made it into the next branch and will hopefully be included in Git v2.28.0. If not, my plan is to include it in Git for Windows v2.28.0, anyway.

@sachinrk
Copy link
Author

sachinrk commented Jul 2, 2020 via email

dscho added a commit to dscho/git that referenced this issue Jul 3, 2020
In git-for-windows#2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed: it reported intent-to-add files
with the empty _tree_ as the post-image OID, when we need to show
an all-zero (or, "null") OID instead, to indicate to the caller that
they have to look at the worktree file.

The symptom of that problem shown by `git difftool` was this:

	error: unable to read sha1 file of <path> (<empty-tree-OID>)
	error: could not write '<filename>'

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho dscho added this to the Next release milestone Jul 3, 2020
dscho added a commit that referenced this issue Jul 8, 2020
In #2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed: it reported intent-to-add files
with the empty _tree_ as the post-image OID, when we need to show
an all-zero (or, "null") OID instead, to indicate to the caller that
they have to look at the worktree file.

The symptom of that problem shown by `git difftool` was this:

	error: unable to read sha1 file of <path> (<empty-tree-OID>)
	error: could not write '<filename>'

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@dscho
Copy link
Member

dscho commented Jul 8, 2020

@sachinrk would you mind testing the latest snapshot?

@sachinrk
Copy link
Author

sachinrk commented Jul 8, 2020 via email

@dscho
Copy link
Member

dscho commented Jul 8, 2020

Won't mind at all. What do I have to do?

Just install the latest snapshot and re-test whether the bug you reported is fixed or not ;-)

@sachinrk
Copy link
Author

sachinrk commented Jul 8, 2020

Can confirm the issue is resolved with the latest git for windows snapshot

d:\RoutingPlane\sources\test\CAFE\NanoProxyTests>git --version
git version 2.27.0.windows.1.18.g51f9af323b.20200707200701

d:\RoutingPlane\sources\test\CAFE\NanoProxyTests>notepad++ a.txt

d:\RoutingPlane\sources\test\CAFE\NanoProxyTests>git st
On branch user/sakul/entity_body_support
Your branch is ahead of 'origin/user/sakul/entity_body_support' by 1 commit.
(use "git push" to publish your local commits)

Untracked files:
(use "git add ..." to include in what will be committed)
a.txt

nothing added to commit but untracked files present (use "git add" to track)

d:\RoutingPlane\sources\test\CAFE\NanoProxyTests>git add -N a.txt

d:\RoutingPlane\sources\test\CAFE\NanoProxyTests>git st
On branch user/sakul/entity_body_support
Your branch is ahead of 'origin/user/sakul/entity_body_support' by 1 commit.
(use "git push" to publish your local commits)

Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git restore ..." to discard changes in working directory)
new file: a.txt

no changes added to commit (use "git add" and/or "git commit -a")

d:\RoutingPlane\sources\test\CAFE\NanoProxyTests>git difftool -d

d:\RoutingPlane\sources\test\CAFE\NanoProxyTests>git difftool -d

d:\RoutingPlane\sources\test\CAFE\NanoProxyTests>

dscho added a commit that referenced this issue Jul 17, 2020
In #2677, a `git difftool
-d` problem was reported. The underlying cause was a bug in `git
diff-files --raw` that we just fixed: it reported intent-to-add files
with the empty _tree_ as the post-image OID, when we need to show
an all-zero (or, "null") OID instead, to indicate to the caller that
they have to look at the worktree file.

The symptom of that problem shown by `git difftool` was this:

	error: unable to read sha1 file of <path> (<empty-tree-OID>)
	error: could not write '<filename>'

Make sure that the reported `difftool` problem stays fixed.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
@sachinrk
Copy link
Author

sachinrk commented Jul 20, 2020

Is it possible to port this fix to git used with vfs? specifically - 2.23.0.vfs.1.1 and release an update?

@dscho
Copy link
Member

dscho commented Aug 10, 2020

@sachinrk I believe that the latest VFS release comes with this fix.

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.

3 participants