Re: [PATCH] submodule: implement `module_name` as a builtin helper

[Junio C Hamano <gitster@pobox.com>]

Stefan Beller <sbeller@google.com> writes:

> On Fri, Aug 7, 2015 at 1:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jens Lehmann <Jens.Lehmann@web.de> writes:
>>
>> This change...
>>
>>>> @@ -723,10 +733,8 @@ int fetch_populated_submodules(const struct argv_array *options,
>>>>              if (!S_ISGITLINK(ce->ce_mode))
>>>>                      continue;
>>>>
>>>> -            name = ce->name;
>>>> -            name_for_path = unsorted_string_list_lookup(&config_name_for_path, ce->name);
>>>> -            if (name_for_path)
>>>> -                    name = name_for_path->util;
>>>> +            name_for_path = submodule_name_for_path(ce->name);
>>>> +            name =  name_for_path ? name_for_path : ce->name;
>>
>> ... interacts with Heiko's cached submodule config work that seems
>> to have stalled.
>
> We can drop that hunk as it only uses the new method
> `submodule_name_for_path` but doesn't change functionality.
> So if you want to keep Heikos work, I'll just resend the patch
> without that hunk.

Does such a result even make sense?  Note that I wasn't talking
about textual conflict.

If we followed what you just said, that patch will try to directly
read the data in config_name_for_path string list, which is removed
by Heiko's series, if I am reading it right.

In the new world order with Heiko's series, the way you grab
submodule configuration data is to call submodule_from_path() or
submodule_from_name() and they allow you to read from .gitmodules
either in a commit (for historical state), the index, or from the
working tree.  Which should be much cleaner and goes in the right
direction in the longer term.

And the best part of the story is that your module_name would be
just calling submodule_from_path() and peeking into a field.

> 2) Come up with a good thread pool abstraction
>    (Started as "[RFC/PATCH 0/4] parallel fetch for submodules" )
>    This abstraction (if done right) will allow us to use it in different places
>    easily. I started it as part of "git fetch --recurse-submodules" because
>    it is submodule related and reasonably sized

I personally think this gives the most bang-for-buck.  Write that
and expose it as "git submodule for-each-parallel", which takes the
shell scriptlet that currently is the loop body of "while read mode
sha1 stage sm_path" in update and clone.  You will have immediate
and large payback.

And you do not even need module_list and module_name written in C in
order to do so.  Not that these two are not trivial to implement, but
the payoff from them is not as large as from for-each-parallel ;-)