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

From: David Aguilar <davvid@gmail.com>
Date: Apr 11, 2012, 09:02 UTC
Message-ID: <CAJDDKr7XXqWBRVXhEenWVcE+7526sYVV821Ytty0xbZBvshd-w@mail.gmail.com>
In-Reply-To: <CAFouetijF5vt9r1GfbntJ0BEqoo0FpztkeqrmeN=-6DK1Q_XRA@mail.gmail.com>

On Tue, Apr 10, 2012 at 10:24 AM, Tim Henigan <tim.henigan@gmail.com> wrote:

Show 34 quoted lines

> On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar <davvid@gmail.com> wrote:
>> On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan <tim.henigan@gmail.com> wrote:
>>> diff --git a/git-difftool.perl b/git-difftool.perl
>>> index d4fe998..5bb01e1 100755
>>> --- a/git-difftool.perl
>>> +++ b/git-difftool.perl
>>> @@ -1,21 +1,29 @@
>
> ...snip...
>
>>> +       $ENV{GIT_DIR} = $repo->repo_path();
>>> +       $ENV{GIT_INDEX_FILE} = "$tmpdir/lindex";
>>> +       ($inpipe, $ctx) = $repo->command_input_pipe(qw/update-index -z --index-info/);
>>> +       print($inpipe $lindex);
>>> +       $repo->command_close_pipe($inpipe, $ctx);
>>> +       system(('git', 'checkout-index', '--all', "--prefix=$ldir/"));
>
> ...snip...
>
>>> +       # If the diff including working copy files and those
>>> +       # files were modified during the diff, then the changes
>>> +       # should be copied back to the working tree
>>> +       my $repo = Git->repository();
>>> +       my $workdir = $repo->repo_path() . "/..";
>>
>> Does this work when $GIT_WORK_TREE / core.worktree are defined?
>
> Should I also be concerned that the existing code always overrides $GIT_DIR?
> For example, should I squash in the following before calling 'git update-index'?
>
> -       $ENV{GIT_DIR} = $repo->repo_path();
> +      if (not defined($ENV{GIT_DIR})) {
> +              $ENV{GIT_DIR} = $repo->repo_path();
> +      }

I think the right thing to do would be to not override GIT_DIR at all.
 I haven't read it deeply enough to know whether it was being
overridden for a specific reason, but I think it should be safe to
leave it as-is.

Git.pm ends up overriding these variables itself anyways when calling commands.

The GIT_WORK_TREE check should use $repo->wc_path(). Git.pm's already done all the hard work ;-)

-- 
David