Re: [PATCH v2] add test to demonstrate that shallow recursive clones fail

[Stefan Beller <sbeller@google.com>]

On Tue, Nov 17, 2015 at 12:39 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
> Am 17.11.2015 um 21:04 schrieb Stefan Beller:
>>
>> On Tue, Nov 17, 2015 at 11:46 AM, Jens Lehmann <Jens.Lehmann@web.de>
>> wrote:
>>>
>>>
>>> But for quite some time you'll have older servers out there that
>>> don't support fetching a single sha1 or aren't configured to do so.
>>
>>
>> Only when talking about the open source side. If you have all the
>> submodules/superprojects on your companies mirror, you can control
>> the git installations there.
>
>
> Sure. But that doesn't mean we should make life harder for the open
> source side, no? We'll have to support both for quite some time.
>
>>> Wouldn't it be better to give the user an appropriate warning and
>>> fall back to cloning everything for those submodules while using the
>>> optimized new method for all others and the superproject? Otherwise
>>> you won't be able to limit the depth if only a single submodule
>>> server doesn't support fetching a single sha1.
>>>
>>
>> I think warnings are fine, but no fallbacks. The warning could look like:
>>
>>      Server for submodule <foo> doesn't support fetching by sha1.
>>      Fetch again without depth argument.
>>
>> and keep going with the other submodules. This would allow the user
>> to make an informed decision if they want to have the fallback solution
>> (which requires more band width, disk space)
>
>
> No, this is a regression. This worked before but now some submodules
> are missing from the clone. And if that happens inside a Jenkins
> script I doubt that Jenkins can make an informed decision, that job
> will simply fail.
>
>> On the other hand, that's what people do today, so it's not that bad
>> either,
>> so I guess falling bad would work too.
>
>
> Not that bad? I don't see any other sane way. Don't break formerly
> working use cases without a very good reason. Fall back to what we
> did before (even if it is suboptimal) and only then use the new
> optimized (and admittedly better) feature when it is available.

I assumed we'd have yet another flag to activate the new behavior,
but if you want to roll out that new feature as a default, I agree on
needing the fallback.