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

[Jens Lehmann <Jens.Lehmann@web.de>]

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.