From: Mark Jamsek <mark@jamsek.com>
Subject: Re: gotwebd got_get_repo_age simplification
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Thu, 21 Nov 2024 20:56:50 +1100

Stefan Sperling <stsp@stsp.name> wrote:
> Instead of sorting refs by name and then hunting for the youngest
> commits in that list, obtain an appropriately sorted list via
> got_ref_cmp_by_commit_timestamp_descending and then pick the first
> ref from this list.
> 
> And in the case were we already know which ref to check, just open
> this ref directly instead of listing all the refs.
> 
> Easier to read and should improve performance in both cases.
> 
> This diff is hard to read. I'd recommend applying it and reviewing
> the result.
> 
> ok?

Reads much nicer! But this is giving me different results for a fossil
repo converted to git that I moved into my /got/public repos a couple
days ago for testing the test harness. I'm not sure if replacing
got_object_commit_get_committer_time() with got_ref_get_mtime() is right
as they can produce different results.

Before this change, various branches are correctly listed as X years,
Y months, or Z days ago, but with the diff they're all showing as 2 days
ago, which would've been when I copied the repo into /got/public.

> diff /home/stsp/src/got
> commit - ece731b025b35fd112ea1faebfabb163b61aacbe
> path + /home/stsp/src/got
> blob - f4a93f9f22c2a2ae93a2bf822794bade8af45b6c
> file + gotwebd/got_operations.c
> --- gotwebd/got_operations.c
> +++ gotwebd/got_operations.c
> @@ -116,60 +116,46 @@ got_get_repo_owner(char **owner, struct request *c)
>  	return NULL;
>  }
>  
> +/*
> + * Find the youngest branch tip in the repository, or the age of
> + * a specific branch tip if a name was provided by the caller.
> + */
>  const struct got_error *
>  got_get_repo_age(time_t *repo_age, struct request *c, const char *refname)
>  {
>  	const struct got_error *error = NULL;
>  	struct transport *t = c->t;
>  	struct got_repository *repo = t->repo;
> -	struct got_commit_object *commit = NULL;
> -	struct got_reflist_head refs;
> -	struct got_reflist_entry *re;
> -	time_t committer_time = 0, cmp_time = 0;
>  
> -	TAILQ_INIT(&refs);
> -
>  	*repo_age = 0;
>  
> -	error = got_ref_list(&refs, repo, "refs/heads",
> -	    got_ref_cmp_by_name, NULL);
> -	if (error)
> -		goto done;
> +	if (refname) {
> +		struct got_reference *ref;
> +		
> +		error = got_ref_open(&ref, repo, refname, 0);
> +		if (error)
> +			return error;
>  
> -	/*
> -	 * Find the youngest branch tip in the repository, or the age of
> -	 * the a specific branch tip if a name was provided by the caller.
> -	 */
> -	TAILQ_FOREACH(re, &refs, entry) {
> -		struct got_object_id *id = NULL;
> +		*repo_age = got_ref_get_mtime(ref);
> +		got_ref_close(ref);
> +	} else {
> +		struct got_reflist_head refs;
> +		struct got_reflist_entry *re;
>  
> -		if (refname && strcmp(got_ref_get_name(re->ref), refname) != 0)
> -			continue;
> +		TAILQ_INIT(&refs);
>  
> -		error = got_ref_resolve(&id, repo, re->ref);
> +		error = got_ref_list(&refs, repo, "refs/heads",
> +		    got_ref_cmp_by_commit_timestamp_descending, repo);
>  		if (error)
> -			goto done;
> +			return error;
>  
> -		error = got_object_open_as_commit(&commit, repo, id);
> -		free(id);
> -		if (error)
> -			goto done;
> -
> -		committer_time =
> -		    got_object_commit_get_committer_time(commit);
> -		got_object_commit_close(commit);
> -		if (cmp_time < committer_time)
> -			cmp_time = committer_time;
> -
> -		if (refname)
> -			break;
> +		re = TAILQ_FIRST(&refs);
> +		if (re)
> +			*repo_age = got_ref_get_mtime(re->ref);
> +		got_ref_list_free(&refs);
>  	}
>  
> -	if (cmp_time != 0)
> -		*repo_age = cmp_time;
> - done:
> -	got_ref_list_free(&refs);
> -	return error;
> +	return NULL;
>  }
>  
>  static const struct got_error *


-- 
Mark Jamsek <https://bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68