On Thu, Nov 21, 2024 at 08:56:50PM +1100, Mark Jamsek wrote:
> 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.

Right, the reference's mtime is an on-disk timestamp of the ref file,
which does not correspond to the commit object's timestamp.

Fixed version below.

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,59 +116,82 @@ got_get_repo_owner(char **owner, struct request *c)
 	return NULL;
 }
 
+static const struct got_error *
+get_ref_commit_timestamp(time_t *timestamp,
+    struct got_reference *ref, struct got_repository *repo)
+{
+	const struct got_error *error;
+	struct got_object_id *id = NULL;
+	int obj_type;
+	struct got_commit_object *commit = NULL;
+
+	/* We might have a cached timestamp available. */
+	*timestamp = got_ref_get_cached_committer_time(ref);
+	if (*timestamp != 0)
+		return NULL;
+
+	error = got_ref_resolve(&id, repo, ref);
+	if (error)
+		return error;
+
+	error = got_object_get_type(&obj_type, repo, id);
+	if (error || obj_type != GOT_OBJ_TYPE_COMMIT)
+		goto done;
+
+	error = got_object_open_as_commit(&commit, repo, id);
+	if (error)	
+		goto done;
+
+	*timestamp = got_object_commit_get_committer_time(commit);
+done:
+	free(id);
+	if (commit)
+		got_object_commit_close(commit);
+
+	return error;
+}
+
+/*
+ * 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;
+		error = get_ref_commit_timestamp(repo_age, ref, repo);
+		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) {
+			error = get_ref_commit_timestamp(repo_age,
+			    re->ref, repo);
+		}
+		got_ref_list_free(&refs);
 	}
 
-	if (cmp_time != 0)
-		*repo_age = cmp_time;
- done:
-	got_ref_list_free(&refs);
 	return error;
 }
 
blob - 0e987a302c6c21819424c0bbf8c957b06744abd8
file + include/got_reference.h
--- include/got_reference.h
+++ include/got_reference.h
@@ -66,6 +66,17 @@ const char *got_ref_get_symref_target(struct got_refer
 time_t got_ref_get_mtime(struct got_reference *);
 
 /*
+ * Get the modification timestamp of the commit pointed at by the
+ * given reference. Will return zero if the commit timestamp is
+ * unknown or if the reference does not point at a commit.
+ *
+ * The cached timestamp will be known after got_ref_list() was called
+ * with either got_ref_cmp_by_commit_timestamp_descending() or
+ * with got_ref_cmp_tags().
+ */
+time_t got_ref_get_cached_committer_time(struct got_reference *);
+
+/*
  * Create a duplicate copy of a reference.
  * The caller must dispose of this copy with got_ref_close().
  */
blob - 3b728f8e2874054c0698accbbe24e51d69d93929
file + lib/reference.c
--- lib/reference.c
+++ lib/reference.c
@@ -644,6 +644,12 @@ got_ref_get_mtime(struct got_reference *ref)
 	return ref->mtime;
 }
 
+time_t
+got_ref_get_cached_committer_time(struct got_reference *ref)
+{
+	return ref->committer_time;
+}
+
 const struct got_error *
 got_ref_cmp_by_name(void *arg, int *cmp, struct got_reference *re1,
     struct got_reference* re2)