From: Jens Lehmann Date: Tue, 17 Nov 2015 20:39:45 GMT Subject: Re: [PATCH v2] add test to demonstrate that shallow recursive clones fail Message-ID: <564B9091.7070902@web.de> In-Reply-To: Am 17.11.2015 um 21:04 schrieb Stefan Beller: > On Tue, Nov 17, 2015 at 11:46 AM, Jens Lehmann 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 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.