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

From: Stefan Beller <sbeller@google.com>
Date: Aug 6, 2016, 00:22 UTC
Message-ID: <CAGZ79kYsdqffWykzcEQcd2HigYqVNt0Y1H0Biw3QA2RdQaMOOw@mail.gmail.com>
In-Reply-To: <xmqq1t22x6fj.fsf@gitster.mtv.corp.google.com>

On Fri, Aug 5, 2016 at 2:16 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 10 quoted lines

> 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.

Show 15 quoted lines

>
>> @@ -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.

Show 5 quoted lines

> 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