Re: [PATCH] improve nginx-service

[Ludovic Courtès]

Julien Lepiller <address@hidden> skribis:

> On Thu, 27 Oct 2016 14:41:18 +0200
> address@hidden (Ludovic Courtès) wrote:
>
>> [...]
>>
>> What I had in mind was just to add ‘compose’ and ‘extend’ to
>> ‘nginx-service-type’ (this is where we define how extensions are
>> handled).  There’s a bit of extra bookeeping to do, in particular
>> moving the list of vhosts to <nginx-configuration>, as in this
>> untested patch:
>> 

[...]

> Actually your patch didn't help me except for syntax simplification.
> Your answer was not really helpful,

Fair enough.  :-)

> but it forced me to consider what I really wanted. So my use case
> would be that I would like to run a webservice from the store (all
> configuration should be made via guix).  But my problem is that I
> don't know how to get the path to the store that I could pass as the
> root parameter in nginx-vhost-config.

OK, that doesn’t have much to do with making nginx-service-type
extensible.  But we can do both!

If the vhost’s root directory is immutable, you can of course add it to
the store via ‘computed-file’ or similar, and then write:

  (nginx-vhost-configuration
    (root #$(computed-file "root" …)))

If it’s mutable, as is often going to be the case, you can simply pass
its file name:

  (nginx-vhost-configuration
    (root "/var/www/the-service"))

In that case, it’s up to you to make sure that /var/www/the-service
exists and contains the right thing.

Does that answer your question?

> Also, I don't know what the best solution would be to get a configured
> web service in the store. Configuration is usually in a file in the
> root directory, and sometimes a default file is already present and you
> are supposed to modify it by hand. The issue here is that the store is
> read-only. How could you create a configuration file that can be found
> in the package's directory (using a guix service)? Would that mean that
> I have to copy the whole package and change just one file? Is there
> anything better, or do I have no other choice than install it outside of
> guix?

All the daemons (almost all of them) started on GuixSD get their config
file from the store.  When you wrote:

  #~(system* #$(file-append nginx "/bin/nginx") "-c" #$config-file)

that translated to:

  (system* "/gnu/store/…/bin/nginx" "-c" "/gnu/store/…-config")

where /gnu/store/…-config is a file that was generated using
‘computed-file’ or similar.

I think the same approach should work for a Web service, but without
being more specific I can’t tell.

> From 25a296057969a35b86ea7371577504c43bf96334 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <address@hidden>
> Date: Thu, 27 Oct 2016 19:47:27 +0200
> Subject: [PATCH] service: Make nginx-service extensible
>
> gnu/services/web.scm (nginx-service-type): Make extensible

[...]

> address@hidden [Scheme Variable] nginx-service-type
          ^
Should be braces.

> +This is the type for the @code{nginx} web server.

“nginx” (the proper name, not the command name.)

>    (nginx         nginx-configuration-nginx)         ;<package>
>    (log-directory nginx-configuration-log-directory) ;string
>    (run-directory nginx-configuration-run-directory) ;string
> +  (vhosts        nginx-configuration-vhosts)        ;list of 
> <nginx-vhost-configuration>

Please add a default value like at
<https://lists.gnu.org/archive/html/guix-devel/2016-10/msg01380.html>.

I hindsight, I wonder why I put that one-element list as the default;
shouldn’t it be the empty list?

>    (file          nginx-configuration-file))         ;string | file-like
>  
>  (define (config-domain-strings names)
> @@ -102,11 +104,13 @@ of index files."
>     "      server_name " (config-domain-strings
>                           (nginx-vhost-configuration-server-name vhost))
>                          ";\n"
> -   (if (nginx-vhost-configuration-ssl-certificate vhost)
> +   (if (and (nginx-vhost-configuration-https-port vhost)
> +            (nginx-vhost-configuration-ssl-certificate vhost))
>         (string-append "      ssl_certificate "
>                        (nginx-vhost-configuration-ssl-certificate vhost) 
> ";\n")
>         "")
> -   (if (nginx-vhost-configuration-ssl-certificate-key vhost)
> +   (if (and (nginx-vhost-configuration-https-port vhost)
> +            (nginx-vhost-configuration-ssl-certificate-key vhost))

Could you remove these changes?  I think they are unrelated.

>  (define* (nginx-service #:key (nginx nginx)
>                          (log-directory "/var/log/nginx")
>                          (run-directory "/var/run/nginx")
> -                        (vhost-list (list (nginx-vhost-configuration)))
> -                        (config-file
> -                         (default-nginx-config log-directory run-directory 
> vhost-list)))
> +                        (vhost-list (list (nginx-vhost-configuration
> +                                            (https-port #f))))

Please remove the ‘https-port’ setting.

Apart from that it LGTM.  Could you send an updated patch?

Thanks!

Ludo’.