From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35191) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1epirO-0006yg-RI for guix-patches@gnu.org; Sat, 24 Feb 2018 18:04:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1epirL-0002ll-NP for guix-patches@gnu.org; Sat, 24 Feb 2018 18:04:06 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:51271) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1epirL-0002lh-Iz for guix-patches@gnu.org; Sat, 24 Feb 2018 18:04:03 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1epirJ-0002Y1-P1 for guix-patches@gnu.org; Sat, 24 Feb 2018 18:04:03 -0500 Subject: [bug#29820] [PATCH] services: cgit: Add more configuration fields. Resent-Message-ID: References: <87incy9yv6.fsf@gmail.com> <87h8sdb6qa.fsf@lassieur.org> <87po6yu7h9.fsf@gmail.com> <871sjdz8ca.fsf@lassieur.org> <87r2q6zp2l.fsf@gmail.com> From: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur In-reply-to: <87r2q6zp2l.fsf@gmail.com> Date: Sun, 25 Feb 2018 00:03:49 +0100 Message-ID: <87h8q6at2i.fsf@lassieur.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Oleg Pykhalov Cc: 29820@debbugs.gnu.org Hi Oleg, Now is my turn to be late :-) I'm really sorry. Oleg Pykhalov 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 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!