[bug#29820] [PATCH] services: cgit: Add more configuration fields.

[Clément Lassieur]

Hi Oleg,

Now is my turn to be late :-)  I'm really sorry.

Oleg Pykhalov <address@hidden> writes:

> Hello Clément,
>
> apologies for such a long pause.  I tried to implement all we talked
> about, but I kinda stuck.  What do you think about merging and not hold
> it more for a small issue with project-list?  The patch is attached.

Sure, it's very helpful, thank you for this work!

> The test suite succeeds:
> --8<---------------cut here---------------start------------->8---
> ./pre-inst-env env GUIX_PACKAGE_PATH= make check-system TESTS=cgit
> --8<---------------cut here---------------end--------------->8---
>
> Clément Lassieur <address@hidden> writes:
>
> [...]
>
>> If you stick with depending on the fields order, then could you add
>> very clear comments so that people don't add fields at the wrong
>> place?  WDYT?
>
> I think we could stick with ordering fields and comments.
>
> I'll add a note commentary about order at the head of the file.

Cool thanks!

>>>> I think the official project uses 'cgit' instead of 'Cgit' (there are
>>>> other occurrences where you use 'Cgit').
>>>
>>> Ludovic asked to capitalize cgit in
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28283#14
>>
>> But he was only talking about titles wasn't he?
>
> I think not only, because we have Cgit everywhere in the current
> documentation.

Then it should be changed to 'cgit', there is no need to copy
documentation mistakes :-).  This is an example of commit from Ludovic
where he doesn't capitalize 'zlib':
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=06e3a5181efa0ea83bb6608d3cbfba5caa56d7e9.

>>>>> +  (repository-directory
>>>>> +   (repository-directory "/srv/git")
>>>>> +   "Name of the directory to scan for repositories.")
>>>>
>>>> I believe it would be clearer if it was named the same way cgit names
>>>> it: scan-path.
>>>
>>> Ludovic asked to rename it in
>>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28283#14
>>>
>>> I don't know should we stay close to cgit naming conventions or not.
>>> Thoughts?
>>
>> At least it should be possible to grep 'scan_path' in the documentation,
>> so that users can easily find what to use instead of 'scan-path'.  Could
>> you say 'scan_path' is the original name in the docstring?
>
> Good idea, thank you!  I'll add a '(represents @code{scan-path})' to the
> description of the 'repository-directory' field.

Ok!

>>>>> +  (project-list
>>>>> +   (list '())
>>>>> +   "A list of subdirectories inside of @code{repository-directory}, 
>>>>> relative
>>>>> +to it, that should loaded as Git repositories.")
>>>>
>>>> I forgot one thing: 'project-list' is a file, not a list of strings.  I
>>>> agree it's weird that cgit's documentation doesn't say it's a file.  I
>>>> see two solutions:
>>>
>>> Sorry, it's not clear for me.  As I understand from CGITRC(5) it's a
>>> list like:
>>>
>>>     project-list=/share/cgit/cgit.png /share/cgit/cgit.jpg
>>>
>>> relative to /srv/git (in our case).
>>
>> CGITRC isn't clear.  It's really a file containing the list of git
>> directories.  For example:
>>
>> /etc/cgit/project-list:
>>
>> a/b/foo.git
>> c/bar.git
>> baz.git
>>
>> And
>>
>> project-list=/etc/cgit/project-list
>>
>>>> 1. Change the type to 'string', so that people can set a file name.
>>>>
>>>> 2. Use a list type that would transparently transform its values into a
>>>>    file in the store, with the generated cgitrc file pointing to it.
>>>>
>>>> The second solution is better because the user won't need to create the
>>>> file.
>>>
>>> I choose 1st for now, because 2nd I don't understand what need to be
>>> produced at the end.  Could you give me an example?
>>
>> With 2nd, users would write a configuration like
>>
>> (project-list '("a/b/foo.git"
>>                 "c/bar.git"
>>                 "baz.git"))
>>
>> And 'guix system reconfigure' would create the file
>> /gnu/store/xxxxxxx-project-list containing those three lines.  The
>> generated cgitrc file would contain:
>>
>> project-list=/gnu/store/xxxxxxx-project-list
>>
>> You could use a type whose serializer would call the 'plain-file'
>> procedure.
>
> Will be in a TODO list until I get more familiar with Guix or somebody
> else add this.

It's actually more complicated than I thought, because file-like objects
can't be serialized as strings.  So the serialization mechanism would
need to be a bit reworked, so that it uses lists instead of strings, but
it could be done later.

I guess the most sensible thing to do is to comment the 'project-list'
field, with a TODO note, explaining that cgit expects a file name that
should be created from a list of strings provided by the user.

>>>> Also, could you add a way to use an opaque configuration file?  It would
>>>> be helpful for users who don't have time to update their configuration,
>>>> or in case there are new cgit configurations that are not yet
>>>> implemented by the Scheme service.
>>>
>>> Sure.
>>>
>>> Is the following order is OK?
>>>
>>>     (serialize-configuration
>>>      (cgit-configuration
>>>       (extra-options (list "soo=do"))
>>>       (repositories (list
>>>                      (repository-cgit-configuration
>>>                       (module-link-path '("/super/cow" "moo"))
>>>                       (extra-options (list "goo=foo"))))))
>>>      cgit-configuration-fields)
>>>
>>>     …
>>>     repo.extra-options=goo=foo
>>>     extra-options=soo=do
>>>     # END OF FILE
>>
>> I was more thinking about something like in the Dovecot service where
>> you can pass the whole file as a string.
>
> OK, thank you for a reference to Dovecot example.  I'll add this.

Great!