Re: [PATCH 2/3] Teach read_tree_recursive() how to traverse into submodules

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi,

On Sun, 18 Jan 2009, Lars Hjemli wrote:

> On Sun, Jan 18, 2009 at 16:48, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > On Sun, 18 Jan 2009, Lars Hjemli wrote:
> >
> >> +                  struct tree **subtree)
> >> +{
> >> +     unsigned char sha1[20];
> >> +     int linked_odb = 0;
> >> +     struct commit *commit;
> >> +     void *buffer;
> >> +     enum object_type type;
> >> +     unsigned long size;
> >> +
> >> +     hashcpy(sha1, commit_sha1);
> >> +     if (!add_gitlink_odb(path)) {
> >> +             linked_odb = 1;
> >> +             if (resolve_gitlink_ref(path, "HEAD", sha1))
> >> +                     die("Unable to lookup HEAD in %s", path);
> >> +     }
> >
> > Why would you want to continue if add_gitlink_odb() did not find a checked
> > out submodule?
> >
> > Seems you want to fall back to look in the superproject's object database.
> > But I think that is wrong, as I have a superproject with many platform
> > dependent submodules, only one of which is checked out, and for
> > convenience, the submodules all live in the superproject's repository.
> 
> Actually, I want this to work for bare repositories by specifying the
> submodule odbs in the alternates file. So if the current submodule odb
> wasn't found my plan was to check if the commit object was accessible
> anyways but don't die() if it wasn't.

Please make that an explicit option (cannot think of a good name, though), 
otherwise I will not be able to use your feature.  Making it the default 
would be inconsistent with the rest of our submodules framework.

> >> +     commit = lookup_commit(sha1);
> >> +     if (!commit)
> >> +             die("traverse_gitlink(): internal error");
> >
> > s/internal error/could not access commit '%s' of submodule '%s'",
> >                        sha1_to_hex(sha1), path);/
> 
> Ok (I belive this codepath is virtually impossible to hit, hence the
> "internal error", but I could of course be mistaken).

You make it a function that is exported to other parts of Git in cache.h.  
So you might just as well expect it to be used by other parts at some 
stage.

Ciao,
Dscho