Re: Determining whether you have a commit locally, in a partial clone?

From: Jeff King <peff@peff.net>
Date: Jun 21, 2023, 06:54 UTC
Message-ID: <20230621065446.GB607974@coredump.intra.peff.net>
In-Reply-To: <xmqqo7lam1ei.fsf@gitster.g>

On Tue, Jun 20, 2023 at 11:31:01AM -0700, Junio C Hamano wrote:

> In hindsight, I think (1) the first one should probably fail the
> "git log" process (not just the lazy fetch subprocess), and (2)
> there should be an explicit way, e.g. giving an empty string, to
> "clear" the list of .url accumulated so far.

Agreed. I think in general that any "list" config that is not doing last-one-wins should have let a blank entry reset the list.

> (2) may look something silly like this:

I know you said "silly", but in case anybody wants to follow up on this...

Show 12 quoted lines

> diff --git c/remote.c w/remote.c
> index 0764fca0db..ecc146856a 100644
> --- c/remote.c
> +++ w/remote.c
> @@ -64,12 +64,22 @@ static const char *alias_url(const char *url, struct rewrites *r)
>  
>  static void add_url(struct remote *remote, const char *url)
>  {
> +	if (!*url) {
> +		remote->url_nr = 0;
> +		return;
> +	}

...it probably needs to free() the entries from 0..url_nr, and then also free the unused empty-string "url".

But I think there's some questionable memory-ownership stuff going on here. The main config caller passes in a newly allocated string, handing over ownership to this list (despite the "const" parameter!). But in add_url_alias(), we take a single allocated "url" string and put it in both the "url" and "pushurl" lists. The former goes through alias_url() which sometimes allocates a new string and sometimes does not.

Probably both of these would be better off as strvecs or string_lists.

-Peff