Re: [PATCH v2] builtin/push: call set_refspecs after validating remote

From: Jeff King <peff@peff.net>
Date: Jul 9, 2024, 23:55 UTC
Message-ID: <20240709235507.GB1525171@coredump.intra.peff.net>
In-Reply-To: <xmqq4j8yflrq.fsf@gitster.g>

On Tue, Jul 09, 2024 at 11:44:25AM -0700, Junio C Hamano wrote:

Show 19 quoted lines

> In any case, an obvious additional fix on top of your change might
> be to do something like this:
> 
>         diff --git i/remote.c w/remote.c
>         index 5fa046c8f8..d7f9ba3571 100644
>         --- i/remote.c
>         +++ w/remote.c
>         @@ -682,7 +682,7 @@ remotes_remote_get_1(
>                 struct remote *ret;
>                 int name_given = 0;
> 
>         -	if (name)
>         +	if (name && *name)
>                         name_given = 1;
>                 else
>                         name = get_default(remote_state, remote_state->current_branch,
> 
> which would give us the default remote name, and we would not call
> add_url_alias() with a bogus empty string to nuke the list.

FWIW, I almost suggested something like this earlier. The outcome will be the same (remote_get(), etc, will return NULL), but I think it removes the "this is surprising" comment from my earlier email and makes things much more explicit.

(I also agree with everything else you said in your review).

-Peff