From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?utf-8?Q?Court=C3=A8s?=) Subject: Re: [PATCH] improve nginx-service Date: Thu, 27 Oct 2016 14:41:18 +0200 Message-ID: <87y41aklqp.fsf@gnu.org> References: <20161016143347.38d8a6f2@polymos.lepiller.eu> <874m483vap.fsf@gnu.org> <20161020143744.516a1184@polymos.lepiller.eu> <87shrlzd0w.fsf@gnu.org> <20161026214507.45445d14@lepiller.eu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:51645) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzjzq-0001Lh-BH for guix-devel@gnu.org; Thu, 27 Oct 2016 08:41:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bzjzl-0007U7-IK for guix-devel@gnu.org; Thu, 27 Oct 2016 08:41:26 -0400 In-Reply-To: <20161026214507.45445d14@lepiller.eu> (Julien Lepiller's message of "Wed, 26 Oct 2016 21:45:07 +0200") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Julien Lepiller Cc: guix-devel@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi! Julien Lepiller skribis: > On Mon, 24 Oct 2016 22:51:27 +0200 > ludo@gnu.org (Ludovic Court=C3=A8s) wrote: [...] >> >> I suppose we could make =E2=80=98nginx-service-type=E2=80=99 extensib= le (info >> >> "(guix) Service Types and Services") so that people can write >> >> services that define new vhosts?=20=20 >> > >> > You mean something like udev-service-type where you could extend it >> > with a list of vhosts?=20=20 >>=20 >> 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 , 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): > #< name: "default-nginx-vhost" gexp: # (mkdir #) (symlink # name: > "index.html" content: "[...]" references: ()>:out> (string-append > # "/index.html"))) 5959270> options: (#:local-build? > #t)> I think the backtrace, ahem, very clearly shows what=E2=80=99s wrong. (Suddenly I=E2=80=99m scared: what if this backtrace gave you the same feel= ing that I have when I get a C++ compilation error?!) > From 13748b6bfffef19080f4fa3bde2a6ae7d5c8d067 Mon Sep 17 00:00:00 2001 > From: Julien Lepiller > 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 =E2=80=98compose=E2=80=99 and =E2=80=98e= xtend=E2=80=99 to =E2=80=98nginx-service-type=E2=80=99 (this is where we define how extension= s are handled). There=E2=80=99s a bit of extra bookeeping to do, in particular m= oving the list of vhosts to , as in this untested patch: --=-=-= Content-Type: text/x-patch Content-Disposition: inline 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) ; (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 + (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 log-directory run-directory config-file) + (($ 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)))) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable With that in place, it becomes possible to write another service that provides additional vhosts, like: (define vh (nginx-vhost-configuration =E2=80=A6)) (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=E2=80=99. --=-=-=--