From: Mark Jamsek Subject: Re: gotwebd got_get_repo_age simplification To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Thu, 21 Nov 2024 21:35:47 +1100 Stefan Sperling wrote: > On Thu, Nov 21, 2024 at 08:56:50PM +1100, Mark Jamsek wrote: > > Stefan Sperling 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. Great! Still reads much nicer and this indeed works for me, thanks :) ok with or without the stray tabs (annotated inline) removed. > 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) trailing tab here^ > + 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; > + \t \t two tabs here > + 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) -- Mark Jamsek GPG: F2FF 13DE 6A06 C471 CA80 E6E2 2930 DC66 86EE CF68