Re: [PATCH 2/2] difftool --dir-diff: symlink all files matching the working tree

[John Keeping]

On Wed, Mar 13, 2013 at 08:41:29PM -0700, David Aguilar wrote:
 On Wed, Mar 13, 2013 at 1:33 PM, John Keeping j...@keeping.me.uk wrote:
  diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
  index eb1d3f8..8102ce1 100755
  --- a/t/t7800-difftool.sh
  +++ b/t/t7800-difftool.sh
  @@ -370,6 +370,20 @@ test_expect_success PERL 'difftool --dir-diff' '
  echo $diff | stdin_contains file
   '
 
  +write_script .git/CHECK_SYMLINKS \EOF 
 
 Tiny nit.  Is there any downside to leaving this file
 at the root instead of inside the .git dir?

I followed what some of the other uses of write_script (in other tests)
did.  I think putting it under .git is slightly better because it won't
show up as untracked in the repository but that shouldn't matter here,
so I'm happy to change it in a re-roll.

  +#!/bin/sh
  +test -L $2/file 
  +test -L $2/file2 
  +test -L $2/sub/sub
  +echo $?
  +EOF
  +
  +test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without 
  unstaged changes' '
  +   result=$(git difftool --dir-diff --symlink \
  +   --extcmd ./.git/CHECK_SYMLINKS branch HEAD) 
  +   test $result = 0
  +'
  +
 
 How about something like this?
 
 +   echo 0 expect 
 +   git difftool --dir-diff --symlink \
 +   --extcmd ./CHECK_SYMLINKS branch HEAD actual 
 +   test_cmp expect actual
 
 (sans gmail whitespace damage) so that we can keep it chained with .

I hadn't considered using test_cmp, if we go that way I wonder if we can
do slightly better for future debugging.  Something like this perhaps?

+write_script .git/CHECK_SYMLINKS \EOF 
+for f in file file2 sub/sub
+do
+   echo $f
+   readlink $2/$f
+done actual
+EOF
+
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without 
unstaged changes' '
+   cat EOF expect 
+file
+$(pwd)/file
+file2
+$(pwd)/file2
+sub/sub
+$(pwd)/sub/sub
+EOF
+   git difftool --dir-diff --symlink \
+   --extcmd ./.git/CHECK_SYMLINKS branch HEAD 
+   test_cmp actual expect
+'

 Ah.. it seems your branch is based on master, perhaps?

 There's stuff cooking in next for difftool's tests.
 I'm not sure if this patch is based on top of them.
 Can you rebase the tests so that the chaining is done like it is in 'next'?

Yes it is based on master.  The cleanup on next looks good, I'll base
the re-roll on that.


John
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] difftool --dir-diff: symlink all files matching the working tree

[Junio C Hamano]

John Keeping j...@keeping.me.uk writes:

 +write_script .git/CHECK_SYMLINKS \EOF 
 +#!/bin/sh
 +test -L $2/file 
 +test -L $2/file2 
 +test -L $2/sub/sub
 +echo $?
 +EOF

Please drop #!/bin/sh from the above; it is misleading and
pointless.

After all, you are using write_script to avoid having to know
where the user's shell is.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] difftool --dir-diff: symlink all files matching the working tree

[John Keeping]

Some users like to edit files in their diff tool when using git
difftool --dir-diff --symlink to compare against the working tree but
difftool currently only created symlinks when a file contains unstaged
changes.

Change this behaviour so that symlinks are created whenever the
right-hand side of the comparison has the same SHA1 as the file in the
working tree.

Note that textconv filters are handled in the same way as by git-diff
and if a clean filter is not the inverse of its smudge filter we already
get a null SHA1 from diff --raw and will symlink the file without
going through the new hash-object based check.

Reported-by: Matt McClure matthewlmccl...@gmail.com
Signed-off-by: John Keeping j...@keeping.me.uk
---
 Documentation/git-difftool.txt |  4 +++-
 git-difftool.perl  | 21 ++---
 t/t7800-difftool.sh| 14 ++
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index e575fea..8361e6e 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -72,7 +72,9 @@ with custom merge tool commands and has the same value as 
`$MERGED`.
 --symlinks::
 --no-symlinks::
'git difftool''s default behavior is create symlinks to the
-   working tree when run in `--dir-diff` mode.
+   working tree when run in `--dir-diff` mode and the right-hand
+   side of the comparison yields the same content as the file in
+   the working tree.
 +
 Specifying `--no-symlinks` instructs 'git difftool' to create copies
 instead.  `--no-symlinks` is the default on Windows.
diff --git a/git-difftool.perl b/git-difftool.perl
index 0a90de4..5f093ae 100755
--- a/git-difftool.perl
+++ b/git-difftool.perl
@@ -83,6 +83,21 @@ sub exit_cleanup
exit($status | ($status  8));
 }
 
+sub use_wt_file
+{
+   my ($repo, $workdir, $file, $sha1, $symlinks) = @_;
+   my $null_sha1 = '0' x 40;
+
+   if ($sha1 eq $null_sha1) {
+   return 1;
+   } elsif (not $symlinks) {
+   return 0;
+   }
+
+   my $wt_sha1 = $repo-command_oneline('hash-object', $workdir/$file);
+   return $sha1 eq $wt_sha1;
+}
+
 sub setup_dir_diff
 {
my ($repo, $workdir, $symlinks) = @_;
@@ -159,10 +174,10 @@ EOF
}
 
if ($rmode ne $null_mode) {
-   if ($rsha1 ne $null_sha1) {
-   $rindex .= $rmode $rsha1\t$dst_path\0;
-   } else {
+   if (use_wt_file($repo, $workdir, $dst_path, $rsha1, 
$symlinks)) {
push(@working_tree, $dst_path);
+   } else {
+   $rindex .= $rmode $rsha1\t$dst_path\0;
}
}
}
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index eb1d3f8..8102ce1 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -370,6 +370,20 @@ test_expect_success PERL 'difftool --dir-diff' '
echo $diff | stdin_contains file
 '
 
+write_script .git/CHECK_SYMLINKS \EOF 
+#!/bin/sh
+test -L $2/file 
+test -L $2/file2 
+test -L $2/sub/sub
+echo $?
+EOF
+
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff --symlink without 
unstaged changes' '
+   result=$(git difftool --dir-diff --symlink \
+   --extcmd ./.git/CHECK_SYMLINKS branch HEAD) 
+   test $result = 0
+'
+
 test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
diff=$(git difftool --dir-diff --prompt --extcmd ls branch) 
echo $diff | stdin_contains sub 
-- 
1.8.2.rc2.4.g7799588

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html