From: David Aguilar Date: Wed, 18 Apr 2012 03:23:33 GMT Subject: Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs Message-ID: In-Reply-To: On Tue, Apr 17, 2012 at 6:25 AM, Tim Henigan wrote: > On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar wrote: >> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar wrote: >>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan wrote: >>> >>> I started testing this patch.  I started on the commit before what's >>> in pu and then applied this patch: >>> >>> $ git checkout e9653615fafcbac6109da99fac4fa66b0b432048 >>> $ git am difftool.patch >>> >>> The basics work and I know folks will be really happy when this >>> feature lands.  Folks have personally asked me for this feature in the >>> past.  I dig it.  I'd also like to help pursue using symlinks sometime >>> in the future if that sounds like a reasonable thing to you, but the >>> stabilizing the existing implementation is more important right now. > > I appreciate everyone's patience as this feature continues to develop. >  It has not gone as smoothly as I hoped ;) > > >>> I ran into some issues when trying it against a few random commits.  I >>> went pretty far back in git's history to see what would happen. >>> >>> $ git difftool --dir-diff e5b06629de847663aaf0f7daae8de81338da3901 | tail >>> Use of uninitialized value $rmode in string eq at >>> /home/david/src/git/git-difftool line 96. > > I ran the same test using both my local branch [1] and the tip of > Junio's th/difftool-diffall branch [2].   In both cases, I see the > "RelNotes: no such file" error (more on that below), but I do not see > uninitialized value errors. > > The section of code that reads the mode, SHA1 and path info from the > diff output looks like this: > >    my @rawdiff = split('\0', $diffrtn); > >    for (my $i=0; $i<$#rawdiff; $i+=2) { >        my ($lmode, $rmode, $lsha1, $rsha1, $status) = split(' ', > substr($rawdiff[$i], 1)); >        ... > > If the "$lmode, ..." variables are not defined, then the output of > 'git diff --raw' is malformed in some way (perhaps some error?).  At > the very end of your output, I saw this as well: > >>> fatal: malformed index info /t9800-git-p4-basic.sh      :100755 100755 >>> a25f18d36a196a4b85f6cac15a6a081744fe8fa1 >>> d41470541650590355bf0de1a1b556b3502492b5 M >>> update-index -z --index-info: command returned error: 128 > > Would it be possible for you to try either Junio's or my branch to > insure something did not go wrong with your locally applied patch? > Also, which platform are you testing on? Linux. I also test on OS X occasionally, but it's not my primary platform. I think I narrowed it down. I have this in my ~/.gitconfig -- [diff] renames = copy -- That means we should be able to reproduce this by doing: $ git difftool --dir-diff -M -C baf5aaa33383af656a34b7ba9039e9eb3c9e678c That ends up calling `git diff --raw -M -C baf5aaa33383af656a34b7ba9039e9eb3c9e678c`, whose output contains: ...[snip]... :000000 100755 0000000... 05824fa... A t/lib-gpg.sh :100644 100644 83855fa... 83855fa... R100 t/t7004/pubring.gpg t/lib-gpg/pubring.gpg :100644 100644 8fed133... 8fed133... R100 t/t7004/random_seed t/lib-gpg/random_seed :100644 100644 d831cd9... d831cd9... R100 t/t7004/secring.gpg t/lib-gpg/secring.gpg :100644 100644 abace96... abace96... R100 t/t7004/trustdb.gpg t/lib-gpg/trustdb.gpg :100644 100644 3f24384... f7dc078... M t/lib-httpd.sh ...[snip]... I suspect the R100 lines are the ones that are throwing it off. >> xxdiff still gives an error message about "$tmp/left/RelNotes: No such >> file or directory" with --no-renames so we may want to touch some >> dummy files to make the tools happy. > > I get the same error.  I looked into it and found that "RelNotes" is a > symbolic link.  If we look at the standard diff of the example you > gave, we see the following: > >    $ git diff e5b0662 -- RelNotes >    diff --git a/RelNotes b/RelNotes >    index 7d92769..2c2a169 120000 >    --- a/RelNotes >    +++ b/RelNotes >    @@ -1 +1 @@ >    -Documentation/RelNotes/1.7.8.txt >    \ No newline at end of file >    +Documentation/RelNotes/1.7.10.txt >    \ No newline at end of file > > When 'git-difftool' executes this command, it sees that the "RelNotes" > file changed, but it is not smart enough to copy the link target.  So > the "/tmp/git.diffall.XXXXX/left" directory has "RelNotes" in it, but > not the target of the link "Documentation/RelNotes/1.7.8.txt". > > In xxdiff, this manifests as a "file not found" error.  In meld, it is > shown as a "Dangling symlink". > > I will look into ways to deal with this, probably adding special logic > to deal with file modes of "120000". Sounds reasonable. Thanks Tim, -- David