Re: [PATCH] builtin-merge: missing structure bzero.

[Pierre Habouzit <madcoder@debian.org>]

[-- Attachment #1: Type: text/plain, Size: 4378 bytes --]

On Mon, Jul 21, 2008 at 06:18:50PM +0000, Pierre Habouzit wrote:
> On Mon, Jul 21, 2008 at 05:21:19PM +0000, Miklos Vajna wrote:
> > On Mon, Jul 21, 2008 at 07:03:50PM +0200, Pierre Habouzit <madcoder@debian.org> wrote:
> > > This cause segfaults when replacing a directory with a submodule in a
> > > fast-forward.
> > 
> > Thanks.
> > 
> > > +test_expect_failure 'Replace a directory with a submodule, with a file conflict' '
> > > +	mkdir test &&
> > > +	cd test &&
> > > +	: create our repository with a sub/a file &&
> > > +	git init &&
> > > +	mkdir sub && echo a > sub/a &&
> > > +	git add sub && git commit -asm"initial repository" &&
> > > +	: save this state in a new branch &&
> > > +	git branch temp &&
> > > +	: then replace sub with it &&
> > > +	git rm -rf sub &&
> > > +        git submodule add -- "$(pwd)/../submodule/.git/" sub &&
> > > +	git commit -asm "replace sub/ with a submodule" &&
> > > +	: then try to update the "temp" branch &&
> > > +	git checkout temp &&
> > 
> > It seems this one fails. I guess this will be a problem in the low-level
> > merge code (read-tree -m) and not in builtin-merge.
> 
>   Yeah, I saw that afterwards, the error was misleading (as it tells
> about some "merge" issue), but when I tried to debug it, it was indeed
> in git checkout. The easiest way to reproduce, is to have a submodule
> that replace a file that was previously versionned (which is something
> that will happen in real life when you take out a subdirectory of a
> project to make it live into a submodule) and that you then git checkout
> HEAD~1.

Actually, the issue is quite obvious when thinking. At git-checkout
time, git doesn't recurse (yet) into submodules, hence when we checkout
'temp' that is basically master~1 that basically does that:

"sub/" is a submodule, and gets replaced by sub/ a tree with "a" in it,
and the submodule _also_ has "a". As submodules are ignored silently, it
just:

  (1) forgets about the submodule at once, leaving all the submodules
      files into the wild, and sub/a (that was in the submodule) is now
      an untracked file.

  (2) tries to checkout sub/a and sees a file currently untracked and
      barfs.

So it boils down to the fact that git-checkout does nothing with
submodules. I really think we should address that, it's a major issue
wrt submodules and usability.


I believe we should for now require that all submodules are clean for
checkout to be able to work, and then we have those cases:

  * submodule -> directory: ignore any file that is either versionned or
    ignored in the submodule wrt checkout issues. If no issue arises
    with untracked non ignored files in the submodule, then basically:
    (1) in the submodule: git ls-files | xargs rm -f ; rm -rf .git 
    (2) proceed with the checkout.

  * submodule -> blob: refuse to proceed if there is any untracked non
    ignored file in the submodule, else just rm -rf it, and proceed.

  * submodule h1 -> h2: if "git checkout h2" will work in the submodule,
    then we have no problem with the checkout, and do proceed. Though,
    if the checkout isn't possible because "h2" is unknown, then instead
    of a "cannot read h2" error, one should suggest the user to update
    its submodules (he probably lacks some objects and needs to fetch).

    Special case: if the submodule isn't initialized, proceed and warn
    about the submodule being uninitialized.

  * {blob,directory} -> submodule already work properly, though
    suggesting the user to run "git submodule update -i" when we *know*
    we have uninitialized submodules because of a checkout is a good
    idea.

  Note that uninitialized submodules are not special cases for the two
first cases, because uninitialized submodules are not different from a
directory with possibly untracked files in it. An uninitialized
submodule probably need to be dealt with as if it's clean from many
places actually.


  The issue is that I tried to grok upack-trees.c, but the code is quite
dense, and how to hack this efficiently still eludes me, because I don't
really get the big picture enough yet.


-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]