Re: Fwd: Git Directory Diff for submodule

On Thu, Feb 20, 2014 at 02:41:23PM -0800, David Aguilar wrote:

On Thu, Feb 20, 2014 at 1:03 PM, Jens Lehmann [off-list ref] wrote:

quoted

Sorry for the late reply, but here we go ...

Am 10.02.2014 07:33, schrieb Gábor Lipták:

quoted

Hi Jens,

So "git status" says:

liptak@liptak-kubuntu:~/Projects/MAIN_MODULE/platform/SUBMODULE
[master]$ git status
# On branch master
# Your branch is up-to-date with 'origin/master'.
#
# Changes not staged for commit:
#   (use "git add <file>..." to update what will be committed)
#   (use "git checkout -- <file>..." to discard changes in working
directory)
#
#       modified:   xxxxxx.java
#       modified:   xxxxxxx.java
# ...
# ...
# ...
# ...
# ...
#
no changes added to commit (use "git add" and/or "git commit -a")

git config core.worktree gives back: "../../../../platform/SUBMODULE"

Which looks a bit strange but is perfectly ok for a repository
that uses a gitfile, as the core.worktree setting is relative
to the git directory the gitfile references and not the directory
the gitfile lives in. A quick glance at the find_worktree
subroutine in git-difftool.perl makes me think that difftool is
not aware of that fact. David, does that make sense?

That does make sense. It sounds like that may need to be adjusted.

What does `git rev-parse --show-toplevel` print? It seems like the
find_worktree() logic needs to be extended to handle .git files.

Having tried it with a submodule here, it looks like rev-parse gets this
right.

quoted

quoted

The submodule was inited simply with "git submodule init" +
"git.submodule update"

Or possibly, as you mention below, it could be that "git submodule
init" ended up writing the wrong core.worktree value. I'm not very
familiar with how they are initialized, but the paths do seem sane, so
I doesn't seem like that's the issue.

If it's a problem in difftool we can probably find a way to do the
right thing. It looks like the core.worktree is relative to the
.git/modules/XXX directory rather than the submodule (and
super-project)'s worktree.

Here's our current logic:

sub find_worktree
{
    my ($repo) = @_;

    # Git->repository->wc_path() does not honor changes to the working
    # tree location made by $ENV{GIT_WORK_TREE} or the 'core.worktree'
    # config variable.
    my $worktree;
    my $env_worktree = $ENV{GIT_WORK_TREE};
    my $core_worktree = Git::config('core.worktree');

    if (defined($env_worktree) and (length($env_worktree) > 0)) {
        $worktree = $env_worktree;
    } elsif (defined($core_worktree) and (length($core_worktree) > 0)) {
        $worktree = $core_worktree;
    } else {
        $worktree = $repo->wc_path();
    }
    return $worktree;
}

John, any thoughts?

I don't really like reimplementing logic from core Git here, so I think
it might be best to just call "git rev-parse --show-toplevel" here; it
would also be sufficient to make sure we resolve $core_worktree against
$GIT_DIR - should we be doing that for $env_worktree as well?  It looks
like git-rev-parse treats a relative GIT_WORK_TREE as relative to the
current working directory, so we're OK there.

Does anyone know why repository->wc_path() does not obey the config
variable or $GIT_WORK_TREE?  It seems like the most correct fix is to
fix it there.