git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Lars Hjemli <hjemli@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC/PATCH 1/3] tree.c: add support for traversal of submodules
Date: Mon, 12 Jan 2009 02:07:19 -0800	[thread overview]
Message-ID: <7vab9wj0rs.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <8c5c35580901120104u418d8d73mad4ab7d71fe8c3f8@mail.gmail.com> (Lars Hjemli's message of "Mon, 12 Jan 2009 10:04:29 +0100")

Lars Hjemli <hjemli@gmail.com> writes:

> On Mon, Jan 12, 2009 at 04:15, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> When the user explicitly asks you to traverse into submodules and the
>> necessary commit is not available in a submodule, the code goes on without
>> complaining.  I am not saying it is bad, but I wonder if we would want to
>> distinguish these three cases:
>>
>>  (1) the submodule is initialized and the necessary commit is there.
>>
>>  (2) the submodule is initialized, but the necessary commit is missing.
>>
>>  (3) the submodule is not even initialized (aka "the user is not
>>     interested in it"); there is only an empty directory.
>>
>> I think it is perfectly fine not to say anything for (3) but I am unsure
>> about the second case.
>
> Do we want to impose the porcelainish rules of git-submodule
> (.gitmodules, .git/config) in read_tree_recursive()?
>
> If so, I guess a new submodule.h might provide something like this
> (disclaimer: coded in gmail):

I do not see why you would need anything more than we already have to tell
(3) from (1) and (2).  And I do not see why you need to have the Porcelain
policy in the picture for telling these three cases apart, either.

For example, there is this code in read-cache.c::ce_compare_gitlink():

        static int ce_compare_gitlink(struct cache_entry *ce)
        {
                unsigned char sha1[20];

                /*
                 * We don't actually require that the .git directory
                 * under GITLINK directory be a valid git directory. It
                 * might even be missing (in case nobody populated that
                 * sub-project).
                 *
                 * If so, we consider it always to match.
                 */
                if (resolve_gitlink_ref(ce->name, "HEAD", sha1) < 0)
                        return 0;
                return hashcmp(sha1, ce->sha1);
        }

It asks resolve_gitlink_ref() to see if the directory (where the submodule
checkout _might_ be present if the user is interested in it) has .git/HEAD
that resolves.  If so, the user has a checkout and is interested in it.
Otherwise, there is no checkout, in other words, we have case (3) above.

Whether you force the user to link the submodule object store to the
primary one as alternates, or do that for the user temporarily inside the
process [*1*], you would then be able to tell (1) and (2) apart by asking
has_sha1_file() if you can see the commit.

One thing that is unclear is to me is for whom the commit is missing (or
present).  I think the outline I gave above follows the design of your
patch to assume that the commit may (or may not) be available to the
superproject and traverse into the commit when that is the case.  It does
not mean the commit is available to the submodule itself (the commit may
have found in the primary project itself, not via the alternates), but
such an arrangement makes it somewhat useless.

What's the typical direction of using alternates in a setting with
superproject with a submodule?  Do people have alternates in the submodule
repository that borrows from the superproject repository?  Or the other
way around?  What's the rationale for having such alternates for normal
use case?  I am suspecting that there is no reason (other than this
"recursive tree traversal") to have an alternates file in either
direction, but I also strongly suspect that I am missing some unwritten
assumption you are making.

[Footnote]

*1* If you want to recurse seemlessly, it might make sense to add (during
the course of this "recursive tree traversal") the object store of the
submodule repository to the process'es list of alternate object databases
yourself, instead of forcing the user to do so permanently by mucking with
the alternates list of the primary project repository.  But that is an
independent issue.

  reply	other threads:[~2009-01-12 10:08 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-11 23:45 [RFC/PATCH 0/3] Enable in-process submodule traversal Lars Hjemli
2009-01-11 23:45 ` [RFC/PATCH 1/3] tree.c: add support for traversal of submodules Lars Hjemli
2009-01-11 23:45   ` [RFC/PATCH 2/3] archive.c: enable " Lars Hjemli
2009-01-11 23:45     ` [RFC/PATCH 3/3] builtin-ls-tree: " Lars Hjemli
2009-01-12  3:15   ` [RFC/PATCH 1/3] tree.c: add support for " Junio C Hamano
2009-01-12  9:04     ` Lars Hjemli
2009-01-12 10:07       ` Junio C Hamano [this message]
2009-01-12 10:59         ` Lars Hjemli
2009-01-12  2:07 ` [RFC/PATCH 0/3] Enable in-process submodule traversal Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vab9wj0rs.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=hjemli@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).