Re: [PATCH v3] builtin/submodule--helper.c: handle missing submodule URLs

From: Taylor Blau <me@ttaylorr.com>
Date: May 24, 2023, 20:36 UTC
Message-ID: <ZG51XsEq4JB9IK33@nand.local>
In-Reply-To: <c8d32d5a-9803-9eb7-baaa-49b4521f0c37@web.de>

On Wed, May 24, 2023 at 10:29:39PM +0200, René Scharfe wrote:

Show 8 quoted lines

> Am 24.05.23 um 21:51 schrieb Taylor Blau:
>
> > By adding a NULL-ness check on `sub->url`, we'll fall into the 'else'
> > branch, setting `url` to `sub->url` (which is NULL). Before attempting
> > to invoke `git submodule--helper clone`, check whether `url` is NULL,
> > and die() if it is.
>
> Why die() here instead of just warn and skip as well?

That's a good point. When I read prepare_to_clone_next_submodule(), I thought that it was already too late to skip that submodule. But my understanding was incorrect, we could easily issue a warning() and return '0', which would indicate to skip it.

But I concur with Junio earlier in the thread that we don't have to rush things to get this into -rc2 or even 2.41, since this bug has been with us since v2.20.0.

Thanks, Taylor