[PATCH 1/2] difftool: initialize variables for readability

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] difftool: initialize variables for readability

David Aguilar
The code always goes into one of the two conditional blocks but make it
clear that not doing so is an error condition by setting $ok to 0.

Signed-off-by: David Aguilar <[hidden email]>
---
 git-difftool.perl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-difftool.perl b/git-difftool.perl
index 488d14b..8cf0040 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -273,7 +273,7 @@ EOF
  # temporary file to both the left and right directories to show the
  # change in the recorded SHA1 for the submodule.
  for my $path (keys %submodule) {
- my $ok;
+ my $ok = 0;
  if (defined($submodule{$path}{left})) {
  $ok = write_to_file("$ldir/$path",
  "Subproject commit $submodule{$path}{left}");
@@ -289,7 +289,7 @@ EOF
  # shows only the link itself, not the contents of the link target.
  # This loop replicates that behavior.
  for my $path (keys %symlink) {
- my $ok;
+ my $ok = 0;
  if (defined($symlink{$path}{left})) {
  $ok = write_to_file("$ldir/$path",
  $symlink{$path}{left});
--
2.8.2.404.gdfc6a507

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2] difftool: handle unmerged files in dir-diff mode

David Aguilar
When files are unmerged they can show up as both unmerged and
modified in the output of `git diff --raw`.  This causes
difftool's dir-diff to create filesystem entries for the same
path twice, which fails when it encounters a duplicate path.

Ensure that each worktree path is only processed once.
Add a test to demonstrate the breakage.

Reported-by: Jan Smets <[hidden email]>
Signed-off-by: David Aguilar <[hidden email]>
---
 git-difftool.perl   |  5 +++++
 t/t7800-difftool.sh | 23 +++++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/git-difftool.perl b/git-difftool.perl
index 8cf0040..ebd13ba 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -138,6 +138,7 @@ sub setup_dir_diff
  my %submodule;
  my %symlink;
  my @working_tree = ();
+ my %working_tree_dups = ();
  my @rawdiff = split('\0', $diffrtn);
 
  my $i = 0;
@@ -188,6 +189,10 @@ EOF
  }
 
  if ($rmode ne $null_mode) {
+ # Avoid duplicate working_tree entries
+ if ($working_tree_dups{$dst_path}++) {
+ next;
+ }
  my ($use, $wt_sha1) = use_wt_file($repo, $workdir,
   $dst_path, $rsha1);
  if ($use) {
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index ff7a9e9..7ce4cd7 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -419,6 +419,29 @@ run_dir_diff_test 'difftool --dir-diff when worktree file is missing' '
  grep file2 output
 '
 
+run_dir_diff_test 'difftool --dir-diff with unmerged files' '
+ test_when_finished git reset --hard &&
+ test_config difftool.echo.cmd "echo ok" &&
+ git checkout -B conflict-a &&
+ git checkout -B conflict-b &&
+ git checkout conflict-a &&
+ echo a >>file &&
+ git add file &&
+ git commit -m conflict-a &&
+ git checkout conflict-b &&
+ echo b >>file &&
+ git add file &&
+ git commit -m conflict-b &&
+ git checkout master &&
+ git merge conflict-a &&
+ test_must_fail git merge conflict-b &&
+ cat >expect <<-EOF &&
+ ok
+ EOF
+ git difftool --dir-diff $symlinks -t echo >actual &&
+ test_cmp expect actual
+'
+
 write_script .git/CHECK_SYMLINKS <<\EOF
 for f in file file2 sub/sub
 do
--
2.8.2.404.gdfc6a507

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/2] difftool: handle unmerged files in dir-diff mode

Junio C Hamano
Thanks, will queue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html