Re: Fwd: Git Directory Diff for submodule

From: John Keeping <john@keeping.me.uk>
Date: Feb 21, 2014, 09:27 UTC
Message-ID: <20140221092725.GE1048@serenity.lan>
In-Reply-To: <CAJDDKr5czZezBZC1HtS9EjKvpPn-sxQTJOWLABRBVw2ei2BFVg@mail.gmail.com>

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

Show 41 quoted lines

> On Thu, Feb 20, 2014 at 1:03 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> > Sorry for the late reply, but here we go ...
> >
> > Am 10.02.2014 07:33, schrieb Gábor Lipták:
> >> 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.

Show 37 quoted lines

> >> 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.