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

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> Since 9badf97c4 (remote: allow resetting url list),

Please do not be original in places where it shouldn't matter.  Use
"git show -s --format=reference" that includes the datestamp to help
readers judge how old the problem is.

> we reset the remote
> URL if the provided URL is empty. This means any caller of
> `remotes_remote_get()` would now get a NULL remote.

"NULL remote" meaning?

If you have this:

 [remote "multi"]
	url = wrong-one
	url = wrong-two
	url =

and ask "remotes_remote_get()" to give you the remote "multi", you'd
get a remote whose URL array has no elements.  Is that what you are
referring to?

> The 'builtin/push.c' code, calls 'set_refspecs' before validating the
> remote.

There is a comment about "lazily grab remote", so it is very
understandable.

> This worked earlier since we would get a remote, albeit with an
> empty URL. With the new changes, we get a NULL remote and this crashes.

You'd really really need to clarify what you mean by "a NULL remote"
if you want the proposed log message and the change to be
understood.  The change made by 9badf97c (remote: allow resetting
url list, 2024-06-14), as far as I can tell, can make the strvecs
that hold URL and pushURL in a remote structure empty, but it does
not otherwise destroy the remote structure, or nullify a pointer
that points at the remote structure.  So I am completely lost here.