From: Tracey Emery <tracey@traceyemery.net>
Subject: Re: gotwebd: templateify gotweb_render_tree
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Thu, 5 Jan 2023 07:41:05 -0700

On Thu, Jan 05, 2023 at 12:41:25PM +0100, Omar Polo wrote:
> shouldn't introduce any difference with the current output, links for
> the BLOBRAW page will be added as follow-up.
> 
> my instance is running with latest main + this applied.
> 
> I don't like too much changing the functions in got_operations.c from
> `const struct got_error *' to int to please the template, but for the
> moment is the quickest way to move forward.  Once everything is

Yeah, let's do what it takes to get forward movement to finish
templates.

> running with the templates I'd like to clean this a bit more so that
> we first acquire the resources needed (i.e. open the tree) and only
> then output.  This way the only failures in the template code path
> will be from ENOMEM and clients closing the connection before we've
> finished.  (see for example how BLOB is handled, that's the idea.)
> 
> ok?

ok

> 
> diff /home/op/w/got
> commit - 5eeb13e79b508eacc001662d742ea375c069d7f7
> path + /home/op/w/got
> blob - 9d187ce1e16a66d113377842fd55ccbd016fd2e3
> file + gotwebd/got_operations.c
> --- gotwebd/got_operations.c
> +++ gotwebd/got_operations.c
> @@ -793,8 +793,9 @@ const struct got_error *
>  	return error;
>  }
>  
> -const struct got_error *
> -got_output_repo_tree(struct request *c)
> +int
> +got_output_repo_tree(struct request *c,
> +    int (*cb)(struct template *, struct got_tree_entry *))
>  {
>  	const struct got_error *error = NULL;
>  	struct transport *t = c->t;
> @@ -805,10 +806,10 @@ got_output_repo_tree(struct request *c)
>  	struct got_object_id *tree_id = NULL, *commit_id = NULL;
>  	struct got_reflist_head refs;
>  	struct got_tree_object *tree = NULL;
> +	struct got_tree_entry *te;
>  	struct repo_dir *repo_dir = t->repo_dir;
> -	const char *name, *folder;
>  	char *escaped_name = NULL, *path = NULL;
> -	int nentries, i, r;
> +	int nentries, i;
>  
>  	TAILQ_INIT(&refs);
>  
> @@ -845,106 +846,10 @@ got_output_repo_tree(struct request *c)
>  
>  	nentries = got_object_tree_get_nentries(tree);
>  
> -	folder = qs->folder ? qs->folder : "";
> -
>  	for (i = 0; i < nentries; i++) {
> -		const char *modestr;
> -		struct got_tree_entry *te;
> -		mode_t mode;
> -
>  		te = got_object_tree_get_entry(tree, i);
> -
> -		mode = got_tree_entry_get_mode(te);
> -		if (got_object_tree_entry_is_submodule(te))
> -			modestr = "$";
> -		else if (S_ISLNK(mode))
> -			modestr = "@";
> -		else if (S_ISDIR(mode))
> -			modestr = "/";
> -		else if (mode & S_IXUSR)
> -			modestr = "*";
> -		else
> -			modestr = "";
> -
> -		name = got_tree_entry_get_name(te);
> -		error = gotweb_escape_html(&escaped_name, name);
> -		if (error)
> -			goto done;
> -
> -		if (S_ISDIR(mode)) {
> -			struct gotweb_url url = {
> -				.index_page = -1,
> -				.page = -1,
> -				.action = TREE,
> -				.commit = rc->commit_id,
> -				.path = qs->path,
> -				/* `folder' is filled later */
> -			};
> -			char *path = NULL;
> -
> -			if (fcgi_printf(c,"<div class='tree_wrapper'>\n"
> -			    "<div class='tree_line'>") == -1)
> -				goto done;
> -
> -			if (asprintf(&path, "%s/%s", folder, name) == -1) {
> -				error = got_error_from_errno("asprintf");
> -				goto done;
> -			}
> -			url.folder = path;
> -			r = gotweb_link(c, &url, "%s%s", escaped_name,
> -			    modestr);
> -			free(path);
> -			if (r == -1)
> -				goto done;
> -
> -			if (fcgi_printf(c, "</div>\n" /* .tree_line */
> -			    "<div class='tree_line_blank'>&nbsp;</div>\n"
> -			    "</div>\n") == -1)
> -				goto done;
> -		} else {
> -			struct gotweb_url url = {
> -				.index_page = -1,
> -				.page = -1,
> -				.path = qs->path,
> -				.commit = rc->commit_id,
> -				.folder = folder,
> -				.file = name,
> -			};
> -
> -			if (fcgi_printf(c, "<div class='tree_wrapper'>\n"
> -			    "<div class='tree_line'>") == -1)
> -				goto done;
> -
> -			url.action = BLOB;
> -			r = gotweb_link(c, &url, "%s%s", escaped_name,
> -			    modestr);
> -			if (r == -1)
> -				goto done;
> -
> -			if (fcgi_printf(c, "</div>\n" /* .tree_line */
> -			    "<div class='tree_line_blank'>") == -1)
> -				goto done;
> -
> -			url.action = COMMITS;
> -			r = gotweb_link(c, &url, "commits");
> -			if (r == -1)
> -				goto done;
> -
> -			if (fcgi_printf(c, " | ") == -1)
> -				goto done;
> -
> -			url.action = BLAME;
> -			r = gotweb_link(c, &url, "blame");
> -			if (r == -1)
> -				goto done;
> -
> -			if (fcgi_printf(c,
> -			    "</div>\n"		/* .tree_line_blank */
> -			    "</div>\n") == -1)	/* .tree_wrapper */
> -				goto done;
> -		}
> -		free(escaped_name);
> -		escaped_name = NULL;
> +		if (cb(c->tp, te) == -1)
> +			break;
>  	}
>  done:
>  	free(escaped_name);
> @@ -956,7 +861,11 @@ done:
>  		got_object_tree_close(tree);
>  	free(commit_id);
>  	free(tree_id);
> -	return error;
> +	if (error) {
> +		log_warnx("%s: %s", __func__, error->msg);
> +		return -1;
> +	}
> +	return 0;
>  }
>  
>  const struct got_error *
> blob - 37d8624ec792d91e8f7438d5cc3682be3a54d179
> file + gotwebd/gotweb.c
> --- gotwebd/gotweb.c
> +++ gotwebd/gotweb.c
> @@ -98,7 +98,6 @@ static const struct got_error *gotweb_render_tree(stru
>  static const struct got_error *gotweb_render_summary(struct request *);
>  static const struct got_error *gotweb_render_tag(struct request *);
>  static const struct got_error *gotweb_render_tags(struct request *);
> -static const struct got_error *gotweb_render_tree(struct request *);
>  static const struct got_error *gotweb_render_branches(struct request *);
>  
>  static void gotweb_free_querystring(struct querystring *);
> @@ -306,11 +305,13 @@ render:
>  		}
>  		break;
>  	case TREE:
> -		error = gotweb_render_tree(c);
> +		error = got_get_repo_commits(c, 1);
>  		if (error) {
>  			log_warnx("%s: %s", __func__, error->msg);
>  			goto err;
>  		}
> +		if (gotweb_render_tree(c->tp) == -1)
> +			goto err;
>  		break;
>  	case ERR:
>  	default:
> @@ -1120,63 +1121,6 @@ gotweb_render_tree(struct request *c)
>  }
>  
>  static const struct got_error *
> -gotweb_render_tree(struct request *c)
> -{
> -	const struct got_error *error = NULL;
> -	struct transport *t = c->t;
> -	struct repo_commit *rc = NULL;
> -	char *age = NULL, *msg = NULL;
> -	int r;
> -
> -	error = got_get_repo_commits(c, 1);
> -	if (error)
> -		return error;
> -
> -	rc = TAILQ_FIRST(&t->repo_commits);
> -
> -	error = gotweb_get_time_str(&age, rc->committer_time, TM_LONG);
> -	if (error)
> -		goto done;
> -
> -	error = gotweb_escape_html(&msg, rc->commit_msg);
> -	if (error)
> -		goto done;
> -
> -	r = fcgi_printf(c, "<div id='tree_title_wrapper'>\n"
> -	    "<div id='tree_title'>Tree</div>\n"
> -	    "</div>\n"		/* #tree_title_wrapper */
> -	    "<div id='tree_content'>\n"
> -	    "<div id='tree_header_wrapper'>\n"
> -	    "<div id='tree_header'>\n"
> -	    "<div id='header_tree_title'>Tree:</div>\n"
> -	    "<div id='header_tree'>%s</div>\n"
> -	    "<div class='header_age_title'>Date:</div>\n"
> -	    "<div class='header_age'>%s</div>\n"
> -	    "<div id='header_commit_msg_title'>Message:</div>\n"
> -	    "<div id='header_commit_msg'>%s</div>\n"
> -	    "</div>\n"		/* #tree_header */
> -	    "</div>\n"		/* #tree_header_wrapper */
> -	    "<div class='dotted_line'></div>\n"
> -	    "<div id='tree'>\n",
> -	    rc->tree_id,
> -	    age,
> -	    msg);
> -	if (r == -1)
> -		goto done;
> -
> -	error = got_output_repo_tree(c);
> -	if (error)
> -		goto done;
> -
> -	fcgi_printf(c, "</div>\n"); /* #tree */
> -	fcgi_printf(c, "</div>\n"); /* #tree_content */
> -done:
> -	free(age);
> -	free(msg);
> -	return error;
> -}
> -
> -static const struct got_error *
>  gotweb_render_diff(struct request *c)
>  {
>  	const struct got_error *error = NULL;
> blob - e4e23bf9c198a25c2338660068796cd385a8498c
> file + gotwebd/gotwebd.h
> --- gotwebd/gotwebd.h
> +++ gotwebd/gotwebd.h
> @@ -117,6 +117,7 @@ struct got_blob_object;
>  
>  /* Forward declaration */
>  struct got_blob_object;
> +struct got_tree_entry;
>  
>  enum imsg_type {
>  	IMSG_CFG_SRV = IMSG_PROC_MAX,
> @@ -467,6 +468,7 @@ int	gotweb_render_rss(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_tree(struct template *);
>  int	gotweb_render_rss(struct template *);
>  
>  /* parse.y */
> @@ -495,7 +497,8 @@ const struct got_error *got_output_repo_tree(struct re
>  const struct got_error *got_get_repo_tags(struct request *, int);
>  const struct got_error *got_get_repo_heads(struct request *);
>  const struct got_error *got_output_repo_diff(struct request *);
> -const struct got_error *got_output_repo_tree(struct request *);
> +int got_output_repo_tree(struct request *,
> +    int (*cb)(struct template *, struct got_tree_entry *));
>  const struct got_error *got_open_blob_for_output(struct got_blob_object **,
>      int *, int *, struct request *);
>  const struct got_error *got_output_file_blob(struct request *);
> blob - 5dac668e8a6c3b5de586bc8484746a6397e29dfb
> file + gotwebd/pages.tmpl
> --- gotwebd/pages.tmpl
> +++ gotwebd/pages.tmpl
> @@ -18,20 +18,26 @@
>  
>  #include <sys/types.h>
>  #include <sys/queue.h>
> +#include <sys/stat.h>
>  
>  #include <ctype.h>
>  #include <event.h>
>  #include <stdint.h>
> +#include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <sha1.h>
>  #include <imsg.h>
>  
> +#include "got_object.h"
> +
>  #include "proc.h"
>  
>  #include "gotwebd.h"
>  #include "tmpl.h"
>  
>  static int gotweb_render_blob_line(struct template *, const char *, size_t);
> +static int gotweb_render_tree_item(struct template *, struct got_tree_entry *);
>  
>  static inline int rss_tag_item(struct template *, struct repo_tag *);
>  static inline int rss_author(struct template *, char *);
> @@ -456,6 +462,112 @@ gotweb_render_age(struct template *tp, time_t time, in
>  </div>
>  {{ end }}
>  
> +{{ define gotweb_render_tree(struct template *tp) }}
> +{!
> +	struct request		*c = tp->tp_arg;
> +	struct transport	*t = c->t;
> +	struct repo_commit	*rc = TAILQ_FIRST(&t->repo_commits);
> +!}
> +<div id="tree_title_wrapper">
> +  <div id="tree_title">Tree</div>
> +</div>
> +<div id="tree_content">
> +  <div id="tree_header_wrapper">
> +    <div id="tree_header">
> +      <div id="header_tree_title">Tree:</div>
> +      <div id="header_tree">{{ rc->tree_id }}</div>
> +      <div class="header_age_title">Date:</div>
> +      <div class="header_age">
> +        {{ render gotweb_render_age(tp, rc->committer_time, TM_LONG) }}
> +      </div>
> +      <div id="header_commit_msg_title">Message:</div>
> +      <div id="header_commit_msg">{{ rc->commit_msg }}</div>
> +    </div>
> +  </div>
> +  <div class="dotted_line"></div>
> +  <div id="tree">
> +    {{ render got_output_repo_tree(c, gotweb_render_tree_item) }}
> +  </div>
> +</div>
> +{{ end }}
> +
> +{{ define gotweb_render_tree_item(struct template *tp,
> +    struct got_tree_entry *te) }}
> +{!
> +	struct request		*c = tp->tp_arg;
> +	struct transport	*t = c->t;
> +	struct querystring	*qs = t->qs;
> +	struct repo_commit	*rc = TAILQ_FIRST(&t->repo_commits);
> +	const char		*modestr = "";
> +	const char		*name;
> +	const char		*folder;
> +	char			*dir = NULL;
> +	mode_t			 mode;
> +	struct gotweb_url	 url = {
> +	       .index_page = -1,
> +	       .page = -1,
> +	       .commit = rc->commit_id,
> +	       .path = qs->path,
> +	};
> +
> +	name = got_tree_entry_get_name(te);
> +	mode = got_tree_entry_get_mode(te);
> +
> +	folder = qs->folder ? qs->folder : "";
> +	if (S_ISDIR(mode)) {
> +		if (asprintf(&dir, "%s/%s", folder, name) == -1)
> +			return (-1);
> +
> +		url.action = TREE;
> +		url.folder = dir;
> +	} else {
> +		url.action = BLOB;
> +		url.folder = folder;
> +		url.file = name;
> +	}
> +
> +	if (got_object_tree_entry_is_submodule(te))
> +		modestr = "$";
> +	else if (S_ISLNK(mode))
> +		modestr = "@";
> +	else if (S_ISDIR(mode))
> +		modestr = "/";
> +	else if (mode & S_IXUSR)
> +		modestr = "*";
> +!}
> +<div class="tree_wrapper">
> +  {{ if S_ISDIR(mode) }}
> +    <div class="tree_line">
> +      <a href="{{ render gotweb_render_url(c, &url) }}">
> +        {{ name }}{{ modestr }}
> +      </a>
> +    </div>
> +    <div class="tree_line_blank">&nbsp;</div>
> +  {{ else }}
> +    <div class="tree_line">
> +      <a href="{{ render gotweb_render_url(c, &url) }}">
> +        {{ name }}{{ modestr }}
> +      </a>
> +    </div>
> +    <div class="tree_line_blank">
> +      {! url.action = COMMITS; !}
> +      <a href="{{ render gotweb_render_url(c, &url) }}">
> +        commits
> +      </a>
> +      {{ " | " }}
> +      {! url.action = BLAME; !}
> +      <a href="{{ render gotweb_render_url(c, &url) }}">
> +        blame
> +      </a>
> +    </div>
> +  {{ end }}
> +</div>
> +{{ finally }}
> +{!
> +	free(dir);
> +!}
> +{{ end }}
> +
>  {{ define gotweb_render_rss(struct template *tp) }}
>  {!
>  	struct request		*c = tp->tp_arg;
> 

-- 

Tracey Emery