unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] improve nginx-service
@ 2016-10-16 12:33 Julien Lepiller
  2016-10-19 21:04 ` Ludovic Courtès
  0 siblings, 1 reply; 29+ messages in thread
From: Julien Lepiller @ 2016-10-16 12:33 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 186 bytes --]

Hi,

this patch fixes a problem with nginx configuration (ensuring some
directories are available to nginx) and adds vhost support in its
configuration. Also updates the doc accordingly.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-services-improve-nginx-service-configuration.patch --]
[-- Type: text/x-patch, Size: 9547 bytes --]

From 613d5db739d4010be2037fd2fcfc70baca4625aa Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Mon, 26 Sep 2016 23:55:58 +0200
Subject: [PATCH] services: improve nginx-service configuration

* gnu/services/web.scm (<nginx-vhost-configuration>): New record type.
* doc/guix.texi (Web Services): Document 'nginx-vhost-configuration'.
---
 doc/guix.texi        | 44 ++++++++++++++++++++++++++++
 gnu/services/web.scm | 81 +++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 121 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 47fc199..2e7af90 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10370,6 +10370,7 @@ The @code{(gnu services web)} module provides the following service:
 @deffn {Scheme Procedure} nginx-service [#:nginx nginx] @
        [#:log-directory ``/var/log/nginx''] @
        [#:run-directory ``/var/run/nginx''] @
+       [#:vhost-list (list (nginx-vhost-configuration))] @
        [#:config-file]
 
 Return a service that runs @var{nginx}, the nginx web server.
@@ -10380,8 +10381,51 @@ files are written to @var{run-directory}.  For proper operation, these
 arguments should match what is in @var{config-file} to ensure that the
 directories are created when the service is activated.
 
+As an alternative to using a @var{config-file}, @var{vhost-list} can be
+used to specify the list of vhosts required on the host.  For this to work,
+use the default value for @var{config-file}.
+
 @end deffn
 
+@deftp {Data Type} nginx-vhost-configuration
+Data type representing the configuration of an nginx virtual host.
+This type has the following parameters:
+@table @asis
+@item @code{http-port} (default: 80)
+Nginx will listen for http connection on this port. Set it at #f if nginx should
+not listen for http (non secure) connection for this vhost.
+
+@item @code{https-port} (default: 443)
+Nginx will listen for https connection on this port. Set it at #f if nginx
+should not listen for https (secure) connection for this vhost.
+
+Note that nginx can listen for http and https connections in the same vhost.
+
+@item @code{server-name} (default: @code{(list 'default)})
+A list of server names this vhost represents. @code{'default} represents the
+default vhost for connections matching no other vhost.
+
+@item @code{root} (default: ``/srv/http'')
+Root of the website nginx will serve.
+
+@item @code{index} (default: @code{(list ``index.html'')})
+Index files to look for when clients ask for a directory. If it cannot be found,
+Nginx will send the list of files in the directory.
+
+@item @code{ssl-certificate} (default: ``/etc/nginx/cert.pem'')
+Where to find the certificate for secure connections. Set it to #f if you don't
+have a certificate or you don't want to use https.
+
+@item @code{ssl-certificate-key} (default: ``/etc/nginx/key.pem'')
+Where to find the private key for secure connections. Set it to #f if you don't
+have a key or you don't want to use https.
+
+@item @code{server-tokens?} (default: #f)
+Whether the server should add its configuration to response.
+
+@end table
+@end deftp
+
 @node Network File System
 @subsubsection Network File System
 @cindex NFS
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 0a2a09b..dca4972 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 ng0 <ng0@we.make.ritual.n0.is>
+;;; Copyright © 2016 Julien Lepiller <julien@lepiller.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -29,6 +30,8 @@
   #:use-module (ice-9 match)
   #:export (nginx-configuration
             nginx-configuration?
+            nginx-vhost-configuration
+            nginx-vhost-configuration?
             nginx-service
             nginx-service-type))
 
@@ -38,6 +41,26 @@
 ;;;
 ;;; Code:
 
+(define-record-type* <nginx-vhost-configuration>
+  nginx-vhost-configuration make-nginx-vhost-configuration
+  nginx-vhost-configuration?
+  (http-port           nginx-vhost-configuration-http-port
+                       (default 80))
+  (https-port          nginx-vhost-configuration-https-port
+                       (default 443))
+  (server-name         nginx-vhost-configuration-server-name
+                       (default (list 'default)))
+  (root                nginx-vhost-configuration-root
+                       (default "/srv/http"))
+  (index               nginx-vhost-configuration-index
+                       (default (list "index.html")))
+  (ssl-certificate     nginx-vhost-configuration-ssl-certificate
+                       (default "/etc/nginx/cert.pem"))
+  (ssh-certificate-key nginx-vhost-configuration-ssl-certificate-key
+                       (default "/etc/nginx/key.pem"))
+  (server-tokens?      nginx-vhost-configuration-server-tokens?
+                       (default #f)))
+
 (define-record-type* <nginx-configuration>
   nginx-configuration make-nginx-configuration
   nginx-configuration?
@@ -46,16 +69,59 @@
   (run-directory nginx-configuration-run-directory) ;string
   (file          nginx-configuration-file))         ;string | file-like
 
-(define (default-nginx-config log-directory run-directory)
+(define (list-names names)
+ (match names
+  ('() "")
+  ((head tail ...) (string-append (if (eq? head 'default) "_" head)
+                                  " "
+                                  (list-names tail)))))
+
+(define (default-nginx-vhost-config vhost)
+  (string-append 
+   "    server {\n"
+   (if (nginx-vhost-configuration-http-port vhost)
+       (string-append "      listen "
+                      (number->string (nginx-vhost-configuration-http-port vhost))
+                      ";\n")
+       "")
+   (if (nginx-vhost-configuration-https-port vhost)
+       (string-append "      listen "
+                      (number->string (nginx-vhost-configuration-https-port vhost))
+                      " ssl;\n")
+       "")
+   "      server_name " (list-names (nginx-vhost-configuration-server-name vhost))
+                        ";\n"
+   (if (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)
+       (string-append "      ssl_certificate_key "
+                      (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
+       "")
+   "      root " (nginx-vhost-configuration-root vhost) ";\n"
+   "      index " (list-names (nginx-vhost-configuration-index vhost)) ";\n"
+   "      server_tokens " (if (nginx-vhost-configuration-server-tokens? vhost)
+                              "on" "off") ";\n"
+   "    }\n"))
+
+(define (default-nginx-config log-directory run-directory vhost-list)
   (plain-file "nginx.conf"
               (string-append
                "user nginx nginx;\n"
                "pid " run-directory "/pid;\n"
                "error_log " log-directory "/error.log info;\n"
                "http {\n"
+               "    client_body_temp_path " run-directory "/client_body_temp;\n"
+               "    proxy_temp_path " run-directory "/proxy_temp;\n"
+               "    fastcgi_temp_path " run-directory "/fastcgi_temp;\n"
+               "    uwsgi_temp_path " run-directory "/uwsgi_temp;\n"
+               "    scgi_temp_path " run-directory "/scgi_temp;\n"
                "    access_log " log-directory "/access.log;\n"
-               "    root /var/www;\n"
-               "    server {}\n"
+               (let ((http (map default-nginx-vhost-config vhost-list)))
+                    (do ((http http (cdr http)) 
+                         (block "" (string-append (car http) "\n" block )))
+                        ((null? http) block)))
                "}\n"
                "events {}\n")))
 
@@ -79,6 +145,12 @@
          (mkdir-p #$log-directory)
          (format #t "creating nginx run directory '~a'~%" #$run-directory)
          (mkdir-p #$run-directory)
+         (format #t "creating nginx temp directories '~a/{client_body,proxy,fastcgi,uwsgi,scgi}_temp'~%" #$run-directory)
+         (mkdir-p (string-append #$run-directory "/client_body_temp"))
+         (mkdir-p (string-append #$run-directory "/proxy_temp"))
+         (mkdir-p (string-append #$run-directory "/fastcgi_temp"))
+         (mkdir-p (string-append #$run-directory "/uwsgi_temp"))
+         (mkdir-p (string-append #$run-directory "/scgi_temp"))
          ;; Check configuration file syntax.
          (system* (string-append #$nginx "/sbin/nginx")
                   "-c" #$config-file "-t")))))
@@ -114,8 +186,9 @@
 (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)))
+                         (default-nginx-config log-directory run-directory vhost-list)))
   "Return a service that runs NGINX, the nginx web server.
 
 The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-10-16 12:33 [PATCH] improve nginx-service Julien Lepiller
@ 2016-10-19 21:04 ` Ludovic Courtès
  2016-10-20 12:37   ` Julien Lepiller
  0 siblings, 1 reply; 29+ messages in thread
From: Ludovic Courtès @ 2016-10-19 21:04 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: guix-devel, David Thompson

Hi Julien,

This looks like a great improvement to me!  Sounds nicer than fiddling
with config files.

I suppose we could make ‘nginx-service-type’ extensible (info "(guix)
Service Types and Services") so that people can write services that
define new vhosts?

Julien Lepiller <julien@lepiller.eu> skribis:

> From 613d5db739d4010be2037fd2fcfc70baca4625aa Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Mon, 26 Sep 2016 23:55:58 +0200
> Subject: [PATCH] services: improve nginx-service configuration
>
> * gnu/services/web.scm (<nginx-vhost-configuration>): New record type.
> * doc/guix.texi (Web Services): Document 'nginx-vhost-configuration'.

Please mention the other changes in web.scm: new procedures, changes in
existing procedures, etc.

> +As an alternative to using a @var{config-file}, @var{vhost-list} can be
> +used to specify the list of vhosts required on the host.  For this to work,

“the list of @dfn{virtual hosts} or @dfn{vhosts}”

> +@deftp {Data Type} nginx-vhost-configuration
> +Data type representing the configuration of an nginx virtual host.

@dfn{virtual host}

> +This type has the following parameters:
> +@table @asis
> +@item @code{http-port} (default: 80)

@code{80}; likewise for all the other default values.

> +Nginx will listen for http connection on this port. Set it at #f if nginx should
> +not listen for http (non secure) connection for this vhost.

s/http/HTTP/ and s/#f/@code{#f}/
Please leave two spaces after an end-of-sentence period.

> +(define (list-names names)
> + (match names
> +  ('() "")
> +  ((head tail ...) (string-append (if (eq? head 'default) "_" head)
> +                                  " "
> +                                  (list-names tail)))))

Maybe call it ‘config-value-strings’?  Please add a docstring.  I think
it’d be more readable like this:

  (define (config-value-strings names)
    "Return a string denoting the nginx config representation of NAMES,
   a list of foobars…"
    (string-concatenate
     (map (match-lambda
            ('default "_")
            ((? string? str) str))
          names)))

Could you send an updated patch?

Unless David, who initially wrote this service has objections, this
patch looks good to me with the changes as suggested above.

Thanks!

Ludo’.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-10-19 21:04 ` Ludovic Courtès
@ 2016-10-20 12:37   ` Julien Lepiller
  2016-10-24 20:51     ` Ludovic Courtès
  0 siblings, 1 reply; 29+ messages in thread
From: Julien Lepiller @ 2016-10-20 12:37 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2812 bytes --]

On Wed, 19 Oct 2016 23:04:14 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Hi Julien,
> 
> This looks like a great improvement to me!  Sounds nicer than fiddling
> with config files.
> 
> I suppose we could make ‘nginx-service-type’ extensible (info "(guix)
> Service Types and Services") so that people can write services that
> define new vhosts?

You mean something like udev-service-type where you could extend it
with a list of vhosts?

Except for that comment, I modified the patch following your advice.

> 
> Julien Lepiller <julien@lepiller.eu> skribis:
> 
> > From 613d5db739d4010be2037fd2fcfc70baca4625aa Mon Sep 17 00:00:00
> > 2001 From: Julien Lepiller <julien@lepiller.eu>
> > Date: Mon, 26 Sep 2016 23:55:58 +0200
> > Subject: [PATCH] services: improve nginx-service configuration
> >
> > * gnu/services/web.scm (<nginx-vhost-configuration>): New record
> > type.
> > * doc/guix.texi (Web Services): Document
> > 'nginx-vhost-configuration'.  
> 
> Please mention the other changes in web.scm: new procedures, changes
> in existing procedures, etc.
> 
> > +As an alternative to using a @var{config-file}, @var{vhost-list}
> > can be +used to specify the list of vhosts required on the host.
> > For this to work,  
> 
> “the list of @dfn{virtual hosts} or @dfn{vhosts}”
> 
> > +@deftp {Data Type} nginx-vhost-configuration
> > +Data type representing the configuration of an nginx virtual
> > host.  
> 
> @dfn{virtual host}
> 
> > +This type has the following parameters:
> > +@table @asis
> > +@item @code{http-port} (default: 80)  
> 
> @code{80}; likewise for all the other default values.
> 
> > +Nginx will listen for http connection on this port. Set it at #f
> > if nginx should +not listen for http (non secure) connection for
> > this vhost.  
> 
> s/http/HTTP/ and s/#f/@code{#f}/
> Please leave two spaces after an end-of-sentence period.
> 
> > +(define (list-names names)
> > + (match names
> > +  ('() "")
> > +  ((head tail ...) (string-append (if (eq? head 'default) "_" head)
> > +                                  " "
> > +                                  (list-names tail)))))  
> 
> Maybe call it ‘config-value-strings’?  Please add a docstring.  I
> think it’d be more readable like this:
> 
>   (define (config-value-strings names)
>     "Return a string denoting the nginx config representation of
> NAMES, a list of foobars…"
>     (string-concatenate
>      (map (match-lambda
>             ('default "_")
>             ((? string? str) str))
>           names)))
> 
> Could you send an updated patch?
> 
> Unless David, who initially wrote this service has objections, this
> patch looks good to me with the changes as suggested above.
> 
> Thanks!
> 
> Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-services-improve-nginx-service-configuration.patch --]
[-- Type: text/x-patch, Size: 10278 bytes --]

From 8bda6fdd53b3cc7470fac67228a88e0d165134dd Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Mon, 26 Sep 2016 23:55:58 +0200
Subject: [PATCH] services: improve nginx-service configuration

* gnu/services/web.scm (<nginx-vhost-configuration>): New record type.
(config-domain-strings): New procedure.
(config-index-strings): New procedure.
(default-nginx-vhost-config): New procedure.
(default-nginx-config): Add vhost support and temporary directories
(nginx-activation): Create temporary directories
(nginx-service): Add vhost-list key.
* doc/guix.texi (Web Services): Document 'nginx-vhost-configuration'.
---
 doc/guix.texi        | 46 ++++++++++++++++++++++++++
 gnu/services/web.scm | 92 +++++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 134 insertions(+), 4 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 0c5d641..c36a29c 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10394,6 +10394,7 @@ The @code{(gnu services web)} module provides the following service:
 @deffn {Scheme Procedure} nginx-service [#:nginx nginx] @
        [#:log-directory ``/var/log/nginx''] @
        [#:run-directory ``/var/run/nginx''] @
+       [#:vhost-list (list (nginx-vhost-configuration))] @
        [#:config-file]
 
 Return a service that runs @var{nginx}, the nginx web server.
@@ -10404,8 +10405,53 @@ files are written to @var{run-directory}.  For proper operation, these
 arguments should match what is in @var{config-file} to ensure that the
 directories are created when the service is activated.
 
+As an alternative to using a @var{config-file}, @var{vhost-list} can be
+used to specify the list of @dfn{virtual hosts} required on the host.  For
+this to work, use the default value for @var{config-file}.
+
 @end deffn
 
+@deftp {Data Type} nginx-vhost-configuration
+Data type representing the configuration of an nginx virtual host.
+This type has the following parameters:
+@table @asis
+@item @code{http-port} (default: @code{80})
+Nginx will listen for HTTP connection on this port.  Set it at @code{#f} if
+nginx should not listen for HTTP (non secure) connection for this
+@dfn{virtual host}.
+
+@item @code{https-port} (default: @code{443})
+Nginx will listen for HTTPS connection on this port.  Set it at @code{#f} if
+nginx should not listen for HTTPS (secure) connection for this @dfn{virtual host}.
+
+Note that nginx can listen for HTTP and HTTPS connections in the same
+@dfn{virtual host}.
+
+@item @code{server-name} (default: @code{(list 'default)})
+A list of server names this vhost represents. @code{'default} represents the
+default vhost for connections matching no other vhost.
+
+@item @code{root} (default: @code{``/srv/http''})
+Root of the website nginx will serve.
+
+@item @code{index} (default: @code{(list ``index.html'')})
+Index files to look for when clients ask for a directory.  If it cannot be found,
+Nginx will send the list of files in the directory.
+
+@item @code{ssl-certificate} (default: @code{``/etc/nginx/cert.pem''})
+Where to find the certificate for secure connections.  Set it to @code{#f} if
+you don't have a certificate or you don't want to use HTTPS.
+
+@item @code{ssl-certificate-key} (default: @code{``/etc/nginx/key.pem''})
+Where to find the private key for secure connections.  Set it to @code{#f} if
+you don't have a key or you don't want to use HTTPS.
+
+@item @code{server-tokens?} (default: @code{#f})
+Whether the server should add its configuration to response.
+
+@end table
+@end deftp
+
 @node Network File System
 @subsubsection Network File System
 @cindex NFS
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 0a2a09b..49a2962 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2015, 2016 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2016 ng0 <ng0@we.make.ritual.n0.is>
+;;; Copyright © 2016 Julien Lepiller <julien@lepiller.eu>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -29,6 +30,8 @@
   #:use-module (ice-9 match)
   #:export (nginx-configuration
             nginx-configuration?
+            nginx-vhost-configuration
+            nginx-vhost-configuration?
             nginx-service
             nginx-service-type))
 
@@ -38,6 +41,26 @@
 ;;;
 ;;; Code:
 
+(define-record-type* <nginx-vhost-configuration>
+  nginx-vhost-configuration make-nginx-vhost-configuration
+  nginx-vhost-configuration?
+  (http-port           nginx-vhost-configuration-http-port
+                       (default 80))
+  (https-port          nginx-vhost-configuration-https-port
+                       (default 443))
+  (server-name         nginx-vhost-configuration-server-name
+                       (default (list 'default)))
+  (root                nginx-vhost-configuration-root
+                       (default "/srv/http"))
+  (index               nginx-vhost-configuration-index
+                       (default (list "index.html")))
+  (ssl-certificate     nginx-vhost-configuration-ssl-certificate
+                       (default "/etc/nginx/cert.pem"))
+  (ssl-certificate-key nginx-vhost-configuration-ssl-certificate-key
+                       (default "/etc/nginx/key.pem"))
+  (server-tokens?      nginx-vhost-configuration-server-tokens?
+                       (default #f)))
+
 (define-record-type* <nginx-configuration>
   nginx-configuration make-nginx-configuration
   nginx-configuration?
@@ -46,16 +69,70 @@
   (run-directory nginx-configuration-run-directory) ;string
   (file          nginx-configuration-file))         ;string | file-like
 
-(define (default-nginx-config log-directory run-directory)
+(define (config-domain-strings names)
+ "Return a string denoting the nginx config representation of NAMES, a list
+of domain names."
+ (string-concatenate
+  (map (match-lambda
+        ('default "_")
+        ((? string? str) str))
+       names)))
+
+(define (config-index-strings names)
+ "Return a string denoting the nginx config representation of NAMES, a list
+of index files."
+ (string-concatenate
+  (map (match-lambda
+        ((? string? str) str))
+       names)))
+
+(define (default-nginx-vhost-config vhost)
+  (string-append 
+   "    server {\n"
+   (if (nginx-vhost-configuration-http-port vhost)
+       (string-append "      listen "
+                      (number->string (nginx-vhost-configuration-http-port vhost))
+                      ";\n")
+       "")
+   (if (nginx-vhost-configuration-https-port vhost)
+       (string-append "      listen "
+                      (number->string (nginx-vhost-configuration-https-port vhost))
+                      " ssl;\n")
+       "")
+   "      server_name " (config-domain-strings
+                         (nginx-vhost-configuration-server-name vhost))
+                        ";\n"
+   (if (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)
+       (string-append "      ssl_certificate_key "
+                      (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
+       "")
+   "      root " (nginx-vhost-configuration-root vhost) ";\n"
+   "      index " (config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
+   "      server_tokens " (if (nginx-vhost-configuration-server-tokens? vhost)
+                              "on" "off") ";\n"
+   "    }\n"))
+
+(define (default-nginx-config log-directory run-directory vhost-list)
   (plain-file "nginx.conf"
               (string-append
                "user nginx nginx;\n"
                "pid " run-directory "/pid;\n"
                "error_log " log-directory "/error.log info;\n"
                "http {\n"
+               "    client_body_temp_path " run-directory "/client_body_temp;\n"
+               "    proxy_temp_path " run-directory "/proxy_temp;\n"
+               "    fastcgi_temp_path " run-directory "/fastcgi_temp;\n"
+               "    uwsgi_temp_path " run-directory "/uwsgi_temp;\n"
+               "    scgi_temp_path " run-directory "/scgi_temp;\n"
                "    access_log " log-directory "/access.log;\n"
-               "    root /var/www;\n"
-               "    server {}\n"
+               (let ((http (map default-nginx-vhost-config vhost-list)))
+                    (do ((http http (cdr http)) 
+                         (block "" (string-append (car http) "\n" block )))
+                        ((null? http) block)))
                "}\n"
                "events {}\n")))
 
@@ -79,6 +156,12 @@
          (mkdir-p #$log-directory)
          (format #t "creating nginx run directory '~a'~%" #$run-directory)
          (mkdir-p #$run-directory)
+         (format #t "creating nginx temp directories '~a/{client_body,proxy,fastcgi,uwsgi,scgi}_temp'~%" #$run-directory)
+         (mkdir-p (string-append #$run-directory "/client_body_temp"))
+         (mkdir-p (string-append #$run-directory "/proxy_temp"))
+         (mkdir-p (string-append #$run-directory "/fastcgi_temp"))
+         (mkdir-p (string-append #$run-directory "/uwsgi_temp"))
+         (mkdir-p (string-append #$run-directory "/scgi_temp"))
          ;; Check configuration file syntax.
          (system* (string-append #$nginx "/sbin/nginx")
                   "-c" #$config-file "-t")))))
@@ -114,8 +197,9 @@
 (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)))
+                         (default-nginx-config log-directory run-directory vhost-list)))
   "Return a service that runs NGINX, the nginx web server.
 
 The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-10-20 12:37   ` Julien Lepiller
@ 2016-10-24 20:51     ` Ludovic Courtès
  2016-10-26 19:45       ` Julien Lepiller
  0 siblings, 1 reply; 29+ messages in thread
From: Ludovic Courtès @ 2016-10-24 20:51 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1420 bytes --]

Hi,

Julien Lepiller <julien@lepiller.eu> skribis:

> On Wed, 19 Oct 2016 23:04:14 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:
>
>> Hi Julien,
>> 
>> This looks like a great improvement to me!  Sounds nicer than fiddling
>> with config files.
>> 
>> I suppose we could make ‘nginx-service-type’ extensible (info "(guix)
>> Service Types and Services") so that people can write services that
>> define new vhosts?
>
> You mean something like udev-service-type where you could extend it
> with a list of vhosts?

Yes, exactly.  So for example one could write a service for some
high-level Web service that would in turn create an nginx vhost.
WDYT?

> From 8bda6fdd53b3cc7470fac67228a88e0d165134dd Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Mon, 26 Sep 2016 23:55:58 +0200
> Subject: [PATCH] services: improve nginx-service configuration
>
> * gnu/services/web.scm (<nginx-vhost-configuration>): New record type.
> (config-domain-strings): New procedure.
> (config-index-strings): New procedure.
> (default-nginx-vhost-config): New procedure.
> (default-nginx-config): Add vhost support and temporary directories
> (nginx-activation): Create temporary directories
> (nginx-service): Add vhost-list key.
> * doc/guix.texi (Web Services): Document 'nginx-vhost-configuration'.

Applied with the minor changes below.

Thank you!

Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3192 bytes --]

diff --git a/doc/guix.texi b/doc/guix.texi
index 646808b..1293b8b 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -27,7 +27,8 @@ Copyright @copyright{} 2016 Chris Marusich@*
 Copyright @copyright{} 2016 Efraim Flashner@*
 Copyright @copyright{} 2016 John Darrington@*
 Copyright @copyright{} 2016 ng0@*
-Copyright @copyright{} 2016 Jan Nieuwenhuizen
+Copyright @copyright{} 2016 Jan Nieuwenhuizen@*
+Copyright @copyright{} 2016 Julien Lepiller
 
 Permission is granted to copy, distribute and/or modify this document
 under the terms of the GNU Free Documentation License, Version 1.3 or
@@ -10417,6 +10418,7 @@ this to work, use the default value for @var{config-file}.
 @deftp {Data Type} nginx-vhost-configuration
 Data type representing the configuration of an nginx virtual host.
 This type has the following parameters:
+
 @table @asis
 @item @code{http-port} (default: @code{80})
 Nginx will listen for HTTP connection on this port.  Set it at @code{#f} if
@@ -10434,18 +10436,18 @@ Note that nginx can listen for HTTP and HTTPS connections in the same
 A list of server names this vhost represents. @code{'default} represents the
 default vhost for connections matching no other vhost.
 
-@item @code{root} (default: @code{``/srv/http''})
+@item @code{root} (default: @code{"/srv/http"})
 Root of the website nginx will serve.
 
-@item @code{index} (default: @code{(list ``index.html'')})
+@item @code{index} (default: @code{(list "index.html")})
 Index files to look for when clients ask for a directory.  If it cannot be found,
 Nginx will send the list of files in the directory.
 
-@item @code{ssl-certificate} (default: @code{``/etc/nginx/cert.pem''})
+@item @code{ssl-certificate} (default: @code{"/etc/nginx/cert.pem"})
 Where to find the certificate for secure connections.  Set it to @code{#f} if
 you don't have a certificate or you don't want to use HTTPS.
 
-@item @code{ssl-certificate-key} (default: @code{``/etc/nginx/key.pem''})
+@item @code{ssl-certificate-key} (default: @code{"/etc/nginx/key.pem"})
 Where to find the private key for secure connections.  Set it to @code{#f} if
 you don't have a key or you don't want to use HTTPS.
 
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 49a2962..59e1e54 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -87,7 +87,7 @@ of index files."
        names)))
 
 (define (default-nginx-vhost-config vhost)
-  (string-append 
+  (string-append
    "    server {\n"
    (if (nginx-vhost-configuration-http-port vhost)
        (string-append "      listen "
@@ -130,9 +130,9 @@ of index files."
                "    scgi_temp_path " run-directory "/scgi_temp;\n"
                "    access_log " log-directory "/access.log;\n"
                (let ((http (map default-nginx-vhost-config vhost-list)))
-                    (do ((http http (cdr http)) 
-                         (block "" (string-append (car http) "\n" block )))
-                        ((null? http) block)))
+                 (do ((http http (cdr http))
+                      (block "" (string-append (car http) "\n" block )))
+                     ((null? http) block)))
                "}\n"
                "events {}\n")))
 

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-10-24 20:51     ` Ludovic Courtès
@ 2016-10-26 19:45       ` Julien Lepiller
  2016-10-27 12:41         ` Ludovic Courtès
  0 siblings, 1 reply; 29+ messages in thread
From: Julien Lepiller @ 2016-10-26 19:45 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 3863 bytes --]

On Mon, 24 Oct 2016 22:51:27 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Hi,
> 
> Julien Lepiller <julien@lepiller.eu> skribis:
> 
> > On Wed, 19 Oct 2016 23:04:14 +0200
> > ludo@gnu.org (Ludovic Courtès) wrote:
> >  
> >> Hi Julien,
> >> 
> >> This looks like a great improvement to me!  Sounds nicer than
> >> fiddling with config files.
> >> 
> >> I suppose we could make ‘nginx-service-type’ extensible (info
> >> "(guix) Service Types and Services") so that people can write
> >> services that define new vhosts?  
> >
> > You mean something like udev-service-type where you could extend it
> > with a list of vhosts?  
> 
> Yes, exactly.  So for example one could write a service for some
> high-level Web service that would in turn create an nginx vhost.
> WDYT?

Sounds great. I tried to work on it, but I'm stuck.

I'm trying to implement a service that extends nginx-service-type with
a vhost (default-nginx-vhost-service-type), and it creates a root
directory for a default empty index file. Now I would like to pass the
absolute path of this directory to the <nginx-vhost-configuration>, but
it is too early for that. System configuration with this new service
fails with:

Backtrace:
In guix/ui.scm:
1220: 19 [run-guix-command system "reconfigure" "/etc/config.scm"]
In ice-9/boot-9.scm:
 157: 18 [catch srfi-34 #<procedure 56353c0 at guix/ui.scm:426:2
()> ...] 157: 17 [catch system-error ...]
In guix/scripts/system.scm:
 870: 16 [#<procedure 560f270 at guix/scripts/system.scm:862:2 ()>]
 776: 15 [process-action reconfigure ("/etc/config.scm") ...]
In guix/store.scm:
1182: 14 [run-with-store # ...]
In guix/scripts/system.scm:
 788: 13 [#<procedure 5be5180 at guix/scripts/system.scm:780:8 (state)>
#] 552: 12 [perform-action reconfigure # # ...]
In gnu/system.scm:
 627: 11 [operating-system-derivation # # #f]
In gnu/services.scm:
 585: 10 [loop #]
In srfi/srfi-1.scm:
 578: 9 [map #<procedure loop (sink)> (# # #)]
In gnu/services.scm:
 585: 8 [loop #<<service> type: # parameters: #>]
In srfi/srfi-1.scm:
 578: 7 [map #<procedure loop (sink)> (# # # # ...)]
In gnu/services.scm:
 585: 6 [loop #<<service> type: # parameters: ()>]
In srfi/srfi-1.scm:
 578: 5 [map #<procedure 55bd960 at gnu/services.scm:574:4 (service)>
(# # # # ...)] In gnu/services/web.scm:
 189: 4 [nginx-shepherd-service #]
 181: 3 [nginx-action "-p" "/var/run/nginx"]
 141: 2 [nginx-config-file "/var/log/nginx" "/var/run/nginx" ...]
In srfi/srfi-1.scm:
 576: 1 [map #<procedure default-nginx-vhost-config (vhost)> (# #)]
In unknown file:
   ?: 0 [string-append "    server {\n" "      listen 80;\n" ...]

ERROR: In procedure string-append:
ERROR: In procedure string-append: Wrong type (expecting string):
#<<computed-file> name: "default-nginx-vhost" gexp: #<gexp (begin
(mkdir #<gexp-output out>) (symlink #<gexp-input #<<plain-file> name:
"index.html" content: "[...]" references: ()>:out> (string-append
#<gexp-output out> "/index.html"))) 5959270> options: (#:local-build?
#t)>

> 
> > From 8bda6fdd53b3cc7470fac67228a88e0d165134dd Mon Sep 17 00:00:00
> > 2001 From: Julien Lepiller <julien@lepiller.eu>
> > Date: Mon, 26 Sep 2016 23:55:58 +0200
> > Subject: [PATCH] services: improve nginx-service configuration
> >
> > * gnu/services/web.scm (<nginx-vhost-configuration>): New record
> > type. (config-domain-strings): New procedure.
> > (config-index-strings): New procedure.
> > (default-nginx-vhost-config): New procedure.
> > (default-nginx-config): Add vhost support and temporary directories
> > (nginx-activation): Create temporary directories
> > (nginx-service): Add vhost-list key.
> > * doc/guix.texi (Web Services): Document
> > 'nginx-vhost-configuration'.  
> 
> Applied with the minor changes below.
> 
> Thank you!
> 
> Ludo’.
> 


[-- Attachment #2: 0001-services-Make-nginx-service-type-extensible.patch --]
[-- Type: text/x-patch, Size: 7438 bytes --]

From 13748b6bfffef19080f4fa3bde2a6ae7d5c8d067 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Wed, 26 Oct 2016 21:33:34 +0200
Subject: [PATCH] services: Make nginx-service-type extensible

gnu/services/web.scm (nginx-service): Removed.
gnu/services/web.scm (nginx-service-sytsem): Make extensible.
gnu/services/web.scm (default-nginx-vhost-service-type): New variable.
---
 gnu/services/web.scm | 87 ++++++++++++++++++++++++++++++++--------------------
 1 file changed, 53 insertions(+), 34 deletions(-)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 59e1e54..0f06c99 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -28,12 +28,13 @@
   #:use-module (guix records)
   #:use-module (guix gexp)
   #:use-module (ice-9 match)
+  #:use-module (srfi srfi-1)
   #:export (nginx-configuration
             nginx-configuration?
             nginx-vhost-configuration
             nginx-vhost-configuration?
-            nginx-service
-            nginx-service-type))
+            nginx-service-type
+            default-nginx-vhost-service-type))
 
 ;;; Commentary:
 ;;;
@@ -64,10 +65,14 @@
 (define-record-type* <nginx-configuration>
   nginx-configuration make-nginx-configuration
   nginx-configuration?
-  (nginx         nginx-configuration-nginx)         ;<package>
-  (log-directory nginx-configuration-log-directory) ;string
-  (run-directory nginx-configuration-run-directory) ;string
-  (file          nginx-configuration-file))         ;string | file-like
+  (nginx         nginx-configuration-nginx         ;<package>
+                 (default nginx))
+  (log-directory nginx-configuration-log-directory ;string
+                 (default "/var/log/nginx"))
+  (run-directory nginx-configuration-run-directory ;string
+                 (default "/var/run/nginx"))
+  (vhosts        nginx-configuration-vhosts        ;list
+                 (default (list (nginx-vhost-configuration (https-port #f))))))
 
 (define (config-domain-strings names)
  "Return a string denoting the nginx config representation of NAMES, a list
@@ -102,11 +107,15 @@ 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-ssl-certificate vhost)
+         (nginx-vhost-configuration-https-port 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-ssl-certificate-key vhost)
+         (nginx-vhost-configuration-https-port vhost))
        (string-append "      ssl_certificate_key "
                       (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
        "")
@@ -116,7 +125,7 @@ of index files."
                               "on" "off") ";\n"
    "    }\n"))
 
-(define (default-nginx-config log-directory run-directory vhost-list)
+(define (nginx-config-file log-directory run-directory vhost-list)
   (plain-file "nginx.conf"
               (string-append
                "user nginx nginx;\n"
@@ -148,7 +157,7 @@ of index files."
 
 (define nginx-activation
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts)
      #~(begin
          (use-modules (guix build utils))
 
@@ -161,20 +170,20 @@ of index files."
          (mkdir-p (string-append #$run-directory "/proxy_temp"))
          (mkdir-p (string-append #$run-directory "/fastcgi_temp"))
          (mkdir-p (string-append #$run-directory "/uwsgi_temp"))
-         (mkdir-p (string-append #$run-directory "/scgi_temp"))
-         ;; Check configuration file syntax.
-         (system* (string-append #$nginx "/sbin/nginx")
-                  "-c" #$config-file "-t")))))
+         (mkdir-p (string-append #$run-directory "/scgi_temp"))))))
 
 (define nginx-shepherd-service
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts)
      (let* ((nginx-binary (file-append nginx "/sbin/nginx"))
             (nginx-action
              (lambda args
                #~(lambda _
                    (zero?
-                    (system* #$nginx-binary "-c" #$config-file #$@args))))))
+                    (system* #$nginx-binary "-c" #$(nginx-config-file
+                                                     log-directory
+                                                     run-directory
+                                                     vhosts) #$@args))))))
 
        ;; TODO: Add 'reload' action.
        (list (shepherd-service
@@ -192,21 +201,31 @@ of index files."
                        (service-extension activation-service-type
                                           nginx-activation)
                        (service-extension account-service-type
-                                          (const %nginx-accounts))))))
-
-(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)))
-  "Return a service that runs NGINX, the nginx web server.
-
-The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
-files in LOG-DIRECTORY, and stores temporary runtime files in RUN-DIRECTORY."
-  (service nginx-service-type
-           (nginx-configuration
-            (nginx nginx)
-            (log-directory log-directory)
-            (run-directory run-directory)
-            (file config-file))))
+                                          (const %nginx-accounts))))
+                (compose concatenate)
+                (extend (lambda (config additional-vhosts)
+                          (match config
+                            (($ <nginx-configuration> nginx log-directory
+                                                      run-directory vhosts)
+                             (nginx-configuration
+                               (nginx nginx)
+                               (log-directory log-directory)
+                               (run-directory run-directory)
+                               (vhosts (append vhosts additional-vhosts)))))))))
+
+(define (default-nginx-vhost-configuration root)
+ (lambda (_)
+   (list (nginx-vhost-configuration (https-port #f)
+                                    (root root)))))
+
+(define default-nginx-vhost-service-type
+  (let* ((index (plain-file "index.html"
+                  "<!DOCTYPE html><html><head></head><body></body></html>"))
+         (root (computed-file "default-nginx-vhost"
+                 #~(begin
+                     (mkdir #$output)
+                     (symlink #$index (string-append #$output "/index.html"))))))
+    (service-type (name 'default-nginx-vhost)
+                  (extensions
+                   (list (service-extension nginx-service-type
+                          (default-nginx-vhost-configuration root)))))))
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-10-26 19:45       ` Julien Lepiller
@ 2016-10-27 12:41         ` Ludovic Courtès
  2016-10-27 17:59           ` Julien Lepiller
  0 siblings, 1 reply; 29+ messages in thread
From: Ludovic Courtès @ 2016-10-27 12:41 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]

Hi!

Julien Lepiller <julien@lepiller.eu> skribis:

> On Mon, 24 Oct 2016 22:51:27 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:

[...]

>> >> I suppose we could make ‘nginx-service-type’ extensible (info
>> >> "(guix) Service Types and Services") so that people can write
>> >> services that define new vhosts?  
>> >
>> > You mean something like udev-service-type where you could extend it
>> > with a list of vhosts?  
>> 
>> Yes, exactly.  So for example one could write a service for some
>> high-level Web service that would in turn create an nginx vhost.
>> WDYT?
>
> Sounds great. I tried to work on it, but I'm stuck.
>
> I'm trying to implement a service that extends nginx-service-type with
> a vhost (default-nginx-vhost-service-type), and it creates a root
> directory for a default empty index file. Now I would like to pass the
> absolute path of this directory to the <nginx-vhost-configuration>, but
> it is too early for that. System configuration with this new service
> fails with:
>
> Backtrace:
> In guix/ui.scm:
> 1220: 19 [run-guix-command system "reconfigure" "/etc/config.scm"]

[...]

> ERROR: In procedure string-append:
> ERROR: In procedure string-append: Wrong type (expecting string):
> #<<computed-file> name: "default-nginx-vhost" gexp: #<gexp (begin
> (mkdir #<gexp-output out>) (symlink #<gexp-input #<<plain-file> name:
> "index.html" content: "[...]" references: ()>:out> (string-append
> #<gexp-output out> "/index.html"))) 5959270> options: (#:local-build?
> #t)>

I think the backtrace, ahem, very clearly shows what’s wrong.

(Suddenly I’m scared: what if this backtrace gave you the same feeling
that I have when I get a C++ compilation error?!)

> From 13748b6bfffef19080f4fa3bde2a6ae7d5c8d067 Mon Sep 17 00:00:00 2001
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Wed, 26 Oct 2016 21:33:34 +0200
> Subject: [PATCH] services: Make nginx-service-type extensible
>
> gnu/services/web.scm (nginx-service): Removed.
> gnu/services/web.scm (nginx-service-sytsem): Make extensible.
> gnu/services/web.scm (default-nginx-vhost-service-type): New variable.

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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3591 bytes --]

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 59e1e54..a319450 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -27,6 +27,7 @@
   #:use-module (gnu packages web)
   #:use-module (guix records)
   #:use-module (guix gexp)
+  #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
   #:export (nginx-configuration
             nginx-configuration?
@@ -67,7 +68,9 @@
   (nginx         nginx-configuration-nginx)         ;<package>
   (log-directory nginx-configuration-log-directory) ;string
   (run-directory nginx-configuration-run-directory) ;string
-  (file          nginx-configuration-file))         ;string | file-like
+  (file          nginx-configuration-file)          ;string | file-like
+  (vhosts        nginx-configuration-vhosts ;list of <nginx-vhost-configuration>
+                 (default (list (nginx-vhost-configuration)))))
 
 (define (config-domain-strings names)
  "Return a string denoting the nginx config representation of NAMES, a list
@@ -148,7 +151,8 @@ of index files."
 
 (define nginx-activation
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory
+                              config-file vhosts)
      #~(begin
          (use-modules (guix build utils))
 
@@ -164,7 +168,10 @@ of index files."
          (mkdir-p (string-append #$run-directory "/scgi_temp"))
          ;; Check configuration file syntax.
          (system* (string-append #$nginx "/sbin/nginx")
-                  "-c" #$config-file "-t")))))
+                  "-c" #$(or config-file
+                             (default-nginx-config log-directory
+                               run-directory vhosts))
+                  "-t")))))
 
 (define nginx-shepherd-service
   (match-lambda
@@ -192,14 +199,24 @@ of index files."
                        (service-extension activation-service-type
                                           nginx-activation)
                        (service-extension account-service-type
-                                          (const %nginx-accounts))))))
+                                          (const %nginx-accounts))))
+
+                ;; Concatenate the list of vhosts we're given
+                (compose concatenate)
+
+                ;; Append the list of vhosts we were passed to those already
+                ;; in the config.
+                (extend (lambda (config vhosts)
+                          (nginx-configuration
+                           (inherit config)
+                           (vhosts (append (nginx-configuration-vhosts config)
+                                           vhosts)))))))
 
 (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)))
+                        (config-file #f))
   "Return a service that runs NGINX, the nginx web server.
 
 The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
@@ -207,6 +224,7 @@ files in LOG-DIRECTORY, and stores temporary runtime files in RUN-DIRECTORY."
   (service nginx-service-type
            (nginx-configuration
             (nginx nginx)
+            (vhosts vhost-list)
             (log-directory log-directory)
             (run-directory run-directory)
             (file config-file))))

[-- Attachment #3: Type: text/plain, Size: 544 bytes --]


With that in place, it becomes possible to write another service that
provides additional vhosts, like:


  (define vh
    (nginx-vhost-configuration …))

  (define foo
    (service-type (name 'foo)
                  (extensions (list (service-extension nginx-service-type
                                                       (const (list vh)))))))

or simply:

  (simple-service 'my-extra-vhost nginx-service-type (list vh))

Does that make sense?

Of course feel free to start from the patch above!

HTH,
Ludo’.

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-10-27 12:41         ` Ludovic Courtès
@ 2016-10-27 17:59           ` Julien Lepiller
  2016-10-30 21:46             ` Ludovic Courtès
  0 siblings, 1 reply; 29+ messages in thread
From: Julien Lepiller @ 2016-10-27 17:59 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1564 bytes --]

On Thu, 27 Oct 2016 14:41:18 +0200
ludo@gnu.org (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:
> 

What do you think of this patch? I only had to add one (or) so the same
config file would be used during test and running (I hope it does not
create another one?).

Actually your patch didn't help me except for syntax simplification.
Your answer was not really helpful, 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.

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?

[-- Attachment #2: 0001-service-Make-nginx-service-extensible.patch --]
[-- Type: text/x-patch, Size: 6588 bytes --]

From 25a296057969a35b86ea7371577504c43bf96334 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
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
---
 doc/guix.texi        | 15 ++++++++++++++-
 gnu/services/web.scm | 39 +++++++++++++++++++++++++++++----------
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 1293b8b..b6e62b3 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10399,7 +10399,7 @@ The @code{(gnu services web)} module provides the following service:
        [#:log-directory ``/var/log/nginx''] @
        [#:run-directory ``/var/run/nginx''] @
        [#:vhost-list (list (nginx-vhost-configuration))] @
-       [#:config-file]
+       [#:config-file @code{#f}]
 
 Return a service that runs @var{nginx}, the nginx web server.
 
@@ -10415,6 +10415,19 @@ this to work, use the default value for @var{config-file}.
 
 @end deffn
 
+@deffn [Scheme Variable] nginx-service-type
+This is the type for the @code{nginx} web server.
+
+This service can be extended to add more vhosts than the default one.
+
+@example
+(simple-service 'my-extra-vhost nginx-service-type
+                (list (nginx-vhost-configuration (https-port #f)
+                                                 (root "/var/www/extra-website"))))
+@end example
+
+@end deffn
+
 @deftp {Data Type} nginx-vhost-configuration
 Data type representing the configuration of an nginx virtual host.
 This type has the following parameters:
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 59e1e54..0fb1a0b 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -28,6 +28,7 @@
   #:use-module (guix records)
   #:use-module (guix gexp)
   #:use-module (ice-9 match)
+  #:use-module (srfi srfi-1)
   #:export (nginx-configuration
             nginx-configuration?
             nginx-vhost-configuration
@@ -67,6 +68,7 @@
   (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>
   (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))
        (string-append "      ssl_certificate_key "
                       (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
        "")
@@ -148,7 +152,7 @@ of index files."
 
 (define nginx-activation
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts config-file)
      #~(begin
          (use-modules (guix build utils))
 
@@ -164,17 +168,25 @@ of index files."
          (mkdir-p (string-append #$run-directory "/scgi_temp"))
          ;; Check configuration file syntax.
          (system* (string-append #$nginx "/sbin/nginx")
-                  "-c" #$config-file "-t")))))
+                   "-c" #$(or config-file
+                              (default-nginx-config log-directory
+                                run-directory vhosts))
+                   "-t")))))
 
 (define nginx-shepherd-service
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts config-file)
      (let* ((nginx-binary (file-append nginx "/sbin/nginx"))
             (nginx-action
              (lambda args
                #~(lambda _
                    (zero?
-                    (system* #$nginx-binary "-c" #$config-file #$@args))))))
+                    (system* #$nginx-binary "-c" #$(or config-file
+                                                       (default-nginx-config
+                                                         log-directory
+                                                         run-directory
+                                                         vhosts))
+                             #$@args))))))
 
        ;; TODO: Add 'reload' action.
        (list (shepherd-service
@@ -192,14 +204,20 @@ of index files."
                        (service-extension activation-service-type
                                           nginx-activation)
                        (service-extension account-service-type
-                                          (const %nginx-accounts))))))
+                                          (const %nginx-accounts))))
+                (compose concatenate)
+                (extend (lambda (config vhosts)
+                          (nginx-configuration
+                            (inherit config)
+                            (vhosts (append (nginx-configuration-vhosts config)
+                                            vhosts)))))))
 
 (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))))
+                        (config-file #f))
   "Return a service that runs NGINX, the nginx web server.
 
 The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
@@ -209,4 +227,5 @@ files in LOG-DIRECTORY, and stores temporary runtime files in RUN-DIRECTORY."
             (nginx nginx)
             (log-directory log-directory)
             (run-directory run-directory)
+            (vhosts vhost-list)
             (file config-file))))
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-10-27 17:59           ` Julien Lepiller
@ 2016-10-30 21:46             ` Ludovic Courtès
  2016-11-02  8:22               ` Hartmut Goebel
  0 siblings, 1 reply; 29+ messages in thread
From: Ludovic Courtès @ 2016-10-30 21:46 UTC (permalink / raw)
  To: Julien Lepiller; +Cc: guix-devel

Julien Lepiller <julien@lepiller.eu> skribis:

> On Thu, 27 Oct 2016 14:41:18 +0200
> ludo@gnu.org (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 <julien@lepiller.eu>
> 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

[...]

> +@deffn [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’.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-10-30 21:46             ` Ludovic Courtès
@ 2016-11-02  8:22               ` Hartmut Goebel
  2016-11-03 14:54                 ` Ludovic Courtès
  0 siblings, 1 reply; 29+ messages in thread
From: Hartmut Goebel @ 2016-11-02  8:22 UTC (permalink / raw)
  To: guix-devel

Hi,
> 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" …)))
Related question: I've but the content of some example website into a
package. How can I use that package's directory as "root"? I tried
these, but none worked:

   (root (package-file demo-website))
   (root #$(file-append demo-website "/"))
   (root (assoc-ref demo-website "out"))
   (root #$demo-website)

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-02  8:22               ` Hartmut Goebel
@ 2016-11-03 14:54                 ` Ludovic Courtès
  2016-11-03 22:38                   ` Hartmut Goebel
  0 siblings, 1 reply; 29+ messages in thread
From: Ludovic Courtès @ 2016-11-03 14:54 UTC (permalink / raw)
  To: Hartmut Goebel; +Cc: guix-devel

Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:

> Hi,
>> 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" …)))

Oops, there shouldn’t be a #$ here since we’re not inside #~.

> Related question: I've but the content of some example website into a
> package. How can I use that package's directory as "root"? I tried
> these, but none worked:
>
>    (root (package-file demo-website))
>    (root #$(file-append demo-website "/"))
>    (root (assoc-ref demo-website "out"))
>    (root #$demo-website)

Assuming ‘demo-website’ is bound to a package object, you should be able
to do:

  (root (file-append demo-website "/"))

or simply:

  (root demo-website)

The #$ above are invalid since #$ is meant to be used within #~.
Likewise, the first argument to ‘assoc-ref’ must be an association list,
which ‘demo-website’ is not.  As for ‘package-file’, it’s not enough: it
returns the file name of the package, but doesn’t make the package a
dependency of the vhost config.

HTH!

Ludo’.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-03 14:54                 ` Ludovic Courtès
@ 2016-11-03 22:38                   ` Hartmut Goebel
  2016-11-04 13:21                     ` Ludovic Courtès
  0 siblings, 1 reply; 29+ messages in thread
From: Hartmut Goebel @ 2016-11-03 22:38 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel

Am 03.11.2016 um 15:54 schrieb Ludovic Courtès:
> Assuming ‘demo-website’ is bound to a package object, you should be able
> to do:
>
>   (root (file-append demo-website "/"))
>
> or simply:
>
>   (root demo-website)

Hmm, this does not work for me:


ice-9/boot-9.scm:702:27: In procedure map:
ice-9/boot-9.scm:702:27: In procedure string-append: Wrong type
(expecting string): #<package taler-demo-landing-page@0.0.0-1.105bea68
./taler/packages/demo.scm:100 3e2a600>


I tried with current master branch.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-03 22:38                   ` Hartmut Goebel
@ 2016-11-04 13:21                     ` Ludovic Courtès
  2016-11-04 18:01                       ` Julien Lepiller
  0 siblings, 1 reply; 29+ messages in thread
From: Ludovic Courtès @ 2016-11-04 13:21 UTC (permalink / raw)
  To: Hartmut Goebel; +Cc: guix-devel

Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:

> Am 03.11.2016 um 15:54 schrieb Ludovic Courtès:
>> Assuming ‘demo-website’ is bound to a package object, you should be able
>> to do:
>>
>>   (root (file-append demo-website "/"))
>>
>> or simply:
>>
>>   (root demo-website)
>
> Hmm, this does not work for me:
>
>
> ice-9/boot-9.scm:702:27: In procedure map:
> ice-9/boot-9.scm:702:27: In procedure string-append: Wrong type
> (expecting string): #<package taler-demo-landing-page@0.0.0-1.105bea68
> ./taler/packages/demo.scm:100 3e2a600>
>
>
> I tried with current master branch.

Doh!  Indeed, ‘default-nginx-vhost-config’ must be rewritten to do
something like:

  #~(string-append #$foo #$bar …)

instead of:

  (string-append foo bar …)

More precisely, it could ‘string-append’ all the string literals yet use
a #~(string-append …) gexp for ‘root’.

Julien?  :-)

Ludo’.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-04 13:21                     ` Ludovic Courtès
@ 2016-11-04 18:01                       ` Julien Lepiller
  2016-11-04 21:28                         ` Hartmut Goebel
  0 siblings, 1 reply; 29+ messages in thread
From: Julien Lepiller @ 2016-11-04 18:01 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

On Fri, 04 Nov 2016 14:21:43 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:
> 
> > Am 03.11.2016 um 15:54 schrieb Ludovic Courtès:  
> >> Assuming ‘demo-website’ is bound to a package object, you should
> >> be able to do:
> >>
> >>   (root (file-append demo-website "/"))
> >>
> >> or simply:
> >>
> >>   (root demo-website)  
> >
> > Hmm, this does not work for me:
> >
> >
> > ice-9/boot-9.scm:702:27: In procedure map:
> > ice-9/boot-9.scm:702:27: In procedure string-append: Wrong type
> > (expecting string): #<package
> > taler-demo-landing-page@0.0.0-1.105bea68 ./taler/packages/demo.scm:100
> > 3e2a600>
> >
> >
> > I tried with current master branch.  
> 
> Doh!  Indeed, ‘default-nginx-vhost-config’ must be rewritten to do
> something like:
> 
>   #~(string-append #$foo #$bar …)
> 
> instead of:
> 
>   (string-append foo bar …)
> 
> More precisely, it could ‘string-append’ all the string literals yet
> use a #~(string-append …) gexp for ‘root’.
> 
> Julien?  :-)

I think it's good now. There are three patches:
1. making the service extensible (mostly what you wrote last time)
   I used an empty list as the default list of vhosts.
2. a patch to fix an issue when multiple index files and server name
   were specified (there were no space in between)
3. a patch to allow any configuration option to come from the store (I
   couldn't write something specific to the root option, but I think
   it's fine this way). I tested with the example I added in the
   documentation, and it works fine.

I hope I did it right :)

> 
> Ludo’.


[-- Attachment #2: 0001-Make-nginx-service-extensible.patch --]
[-- Type: text/x-patch, Size: 5659 bytes --]

From 55ccf3041bff562345816b500ee5a2aeda9e3226 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Thu, 3 Nov 2016 18:11:01 +0100
Subject: [PATCH 1/3] Make nginx-service extensible

* gnu/services/web.scm (nginx-service-type): Make extensible.
---
 doc/guix.texi        | 15 ++++++++++++++-
 gnu/services/web.scm | 33 +++++++++++++++++++++++++--------
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 1075a7e..5c42140 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10401,7 +10401,7 @@ The @code{(gnu services web)} module provides the following service:
        [#:log-directory ``/var/log/nginx''] @
        [#:run-directory ``/var/run/nginx''] @
        [#:vhost-list (list (nginx-vhost-configuration))] @
-       [#:config-file]
+       [#:config-file @code{#f}]
 
 Return a service that runs @var{nginx}, the nginx web server.
 
@@ -10417,6 +10417,19 @@ this to work, use the default value for @var{config-file}.
 
 @end deffn
 
+@deffn {Scheme Variable} nginx-service-type
+This is the type for the nginx web server.
+
+This service can be extended to add more vhosts than the default one.
+
+@example
+(simple-service 'my-extra-vhost nginx-service-type
+                (list (nginx-vhost-configuration (https-port #f)
+                                                 (root "/var/www/extra-website"))))
+@end example
+
+@end deffn
+
 @deftp {Data Type} nginx-vhost-configuration
 Data type representing the configuration of an nginx virtual host.
 This type has the following parameters:
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 59e1e54..50f83f3 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -28,6 +28,7 @@
   #:use-module (guix records)
   #:use-module (guix gexp)
   #:use-module (ice-9 match)
+  #:use-module (srfi srfi-1)
   #:export (nginx-configuration
             nginx-configuration?
             nginx-vhost-configuration
@@ -67,6 +68,8 @@
   (nginx         nginx-configuration-nginx)         ;<package>
   (log-directory nginx-configuration-log-directory) ;string
   (run-directory nginx-configuration-run-directory) ;string
+  (vhosts        nginx-configuration-vhosts
+                 (default '()))   ;list of <nginx-vhost-configuration>
   (file          nginx-configuration-file))         ;string | file-like
 
 (define (config-domain-strings names)
@@ -148,7 +151,7 @@ of index files."
 
 (define nginx-activation
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts config-file)
      #~(begin
          (use-modules (guix build utils))
 
@@ -164,17 +167,25 @@ of index files."
          (mkdir-p (string-append #$run-directory "/scgi_temp"))
          ;; Check configuration file syntax.
          (system* (string-append #$nginx "/sbin/nginx")
-                  "-c" #$config-file "-t")))))
+                   "-c" #$(or config-file
+                              (default-nginx-config log-directory
+                                run-directory vhosts))
+                   "-t")))))
 
 (define nginx-shepherd-service
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts config-file)
      (let* ((nginx-binary (file-append nginx "/sbin/nginx"))
             (nginx-action
              (lambda args
                #~(lambda _
                    (zero?
-                    (system* #$nginx-binary "-c" #$config-file #$@args))))))
+                    (system* #$nginx-binary "-c" #$(or config-file
+                                                       (default-nginx-config
+                                                         log-directory
+                                                         run-directory
+                                                         vhosts))
+                             #$@args))))))
 
        ;; TODO: Add 'reload' action.
        (list (shepherd-service
@@ -192,14 +203,19 @@ of index files."
                        (service-extension activation-service-type
                                           nginx-activation)
                        (service-extension account-service-type
-                                          (const %nginx-accounts))))))
+                                          (const %nginx-accounts))))
+                (compose concatenate)
+                (extend (lambda (config vhosts)
+                          (nginx-configuration
+                            (inherit config)
+                            (vhosts (append (nginx-configuration-vhosts config)
+                                            vhosts)))))))
 
 (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 '())
+                        (config-file #f))
   "Return a service that runs NGINX, the nginx web server.
 
 The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
@@ -209,4 +225,5 @@ files in LOG-DIRECTORY, and stores temporary runtime files in RUN-DIRECTORY."
             (nginx nginx)
             (log-directory log-directory)
             (run-directory run-directory)
+            (vhosts vhost-list)
             (file config-file))))
-- 
2.10.2


[-- Attachment #3: 0002-service-Fix-multiple-index-and-server-name.patch --]
[-- Type: text/x-patch, Size: 1077 bytes --]

From e5e123a2ec512a7aed2f1f18ed0f7d2b86ba3a9b Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Thu, 3 Nov 2016 18:11:43 +0100
Subject: [PATCH 2/3] service: Fix multiple index and server name

* gnu/services/web.scm (config-index-strings and config-vhost-strings):
   Add a space between entries.
---
 gnu/services/web.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 50f83f3..e36d284 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -77,8 +77,8 @@
 of domain names."
  (string-concatenate
   (map (match-lambda
-        ('default "_")
-        ((? string? str) str))
+        ('default "_ ")
+        ((? string? str) (string-append str " ")))
        names)))
 
 (define (config-index-strings names)
@@ -86,7 +86,7 @@ of domain names."
 of index files."
  (string-concatenate
   (map (match-lambda
-        ((? string? str) str))
+        ((? string? str) (string-append str " ")))
        names)))
 
 (define (default-nginx-vhost-config vhost)
-- 
2.10.2


[-- Attachment #4: 0003-services-Accept-gexps-as-nginx-configuration-value.patch --]
[-- Type: text/x-patch, Size: 6458 bytes --]

From a1305d1280642bcff46fc736717f119bd4999e74 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Fri, 4 Nov 2016 17:36:21 +0100
Subject: [PATCH 3/3] services: Accept gexps as nginx configuration value.

* gnu/services/web.scm (default-nginx-config): Use computed-file instead of plain-file.
---
 doc/guix.texi        |  8 +++++
 gnu/services/web.scm | 96 +++++++++++++++++++++++++++-------------------------
 2 files changed, 58 insertions(+), 46 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 5c42140..2b07bc4 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10428,6 +10428,14 @@ This service can be extended to add more vhosts than the default one.
                                                  (root "/var/www/extra-website"))))
 @end example
 
+or if you want to make a package available, you can write something like:
+
+@example
+(simple-service 'gtk-doc-vhost nginx-service-type
+                (list (nginx-vhost-configuration 
+                       (root (file-append gtk+2 "/share/gtk-doc/html/gdk2/")))))
+@end example
+
 @end deffn
 
 @deftp {Data Type} nginx-vhost-configuration
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index e36d284..df6e680 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -90,54 +90,58 @@ of index files."
        names)))
 
 (define (default-nginx-vhost-config vhost)
-  (string-append
-   "    server {\n"
-   (if (nginx-vhost-configuration-http-port vhost)
-       (string-append "      listen "
-                      (number->string (nginx-vhost-configuration-http-port vhost))
-                      ";\n")
-       "")
-   (if (nginx-vhost-configuration-https-port vhost)
-       (string-append "      listen "
-                      (number->string (nginx-vhost-configuration-https-port vhost))
-                      " ssl;\n")
-       "")
-   "      server_name " (config-domain-strings
-                         (nginx-vhost-configuration-server-name vhost))
-                        ";\n"
-   (if (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)
-       (string-append "      ssl_certificate_key "
-                      (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
-       "")
-   "      root " (nginx-vhost-configuration-root vhost) ";\n"
-   "      index " (config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
-   "      server_tokens " (if (nginx-vhost-configuration-server-tokens? vhost)
-                              "on" "off") ";\n"
-   "    }\n"))
+  #~(string-append
+     "    server {\n"
+     #$(if (nginx-vhost-configuration-http-port vhost)
+           (string-append "      listen "
+                          (number->string (nginx-vhost-configuration-http-port vhost))
+                          ";\n")
+           "")
+     #$(if (nginx-vhost-configuration-https-port vhost)
+          (string-append "      listen "
+                          (number->string (nginx-vhost-configuration-https-port vhost))
+                          " ssl;\n")
+           "")
+     "      server_name " #$(config-domain-strings
+                             (nginx-vhost-configuration-server-name vhost))
+                          ";\n"
+     #$(if (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)
+           (string-append "      ssl_certificate_key "
+                          (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
+           "")
+     "      root " #$(nginx-vhost-configuration-root vhost) ";\n"
+     "      index " #$(config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
+     "      server_tokens " #$(if (nginx-vhost-configuration-server-tokens? vhost)
+                                  "on" "off") ";\n"
+     "    }\n"))
 
 (define (default-nginx-config log-directory run-directory vhost-list)
-  (plain-file "nginx.conf"
-              (string-append
-               "user nginx nginx;\n"
-               "pid " run-directory "/pid;\n"
-               "error_log " log-directory "/error.log info;\n"
-               "http {\n"
-               "    client_body_temp_path " run-directory "/client_body_temp;\n"
-               "    proxy_temp_path " run-directory "/proxy_temp;\n"
-               "    fastcgi_temp_path " run-directory "/fastcgi_temp;\n"
-               "    uwsgi_temp_path " run-directory "/uwsgi_temp;\n"
-               "    scgi_temp_path " run-directory "/scgi_temp;\n"
-               "    access_log " log-directory "/access.log;\n"
-               (let ((http (map default-nginx-vhost-config vhost-list)))
-                 (do ((http http (cdr http))
-                      (block "" (string-append (car http) "\n" block )))
-                     ((null? http) block)))
-               "}\n"
-               "events {}\n")))
+  (computed-file
+    "nginx.conf"
+    #~(call-with-output-file #$output
+        (lambda (port)
+          (format port
+                  (string-append
+                     "user nginx nginx;\n"
+                     "pid " #$run-directory "/pid;\n"
+                     "error_log " #$log-directory "/error.log info;\n"
+                     "http {\n"
+                     "    client_body_temp_path " #$run-directory "/client_body_temp;\n"
+                     "    proxy_temp_path " #$run-directory "/proxy_temp;\n"
+                     "    fastcgi_temp_path " #$run-directory "/fastcgi_temp;\n"
+                     "    uwsgi_temp_path " #$run-directory "/uwsgi_temp;\n"
+                     "    scgi_temp_path " #$run-directory "/scgi_temp;\n"
+                     "    access_log " #$log-directory "/access.log;\n"
+                     #$(let ((http (map default-nginx-vhost-config vhost-list)))
+                         (do ((http http (cdr http))
+                              (block "" #~(string-append #$(car http) "\n" #$block )))
+                             ((null? http) block)))
+                     "}\n"
+                     "events {}\n"))))))
 
 (define %nginx-accounts
   (list (user-group (name "nginx") (system? #t))
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-04 18:01                       ` Julien Lepiller
@ 2016-11-04 21:28                         ` Hartmut Goebel
  2016-11-04 22:12                           ` Julien Lepiller
  0 siblings, 1 reply; 29+ messages in thread
From: Hartmut Goebel @ 2016-11-04 21:28 UTC (permalink / raw)
  To: guix-devel

Hi Julien

thanks for these patches. I applied them to current master, but I did
not work out how to use the new features. I'd appreciate a more complete
example.

0001-Make-nginx-service-extensible.patch
> +@deffn {Scheme Variable} nginx-service-type +This is the type for the
> nginx web server. + +This service can be extended to add more vhosts
> than the default one. + +@example +(simple-service 'my-extra-vhost
> nginx-service-type + (list (nginx-vhost-configuration (https-port #f)
> + (root "/var/www/extra-website"))))

I do not understand this. Why would I want to to this? Why not just add
the vhost declaration when defining the service in the system
declaration? Is this for vhost-types which have preset values for some
configuration files? Then a different example might be useful, and/or
some more explanation.

It's a bit more obvious with patch 3, but even that example is not that
obvious to me and could use some more explanation.

I've build a simple service like the gtk-doc-vhost in your second
example, but it not manage to make it work: First I got "error: no
target of type 'nginx'", so I re-added "(nginx-service)" to the system
declaration. Then I got

    gnu/services/web.scm:118:34: In procedure default-nginx-vhost-config:
    gnu/services/web.scm:118:34: In procedure struct_vtable: Wrong type
    argument in position 1 (expecting struct):
    (nginx-vhost-configuration (root taler-landing-page))

Si I'd really appreciate seeing a more complete example either in the
documentation of in gn/systems/examples/

> (system* (string-append #$nginx "/sbin/nginx") - "-c" #$config-file
> "-t"))))) + "-c" #$(or config-file + (default-nginx-config
> log-directory + run-directory vhosts)) + "-t")))))

Nitpicking: I'd mode the "-t" to the front, since this is the important
difference.

> (define nginx-shepherd-service (match-lambda - (($
> <nginx-configuration> nginx log-directory run-directory config-file) +
> (($ <nginx-configuration> nginx log-directory run-directory vhosts
> config-file) (let* ((nginx-binary (file-append nginx "/sbin/nginx"))
> (nginx-action (lambda args #~(lambda _ (zero? - (system*
> #$nginx-binary "-c" #$config-file #$@args)))))) + (system*
> #$nginx-binary "-c" #$(or config-file + (default-nginx-config +
> log-directory + run-directory + vhosts))+ #$@args))))))

To avoid duplicate code I suggest moving the "#$(or …)" part into a
private function - if this is possible.



0003-services-Accept-gexps-as-nginx-configuration-value.patch
> +@example +(simple-service 'gtk-doc-vhost nginx-service-type + (list
> (nginx-vhost-configuration

git am says: trailing whitespace

> + " root " #$(nginx-vhost-configuration-root vhost) ";\n" + " index "
> #$(config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
> + " server_tokens " #$(if (nginx-vhost-configuration-server-tokens?
> vhost) + "on" "off") ";\n"

Could you please (maybe in another patch) add a way to include
additional config lines? Both for the main server and each vhost.I'm
gioing to need this for adding locations, backends and such.

-- Regards Hartmut Goebel | Hartmut Goebel |
h.goebel@crazy-compilers.com | | www.crazy-compilers.com | compilers
which you thought are impossible |

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-04 21:28                         ` Hartmut Goebel
@ 2016-11-04 22:12                           ` Julien Lepiller
  2016-11-04 22:34                             ` Hartmut Goebel
  2016-11-04 22:58                             ` Hartmut Goebel
  0 siblings, 2 replies; 29+ messages in thread
From: Julien Lepiller @ 2016-11-04 22:12 UTC (permalink / raw)
  To: guix-devel

On Fri, 4 Nov 2016 22:28:07 +0100
Hartmut Goebel <h.goebel@crazy-compilers.com> wrote:

> Hi Julien
> 
> thanks for these patches. I applied them to current master, but I did
> not work out how to use the new features. I'd appreciate a more
> complete example.
> 
> 0001-Make-nginx-service-extensible.patch
> > +@deffn {Scheme Variable} nginx-service-type +This is the type for
> > the nginx web server. + +This service can be extended to add more
> > vhosts than the default one. + +@example +(simple-service
> > 'my-extra-vhost nginx-service-type + (list
> > (nginx-vhost-configuration (https-port #f)
> > + (root "/var/www/extra-website"))))  
> 
> I do not understand this. Why would I want to to this? Why not just
> add the vhost declaration when defining the service in the system
> declaration? Is this for vhost-types which have preset values for some
> configuration files? Then a different example might be useful, and/or
> some more explanation.

There is no real difference between the two. This is mostly usefull for
implementing new web services in the distro, but it is also good for
separating nginx configuration from web service configuration. I guess
that's a matter of taste.

> 
> It's a bit more obvious with patch 3, but even that example is not
> that obvious to me and could use some more explanation.
> 
> I've build a simple service like the gtk-doc-vhost in your second
> example, but it not manage to make it work: First I got "error: no
> target of type 'nginx'", so I re-added "(nginx-service)" to the system
> declaration. Then I got

Yes, the example adds a service that uses the nginx service, but does
not create it.

> 
>     gnu/services/web.scm:118:34: In procedure
> default-nginx-vhost-config: gnu/services/web.scm:118:34: In procedure
> struct_vtable: Wrong type argument in position 1 (expecting struct):
>     (nginx-vhost-configuration (root taler-landing-page))

What is your configuration exactly, and what is taler-landing-page?

> 
> Si I'd really appreciate seeing a more complete example either in the
> documentation of in gn/systems/examples/
> 
> > (system* (string-append #$nginx "/sbin/nginx") - "-c" #$config-file
> > "-t"))))) + "-c" #$(or config-file + (default-nginx-config
> > log-directory + run-directory vhosts)) + "-t")))))  
> 
> Nitpicking: I'd mode the "-t" to the front, since this is the
> important difference.
> 
> > (define nginx-shepherd-service (match-lambda - (($
> > <nginx-configuration> nginx log-directory run-directory
> > config-file) + (($ <nginx-configuration> nginx log-directory
> > run-directory vhosts config-file) (let* ((nginx-binary (file-append
> > nginx "/sbin/nginx")) (nginx-action (lambda args #~(lambda _ (zero?
> > - (system* #$nginx-binary "-c" #$config-file #$@args)))))) +
> > (system* #$nginx-binary "-c" #$(or config-file +
> > (default-nginx-config + log-directory + run-directory + vhosts))+
> > #$@args))))))  
> 
> To avoid duplicate code I suggest moving the "#$(or …)" part into a
> private function - if this is possible.
> 
> 
> 
> 0003-services-Accept-gexps-as-nginx-configuration-value.patch
> > +@example +(simple-service 'gtk-doc-vhost nginx-service-type + (list
> > (nginx-vhost-configuration  
> 
> git am says: trailing whitespace
> 
> > + " root " #$(nginx-vhost-configuration-root vhost) ";\n" + " index
> > " #$(config-index-strings (nginx-vhost-configuration-index vhost))
> > ";\n"
> > + " server_tokens " #$(if (nginx-vhost-configuration-server-tokens?
> > vhost) + "on" "off") ";\n"  
> 
> Could you please (maybe in another patch) add a way to include
> additional config lines? Both for the main server and each vhost.I'm
> gioing to need this for adding locations, backends and such.

I'd like to get these patches accepted first, but I'm already working
on adding more configuration lines.

Thanks for your review, I will fix that and come with a new patchset
quickly (with better documentation).

> 
> -- Regards Hartmut Goebel | Hartmut Goebel |
> h.goebel@crazy-compilers.com | | www.crazy-compilers.com | compilers
> which you thought are impossible |
> 
> 
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-04 22:12                           ` Julien Lepiller
@ 2016-11-04 22:34                             ` Hartmut Goebel
  2016-11-06 11:11                               ` Julien Lepiller
  2016-11-04 22:58                             ` Hartmut Goebel
  1 sibling, 1 reply; 29+ messages in thread
From: Hartmut Goebel @ 2016-11-04 22:34 UTC (permalink / raw)
  To: guix-devel

Am 04.11.2016 um 23:12 schrieb Julien Lepiller:
>> >     gnu/services/web.scm:118:34: In procedure
>> > default-nginx-vhost-config: gnu/services/web.scm:118:34: In procedure
>> > struct_vtable: Wrong type argument in position 1 (expecting struct):
>> >     (nginx-vhost-configuration (root taler-landing-page))
> What is your configuration exactly, and what is taler-landing-page?
>

The configuration is like in your gtk-doc-vhost, except that I'm not
using gtk+2 and omitting the "string-append" for the path.

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-04 22:12                           ` Julien Lepiller
  2016-11-04 22:34                             ` Hartmut Goebel
@ 2016-11-04 22:58                             ` Hartmut Goebel
  2016-11-06 12:18                               ` Tobias Geerinckx-Rice
  2016-11-06 17:33                               ` Ludovic Courtès
  1 sibling, 2 replies; 29+ messages in thread
From: Hartmut Goebel @ 2016-11-04 22:58 UTC (permalink / raw)
  To: guix-devel

Am 04.11.2016 um 23:12 schrieb Julien Lepiller:
> I'd like to get these patches accepted first, but I'm already working
> on adding more configuration lines.

BTW: I just started typing down a simple "location" record type, and
started to think if these record types are really the way to go for the
configuration.

In nginx AFAIK you can define many options on the server level, on the
vhost level and per location. Additionally locations can be nested. And
some configuration options are quite complex, e.g. "listen" [1] and
maybe worth their own type. So we'll create a lot of code for both the
record types and for converting them to config-file text.

In the long run, we will get maintenance problems, I'm afraid, if we are
only allow options known to us and written into the service. On each
update we'd need to check if any option was added.

So IMHO the recard types are not the way to go. We need a more flexible
way, maybe like used for XML (see default-build.xml in
guix/build/ant-build-system.scm), but with support for default values.
Unfortunatly I'm no scheme hacker and can not propose any solution for
this :-(

[1] https://nginx.org/en/docs/http/ngx_http_core_module.html#listen

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-04 22:34                             ` Hartmut Goebel
@ 2016-11-06 11:11                               ` Julien Lepiller
  0 siblings, 0 replies; 29+ messages in thread
From: Julien Lepiller @ 2016-11-06 11:11 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2628 bytes --]

On Fri, 4 Nov 2016 23:34:31 +0100
Hartmut Goebel <h.goebel@crazy-compilers.com> wrote:

> Am 04.11.2016 um 23:12 schrieb Julien Lepiller:
> >> >     gnu/services/web.scm:118:34: In procedure
> >> > default-nginx-vhost-config: gnu/services/web.scm:118:34: In
> >> > procedure struct_vtable: Wrong type argument in position 1
> >> > (expecting struct): (nginx-vhost-configuration (root
> >> > taler-landing-page))  
> > What is your configuration exactly, and what is taler-landing-page?
> >  
> 
> The configuration is like in your gtk-doc-vhost, except that I'm not
> using gtk+2 and omitting the "string-append" for the path.
> 

I attached new patches that add more documentation (I hope it helps).

Using this configuration works for me:

(nginx-service)
(service (service-type
           (name 'foo)
           (extensions
             (list (service-extension
                     nginx-service-type
                     (const (list
                             ;; a vhost that actually serves html files
                              (nginx-vhost-configuration
                              (https-port #f)
                              (ssl-certificate #f)
                              (ssl-certificate-key #f)
                              (root #~(string-append #$cups
                                         "/share/doc/cups/ja")))
                             ;; just to show how to use a package
                             ;; directly
                             (nginx-vhost-configuration
                              (server-name (list "help"))
                              (https-port #f)
                              (ssl-certificate #f)
                              (ssl-certificate-key #f)
                              (root cups)))))))) #t)

It would work with simple-service too.

It generates this configuration file for nginx:

user nginx nginx;
pid /var/run/nginx/pid;
error_log /var/log/nginx/error.log info;
http {
    client_body_temp_path /var/run/nginx/client_body_temp;
    proxy_temp_path /var/run/nginx/proxy_temp;
    fastcgi_temp_path /var/run/nginx/fastcgi_temp;
    uwsgi_temp_path /var/run/nginx/uwsgi_temp;
    scgi_temp_path /var/run/nginx/scgi_temp;
    access_log /var/log/nginx/access.log;
    server {
      listen 80;
      server_name help ;
      root /gnu/store/l6n860s5fmaxhwcx17mjrfw4wcqx8xy8-cups-2.1.0;
      index index.html ;
      server_tokens off;
    }

    server {
      listen 80;
      server_name _ ;
      root /gnu/store/l6n860s5fmaxhwcx17mjrfw4wcqx8xy8-cups-2.1.0/share/doc/cups/ja;
      index index.html ;
      server_tokens off;
    }

}
events {}


[-- Attachment #2: 0001-Make-nginx-service-extensible.patch --]
[-- Type: text/x-patch, Size: 6433 bytes --]

From 0478b5720a8fcc71d682ccf62bae839a71453b86 Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Sun, 6 Nov 2016 10:26:45 +0100
Subject: [PATCH 1/3] Make nginx-service extensible

* gnu/services/web.scm (nginx-service-type): Make extensible.
---
 doc/guix.texi        | 38 ++++++++++++++++++++++++++++++++++++--
 gnu/services/web.scm | 33 +++++++++++++++++++++++++--------
 2 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 1075a7e..4b60a4a 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10400,8 +10400,8 @@ The @code{(gnu services web)} module provides the following service:
 @deffn {Scheme Procedure} nginx-service [#:nginx nginx] @
        [#:log-directory ``/var/log/nginx''] @
        [#:run-directory ``/var/run/nginx''] @
-       [#:vhost-list (list (nginx-vhost-configuration))] @
-       [#:config-file]
+       [#:vhost-list '()] @
+       [#:config-file @code{#f}]
 
 Return a service that runs @var{nginx}, the nginx web server.
 
@@ -10417,6 +10417,40 @@ this to work, use the default value for @var{config-file}.
 
 @end deffn
 
+@deffn {Scheme Variable} nginx-service-type
+This is the type for the nginx web server.
+
+This service can be extended to add more vhosts than the default ones.  To add
+@dfn{virtual hosts}, you can either put them in the @var{vhost-lists} of the
+@code{nginx-service} procedure, or create a separate service for all or parts
+of them.  For example:
+
+@example
+(define vh nginx-vhost-configuration
+           (root "/var/www/extra-website"))
+(nginx-service)
+(simple-service 'foo nginx-service-type
+                (list vh)) 
+@end example
+
+Is a service that adds a new @dfn{virtual host} to the list of existing ones.
+You can add as many such services as you want.
+
+You can also use a more complete service definition:
+
+@example
+(define foo
+  (service-type (name 'foo)
+                (extensions (list (service-extension nginx-service-type
+                                                     (const (list vh)))))))
+(nginx-service)
+(service foo #t)
+@end example
+
+Note that @code{(nginx-service)} needs to be defined only once.
+
+@end deffn
+
 @deftp {Data Type} nginx-vhost-configuration
 Data type representing the configuration of an nginx virtual host.
 This type has the following parameters:
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 59e1e54..50f83f3 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -28,6 +28,7 @@
   #:use-module (guix records)
   #:use-module (guix gexp)
   #:use-module (ice-9 match)
+  #:use-module (srfi srfi-1)
   #:export (nginx-configuration
             nginx-configuration?
             nginx-vhost-configuration
@@ -67,6 +68,8 @@
   (nginx         nginx-configuration-nginx)         ;<package>
   (log-directory nginx-configuration-log-directory) ;string
   (run-directory nginx-configuration-run-directory) ;string
+  (vhosts        nginx-configuration-vhosts
+                 (default '()))   ;list of <nginx-vhost-configuration>
   (file          nginx-configuration-file))         ;string | file-like
 
 (define (config-domain-strings names)
@@ -148,7 +151,7 @@ of index files."
 
 (define nginx-activation
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts config-file)
      #~(begin
          (use-modules (guix build utils))
 
@@ -164,17 +167,25 @@ of index files."
          (mkdir-p (string-append #$run-directory "/scgi_temp"))
          ;; Check configuration file syntax.
          (system* (string-append #$nginx "/sbin/nginx")
-                  "-c" #$config-file "-t")))))
+                   "-c" #$(or config-file
+                              (default-nginx-config log-directory
+                                run-directory vhosts))
+                   "-t")))))
 
 (define nginx-shepherd-service
   (match-lambda
-    (($ <nginx-configuration> nginx log-directory run-directory config-file)
+    (($ <nginx-configuration> nginx log-directory run-directory vhosts config-file)
      (let* ((nginx-binary (file-append nginx "/sbin/nginx"))
             (nginx-action
              (lambda args
                #~(lambda _
                    (zero?
-                    (system* #$nginx-binary "-c" #$config-file #$@args))))))
+                    (system* #$nginx-binary "-c" #$(or config-file
+                                                       (default-nginx-config
+                                                         log-directory
+                                                         run-directory
+                                                         vhosts))
+                             #$@args))))))
 
        ;; TODO: Add 'reload' action.
        (list (shepherd-service
@@ -192,14 +203,19 @@ of index files."
                        (service-extension activation-service-type
                                           nginx-activation)
                        (service-extension account-service-type
-                                          (const %nginx-accounts))))))
+                                          (const %nginx-accounts))))
+                (compose concatenate)
+                (extend (lambda (config vhosts)
+                          (nginx-configuration
+                            (inherit config)
+                            (vhosts (append (nginx-configuration-vhosts config)
+                                            vhosts)))))))
 
 (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 '())
+                        (config-file #f))
   "Return a service that runs NGINX, the nginx web server.
 
 The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
@@ -209,4 +225,5 @@ files in LOG-DIRECTORY, and stores temporary runtime files in RUN-DIRECTORY."
             (nginx nginx)
             (log-directory log-directory)
             (run-directory run-directory)
+            (vhosts vhost-list)
             (file config-file))))
-- 
2.10.2


[-- Attachment #3: 0002-service-Fix-multiple-nginx-index-and-server-name.patch --]
[-- Type: text/x-patch, Size: 1083 bytes --]

From 3f6ecc72032affa3e6ba1e704f13ac6a20b4f4ed Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Sun, 6 Nov 2016 10:52:34 +0100
Subject: [PATCH 2/3] service: Fix multiple nginx index and server name

* gnu/services/web.scm (config-index strings and config-vhost-strings):
   Add a space between entries.
---
 gnu/services/web.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 50f83f3..e36d284 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -77,8 +77,8 @@
 of domain names."
  (string-concatenate
   (map (match-lambda
-        ('default "_")
-        ((? string? str) str))
+        ('default "_ ")
+        ((? string? str) (string-append str " ")))
        names)))
 
 (define (config-index-strings names)
@@ -86,7 +86,7 @@ of domain names."
 of index files."
  (string-concatenate
   (map (match-lambda
-        ((? string? str) str))
+        ((? string? str) (string-append str " ")))
        names)))
 
 (define (default-nginx-vhost-config vhost)
-- 
2.10.2


[-- Attachment #4: 0003-services-Accept-gexps-as-nginx-configuration-value.patch --]
[-- Type: text/x-patch, Size: 6445 bytes --]

From c7f6944a7483edf2719ecf47a2adf2d4b346700a Mon Sep 17 00:00:00 2001
From: Julien Lepiller <julien@lepiller.eu>
Date: Sun, 6 Nov 2016 11:28:01 +0100
Subject: [PATCH 3/3] services: Accept gexps as nginx configuration value.

* gnu/services/web.scm (default-nginx-config): Use computed-file instead
of plain-file.
---
 doc/guix.texi        |  9 +++++
 gnu/services/web.scm | 96 +++++++++++++++++++++++++++-------------------------
 2 files changed, 59 insertions(+), 46 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 4b60a4a..d125546 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -10433,6 +10433,15 @@ of them.  For example:
                 (list vh)) 
 @end example
 
+or if you want to make a package available, you can write something like:
+
+@example
+(define vh (nginx-vhost-configuration 
+                       (root my-package)))
+(define vh2 (nginx-vhost-configuration 
+                        (root #~(string-append #$cups "/share/doc/cups/ja"))))
+@end example
+
 Is a service that adds a new @dfn{virtual host} to the list of existing ones.
 You can add as many such services as you want.
 
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index e36d284..df6e680 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -90,54 +90,58 @@ of index files."
        names)))
 
 (define (default-nginx-vhost-config vhost)
-  (string-append
-   "    server {\n"
-   (if (nginx-vhost-configuration-http-port vhost)
-       (string-append "      listen "
-                      (number->string (nginx-vhost-configuration-http-port vhost))
-                      ";\n")
-       "")
-   (if (nginx-vhost-configuration-https-port vhost)
-       (string-append "      listen "
-                      (number->string (nginx-vhost-configuration-https-port vhost))
-                      " ssl;\n")
-       "")
-   "      server_name " (config-domain-strings
-                         (nginx-vhost-configuration-server-name vhost))
-                        ";\n"
-   (if (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)
-       (string-append "      ssl_certificate_key "
-                      (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
-       "")
-   "      root " (nginx-vhost-configuration-root vhost) ";\n"
-   "      index " (config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
-   "      server_tokens " (if (nginx-vhost-configuration-server-tokens? vhost)
-                              "on" "off") ";\n"
-   "    }\n"))
+  #~(string-append
+     "    server {\n"
+     #$(if (nginx-vhost-configuration-http-port vhost)
+           (string-append "      listen "
+                          (number->string (nginx-vhost-configuration-http-port vhost))
+                          ";\n")
+           "")
+     #$(if (nginx-vhost-configuration-https-port vhost)
+          (string-append "      listen "
+                          (number->string (nginx-vhost-configuration-https-port vhost))
+                          " ssl;\n")
+           "")
+     "      server_name " #$(config-domain-strings
+                             (nginx-vhost-configuration-server-name vhost))
+                          ";\n"
+     #$(if (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)
+           (string-append "      ssl_certificate_key "
+                          (nginx-vhost-configuration-ssl-certificate-key vhost) ";\n")
+           "")
+     "      root " #$(nginx-vhost-configuration-root vhost) ";\n"
+     "      index " #$(config-index-strings (nginx-vhost-configuration-index vhost)) ";\n"
+     "      server_tokens " #$(if (nginx-vhost-configuration-server-tokens? vhost)
+                                  "on" "off") ";\n"
+     "    }\n"))
 
 (define (default-nginx-config log-directory run-directory vhost-list)
-  (plain-file "nginx.conf"
-              (string-append
-               "user nginx nginx;\n"
-               "pid " run-directory "/pid;\n"
-               "error_log " log-directory "/error.log info;\n"
-               "http {\n"
-               "    client_body_temp_path " run-directory "/client_body_temp;\n"
-               "    proxy_temp_path " run-directory "/proxy_temp;\n"
-               "    fastcgi_temp_path " run-directory "/fastcgi_temp;\n"
-               "    uwsgi_temp_path " run-directory "/uwsgi_temp;\n"
-               "    scgi_temp_path " run-directory "/scgi_temp;\n"
-               "    access_log " log-directory "/access.log;\n"
-               (let ((http (map default-nginx-vhost-config vhost-list)))
-                 (do ((http http (cdr http))
-                      (block "" (string-append (car http) "\n" block )))
-                     ((null? http) block)))
-               "}\n"
-               "events {}\n")))
+  (computed-file
+    "nginx.conf"
+    #~(call-with-output-file #$output
+        (lambda (port)
+          (format port
+                  (string-append
+                     "user nginx nginx;\n"
+                     "pid " #$run-directory "/pid;\n"
+                     "error_log " #$log-directory "/error.log info;\n"
+                     "http {\n"
+                     "    client_body_temp_path " #$run-directory "/client_body_temp;\n"
+                     "    proxy_temp_path " #$run-directory "/proxy_temp;\n"
+                     "    fastcgi_temp_path " #$run-directory "/fastcgi_temp;\n"
+                     "    uwsgi_temp_path " #$run-directory "/uwsgi_temp;\n"
+                     "    scgi_temp_path " #$run-directory "/scgi_temp;\n"
+                     "    access_log " #$log-directory "/access.log;\n"
+                     #$(let ((http (map default-nginx-vhost-config vhost-list)))
+                         (do ((http http (cdr http))
+                              (block "" #~(string-append #$(car http) "\n" #$block )))
+                             ((null? http) block)))
+                     "}\n"
+                     "events {}\n"))))))
 
 (define %nginx-accounts
   (list (user-group (name "nginx") (system? #t))
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-04 22:58                             ` Hartmut Goebel
@ 2016-11-06 12:18                               ` Tobias Geerinckx-Rice
  2016-11-06 17:19                                 ` Ludovic Courtès
  2016-11-06 17:33                               ` Ludovic Courtès
  1 sibling, 1 reply; 29+ messages in thread
From: Tobias Geerinckx-Rice @ 2016-11-06 12:18 UTC (permalink / raw)
  To: julien; +Cc: guix-devel


[-- Attachment #1.1: Type: text/plain, Size: 1089 bytes --]

Hullo Julien,

(Sorry for replying at a random point in the thread — it's the only part
I have in this mailbox.)

On 04/11/16 23:58, Hartmut Goebel wrote:
> Am 04.11.2016 um 23:12 schrieb Julien Lepiller:
>> I'd like to get these patches accepted first, but I'm already 
>> working on adding more configuration lines.
> 
> [...]
> 
> In nginx AFAIK you can define many options on the server level, on 
> the vhost level and per location.

I've written quite a few nginx.confs over the years (too many — a Guixy
front-end would be great!) and have never encountered the term ‘vhost’.
Of course, that proves nothing.

However, a web search for ‘vhost site:nginx.org’ returns the following
as a first result[0]:

   Note: “VirtualHost” is an Apache term. NGINX does not have Virtual
   hosts, it has “Server Blocks” that use the server_name and listen
   directives to bind to tcp sockets.

I don't use Apache, so that explains that.

Kind regards,

T G-R

[0]:
https://www.nginx.com/resources/wiki/start/topics/examples/server_blocks/


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 476 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-06 12:18                               ` Tobias Geerinckx-Rice
@ 2016-11-06 17:19                                 ` Ludovic Courtès
  2016-11-20 12:49                                   ` Julien Lepiller
  0 siblings, 1 reply; 29+ messages in thread
From: Ludovic Courtès @ 2016-11-06 17:19 UTC (permalink / raw)
  To: Tobias Geerinckx-Rice; +Cc: guix-devel

Hi,

Tobias Geerinckx-Rice <me@tobias.gr> skribis:

> However, a web search for ‘vhost site:nginx.org’ returns the following
> as a first result[0]:
>
>    Note: “VirtualHost” is an Apache term. NGINX does not have Virtual
>    hosts, it has “Server Blocks” that use the server_name and listen
>    directives to bind to tcp sockets.
>
> I don't use Apache, so that explains that.

Oh, good to know.  In general I think it’s best to stick to upstream’s
terminology.

Julien, what would you think of changing “virtual host” with “server
blocks” and “vhost” with “server-block” (?) in the code and
documentation?

(This is not a fun suggestion to make, but hey!)

Ludo’.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-04 22:58                             ` Hartmut Goebel
  2016-11-06 12:18                               ` Tobias Geerinckx-Rice
@ 2016-11-06 17:33                               ` Ludovic Courtès
  1 sibling, 0 replies; 29+ messages in thread
From: Ludovic Courtès @ 2016-11-06 17:33 UTC (permalink / raw)
  To: Hartmut Goebel; +Cc: guix-devel

Hartmut Goebel <h.goebel@crazy-compilers.com> skribis:

> BTW: I just started typing down a simple "location" record type, and
> started to think if these record types are really the way to go for the
> configuration.
>
> In nginx AFAIK you can define many options on the server level, on the
> vhost level and per location. Additionally locations can be nested. And
> some configuration options are quite complex, e.g. "listen" [1] and
> maybe worth their own type. So we'll create a lot of code for both the
> record types and for converting them to config-file text.
>
> In the long run, we will get maintenance problems, I'm afraid, if we are
> only allow options known to us and written into the service. On each
> update we'd need to check if any option was added.

You’re right: we never want our Scheme interface to config files to
be less expressive than the original config syntax.

Yet, having a Scheme interface to config files is valuable: people don’t
have to learn a new syntax, you get compile-time checks of field names,
it’s easier to customize config represented as a Scheme object, etc.

So there’s a tension here.

So far, there’s no single approach to this problem in GuixSD.  In some
cases we don’t provide a Scheme interface at all (e.g., syslogd), which
isn’t great.  In other cases (e.g., nginx, Tor), we provide a partial
Scheme interface along with a way to augment the config file using its
native syntax.  In yet other cases (CUPS, Dovecot, added by Andy, or
libc’s NSS and PAM), services have a Scheme interface that cover 100% of
their current config space, sometimes with the added ability to provide
raw inline config.

I think this last approach is the preferred approach, but it’s also more
difficult to get right in some cases.  For instance, nginx config is not
just about key/value arguments: it has blocks, a notion of scope, etc.
So nginx config syntax and semantics is more complicated than key/value
configs such as that of Dovecot or Tor.

> So IMHO the recard types are not the way to go. We need a more flexible
> way, maybe like used for XML (see default-build.xml in
> guix/build/ant-build-system.scm), but with support for default values.

We could obviously use s-expressions for everything (they are
essentially typed XML).  However, people would get zero compile-time
checks and it would also be much harder (and less efficient) to deal
with them.

Ludo’.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-06 17:19                                 ` Ludovic Courtès
@ 2016-11-20 12:49                                   ` Julien Lepiller
  2016-11-22 22:20                                     ` Hartmut Goebel
  0 siblings, 1 reply; 29+ messages in thread
From: Julien Lepiller @ 2016-11-20 12:49 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 2778 bytes --]

On Sun, 06 Nov 2016 18:19:03 +0100
ludo@gnu.org (Ludovic Courtès) wrote:

> Hi,
> 
> Tobias Geerinckx-Rice <me@tobias.gr> skribis:
> 
> > However, a web search for ‘vhost site:nginx.org’ returns the
> > following as a first result[0]:
> >
> >    Note: “VirtualHost” is an Apache term. NGINX does not have
> > Virtual hosts, it has “Server Blocks” that use the server_name and
> > listen directives to bind to tcp sockets.
> >
> > I don't use Apache, so that explains that.  
> 
> Oh, good to know.  In general I think it’s best to stick to upstream’s
> terminology.
> 
> Julien, what would you think of changing “virtual host” with “server
> blocks” and “vhost” with “server-block” (?) in the code and
> documentation?
Ok, sorry for the long delay, I was working on php and other things. So
I've been thinking that we should probably stick more to the way you
would write an nginx configuration file, and have an interface to it.
So here is a web.scm file that implements just that. I have a record
type for configuration entries, and record types for complex
configuration types and blocks. Since many blocks can get the same
configuration option (for instance, http, server and location blocks
can get the "root" option), I define a single procedure that returns
the string corresponding to the configuration line. So for instance,
you could write this:

(nginx-service)
(service (service-type
  (name 'foo)
  (extensions
    (list
      (service-extension
        nginx-service-type
         (const (list (nginx-block-server
                        (blocks (list))
                        (configs (list
                                  (nginx-option (type 'server_name)
                                                (value (list 'default)))
                                  (nginx-option (type 'listen)
                                                (value (nginx-listen)))
                                  (nginx-option (type 'root)
                                                (value "/srv/http"))
                                  (nginx-option (type 'index)
                                                (value (list
                                                  "index.html"))))))))))))

As you can see, it's still a bit verbose, but we could provide a few
helper functions for some common cases.

Also, it is now possible to extend the nginx service with other kind of
blocks (although I implemented only the server block, it could be use
to add upstream blocks for instance).

What do you think? should I continue in that direction, or should I go
back to what I was doing before?

> 
> (This is not a fun suggestion to make, but hey!)
> 
> Ludo’.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: web.scm --]
[-- Type: text/x-scheme, Size: 17063 bytes --]

;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2015 David Thompson <davet@gnu.org>
;;; Copyright © 2015, 2016 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2016 ng0 <ng0@we.make.ritual.n0.is>
;;; Copyright © 2016 Julien Lepiller <julien@lepiller.eu>
;;;
;;; This file is part of GNU Guix.
;;;
;;; GNU Guix is free software; you can redistribute it and/or modify it
;;; under the terms of the GNU General Public License as published by
;;; the Free Software Foundation; either version 3 of the License, or (at
;;; your option) any later version.
;;;
;;; GNU Guix is distributed in the hope that it will be useful, but
;;; WITHOUT ANY WARRANTY; without even the implied warranty of
;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;;; GNU General Public License for more details.
;;;
;;; You should have received a copy of the GNU General Public License
;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.

(define-module (gnu services web)
  #:use-module (gnu services)
  #:use-module (gnu services shepherd)
  #:use-module (gnu system shadow)
  #:use-module (gnu packages admin)
  #:use-module (gnu packages web)
  #:use-module (guix records)
  #:use-module (guix gexp)
  #:use-module (srfi srfi-1)
  #:use-module (ice-9 match)
  #:export (nginx-configuration
            nginx-configuration?
            nginx-block-server
            nginx-access
            nginx-option
            nginx-listen
            nginx-service
            nginx-service-type))

;;; Commentary:
;;;
;;; Web services.
;;;
;;; Code:

(define-record-type* <nginx-configuration>
  nginx-configuration make-nginx-configuration
  nginx-configuration?
  (nginx          nginx-configuration-nginx)         ;<package>
  (log-directory  nginx-configuration-log-directory) ;string
  (run-directory  nginx-configuration-run-directory) ;string
  (http-configs   nginx-configuration-http-configs)
  (events-configs nginx-configuration-events-configs)
  (blocks         nginx-configuration-blocks)
  (file           nginx-configuration-file))         ;string | file-like

(define-record-type* <nginx-option>
  nginx-option make-nginx-option
  nginx-option?
  (type  nginx-option-type)
  (value nginx-option-value))

(define-record-type* <nginx-block-server>
  nginx-block-server make-nginx-block-server
  nginx-block-server?
  (blocks  nginx-block-server-blocks)
  (configs nginx-block-server-configs))

(define-record-type* <nginx-listen>
  nginx-listen make-nginx-listen
  nginx-listen?
  (port    nginx-listen-port
           (default 80))
  (address nginx-listen-address
           (default #f))
  (socket  nginx-listen-socket
           (default #f))
  (ssl?    nginx-listen-ssl?
           (default #f))
  (http2?  nginx-listen-http2?
           (default #f))
  (spdy?   nginx-listen-spdy?
           (default #f))
  (proxy?  nginx-listen-proxy?
           (default #f)))

(define-record-type* <nginx-error-page>
  nginx-error-page make-nginx-error-page
  nginx-error-page?
  (codes    nginx-error-page-codes
            (default (list 404)))
  (response nginx-error-page-response
            (default #f))
  (uri      nginx-error-page-uri
            (default "/404.html")))

(define-record-type* <nginx-access>
  nginx-access make-nginx-access
  nginx-access?
  (deny?  nginx-access-restriction
          (default #t))
  (to     nginx-access-to
          (default 'all))
  (except nginx-access-except
          (default '())))

(define (config-domain-strings names)
 "Return a string denoting the nginx config representation of NAMES, a list
of domain names."
 (string-concatenate
    (map (match-lambda
          ('default "_ ")
          ((? string? str) (string-append str " ")))
         names)))

(define (config-index-strings names)
 "Return a string denoting the nginx config representation of NAMES, a list
of index files."
 (string-concatenate
    (map (match-lambda
          ((? string? str) (string-append str " ")))
         names)))

(define (config-code-strings codes)
 "Return a string denoting the nginx config representation of CODES, a list
of HTTP response code."
 (string-concatenate
    (map (match-lambda
          ((? number? n) (string-append (number->string n) " ")))
         codes)))

(define (nginx-listen-config listen)
  (match listen
   (($ <nginx-listen> port address socket ssl? http2? spdy? proxy?)
     #~(string-append "listen "
        #$(if address
              (if port
                  (string-append address ":" (number->string port))
                  address)
              (if port
                  (number->string port)
                  (string-append "unix:" socket)))
        #$(if ssl? "http2 " (if spdy? "spdy " " "))
        #$(if proxy? "proxy_protocol" "")
        ";"))))

(define (nginx-error-page-config error)
  (match error
    (($ <nginx-error-page> codes response uri)
      #~(string-append "error_page " #$(config-code-strings codes)
                       #$(match response
                           (#f "")
                           ('proxy "=")
                           ((? number? n) (string-append "=" (number->string n))))
                       #$uri ";"))))

(define (nginx-access-config access)
  (match access
    (($ <nginx-access> deny? to except)
      #~(string-append
          #$(let ((except-list
                    (map nginx-access-config except)))
              (do ((except-list except-list (cdr except-list))
                   (block "" #~(string-append #$(car except-list) "\n" #$block )))
                  ((null? except-list) block)))
          #$(if deny? "deny " "allow ")
          #$(match to
              ('all "all")
              ('unix "unix:")
              (_ to))
          ";"))))
          

(define (authorized-option-type type)
  (match type
    ('http (list 'access 'error_page 'etag 'index 'if_modified_since
                 'ignore_invalid_headers 'log_not_found 'log_subrequest
                 'merge_slashes 'port_in_redirect 'recursive_error_pages 'root
                 'server_name_in_redirect 'server_tokens))
    ('server (list 'access 'error_page 'etag 'index 'if_modified_since
                   'ignore_invalid_headers 'listen 'log_not_found 'log_subrequest
                   'merge_slashes 'port_in_redirect 'recursive_error_pages 'root
                   'server_name 'server_name_in_redirect 'server_tokens 'try_files))
    ('location (list 'access 'alias 'error_page 'etag 'if_modified_since 'index
                     'internal 'log_not_found 'log_subrequest 'port_in_redirect
                     'recursive_error_pages 'root 'server_name_in_redirect
                     'server_tokens 'try_files))
    ('if (list 'error_page 'root))
    ('limit_except (list 'access))
    ('events (list))))

(define (assert-good-type conf-type block-type)
  (if (not (memq conf-type (authorized-option-type block-type)))
      (throw 'bad-conf-type
             (string-append (symbol->string conf-type)
                            " is not allowed in a " 
                            (symbol->string block-type)
                            " block."))))

(define (default-nginx-option-config name value)
  #~(string-append #$(symbol->string name) " " #$value ";"))

(define (nginx-option-config option parent-block-type)
  (assert-good-type (nginx-option-type option) parent-block-type)
  (match option
    (($ <nginx-option> type value)
      (match type
        ('access (nginx-access-config value))
        ('error_page (nginx-error-page-config value))
        ('if_modified_since (match value
                             (#f "if_modified_since off;")
                             ('exact "if_modified_since exact;")
                             ('before "if_modified_since before;")))
        ('internal (if value "internal;" ""))
        ('listen (nginx-listen-config value))
        ('server_name #~(string-append "server_name "
                                       #$(config-domain-strings value) ";"))
        ('index #~(string-append "index " #$(config-index-strings value) ";"))
        ('try_files #~(string-append "try_files "
                                     #$(config-index-strings value)) ";")
        (_ (match value
             ((? number? n) (default-nginx-option-config type (number->string n)))
             (#t default-nginx-option-config type "on")
             (#f default-nginx-option-config type "off")
             (_ (default-nginx-option-config type value))))))))

(define (authorized-block-type type)
  (match type
    ('http (list 'server 'types))
    ('location (list 'if 'limit_except 'location 'types))
    ('server (list 'location 'types))))

(define (assert-good-block-type block-type parent-type)
  (if (not (memq block-type (authorized-block-type parent-type)))
      (throw 'bad-block-type
             (string-append (symbol->string block-type)
                            " is not allowed in a " 
                            (symbol->string parent-type)
                            " block."))))

(define (nginx-block-server-config blocks options parent-type)
  (assert-good-block-type 'server parent-type)
  #~(string-append
     "    server {\n"
     #$(let ((config-list
               (map (lambda (option)
                            (nginx-option-config option 'server))
                    options)))
         (do ((config-list config-list (cdr config-list))
              (block "" #~(string-append #$(car config-list) "\n" #$block )))
             ((null? config-list) block)))
     #$(let ((block-list
               (map (lambda (block)
                            (nginx-block-config block 'server)) blocks)))
         (do ((block-list block-list (cdr block-list))
              (block "" #~(string-append #$(car block-list) "\n" #$block )))
             ((null? block-list) block)))
    ; #$(if (eq? options '())
    ;       ""
    ;       #~(string-concatenate
    ;                 #$(map (lambda (option)
    ;                              (nginx-option-config option 'server))
    ;                      options)))
    ; #$(if (eq? blocks '())
    ;       ""
    ;       (string-concatenate
    ;                 (map (lambda (block)
    ;                              (nginx-block-config block 'server))
    ;                      blocks)))
     "    }\n"))

(define (nginx-block-config block parent-type)
  (match block
    (($ <nginx-block-server> blocks options)
      (nginx-block-server-config blocks options parent-type))
    (_ "")))

(define (default-nginx-config log-directory run-directory http-configs
                              events-configs blocks)
  (computed-file "nginx.conf"
    #~(call-with-output-file #$output
        (lambda (port)
          (format port
            (string-append
             "user nginx nginx;\n"
             "pid " #$run-directory "/pid;\n"
             "error_log " #$log-directory "/error.log info;\n"
             "http {\n"
             "    client_body_temp_path " #$run-directory "/client_body_temp;\n"
             "    proxy_temp_path " #$run-directory "/proxy_temp;\n"
             "    fastcgi_temp_path " #$run-directory "/fastcgi_temp;\n"
             "    uwsgi_temp_path " #$run-directory "/uwsgi_temp;\n"
             "    scgi_temp_path " #$run-directory "/scgi_temp;\n"
             "    access_log " #$log-directory "/access.log;\n"
             #$(let ((config-list
                       (map (lambda (option)
                                    (nginx-option-config option 'http))
                            http-configs)))
                 (do ((config-list config-list (cdr config-list))
                      (block "" #~(string-append #$(car config-list) "\n" #$block )))
                     ((null? config-list) block)))
             #$(let ((block-list
                       (map (lambda (block)
                                    (nginx-block-config block 'http)) blocks)))
                 (do ((block-list block-list (cdr block-list))
                      (block "" #~(string-append #$(car block-list) "\n" #$block )))
                     ((null? block-list) block)))
             "}\n"
             "events {\n"
             #$(let ((config-list
                       (map (lambda (option)
                                    (nginx-option-config option 'http))
                            events-configs)))
                 (do ((config-list config-list (cdr config-list))
                      (block "" #~(string-append #$(car config-list) "\n" #$block )))
                     ((null? config-list) block)))
             "}\n"))))))

(define %nginx-accounts
  (list (user-group (name "nginx") (system? #t))
        (user-account
         (name "nginx")
         (group "nginx")
         (system? #t)
         (comment "nginx server user")
         (home-directory "/var/empty")
         (shell (file-append shadow "/sbin/nologin")))))

(define nginx-activation
  (match-lambda
    (($ <nginx-configuration> nginx log-directory run-directory http-configs
                              events-configs blocks file)
     #~(begin
         (use-modules (guix build utils))

         (format #t "creating nginx log directory '~a'~%" #$log-directory)
         (mkdir-p #$log-directory)
         (format #t "creating nginx run directory '~a'~%" #$run-directory)
         (mkdir-p #$run-directory)
         (format #t "creating nginx temp directories '~a/{client_body,proxy,fastcgi,uwsgi,scgi}_temp'~%" #$run-directory)
         (mkdir-p (string-append #$run-directory "/client_body_temp"))
         (mkdir-p (string-append #$run-directory "/proxy_temp"))
         (mkdir-p (string-append #$run-directory "/fastcgi_temp"))
         (mkdir-p (string-append #$run-directory "/uwsgi_temp"))
         (mkdir-p (string-append #$run-directory "/scgi_temp"))
         ;; Check configuration file syntax.
         (system* (string-append #$nginx "/sbin/nginx")
                  "-t" "-c" #$(or file
                                  (default-nginx-config log-directory run-directory
                                    http-configs events-configs blocks)))))))

(define nginx-shepherd-service
  (match-lambda
    (($ <nginx-configuration> nginx log-directory run-directory http-configs
                              events-configs blocks file)
     (let* ((nginx-binary (file-append nginx "/sbin/nginx"))
            (nginx-action
             (lambda args
               #~(lambda _
                   (zero?
                    (system* #$nginx-binary "-c"
                      #$(or file
                            (default-nginx-config log-directory run-directory
                              http-configs events-configs blocks))
                      #$@args))))))

       ;; TODO: Add 'reload' action.
       (list (shepherd-service
              (provision '(nginx))
              (documentation "Run the nginx daemon.")
              (requirement '(user-processes loopback))
              (start (nginx-action "-p" run-directory))
              (stop (nginx-action "-s" "stop"))))))))

(define nginx-service-type
  (service-type (name 'nginx)
                (extensions
                 (list (service-extension shepherd-root-service-type
                                          nginx-shepherd-service)
                       (service-extension activation-service-type
                                          nginx-activation)
                       (service-extension account-service-type
                                          (const %nginx-accounts))))
                (compose concatenate)
                (extend (lambda (config blocks)
                          (nginx-configuration
                            (inherit config)
                            (blocks (append (nginx-configuration-blocks config)
                                            blocks)))))))

(define* (nginx-service #:key (nginx nginx)
                        (log-directory "/var/log/nginx")
                        (run-directory "/var/run/nginx")
                        (http-configs '())
                        (events-configs '())
                        (blocks '())
                        (config-file #f))
  "Return a service that runs NGINX, the nginx web server.

The nginx daemon loads its runtime configuration from CONFIG-FILE, stores log
files in LOG-DIRECTORY, and stores temporary runtime files in RUN-DIRECTORY."
  (service nginx-service-type
           (nginx-configuration
            (nginx nginx)
            (log-directory log-directory)
            (run-directory run-directory)
            (http-configs http-configs)
            (events-configs events-configs)
            (blocks blocks)
            (file config-file))))

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-20 12:49                                   ` Julien Lepiller
@ 2016-11-22 22:20                                     ` Hartmut Goebel
  2016-11-23  9:26                                       ` julien lepiller
  0 siblings, 1 reply; 29+ messages in thread
From: Hartmut Goebel @ 2016-11-22 22:20 UTC (permalink / raw)
  To: guix-devel

Am 20.11.2016 um 13:49 schrieb Julien Lepiller:
> What do you think? should I continue in that direction, or should I go
> back to what I was doing before?

Thanks for working on this. I will give it a try the next days. But I
want to share two points going around in my head:

1) I propose moving the nginx service into a package
gnu/service/nginx.scm. Imagine we will implement an equal complex config
schema for apache. Then we will get a beast of code with lots of useless
"nginx-" prefixes.

2) Is it possible to add some magic to e.g. "nginx-service-type" so that
all identifiers within are searched in the "nginx" scope. Example:

      (service-extension
        nginx-service-type
         (const (list (server    ;; taken from "nginx" scope
                        (blocks (list))
                        (configs (list
                                  (option (type 'server_name)     ;; taken from "nginx" scope
                                          (value (list 'default)))
-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | h.goebel@crazy-compilers.com               |
| www.crazy-compilers.com | compilers which you thought are impossible |

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-22 22:20                                     ` Hartmut Goebel
@ 2016-11-23  9:26                                       ` julien lepiller
  2016-11-25 10:53                                         ` Clément Lassieur
  2016-11-27 21:01                                         ` [PATCH] improve nginx-service Ludovic Courtès
  0 siblings, 2 replies; 29+ messages in thread
From: julien lepiller @ 2016-11-23  9:26 UTC (permalink / raw)
  To: guix-devel

Le 2016-11-22 23:20, Hartmut Goebel a écrit :
> Am 20.11.2016 um 13:49 schrieb Julien Lepiller:
>> What do you think? should I continue in that direction, or should I go
>> back to what I was doing before?
> 
> Thanks for working on this. I will give it a try the next days. But I
> want to share two points going around in my head:
> 
> 1) I propose moving the nginx service into a package
> gnu/service/nginx.scm. Imagine we will implement an equal complex 
> config
> schema for apache. Then we will get a beast of code with lots of 
> useless
> "nginx-" prefixes.
> 
> 2) Is it possible to add some magic to e.g. "nginx-service-type" so 
> that
> all identifiers within are searched in the "nginx" scope. Example:
> 
>       (service-extension
>         nginx-service-type
>          (const (list (server    ;; taken from "nginx" scope
>                         (blocks (list))
>                         (configs (list
>                                   (option (type 'server_name)     ;;
> taken from "nginx" scope
>                                           (value (list 'default)))

I am quite new to scheme, but as far as I can tell, there is no notion 
of scope. Here, (option ...) can be independent from nginx or apache 
(because there is nothing specific), but probably not (server), because 
it is a notion specific to nginx and because what it can contain is 
different in the apache world and nginx world. Also, how would the two 
types be recognized when you want both apache and nginx (probably not 
useful, but it may happen)?

I've been rethinking it, and I would like to use define-configuration 
(from cups and dovecot) because it looks really good. But I don't want 
to define the same config option for each possible block it can appear 
in, and that's why I used (option). So I'm looking for a way to 
dynamically define records for each block type, so they can be used more 
consistently with what we have. Unfortunately, I don't see how I could 
do that, if that is possible at all... Ludo, any idea? I'd like to be 
able to write:

(define-record-type* <nginx-option>
   ...)

(define-syntax option
   (syntax-rule ()
     (option mname mtype mdef mdoc mblocks))
     (nginx-option (name mname) (type mtype) ...)))

(define option-list (list
   (option 'server-name server-name 'default "the name of the server that 
is served by the http block" (list 'http))
   ...))

and then be able to create the block records by filtering that list:

(define-nginx-configuration nginx-http-block
   (filter ... option-list))

(define-nginx-configuration nginx-events-block
   (filter ... option-list))

(define-nginx-configuration nginx-server-block
   (filter ... option-list))

So the user would then be able to write the service configuration as 
they would for any other service we have. What I don't know is how to 
write define-nginx-configuration (ideally it would call 
define-configuration). With it, I can probably do the rest just fine. Or 
maybe there is a better way I don't see yet?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-23  9:26                                       ` julien lepiller
@ 2016-11-25 10:53                                         ` Clément Lassieur
  2016-11-25 11:46                                           ` is using eval good style in guile?(was: [PATCH] improve nginx-service) Hartmut Goebel
  2016-11-27 21:01                                         ` [PATCH] improve nginx-service Ludovic Courtès
  1 sibling, 1 reply; 29+ messages in thread
From: Clément Lassieur @ 2016-11-25 10:53 UTC (permalink / raw)
  To: julien lepiller; +Cc: guix-devel

> I've been rethinking it, and I would like to use define-configuration 
> (from cups and dovecot) because it looks really good. But I don't want 
> to define the same config option for each possible block it can appear 
> in, and that's why I used (option). So I'm looking for a way to 
> dynamically define records for each block type, so they can be used more 
> consistently with what we have. Unfortunately, I don't see how I could 
> do that, if that is possible at all... Ludo, any idea? I'd like to be 
> able to write:
>
> (define-record-type* <nginx-option>
>    ...)
>
> (define-syntax option
>    (syntax-rule ()
>      (option mname mtype mdef mdoc mblocks))
>      (nginx-option (name mname) (type mtype) ...)))
>
> (define option-list (list
>    (option 'server-name server-name 'default "the name of the server that 
> is served by the http block" (list 'http))
>    ...))
>
> and then be able to create the block records by filtering that list:
>
> (define-nginx-configuration nginx-http-block
>    (filter ... option-list))
>
> (define-nginx-configuration nginx-events-block
>    (filter ... option-list))
>
> (define-nginx-configuration nginx-server-block
>    (filter ... option-list))
>
> So the user would then be able to write the service configuration as 
> they would for any other service we have. What I don't know is how to 
> write define-nginx-configuration (ideally it would call 
> define-configuration). With it, I can probably do the rest just fine. Or 
> maybe there is a better way I don't see yet?

I think you are looking for "eval".

(define (list->define-configuration stem fields)
  (eval `(define-configuration ,stem ,@fields) (current-module)))

(list->define-configuration 'some-configuration filtered-list)

I use it in a service I'm working on (Prosody), to handle virtualhosts.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* is using eval good style in guile?(was: [PATCH] improve nginx-service)
  2016-11-25 10:53                                         ` Clément Lassieur
@ 2016-11-25 11:46                                           ` Hartmut Goebel
  2016-11-25 13:29                                             ` is using eval good style in guile? Andy Wingo
  0 siblings, 1 reply; 29+ messages in thread
From: Hartmut Goebel @ 2016-11-25 11:46 UTC (permalink / raw)
  To: guix-devel

[-- Attachment #1: Type: text/plain, Size: 775 bytes --]

Am 25.11.2016 um 11:53 schrieb Clément Lassieur:
> I think you are looking for "eval".
>
> (define (list->define-configuration stem fields)
>   (eval `(define-configuration ,stem ,@fields) (current-module)))

I'm curious: In other programming languages, using eval is regarded bad
programming style. Is this different in guile?

-- 
Schönen Gruß
Hartmut Goebel
Dipl.-Informatiker (univ), CISSP, CSSLP, ISO 27001 Lead Implementer
Information Security Management, Security Governance, Secure Software
Development

Goebel Consult, Landshut
http://www.goebel-consult.de

Blog:
http://www.goebel-consult.de/blog/vortrag-digitalen-selbstverteidigung-fur-unternehmen

Kolumne:
http://www.cissp-gefluester.de/2012-04-compliance-bringt-keine-sicherheit



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 2430 bytes --]

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: is using eval good style in guile?
  2016-11-25 11:46                                           ` is using eval good style in guile?(was: [PATCH] improve nginx-service) Hartmut Goebel
@ 2016-11-25 13:29                                             ` Andy Wingo
  2016-11-26 21:55                                               ` Clément Lassieur
  0 siblings, 1 reply; 29+ messages in thread
From: Andy Wingo @ 2016-11-25 13:29 UTC (permalink / raw)
  To: Hartmut Goebel; +Cc: guix-devel

On Fri 25 Nov 2016 12:46, Hartmut Goebel <h.goebel@goebel-consult.de> writes:

> Am 25.11.2016 um 11:53 schrieb Clément Lassieur:
>> I think you are looking for "eval".
>>
>> (define (list->define-configuration stem fields)
>>   (eval `(define-configuration ,stem ,@fields) (current-module)))
>
> I'm curious: In other programming languages, using eval is regarded bad
> programming style. Is this different in guile?

No.  Eval is generally bad style :)  If it can be avoided it should be
avoided.  I think since this is a definition form, macros are the
appropriate thing if possible (I haven't seen the larger patch).

Andy

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: is using eval good style in guile?
  2016-11-25 13:29                                             ` is using eval good style in guile? Andy Wingo
@ 2016-11-26 21:55                                               ` Clément Lassieur
  0 siblings, 0 replies; 29+ messages in thread
From: Clément Lassieur @ 2016-11-26 21:55 UTC (permalink / raw)
  To: Andy Wingo; +Cc: guix-devel, Hartmut Goebel

Andy Wingo <wingo@igalia.com> writes:

> On Fri 25 Nov 2016 12:46, Hartmut Goebel <h.goebel@goebel-consult.de> writes:
>
>> Am 25.11.2016 um 11:53 schrieb Clément Lassieur:
>>> I think you are looking for "eval".
>>>
>>> (define (list->define-configuration stem fields)
>>>   (eval `(define-configuration ,stem ,@fields) (current-module)))
>>
>> I'm curious: In other programming languages, using eval is regarded bad
>> programming style. Is this different in guile?
>
> No.  Eval is generally bad style :)  If it can be avoided it should be
> avoided.  I think since this is a definition form, macros are the
> appropriate thing if possible (I haven't seen the larger patch).

Oh sorry for that then. The larger patch is there:
http://lists.gnu.org/archive/html/guix-devel/2016-11/msg01048.html

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH] improve nginx-service
  2016-11-23  9:26                                       ` julien lepiller
  2016-11-25 10:53                                         ` Clément Lassieur
@ 2016-11-27 21:01                                         ` Ludovic Courtès
  1 sibling, 0 replies; 29+ messages in thread
From: Ludovic Courtès @ 2016-11-27 21:01 UTC (permalink / raw)
  To: julien lepiller; +Cc: guix-devel

Hi, and sorry for the delay!

julien lepiller <julien@lepiller.eu> skribis:

> Le 2016-11-22 23:20, Hartmut Goebel a écrit :
>> Am 20.11.2016 um 13:49 schrieb Julien Lepiller:
>>> What do you think? should I continue in that direction, or should I go
>>> back to what I was doing before?
>>
>> Thanks for working on this. I will give it a try the next days. But I
>> want to share two points going around in my head:
>>
>> 1) I propose moving the nginx service into a package
>> gnu/service/nginx.scm. Imagine we will implement an equal complex
>> config
>> schema for apache. Then we will get a beast of code with lots of
>> useless
>> "nginx-" prefixes.
>>
>> 2) Is it possible to add some magic to e.g. "nginx-service-type" so
>> that
>> all identifiers within are searched in the "nginx" scope. Example:
>>
>>       (service-extension
>>         nginx-service-type
>>          (const (list (server    ;; taken from "nginx" scope
>>                         (blocks (list))
>>                         (configs (list
>>                                   (option (type 'server_name)     ;;
>> taken from "nginx" scope
>>                                           (value (list 'default)))
>
> I am quite new to scheme, but as far as I can tell, there is no notion
> of scope. Here, (option ...) can be independent from nginx or apache
> (because there is nothing specific), but probably not (server),
> because it is a notion specific to nginx and because what it can
> contain is different in the apache world and nginx world. Also, how
> would the two types be recognized when you want both apache and nginx
> (probably not useful, but it may happen)?
>
> I've been rethinking it, and I would like to use define-configuration
> (from cups and dovecot) because it looks really good. But I don't want
> to define the same config option for each possible block it can appear
> in, and that's why I used (option). So I'm looking for a way to
> dynamically define records for each block type, so they can be used
> more consistently with what we have. Unfortunately, I don't see how I
> could do that, if that is possible at all... Ludo, any idea? I'd like
> to be able to write:
>
> (define-record-type* <nginx-option>
>   ...)
>
> (define-syntax option
>   (syntax-rule ()
>     (option mname mtype mdef mdoc mblocks))
>     (nginx-option (name mname) (type mtype) ...)))
>
> (define option-list (list
>   (option 'server-name server-name 'default "the name of the server
> that is served by the http block" (list 'http))
>   ...))
>
> and then be able to create the block records by filtering that list:
>
> (define-nginx-configuration nginx-http-block
>   (filter ... option-list))
>
> (define-nginx-configuration nginx-events-block
>   (filter ... option-list))
>
> (define-nginx-configuration nginx-server-block
>   (filter ... option-list))
>
> So the user would then be able to write the service configuration as
> they would for any other service we have. What I don't know is how to
> write define-nginx-configuration (ideally it would call
> define-configuration). With it, I can probably do the rest just
> fine. Or maybe there is a better way I don't see yet?

I’ve played a bit with nginx and I think its configuration syntax is far
from trivial.  Services for Dovecot, Kerberos, glibc’s name service
switch, PAM, etc. are pretty clear: it’s quite easy to find out what
data structures the config defines, and to write Scheme bindings for
those structures.

In the case of nginx, it’s much more complex: there’s a notion of scope
and inheritance (some key/value pairs are inherited from the “parent”
context), there are conditionals, variables, keys associated with more
than one value, etc.  Tricky.

So there are two ways to approach it:

  1. Choose a small subset of the functionality, like you’ve done with
     vhosts, and provide bindings for that subset.  It’ll be less
     expressive than nginx’s config files (so users need a way to
     provide nginx-language config when needed), but it can still be
     useful.  And it’s not too hard to implement.

  2. Write bindings for the whole configuration language, such that
     there is no loss of expressivity.  This is the grail, but obviously
     it requires a very good understanding of that language, and it’s
     going to be much more work.

I think it would make sense to stick to #1 in the short- to medium term.
But if you’re an nginx wizard, you might find that #2 is piece of cake;
you’ll have to decide!  :-)

Back to the original comment about “vhost”, I think we could simply
replace “vhost” with “server” in the code, since those
<nginx-vhost-configuration> objects map to ‘server’ blocks.

WDYT?

Thanks,
Ludo’.

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2016-11-27 21:01 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-16 12:33 [PATCH] improve nginx-service Julien Lepiller
2016-10-19 21:04 ` Ludovic Courtès
2016-10-20 12:37   ` Julien Lepiller
2016-10-24 20:51     ` Ludovic Courtès
2016-10-26 19:45       ` Julien Lepiller
2016-10-27 12:41         ` Ludovic Courtès
2016-10-27 17:59           ` Julien Lepiller
2016-10-30 21:46             ` Ludovic Courtès
2016-11-02  8:22               ` Hartmut Goebel
2016-11-03 14:54                 ` Ludovic Courtès
2016-11-03 22:38                   ` Hartmut Goebel
2016-11-04 13:21                     ` Ludovic Courtès
2016-11-04 18:01                       ` Julien Lepiller
2016-11-04 21:28                         ` Hartmut Goebel
2016-11-04 22:12                           ` Julien Lepiller
2016-11-04 22:34                             ` Hartmut Goebel
2016-11-06 11:11                               ` Julien Lepiller
2016-11-04 22:58                             ` Hartmut Goebel
2016-11-06 12:18                               ` Tobias Geerinckx-Rice
2016-11-06 17:19                                 ` Ludovic Courtès
2016-11-20 12:49                                   ` Julien Lepiller
2016-11-22 22:20                                     ` Hartmut Goebel
2016-11-23  9:26                                       ` julien lepiller
2016-11-25 10:53                                         ` Clément Lassieur
2016-11-25 11:46                                           ` is using eval good style in guile?(was: [PATCH] improve nginx-service) Hartmut Goebel
2016-11-25 13:29                                             ` is using eval good style in guile? Andy Wingo
2016-11-26 21:55                                               ` Clément Lassieur
2016-11-27 21:01                                         ` [PATCH] improve nginx-service Ludovic Courtès
2016-11-06 17:33                               ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).