On Wed, Feb 3, 2016 at 4:54 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Stefan Beller <sbel...@google.com> writes:
>
>> On Wed, Feb 3, 2016 at 3:24 PM, Junio C Hamano <gits...@pobox.com> wrote:
>>> Stefan Beller <sbel...@google.com> writes:
>>>
>>>> + if (ce_stage(ce)) {
>>>> + if (pp->recursive_prefix)
>>>> + strbuf_addf(err, "Skipping unmerged
>>>> submodule %s/%s\n",
>>>> + pp->recursive_prefix, ce->name);
>>
>> As a side question: Do we care about proper visual directory
>> separators in Windows?
>
> You know I do not do Windows ;-) I'll leave the question for others
> to answer (I am trying not to be one of the the only small number of
> people who review code around here).
>
>> I never run into this BUG after having proper initialization, so maybe it's
>> not
>> worth carrying this code around. (We have many other places where
>> submodule_from_{path, name} is used unchecked, so why would this place
>> be special?)
>
> That is why I wondered if moudule_list() is a better place to do so.
> That is where the list of everybody works on come from.
I do not think that is a better place as not every consumer of module_list
(and module_list_compute as the nested function) will need to use the
submodule caching API. So these consumers are not interested in possible
bugs in the submodule cache API nor do they want the performance hit
which comes from checking unrelated stuff in there.
As said, I only saw this bug when the cache was not initialized properly,
and then such a bug is to be expected. I'd rather remove it in a reroll.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
