From: David Aguilar Date: Wed, 11 Apr 2012 09:02:26 GMT Subject: Re: [PATCH 8/9 v11] difftool: teach difftool to handle directory diffs Message-ID: In-Reply-To: On Tue, Apr 10, 2012 at 10:24 AM, Tim Henigan wrote: > On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar wrote: >> On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan 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