Re: [RFC/PATCH 1/3] tree.c: add support for traversal of submodules

From: Lars Hjemli <hjemli@gmail.com>
Date: Jan 12, 2009, 09:04 UTC
Message-ID: <8c5c35580901120104u418d8d73mad4ab7d71fe8c3f8@mail.gmail.com>
In-Reply-To: <7vr639kyf0.fsf@gitster.siamese.dyndns.org>

On Mon, Jan 12, 2009 at 04:15, Junio C Hamano <gitster@pobox.com> wrote:

Show 26 quoted lines

> Lars Hjemli <hjemli@gmail.com> writes:
>
>> If the commit referenced by a gitlink is available in the (possibly
>> alternate) object database, read_tree_recursive() is now able to descend
>> into the tree of the linked commit if the flag 'traverse_gitlinks' is
>> turned on.
>>
>> Signed-off-by: Lars Hjemli <hjemli@gmail.com>
>> ---
>>  tree.c |   20 +++++++++++++++++---
>>  tree.h |    1 +
>>  2 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/tree.c b/tree.c
>> index 03e782a..1468e10 100644
>> --- a/tree.c
>> +++ b/tree.c
>> @@ -7,6 +7,7 @@
>>  #include "tree-walk.h"
>>
>>  const char *tree_type = "tree";
>> +int traverse_gitlinks = 0;
>
> I think we tend to put these global settings that will affect everybody in
> environment.c.  You do not have to initialize variable to zero; BSS will
> take care of it.

Ok, I'll add a proper interface in environment.c for this setting.

Show 14 quoted lines

> 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):

	struct submodule {
		int interesting:1;
		char *name;
		char *url;
		char **objectdirs;
		char **paths;
	}

	typedef int (*submodule_cb)(struct submodule *submodule, void *data);

	int load_submodule_config();
	struct submodule *get_submodule_from_path(char *path);
	int add_submodule_objectdb(struct submodule *item);
	int for_each_submodule(submodule_cb cb, void *data);

Then, in read_tree_recursive(), we could check submodule->interesting to decide if we should follow the gitlink. Also, for bare repositories, we'd need to support something like 'submodule.<name>.objectdir' in the config file (and .gitmodules must obviously be read from the objectdb).

If we don't want to impose these rules, the current patch should be minimally sufficient (the user has to edit .git/objects/info/alternates by hand and missing submodule commits are treated as if the submodule is not interesting).

-- 
larsh