On Wed, Apr 11, 2012 at 9:46 AM, Tim Henigan <tim.henigan@xxxxxxxxx> wrote: > On Wed, Apr 11, 2012 at 5:02 AM, David Aguilar <davvid@xxxxxxxxx> wrote: >> On Tue, Apr 10, 2012 at 10:24 AM, Tim Henigan <tim.henigan@xxxxxxxxx> wrote: >>> On Mon, Apr 9, 2012 at 8:14 AM, David Aguilar <davvid@xxxxxxxxx> wrote: >>>> On Wed, Apr 4, 2012 at 12:21 PM, Tim Henigan <tim.henigan@xxxxxxxxx> 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. > > >> 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 -- 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