"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
unbreak gotwebd tag listing
To:
gameoftrees@openbsd.org
Date:
Sat, 07 Sep 2024 10:34:08 +0200

Download raw body.

Thread
I broke this while fixing the links in the README.  The underlying
problem is that got_get_repo_tags() cares whether qs->commit is set.  So
the fix is to set it after calling got_get_repo_tags().

(imho it's unfortunate that we have so much code in got_operations.c that
relies on the content of the querystring, but that's not an excuse for
not having checked properly when doing the change in the first place.)

While here, I couldn't help but also do another change which is a no-op:
don't duplicate the tag skipping logic and remove tags from the queue in
got_operations instead.

ok?


-----------------------------------------------
commit a489eb57352a3f5586e9363358b1c406ea850927
from: Omar Polo <op@omarpolo.com>
date: Sat Sep  7 08:26:03 2024 UTC
 
 gotwebd: centralize the skip tag logic
 
 No need to do it twice, just make sure the tailq has the correct
 entries where it's generated, so in the render code we don't have
 to duplicate the logic.
 
 M  gotwebd/got_operations.c  |  4+  8-
 M  gotwebd/gotweb.c          |  1+  1-
 M  gotwebd/gotwebd.h         |  0+  1-
 M  gotwebd/pages.tmpl        |  2+  9-

4 files changed, 7 insertions(+), 19 deletions(-)

diff 97ddbd299ba74881d7fda4bc0588da7b494ed1af a489eb57352a3f5586e9363358b1c406ea850927
commit - 97ddbd299ba74881d7fda4bc0588da7b494ed1af
commit + a489eb57352a3f5586e9363358b1c406ea850927
blob - 542f0c0f02f83e44d87ede48a28af59748768530
blob + f4a93f9f22c2a2ae93a2bf822794bade8af45b6c
--- gotwebd/got_operations.c
+++ gotwebd/got_operations.c
@@ -510,10 +510,6 @@ got_get_repo_tags(struct request *c, size_t limit)
 	if (limit == 1)
 		chk_multi = 0;
 
-	/*
-	 * XXX: again, see previous message about caching
-	 */
-
 	TAILQ_FOREACH(re, &refs, entry) {
 		struct repo_tag *new_repo_tag = NULL;
 		error = got_init_repo_tag(&new_repo_tag);
@@ -579,13 +575,13 @@ got_get_repo_tags(struct request *c, size_t limit)
 			goto done;
 
 		if (commit_found == 0 && qs->commit != NULL &&
-		    strncmp(id_str, qs->commit, strlen(id_str)) != 0)
+		    strncmp(id_str, qs->commit, strlen(id_str)) != 0) {
+			TAILQ_REMOVE(&t->repo_tags, new_repo_tag, entry);
+			gotweb_free_repo_tag(new_repo_tag);
 			continue;
-		else
+		} else
 			commit_found = 1;
 
-		t->tag_count++;
-
 		/*
 		 * check for one more commit before breaking,
 		 * so we know whether to navigate through briefs
blob - ff01852b75f3531996e1e87a3c719dbe369b65d9
blob + fb2894f0a49e2ecb0e12069bb4354470e9c06e32
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -381,7 +381,7 @@ gotweb_process_request(struct request *c)
 			log_warnx("%s: %s", __func__, error->msg);
 			goto err;
 		}
-		if (c->t->tag_count == 0) {
+		if (TAILQ_EMPTY(&c->t->repo_tags)) {
 			error = got_error_msg(GOT_ERR_BAD_OBJ_ID,
 			    "bad commit id");
 			goto err;
blob - c0047da16df32ddb81665645f2025c12d9cf8250
blob + 59fef09083a448dfb1173969b2c5b5426af439a8
--- gotwebd/gotwebd.h
+++ gotwebd/gotwebd.h
@@ -202,7 +202,6 @@ struct transport {
 	unsigned int		 repos_total;
 	unsigned int		 next_disp;
 	unsigned int		 prev_disp;
-	unsigned int		 tag_count;
 	const struct got_error	*error;
 	struct got_blob_object	*blob;
 	int			 fd;
blob - 6638943209aae86d6f98b5db64e75ad966980813
blob + 9bc9e41186b83a537530e7cfb4e386bdc1bcfb4d
--- gotwebd/pages.tmpl
+++ gotwebd/pages.tmpl
@@ -815,26 +815,19 @@ nextsep(char *s, char **t)
 {!
 	struct request		*c = tp->tp_arg;
 	struct transport	*t = c->t;
-	struct querystring	*qs = t->qs;
 	struct repo_tag		*rt;
-	int			 commit_found;
-
-	commit_found = qs->commit == NULL;
 !}
 <header class='subtitle'>
   <h2>Tags</h2>
 </header>
 <div id="tags_content">
-  {{ if t->tag_count == 0 }}
+  {{ if TAILQ_EMPTY(&t->repo_tags) }}
     <div id="err_content">
       This repository contains no tags
     </div>
   {{ else }}
     {{ tailq-foreach rt &t->repo_tags entry }}
-      {{ if commit_found || !strcmp(qs->commit, rt->commit_id) }}
-        {! commit_found = 1; !}
-        {{ render tag_item(tp, rt) }}
-      {{ end }}
+      {{ render tag_item(tp, rt) }}
     {{ end }}
     {{ render gotweb_render_more(tp, TAGS) }}
   {{ end }}

-----------------------------------------------
commit 4c4f3acfb2df17ea05d5a0b3abd23d26492ceea2 (main)
from: Omar Polo <op@omarpolo.com>
date: Sat Sep  7 08:30:02 2024 UTC
 
 gotwebd: unbreak tags rendering
 
 if qs->commit is set got_get_repo_tags interprets it as an index
 for the tags, i.e. to skip tags until one with that id is found.
 So we can't set qs->commit (intended to unbreak the README links)
 before it, we have to do it afterwards.
 
 M  gotwebd/gotweb.c  |  8+  8-

1 file changed, 8 insertions(+), 8 deletions(-)

diff a489eb57352a3f5586e9363358b1c406ea850927 4c4f3acfb2df17ea05d5a0b3abd23d26492ceea2
commit - a489eb57352a3f5586e9363358b1c406ea850927
commit + 4c4f3acfb2df17ea05d5a0b3abd23d26492ceea2
blob - fb2894f0a49e2ecb0e12069bb4354470e9c06e32
blob + cc06209b1e8c52eb8460afa45b05725c2650d9e2
--- gotwebd/gotweb.c
+++ gotwebd/gotweb.c
@@ -354,6 +354,14 @@ gotweb_process_request(struct request *c)
 		error = got_get_repo_commits(c, srv->summary_commits_display);
 		if (error)
 			goto err;
+		qs->action = TAGS;
+		error = got_get_repo_tags(c, srv->summary_tags_display);
+		if (error) {
+			log_warnx("%s: got_get_repo_tags: %s", __func__,
+			    error->msg);
+			goto err;
+		}
+		qs->action = SUMMARY;
 		commit = TAILQ_FIRST(&c->t->repo_commits);
 		if (commit && qs->commit == NULL) {
 			qs->commit = strdup(commit->commit_id);
@@ -363,14 +371,6 @@ gotweb_process_request(struct request *c)
 				goto err;
 			}
 		}
-		qs->action = TAGS;
-		error = got_get_repo_tags(c, srv->summary_tags_display);
-		if (error) {
-			log_warnx("%s: got_get_repo_tags: %s", __func__,
-			    error->msg);
-			goto err;
-		}
-		qs->action = SUMMARY;
 		if (gotweb_reply(c, 200, "text/html", NULL) == -1)
 			return;
 		gotweb_render_page(c->tp, gotweb_render_summary);