Re: [PATCH 5/6] submodule update: add super-reference flag

[Stefan Beller <sbeller@google.com>]

On Fri, Aug 5, 2016 at 2:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>
> That's a bit sketchy description.  From the title, I expected that
> we would see one additional 'unsigned super_reference : 1;' field in
> some structure, but that is not what is happening.  The log message
> needs to describe what these string list are and why they are needed
> a bit better.

A "--super-reference" allows giving a reference to a superproject instead
of a direct reference.

With --reference you had to point to the direct repository, e.g.

    git clone --reference /usr/share/foo git://xyxxz.example/foo

assumed that /usr/share/foo is either the working dir or the git dir
of said project.

When giving references in the submodule context (this part of the helper
is called from "git submodule update", which itself is called from
"git clone --recursive"  we do not know the place where the submodule
reference is living. As we do know where the superproject reference is
living (i.e. "git clone --recursive --reference <superproject>"),
we can construct one possible path where the submodules are located
at <superproject>/<submodule-path>. To differentiate the reference flag
from a direct reference this will be called "--super-reference" as it
references a super project.

The next patch makes use of it by just passing on the --reference
given to "git clone" as --super-reference to the "submodule update"
procedure, which then uses the code in this patch.

>
> At least, please don't name a multiple_word field "multipleword" ;-)

uh :( OK.

>
>> @@ -715,6 +716,15 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
>>               for_each_string_list_item(item, &suc->references)
>>                       argv_array_pushl(&child->args, "--reference", item->string, NULL);
>>       }
>> +     if (suc->superreferences.nr) {
>> +             struct string_list_item *item;
>> +             for_each_string_list_item(item, &suc->superreferences) {
>> +                     strbuf_reset(&sb);
>> +                     argv_array_pushf(&child->args, "--reference=%s/%s",
>> +                                      relative_path(item->string, suc->prefix, &sb),
>> +                                      sub->path);
>
> The phrase "super reference" made me imagine it is some kind of
> "reference" that is applicable to the superproject,

Oh I see. Except that from the superprojects point of view, it is not a
superproject; it is just a project with submodules. From a submodules point
of view you have a superproject however, so a super-reference is a reference
that points to another superproject, so we can find a sibling reference.

> but this code
> smells like it is a "prefix to create reference suited for use in
> submodule".

Yeah that's it.

> Whatever it is, it should be explained better (than "no
> desciption" which is what we have here ;-), and given a name that
> match the explanation.
>
> Thanks.

OK.

Thanks,
Stefan