From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47435) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eNxTv-0004p8-Kl for guix-patches@gnu.org; Sun, 10 Dec 2017 04:01:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eNxTq-00061o-O4 for guix-patches@gnu.org; Sun, 10 Dec 2017 04:01:07 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:45794) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eNxTq-00061c-Ke for guix-patches@gnu.org; Sun, 10 Dec 2017 04:01:02 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eNxTq-0003X7-78 for guix-patches@gnu.org; Sun, 10 Dec 2017 04:01:02 -0500 Subject: [bug#29466] [PATCH] services: web: Add support for configuring the nginx server names hash. Resent-Message-ID: References: <20171127082338.18504-1-mail@cbaines.net> <87zi77lssn.fsf@gnu.org> From: Christopher Baines In-reply-to: <87zi77lssn.fsf@gnu.org> Date: Sun, 10 Dec 2017 09:00:28 +0000 Message-ID: <87374j6jpv.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 29466@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Ludovic Court=C3=A8s writes: >> The nginx service can fail to start if the server names hash bucket size= is >> too small, which can happen on some systems, and when using QEMU, depend= ing on >> the CPU. >> >> * gnu/services/web.scm (): Add >> server-names-hash-bucket-size and server-names-hash-bucket-max-size. >> (default-nginx-config): Add support for the new hash bucket size param= eters. >> (nginx-service, nginx-activation): Pass the new hash bucket size param= eters >> through to the default-nginx-config procedure. >> * doc/guix.texi (Web Services): Document the new hash bucket size parame= ters. > > LGTM! > > However=E2=80=A6 > >> -(define (default-nginx-config nginx log-directory run-directory server-= list upstream-list) >> +(define (default-nginx-config nginx log-directory run-directory server-= list >> + upstream-list server-names-hash-bucket-si= ze >> + server-names-hash-bucket-max-size) > > That=E2=80=99s too many positional parameters. And should we use a gexp > compiler for anyway? > >> (define nginx-shepherd-service >> (match-lambda >> (($ nginx log-directory run-directory server-= blocks >> - upstream-blocks file) >> + upstream-blocks server-names-hash-bucket-= size >> + server-names-hash-bucket-max-size file) > > Likewise, at this stage, we should probably use =E2=80=98match-record=E2= =80=99 to avoid > mistakes. I've sent a couple of additional patches now, the first makes the change to using match-record, and the second splits out the configuration file part of the record, and adds a gexp compiler for it. Let me know what you think about the 2nd patch? I don't particularly like the duplication between the two records, as both the and records need the package, log directory and run directory for different reasons. In summary: - nginx: used for the nginx binary in the shepherd service log-directory: created in the activation service run-directory: created in the activation service - nginx: used for the mime.types file log-directory: written in to the config file run-directory: written in to the config file It's important that it's possible to specify the log directory and run directory if you don't use the , and instead use something opaque (like a ). But I dislike the duplication that this is adding. I wonder if there is a better way of supporting the use of a raw configuration file, rather than the record. Possibly by providing an easy way to create a custom nginx-service-type, with a different activation phase? I think that would allow for making the original compile to the configuration file, but then have something like this for a custom config file. (service (nginx-service-type-with-custom-activation nginx-service-type #:run-directory "/var/run/nginx-custom" #:log-directory "/var/run/nginx-custom") (local-file "nginx-custom.conf")) One downside with this approach is that service extensions use the service-type, so modifying it would mean that services that extend nginx wouldn't work with this service type (you'd have to have the original as well, or modify the service with the extension). In the 2nd patch, I put in some (bad) handling of the extension case, as with an opaque config file, you can't do anything. Previously, the configuration was changed, but then this is later ignored, as the file takes precedence. Thanks for reviewing, Chris --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKTBAEBCgB9FiEEPonu50WOcg2XVOCyXiijOwuE9XcFAlos96xfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcACgkQXiijOwuE 9XdWFg//RDFnwBWXyGPvwIKAeVxFcJ2c3xsuZHbVHwe4v0L7S6CTaERwSsm7gbQX TnmFj4wfQTS7KD45xv9RYBEaUUJ+KS0cuK0uabrznj94BQHaV+BtLnYFiLhHnQO2 l57LrvyTOBFlK0pK/SOZrIza0XRSyou5GjdMJK7hy/Lo56dyPDVDft71ALdaQCYW AZnHOuyTMsYn9iIKY6mTJ+0d4cfAQ/rCjCAGxQiUaDlvF82e24qzK1x1WGOTQNqH M7zXAOVJ98QuKAKrs4StIXQNXvCUwTSnbtP8HIeCpvCidSKC3dNlSrCKIhoKqoam 8mh4NEyQRyDM9euBiF2CtrJJhrAnrxnG8VnDhwf1ulp6qR9669u/A1cc67ovI/ve 7pEW3o2UbdTjBVnjxcm5hx2loIVPm57hLDCmFYfJwsO5Gbm1baOpI6Ia6/rxRXzD FN7/tBfm+GY/uU/IRbvcJY6iBQMha0/oB/IWW+GD2fqcmvVO22baxmQmqT3Le/lE rZohan3kEkb4TaBejl25MSbc21eok2cxLNhCN9I79AwDfmo7vgYnM+jm/U5pZspT CwDi6tOCstnaiUMVkPuoBbcHTgCVuXa4mr6Wky7KqLsyf2mnhhkGtT14zVQ3XLNX 6MB83QK6/OnlmQsRHoAvDmegrlWehwkLHepdZmXG5gfzDQGSiL0= =Rfzx -----END PGP SIGNATURE----- --=-=-=--