On Fri, Jul 07, 2023 at 05:19:40PM +0200, Omar Polo wrote:
> > +					 * To give admins a chance to create
> > +					 * missing repositories at run-time
> > +					 * we only warn about ENOENT here.
> 
> not a problem of this diff, but this comment is wrong.
> 
> 	% cat >test.conf
> 	repository foo {
> 	        path '/oops.git'
> 	        permit rw op
> 	}
> 	% doas /usr/local/sbin/gotd -d -f ./test.conf
> 	gotd: ./test.conf:2: realpath /oops.git: No such file or directory
> 	% echo $?
> 	1
 
> (yyerror() bumps file->errors, which is then inspected by
> parse_config().)

Oh indeed. I will take a look at this and add test coverage because
this is not the first time that something doesn't work as intended.

> i'm starting to think that we should use log.c for these messages, and
> bump file->errors if we think it's worth.  The upside would be that we
> stop writing to fd 2 on error (which i'm not sure if could cause
> issues when reloading the config.)

Config-reload would require other significant changes since unveil
is locked down after the config is parsed.

> > +	int require_config_file = (proc_id == PROC_GITWRAPPER ? 0 : 1);
> 
> not really an issue, but made me raise an eyebrow.  I'd rewrite it as
> 
> 	int require_config_file = (proc_id != PROC_GITWRAPPER)

Fine, I've made the above tweak before committing.