Download raw body.
gotwebd: finishing the template refactoring
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 :)
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"
gotwebd: finishing the template refactoring