Re: What's cooking in git.git (Jan 2016, #04; Wed, 20)

From: Stefan Beller <sbeller@google.com>
Date: Jan 27, 2016, 18:03 UTC
Message-ID: <CAGZ79kZQ725ErqLseb_3Mp-Yyc=X6dhRfHwARynXUzQigw5dvg@mail.gmail.com>
In-Reply-To: <53B46FC1-7E34-49F1-9955-AE87E27519F5@gmail.com>

On Wed, Jan 27, 2016 at 12:46 AM, Lars Schneider <larsxschneider@gmail.com> wrote:

Show 26 quoted lines

>
> On 26 Jan 2016, at 23:58, Junio C Hamano <gitster@pobox.com> wrote:
>
>> Lars Schneider <larsxschneider@gmail.com> writes:
>>
>>> Hi Junio,
>>>
>>> Did you miss the topic "submodule: extend die message on failed
>>> checkout with depth argument" or do you not agree with it
>>> ($gmane/282779)? Stefan Beller reviewed the commits ($gmane/283666
>>> and $gmane/283851).
>>
>> No, these three are still in my inbox.
>>
>> Quite honestly, "test to demonstrate breakage" alone is not all that
>> interesting to me unless I get a firm impression that somebody is
>> (or will soon be) working on fixing the breakage--which would make
>> it worth keeping track of it in my tree, to allow the eventual fix
>> to be more easily verified and protected from getting broken again.
>> Also "these look good to me" without much explanation is not much of
>> a review, and such a comment on a "demonstrate breakage" did not add
>> to the sense of urgency to the issue, either.
>>
>> That was why it has been sitting near the bottom of the pile.
>
> OK, I get your point. I got the impression that Stefan is on to a fix ($gmane/281260). Looks like he is not and I need to learn more about Git to tackle fixes of this scope.

I was on it and I put it on hold as I wanted to prioritize sb/submodule-parallel-update, sb/submodule-init and the upcoming sb/submodule-groups (maybe that's called sb/submodule-autoInit, its scope is still being bikehedded)

Coming back to $gmane/281260 with a fresh mind, I think this is what we want on the client side:

 1) fetch as we always did. During this fetch we learn from the
    servers capabilities list, if it supports fetching direct sha1s.
 2) if the needed sha1 has been fetched in step 1, stop.
 3) fetch the needed sha1 directly if the server supports it.
 4) if the server doesn't support it, display a message like:
    "Please ask your Git server admin, to run at least Git 2.4.0
    and enable uploadpack.allowReachableSHA1InWant to have
    submodules working properly"

Currently we only perform step 1 and this yields the error when fetching too shallow to have the needed sha1 included.

The server side should be ok as soon as uploadpack.allowReachableSHA1InWant is enabled. I thought we would need to add another option, but it looks like that option is exactly what we need here.

If you want to give it a try, I'll be happy to review patches! (with more than "these look good to me" ;)

Thanks, Stefan

> Would it be an option for you to just drop patch 1/3 from this series and look at the remaining ones? 2/3 fixes an "&& chain issue" ($gmane/282776) and 3/3 prints a explanatory user warning instead of an incomprehensible error ($gmane/282777).
>
> Thanks,
> Lars