Re: [PATCH 8/9 v13] difftool: teach difftool to handle directory diffs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Apr 15, 2012 at 9:01 PM, David Aguilar <davvid@xxxxxxxxx> wrote:
> On Sun, Apr 15, 2012 at 3:20 PM, David Aguilar <davvid@xxxxxxxxx> wrote:
>> On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@xxxxxxxxx> 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?


> I did some more investigating.  I think this happens when the diff
> contains detected renames.
>
> This command made it work:
>
> $ git difftool --dir-diff --no-renames e5b06629de847663aaf0f7daae8de81338da3901
>
> So I think we might need to specify --no-renames when calling git diff --raw.

One of my test repos includes added, removed and renamed files.  They
do not cause any errors.  Just like the output of 'git diff --raw', in
'git difftool --dir-diff' renamed files are shown as the old file name
being deleted and the new file name being added.

I also looked to see if 'git diff --raw' reports a different result
when '--no-renames' is used, but did not see any difference:

    $ git diff --raw e5b0662 > diff_with_renames.txt
    $ git diff --raw --no-renames e5b0662 > diff_without_renames.txt
    $ diff diff_with_renames.txt diff_without_renames.txt


> 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".

[1]: https://github.com/thenigan/git/tree/th/difftool-phase2
[2]: https://github.com/gitster/git/tree/th/difftool-diffall
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]