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

On Wed, Apr 11, 2012 at 9:46 AM, Tim Henigan [off-list ref] wrote:

On Wed, Apr 11, 2012 at 5:02 AM, David Aguilar [off-list ref] wrote:

quoted

On Tue, Apr 10, 2012 at 10:24 AM, Tim Henigan [off-list ref] wrote:

quoted

On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar [off-list ref] wrote:

quoted

On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan [off-list ref] wrote:

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.

I tried to avoid setting $GIT_DIR in earlier versions of the patch.
However as discussed here [1], either 'git update-index' or 'git
checkout-index' did not work as expected without explicitly setting
$GIT_DIR.

If $GIT_DIR is not set, 'update-index' and 'checkout-index' will only
work if 'difftool' is called from the repo root.  If 'difftool' is
called from a subdirectory, then one of the commands fails.

I suspect that when $GIT_INDEX_FILE is set but $GIT_DIR is not, then
$GIT_DIR is assumed to be 'pwd'.  However, I was not able to prove
that.

quoted

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

I also tried this in an earlier version of the patch.  As noted here
[2], I found that when 'difftool' was run from a subdirectory of the
repo root, '$repo->wc_path()' returned the subdirectory rather than
the repo root.

Thinking about this again now, I realize it was a side-effect of
$GIT_DIR being set in the script.  The man page for git config states
that:

   If --git-dir or GIT_DIR is specified but none of --work-tree, GIT_WORK_TREE
   and core.worktree is specified, the current working directory is regarded as
   the top level of your working tree.

So, if I explicitly set $GIT_DIR just for the 'update-index' and
'checkout-index' commands, I need to unset it afterwards.  This should
allow '$repo->wc_path()' to behave as expected.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/193296/focus=193302
[2]: http://thread.gmane.org/gmane.comp.version-control.git/193601/focus=193603

That's right.  If we set it for one command we have to remember to
unset it afterwards else it won't work as expected.

If we're setting GIT_DIR then we should probably set GIT_WORK_TREE
too.  It seems like one way would be to call repo_path() and wc_path()
early, set the variables with the returned values, and then worry
about managing GIT_WORK_TREE before+after calling checkout-index.
That might work.

And in case the code needs it, there's a GIT_PREFIX variable that is
set when the current directory is a subdir beneath the repo root.
-- 
David