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

From: David Aguilar <davvid@gmail.com>
Date: Apr 15, 2012, 22:20 UTC
Message-ID: <CAJDDKr7Uw3Nwg4p7F2zaY8f82j3_tRf3WiiO+YSN+nA6a9wY6w@mail.gmail.com>
In-Reply-To: <1334335002-30806-1-git-send-email-tim.henigan@gmail.com>

On Fri, Apr 13, 2012 at 9:36 AM, Tim Henigan <tim.henigan@gmail.com> wrote:

Show 22 quoted lines

> When 'difftool' is called to compare a range of commits that modify
> more than one file, it opens a separate instance of the diff tool for
> each file that changed.
>
> The new '--dir-diff' option copies all the modified files to a temporary
> location and runs a directory diff on them in a single instance of the
> diff tool.
>
> Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
> ---
>
> This replaces v12 of the script that was sent to the list on April 12, 2011.
>
> Changes in v13:
>
> The 'git diff' command is now called via 'Git->repository->command_oneline'
> again. We need to run the command in a way that allows @ARGV to be given
> as a list, rather than a string, to insure that IFS and shell meta-
> characters are handled properly.  Thanks to Junio Hamano for pointing
> this out [1].
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/195326/focus=195353

Thanks Tim. Sorry for reading this patch out of context and missing the obvious point that it needs the diff output to do something useful in my last review.

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 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. Use of uninitialized value $lsha1 in concatenation (.) or string at /home/david/src/git/git-difftool line 107. Use of uninitialized value $rmode in string ne at /home/david/src/git/git-difftool line 110. Use of uninitialized value $rsha1 in string ne at /home/david/src/git/git-difftool line 111. Use of uninitialized value $rmode in concatenation (.) or string at /home/david/src/git/git-difftool line 112. Use of uninitialized value $rsha1 in concatenation (.) or string at /home/david/src/git/git-difftool line 112. Use of uninitialized value $rmode in string eq at /home/david/src/git/git-difftool line 96. Use of uninitialized value $lsha1 in concatenation (.) or string at /home/david/src/git/git-difftool line 107. Use of uninitialized value $rmode in string ne at /home/david/src/git/git-difftool line 110. Use of uninitialized value $rsha1 in string ne at /home/david/src/git/git-difftool line 111. Use of uninitialized value $rmode in concatenation (.) or string at /home/david/src/git/git-difftool line 112. Use of uninitialized value $rsha1 in concatenation (.) or string at /home/david/src/git/git-difftool line 112. Use of uninitialized value $rmode in string eq at /home/david/src/git/git-difftool line 96. Use of uninitialized value $lsha1 in concatenation (.) or string at /home/david/src/git/git-difftool line 107. Use of uninitialized value $rmode in string ne at /home/david/src/git/git-difftool line 110. Use of uninitialized value $rsha1 in string ne at /home/david/src/git/git-difftool line 111. Use of uninitialized value $rmode in concatenation (.) or string at /home/david/src/git/git-difftool line 112. Use of uninitialized value $rsha1 in concatenation (.) or string at /home/david/src/git/git-difftool line 112. fatal: malformed index info /t9800-git-p4-basic.sh :100755 100755 a25f18d36a196a4b85f6cac15a6a081744fe8fa1 d41470541650590355bf0de1a1b556b3502492b5 M update-index -z --index-info: command returned error: 128

This one works fine but the difftool I used for testing (xxdiff) complained about a missing file: $ git difftool -d 86e15ff4fe9924b73af32d1bebe77eb5592b93cd

You can find more problematic commits by doing `git log -- xdiff`. I was originally going to test --dir-diff in a subdirectory (xdiff in this example) and that's when I found these. I don't think it has anything to do with subdirs; there's probably a commit in the history that changes something (submodules? modes? not sure...) so once we go beyond that point in the history it confuses --dir-diff.

-- 
David