On Fri, Nov 03, 2023 at 09:19:23PM +0100, Martijn van Duren wrote:
> Hello,
> 
> I just started playing with got{,d} for the first time and after
> wrapping my head around the fact that the repository needs to live
> outside the working tree I'm starting to like it.
> 
> In setting up a gotd server, I quickly became bored with the copy-paste
> work of the different per repository settings.
> 
> Diff below allows to declare repository directives in the global scope
> and are stored in a repo_template. If a directive is not specified in
> the repository it will be copied over from the template. The path
> directive will be used as a root-directory and the name of the
> repository will be appended to complete the per repository path.
> 
> thoughts?

Hey Martijn,

I understand the need to avoid repetition. This requirement will probably
come up over and over again so we should definitely do something about this.

I have doubts about appending the repository name to an on-disk path because
the repository name is in a "virtual" path space. It is not an on-disk path.
Consider this example from gotd.conf:

           # This repository can be accessed via
           # ssh://user@example.com/openbsd/ports
           repository "openbsd/ports" {
                   path "/var/git/ports.git"

The repository name is used to construct the repository URL which clients can
fetch from. This URL has no relation to where the repository appears on disk.
I would prefer to keep the URL and on-disk path spaces completely separate.

While at it, we should support multiple repository directories because
people might want to add more partitions when existing ones start running
out of space.

So how about implementing something like this instead:

 directory "/git"  # some repositories are here
 directory "/var/git"  # some more repositories are here

 repository "openbsd/ports" {
   # a relative path is looked up in all directories, first match wins:
   path "ports.git"

   ...
 }

This scheme is backwards compatible with existing configs because so far
gotd.conf has required absolute paths to repositories.

Regarding the new code, could the new protect_set flag be replaced with
TAILQ_EMPTY checks, for example via a helper function like this:

static int
repo_has_protected_refs(struct gotd_repo *repo)
{
  return (TAILQ_EMPTY(&repo->protected_tag_namespaces) &&
      TAILQ_EMPTY(&repo->protected_branch_namespaces) &&
      TAILQ_EMPTY(&repo->protected_branches));
}

I suspect this might reduce the size of the diff and would make the code
easier to follow because there would be less state flags to keep in mind.

Regarding the docs, could you extend the EXAMPLES section in the man page
to cover the new possibilities? I suspect many people jump there and begin
copy-pasting, and only read the rest of the page if they are unsure about
something.

Another problem I noticed while looking at this is that variables don't work
in gotd.conf because symset() is unreachable. I have a diff to make this work
by copying relevant code from gotwebd/parse.y. I had hoped this could help
with avoiding repetition but it won't because variables will not be expanded
inside strings. A directive such as path "$gitrootdir/repo.git" will not
expand $gitrootdir.
Still, variables could be another solution to avoiding repetition.
We could try modifying parse.y to expand variables in strings somehow, but I
don't know how difficult that would be. And maybe there is a good reason why
variables don't get expanded in strings?

Thanks,
Stefan