Download raw body.
gotwebd: finishing the template refactoring
On Fri, Mar 10, 2023 at 06:54:45PM +0100, Omar Polo wrote:
> On 2023/02/21 20:19:32 +0100, Omar Polo <op@omarpolo.com> wrote:
> > Now that we have everything under the templates, I'd like to close the
> > whole refactoring. The idea is to have only one entrypoint,
> > gotweb_render_page() that renders the template (function) given as
> > argument.
> >
> > The idea is to simplify gotweb_process_request() a bit more, handling
> > all the cases inside the big switch. It's also a nice preparation
> > step towards an incoming diff to have gotwebd return a non-200 HTTP
> > code on failure.
> >
> > The only real difference is that gotweb_render_index() now skips
> > directory entries which fails to handle. I don't feel too bad about
> > it. Unlike other parts of got, since gotwebd speaks HTTP, we need to
> > decide up until which point we care about errors and generate page
> > output only after that. In this case, I decided to care about the
> > failure in opening /var/www/got/public and not about failure in
> > reading its content. The follow-up diff regarding the HTTP status
> > code will make this clearer.
> >
> > as usual, my instance is running with this applied.
>
> friendly ping :)
ok
>
> diffstat /home/op/w/gotd
> M gotwebd/config.c | 1+ 0-
> M gotwebd/fcgi.c | 1+ 0-
> M gotwebd/gotweb.c | 75+ 79-
> M gotwebd/gotwebd.c | 1+ 0-
> M gotwebd/gotwebd.h | 10+ 5-
> M gotwebd/pages.tmpl | 9+ 13-
> M gotwebd/parse.y | 3+ 1-
> M gotwebd/sockets.c | 1+ 0-
>
> 8 files changed, 101 insertions(+), 98 deletions(-)
>
> diff /home/op/w/gotd
> commit - 5add7f42e1397d136860680e1f0411db17b4f22c
> path + /home/op/w/gotd
> blob - ec88e1ef5acdb77906f2588283329e2a1f4eda21
> file + gotwebd/config.c
> --- gotwebd/config.c
> +++ gotwebd/config.c
> @@ -37,6 +37,7 @@
> #include <imsg.h>
>
> #include "got_opentemp.h"
> +#include "got_reference.h"
>
> #include "proc.h"
> #include "gotwebd.h"
> blob - ba1803b45941d2c4b83b31f232b8e7edfb92df63
> file + gotwebd/fcgi.c
> --- gotwebd/fcgi.c
> +++ gotwebd/fcgi.c
> @@ -33,6 +33,7 @@
> #include <unistd.h>
>
> #include "got_error.h"
> +#include "got_reference.h"
>
> #include "proc.h"
> #include "gotwebd.h"
> blob - 52aa037e4a8d734d17311aabbd7586d7e23d8fcd
> file + gotwebd/gotweb.c
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -85,7 +85,7 @@ static const struct got_error *gotweb_render_index(str
> char *);
> static const struct got_error *gotweb_assign_querystring(struct querystring **,
> char *, char *);
> -static const struct got_error *gotweb_render_index(struct request *);
> +static int gotweb_render_index(struct template *);
> static const struct got_error *gotweb_init_repo_dir(struct repo_dir **,
> const char *);
> static const struct got_error *gotweb_load_got_path(struct request *c,
> @@ -144,17 +144,12 @@ gotweb_process_request(struct request *c)
> gotweb_process_request(struct request *c)
> {
> const struct got_error *error = NULL, *error2 = NULL;
> - struct got_blob_object *blob = NULL;
> struct server *srv = NULL;
> struct querystring *qs = NULL;
> struct repo_dir *repo_dir = NULL;
> - struct got_reflist_head refs;
> - FILE *fp = NULL;
> uint8_t err[] = "gotwebd experienced an error: ";
> - int r, html = 0, fd = -1;
> + int r, html = 0;
>
> - TAILQ_INIT(&refs);
> -
> /* init the transport */
> error = gotweb_init_transport(&c->t);
> if (error) {
> @@ -217,7 +212,8 @@ gotweb_process_request(struct request *c)
> if (error)
> goto done;
>
> - error2 = got_open_blob_for_output(&blob, &fd, &binary, c);
> + error2 = got_open_blob_for_output(&c->t->blob, &c->t->fd,
> + &binary, c);
> if (error2)
> goto render;
>
> @@ -230,12 +226,12 @@ gotweb_process_request(struct request *c)
> goto done;
>
> for (;;) {
> - error = got_object_blob_read_block(&len, blob);
> + error = got_object_blob_read_block(&len, c->t->blob);
> if (error)
> goto done;
> if (len == 0)
> break;
> - buf = got_object_blob_get_read_buf(blob);
> + buf = got_object_blob_get_read_buf(c->t->blob);
> if (fcgi_gen_binary_response(c, buf, len) == -1)
> goto done;
> }
> @@ -259,7 +255,8 @@ gotweb_process_request(struct request *c)
> if (error)
> goto done;
>
> - error2 = got_open_blob_for_output(&blob, &fd, &binary, c);
> + error2 = got_open_blob_for_output(&c->t->blob, &c->t->fd,
> + &binary, c);
> if (error2)
> goto render;
> if (binary) {
> @@ -289,9 +286,6 @@ render:
> goto done;
> html = 1;
>
> - if (gotweb_render_header(c->tp) == -1)
> - goto err;
> -
> if (error2) {
> error = error2;
> goto err;
> @@ -304,15 +298,15 @@ render:
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
> - if (gotweb_render_blame(c->tp) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_blame) == -1)
> goto done;
> break;
> case BLOB:
> - if (gotweb_render_blob(c->tp, blob) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_blob) == -1)
> goto err;
> break;
> case BRIEFS:
> - if (gotweb_render_briefs(c->tp) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_briefs) == -1)
> goto err;
> break;
> case COMMITS:
> @@ -321,7 +315,7 @@ render:
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
> - if (gotweb_render_commits(c->tp) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_commits) == -1)
> goto err;
> break;
> case DIFF:
> @@ -330,23 +324,28 @@ render:
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
> - error = got_open_diff_for_output(&fp, &fd, c);
> + error = got_open_diff_for_output(&c->t->fp, &c->t->fd, c);
> if (error) {
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
> - if (gotweb_render_diff(c->tp, fp) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_diff) == -1)
> goto err;
> break;
> case INDEX:
> - error = gotweb_render_index(c);
> - if (error) {
> - log_warnx("%s: %s", __func__, error->msg);
> + c->t->nrepos = scandir(srv->repos_path, &c->t->repos, NULL,
> + alphasort);
> + if (c->t->nrepos == -1) {
> + c->t->repos = NULL;
> + error = got_error_from_errno2("scandir",
> + srv->repos_path);
> goto err;
> }
> + if (gotweb_render_page(c->tp, gotweb_render_index) == -1)
> + goto err;
> break;
> case SUMMARY:
> - error = got_ref_list(&refs, c->t->repo, "refs/heads",
> + error = got_ref_list(&c->t->refs, c->t->repo, "refs/heads",
> got_ref_cmp_by_name, NULL);
> if (error) {
> log_warnx("%s: got_ref_list: %s", __func__,
> @@ -361,7 +360,7 @@ render:
> goto err;
> }
> qs->action = SUMMARY;
> - if (gotweb_render_summary(c->tp, &refs) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_summary) == -1)
> goto done;
> break;
> case TAG:
> @@ -375,7 +374,7 @@ render:
> "bad commit id");
> goto err;
> }
> - if (gotweb_render_tag(c->tp) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_tag) == -1)
> goto done;
> break;
> case TAGS:
> @@ -384,7 +383,7 @@ render:
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
> - if (gotweb_render_tags(c->tp) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_tags) == -1)
> goto done;
> break;
> case TREE:
> @@ -393,7 +392,7 @@ render:
> log_warnx("%s: %s", __func__, error->msg);
> goto err;
> }
> - if (gotweb_render_tree(c->tp) == -1)
> + if (gotweb_render_page(c->tp, gotweb_render_tree) == -1)
> goto err;
> break;
> case ERR:
> @@ -421,21 +420,7 @@ done:
> if (html && fcgi_printf(c, "</div>\n") == -1)
> return;
> done:
> - if (blob)
> - got_object_blob_close(blob);
> - if (fp) {
> - error = got_gotweb_flushfile(fp, fd);
> - if (error)
> - log_warnx("%s: got_gotweb_flushfile failure: %s",
> - __func__, error->msg);
> - fd = -1;
> - }
> - if (fd != -1)
> - close(fd);
> - if (html && srv != NULL)
> - gotweb_render_footer(c->tp);
> -
> - got_ref_list_free(&refs);
> + return;
> }
>
> struct server *
> @@ -474,6 +459,7 @@ gotweb_init_transport(struct transport **t)
>
> TAILQ_INIT(&(*t)->repo_commits);
> TAILQ_INIT(&(*t)->repo_tags);
> + TAILQ_INIT(&(*t)->refs);
>
> (*t)->repo = NULL;
> (*t)->repo_dir = NULL;
> @@ -483,6 +469,8 @@ gotweb_init_transport(struct transport **t)
> (*t)->next_disp = 0;
> (*t)->prev_disp = 0;
>
> + (*t)->fd = -1;
> +
> return error;
> }
>
> @@ -770,9 +758,12 @@ gotweb_free_transport(struct transport *t)
> void
> gotweb_free_transport(struct transport *t)
> {
> + const struct got_error *err;
> struct repo_commit *rc = NULL, *trc = NULL;
> struct repo_tag *rt = NULL, *trt = NULL;
> + int i;
>
> + got_ref_list_free(&t->refs);
> TAILQ_FOREACH_SAFE(rc, &t->repo_commits, entry, trc) {
> TAILQ_REMOVE(&t->repo_commits, rc, entry);
> gotweb_free_repo_commit(rc);
> @@ -786,6 +777,22 @@ gotweb_free_transport(struct transport *t)
> free(t->more_id);
> free(t->next_id);
> free(t->prev_id);
> + if (t->blob)
> + got_object_blob_close(t->blob);
> + if (t->fp) {
> + err = got_gotweb_flushfile(t->fp, t->fd);
> + if (err)
> + log_warnx("%s: got_gotweb_flushfile failure: %s",
> + __func__, err->msg);
> + t->fd = -1;
> + }
> + if (t->fd != -1)
> + close(t->fd);
> + if (t->repos) {
> + for (i = 0; i < t->nrepos; ++i)
> + free(t->repos[i]);
> + free(t->repos);
> + }
> free(t);
> }
>
> @@ -848,30 +855,24 @@ static const struct got_error *
> }
> }
>
> -static const struct got_error *
> -gotweb_render_index(struct request *c)
> +static int
> +gotweb_render_index(struct template *tp)
> {
> const struct got_error *error = NULL;
> + struct request *c = tp->tp_arg;
> struct server *srv = c->srv;
> struct transport *t = c->t;
> struct querystring *qs = t->qs;
> struct repo_dir *repo_dir = NULL;
> - struct dirent **sd_dent = NULL;
> - unsigned int d_cnt, d_i, d_disp = 0;
> + struct dirent **sd_dent = t->repos;
> + unsigned int d_i, d_disp = 0;
> unsigned int d_skipped = 0;
> - int type;
> + int type, r;
>
> - d_cnt = scandir(srv->repos_path, &sd_dent, NULL, alphasort);
> - if (d_cnt == -1) {
> - sd_dent = NULL;
> - error = got_error_from_errno2("scandir", srv->repos_path);
> - goto done;
> - }
> -
> if (gotweb_render_repo_table_hdr(c->tp) == -1)
> - goto done;
> + return -1;
>
> - for (d_i = 0; d_i < d_cnt; d_i++) {
> + for (d_i = 0; d_i < t->nrepos; d_i++) {
> if (srv->max_repos > 0 && t->prev_disp == srv->max_repos)
> break;
>
> @@ -884,7 +885,7 @@ gotweb_render_index(struct request *c)
> error = got_path_dirent_type(&type, srv->repos_path,
> sd_dent[d_i]);
> if (error)
> - goto done;
> + continue;
> if (type != DT_DIR) {
> d_skipped++;
> continue;
> @@ -898,50 +899,45 @@ gotweb_render_index(struct request *c)
>
> error = gotweb_init_repo_dir(&repo_dir, sd_dent[d_i]->d_name);
> if (error)
> - goto done;
> + continue;
>
> error = gotweb_load_got_path(c, repo_dir);
> - if (error && error->code == GOT_ERR_NOT_GIT_REPO) {
> - error = NULL;
> + if (error && error->code == GOT_ERR_LONELY_PACKIDX) {
> + if (error->code != GOT_ERR_NOT_GIT_REPO)
> + log_warnx("%s: %s: %s", __func__,
> + sd_dent[d_i]->d_name, error->msg);
> gotweb_free_repo_dir(repo_dir);
> repo_dir = NULL;
> d_skipped++;
> continue;
> }
> - if (error && error->code != GOT_ERR_LONELY_PACKIDX)
> - goto done;
>
> d_disp++;
> t->prev_disp++;
>
> - if (gotweb_render_repo_fragment(c->tp, repo_dir) == -1)
> - goto done;
> -
> + r = gotweb_render_repo_fragment(c->tp, repo_dir);
> gotweb_free_repo_dir(repo_dir);
> - repo_dir = NULL;
> + if (r == -1)
> + return -1;
> +
> t->next_disp++;
> if (d_disp == srv->max_repos_display)
> break;
> }
> - t->repos_total = d_cnt - d_skipped;
> + t->repos_total = t->nrepos - d_skipped;
>
> if (srv->max_repos_display == 0)
> - goto done;
> + return 0;
> if (srv->max_repos > 0 && srv->max_repos < srv->max_repos_display)
> - goto done;
> + return 0;
> if (t->repos_total <= srv->max_repos ||
> t->repos_total <= srv->max_repos_display)
> - goto done;
> + return 0;
>
> if (gotweb_render_navs(c->tp) == -1)
> - goto done;
> -done:
> - if (sd_dent) {
> - for (d_i = 0; d_i < d_cnt; d_i++)
> - free(sd_dent[d_i]);
> - free(sd_dent);
> - }
> - return error;
> + return -1;
> +
> + return 0;
> }
>
> static inline int
> blob - cc0a4954a61a219297305919f2441a682fcb9a78
> file + gotwebd/gotwebd.c
> --- gotwebd/gotwebd.c
> +++ gotwebd/gotwebd.c
> @@ -41,6 +41,7 @@
> #include <util.h>
>
> #include "got_opentemp.h"
> +#include "got_reference.h"
>
> #include "proc.h"
> #include "gotwebd.h"
> blob - 98e9f879c0f9fc272de90ddc649d0c9aa2d88875
> file + gotwebd/gotwebd.h
> --- gotwebd/gotwebd.h
> +++ gotwebd/gotwebd.h
> @@ -186,6 +186,7 @@ struct transport {
> struct transport {
> TAILQ_HEAD(repo_commits_head, repo_commit) repo_commits;
> TAILQ_HEAD(repo_tags_head, repo_tag) repo_tags;
> + struct got_reflist_head refs;
> struct got_repository *repo;
> struct repo_dir *repo_dir;
> struct querystring *qs;
> @@ -196,6 +197,11 @@ struct transport {
> unsigned int next_disp;
> unsigned int prev_disp;
> unsigned int tag_count;
> + struct got_blob_object *blob;
> + int fd;
> + FILE *fp;
> + struct dirent **repos;
> + int nrepos;
> };
>
> enum socket_priv_fds {
> @@ -463,20 +469,19 @@ int gotweb_render_header(struct template *);
> void gotweb_free_transport(struct transport *);
>
> /* pages.tmpl */
> -int gotweb_render_header(struct template *);
> -int gotweb_render_footer(struct template *);
> +int gotweb_render_page(struct template *, int (*)(struct template *));
> int gotweb_render_repo_table_hdr(struct template *);
> int gotweb_render_repo_fragment(struct template *, struct repo_dir *);
> int gotweb_render_briefs(struct template *);
> int gotweb_render_navs(struct template *);
> int gotweb_render_commits(struct template *);
> -int gotweb_render_blob(struct template *, struct got_blob_object *);
> +int gotweb_render_blob(struct template *);
> int gotweb_render_tree(struct template *);
> int gotweb_render_tags(struct template *);
> int gotweb_render_tag(struct template *);
> -int gotweb_render_diff(struct template *, FILE *);
> +int gotweb_render_diff(struct template *);
> int gotweb_render_branches(struct template *, struct got_reflist_head *);
> -int gotweb_render_summary(struct template *, struct got_reflist_head *);
> +int gotweb_render_summary(struct template *);
> int gotweb_render_blame(struct template *);
> int gotweb_render_rss(struct template *);
>
> blob - bea0461c4b1286b162ce1d279fb93dd847ff4ab5
> file + gotwebd/pages.tmpl
> --- gotwebd/pages.tmpl
> +++ gotwebd/pages.tmpl
> @@ -54,7 +54,8 @@ static inline int rss_author(struct template *, char *
>
> !}
>
> -{{ define gotweb_render_header(struct template *tp) }}
> +{{ define gotweb_render_page(struct template *tp,
> + int (*body)(struct template *)) }}
> {!
> struct request *c = tp->tp_arg;
> struct server *srv = c->srv;
> @@ -110,13 +111,7 @@ static inline int rss_author(struct template *, char *
> </div>
> </div>
> <div id="content">
> -{{ end }}
> -
> -{{ define gotweb_render_footer(struct template *tp) }}
> -{!
> - struct request *c = tp->tp_arg;
> - struct server *srv = c->srv;
> -!}
> + {{ render body(tp) }}
> <div id="site_owner_wrapper">
> <div id="site_owner">
> {{ if srv->show_site_owner }}
> @@ -433,11 +428,11 @@ static inline int rss_author(struct template *, char *
> </div>
> {{ end }}
>
> -{{ define gotweb_render_blob(struct template *tp,
> - struct got_blob_object *blob) }}
> +{{ define gotweb_render_blob(struct template *tp) }}
> {!
> struct request *c = tp->tp_arg;
> struct transport *t = c->t;
> + struct got_blob_object *blob = t->blob;
> struct repo_commit *rc = TAILQ_FIRST(&t->repo_commits);
> !}
> <div id="blob_title_wrapper">
> @@ -711,10 +706,11 @@ static inline int rss_author(struct template *, char *
> </div>
> {{ end }}
>
> -{{ define gotweb_render_diff(struct template *tp, FILE *fp) }}
> +{{ define gotweb_render_diff(struct template *tp) }}
> {!
> struct request *c = tp->tp_arg;
> struct transport *t = c->t;
> + FILE *fp = t->fp;
> struct repo_commit *rc = TAILQ_FIRST(&t->repo_commits);
> char *line = NULL;
> size_t linesize = 0;
> @@ -851,12 +847,12 @@ static inline int rss_author(struct template *, char *
> </div>
> {{ end }}
>
> -{{ define gotweb_render_summary(struct template *tp,
> - struct got_reflist_head *refs) }}
> +{{ define gotweb_render_summary(struct template *tp) }}
> {!
> struct request *c = tp->tp_arg;
> struct server *srv = c->srv;
> struct transport *t = c->t;
> + struct got_reflist_head *refs = &t->refs;
> !}
> <div id="summary_wrapper">
> {{ if srv->show_repo_description }}
> blob - cc7ccf7ef44f2f9a5f354959abcc57ee681162bf
> file + gotwebd/parse.y
> --- gotwebd/parse.y
> +++ gotwebd/parse.y
> @@ -47,9 +47,11 @@
> #include <syslog.h>
> #include <unistd.h>
>
> +#include "got_sockaddr.h"
> +#include "got_reference.h"
> +
> #include "proc.h"
> #include "gotwebd.h"
> -#include "got_sockaddr.h"
>
> TAILQ_HEAD(files, file) files = TAILQ_HEAD_INITIALIZER(files);
> static struct file {
> blob - ecb88027e072a125a309c08b985733a77ab2a26d
> file + gotwebd/sockets.c
> --- gotwebd/sockets.c
> +++ gotwebd/sockets.c
> @@ -51,6 +51,7 @@
>
> #include "got_error.h"
> #include "got_opentemp.h"
> +#include "got_reference.h"
> #include "got_repository.h"
>
> #include "proc.h"
>
--
Tracey Emery
gotwebd: finishing the template refactoring