Thank you for your review! I have attached updated patches.
I have some questions. I’d like to answer but not in order. First of
all,
On Fri, Nov 01, 2019 at 03:54:42PM +0100, Ludovic Courtès wrote:
> > + (modules '((guix build utils)
> > + (ice-9 popen)))
> > + (snippet
> > + #~(begin
> > + ;; the nginx source code is part of the module’s source
> > + (format #t "decompressing nginx source code~%")
> > + (call-with-output-file "nginx.tar"
> > + (lambda (out)
> > + (let ((pipe (open-pipe* OPEN_READ
> > + #+(file-append gzip "/bin/gzip")
> > "-cd"
> > + #$(package-source nginx))))
> > + (dump-port pipe out)
> > + (unless (= (status:exit-val (close-pipe pipe)) 0)
> > + (error "gzip decompress failed")))))
> > + (invoke #+(file-append tar "/bin/tar") "xvf" "nginx.tar"
> > + "--strip-components=1")
> > + (delete-file "nginx.tar")
>
> I’d suggest doing it in a phase.
This changes many things. :)
With a phase, `guix build -S` would only return the source files of
nginx-accept-language-module directly but not of statically linked
nginx. I have added a further patch to doc/guix.texi warning of `guix
build -S` not always returning complete and corresponding sources, see
attachment.
The good thing is that with a phase I no longer get an import cycle
because I no longer need a reference to the tar package. So I put
nginx-accept-language-module inside web.scm now and there is no need
for a separate module.
> > + (license (delete-duplicates
> > + (cons license:bsd-2 ;license of nginx-mod-accept-language
> > + (package-license nginx))))))) ;the module’s code is
> > linked
>
> To avoid circular dependencies in top-level references, I suggest
> copying the license of ‘nginx’ instead of writing (package-license
> nginx).
>
I believe this is no longer an issue now that
nginx-accept-language-module is in the same Guile module as nginx, is
it?
> > +++ b/gnu/packages/web-xyz.scm
> > @@ -0,0 +1,175 @@
> > +;;; GNU Guix --- Functional package management for GNU
> > +;;;; TODO should I really add copyright lines for people I copied from??
> > +;;; Copyright © 2014, 2015 Mark H Weaver <m...@netris.org>
> > +;;; Copyright © 2016 Tobias Geerinckx-Rice <m...@tobias.gr>
> > +;;; Copyright © 2017, 2018 Marius Bakke <mba...@fastmail.com>
>
> I don’t think you need to add these 3 lines here; the package definition
> is yours.
>
This does not matter anymore after putting
nginx-accept-language-module in the same Guile module file as nginx.
In general though: IANAL but I have largely copied nginx’ configure
phase, so at least Mark would surely have copyright on parts of it.
However I believe copyright lines have limited legal significance
anyway and adding these authorship lines in a file not added by Mark
seems unreasonable.
> […]
> Perhaps “nginx-accept-language-module”, to match the name of the
> upstream repo?
>
I agree. Arch (who have no package for nginx-accept-language-module)
change their various nginx module package names to be more consistent,
I think, but this is not necessary in Guix, I think.
On Fri, Nov 01, 2019 at 03:58:43PM +0100, Ludovic Courtès wrote:
> Regarding the build system of nginx modules, we’ll see when
> we have more than one module packaged. :-)
>
Good. This module is not typical and writing a build system would be
difficult now.
Regards,
Florian
>From b6da504736866bae655e2b4025729345e1ea19b7 Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflor...@pelzflorian.de>
Date: Sat, 2 Nov 2019 13:13:01 +0100
Subject: [PATCH 1/3] doc: Add warning on the '--source' build option when
linking statically.
* doc/guix.texi (Additional Build Options): Add warning.
---
doc/guix.texi | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/doc/guix.texi b/doc/guix.texi
index da2423b422..30b69d8869 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -8328,6 +8328,12 @@ The returned source tarball is the result of applying
any patches and
code snippets specified in the package @code{origin} (@pxref{Defining
Packages}).
+Note that for statically linked packages, @command{guix build -S} will
+@emph{not} return the complete and corresponding sources since these
+would include the sources of statically linked dependencies. In this
+case, when distributing sources for license compliance, you may want to
+play it safe and use the following @code{--sources} option instead.
+
@item --sources
Fetch and return the source of @var{package-or-derivation} and all their
dependencies, recursively. This is a handy way to obtain a local copy
--
2.23.0
>From 21e6064f42defad1e2d35bbf95a4825fec9e4615 Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflor...@pelzflorian.de>
Date: Sat, 2 Nov 2019 12:43:47 +0100
Subject: [PATCH 2/3] gnu: Add Nginx Accept Language module.
* gnu/packages/web.scm (nginx-accept-language-module): New public variable.
---
gnu/packages/web.scm | 150 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 149 insertions(+), 1 deletion(-)
diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index 45ae63f193..6fcffb3004 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -35,6 +35,7 @@
;;; Copyright © 2019 Alex Griffin <a...@ajgrf.com>
;;; Copyright © 2019 Hartmut Goebel <h.goe...@crazy-compilers.com>
;;; Copyright © 2019 Jakob L. Kreuze <zerodaysford...@sdf.lonestar.org>
+;;; Copyright © 2019 Florian Pelz <pelzflor...@pelzflorian.de>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -134,7 +135,8 @@
#:use-module (gnu packages valgrind)
#:use-module (gnu packages version-control)
#:use-module (gnu packages vim)
- #:use-module (gnu packages xml))
+ #:use-module (gnu packages xml)
+ #:use-module ((srfi srfi-1) #:select (delete-duplicates)))
(define-public httpd
(package
@@ -395,6 +397,152 @@ documentation.")
"This package provides HTML documentation for the nginx web server.")
(license license:bsd-2))))
+(define-public nginx-accept-language-module
+ ;; Upstream has never made a release; use current commit instead.
+ (let ((commit "2f69842f83dac77f7d98b41a2b31b13b87aeaba7")
+ (revision "1"))
+ (package
+ (name "nginx-accept-language-module")
+ (version (git-version "0.0.0" ;upstream has no version number
+ revision commit))
+ (source
+ (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "https://github.com/giom/nginx_accept_language_module.git";)
+ (commit commit)))
+ (file-name (git-file-name name version))
+ (sha256
+ (base32 "1hjysrl15kh5233w7apq298cc2bp4q1z5mvaqcka9pdl90m0vhbw"))))
+ (build-system gnu-build-system)
+ (inputs `(("openssl" ,openssl)
+ ("pcre" ,pcre)
+ ("nginx-sources" ,(package-source nginx))
+ ("zlib" ,zlib)))
+ (arguments
+ `(#:tests? #f ; no test target
+ #:make-flags (list "modules")
+ #:modules ((guix build utils)
+ (guix build gnu-build-system)
+ (ice-9 popen)
+ (ice-9 regex)
+ (ice-9 textual-ports))
+ #:phases
+ (modify-phases %standard-phases
+ (add-after 'unpack 'unpack-nginx-sources
+ (lambda* (#:key inputs native-inputs #:allow-other-keys)
+ (begin
+ ;; The nginx source code is part of the module’s source.
+ (format #t "decompressing nginx source code~%")
+ (call-with-output-file "nginx.tar"
+ (lambda (out)
+ (let* ((gzip (assoc-ref inputs "gzip"))
+ (nginx-srcs (assoc-ref inputs "nginx-sources"))
+ (pipe (open-pipe* OPEN_READ
+ (string-append gzip "/bin/gzip")
+ "-cd"
+ nginx-srcs)))
+ (dump-port pipe out)
+ (unless (= (status:exit-val (close-pipe pipe)) 0)
+ (error "gzip decompress failed")))))
+ (invoke (string-append (assoc-ref inputs "tar") "/bin/tar")
+ "xvf" "nginx.tar" "--strip-components=1")
+ (delete-file "nginx.tar")
+ #t)))
+ (add-after 'unpack 'convert-to-dynamic-module
+ (lambda _
+ (begin
+ (with-atomic-file-replacement "config"
+ (lambda (in out)
+ ;; cf.
https://www.nginx.com/resources/wiki/extending/new_config/
+ (format out "ngx_module_type=HTTP~%")
+ (format out "ngx_module_name=\
+ngx_http_accept_language_module~%")
+ (let* ((str (get-string-all in))
+ (rx (make-regexp
+ "NGX_ADDON_SRCS=\"\\$NGX_ADDON_SRCS (.*)\""))
+ (m (regexp-exec rx str))
+ (srcs (match:substring m 1)))
+ (format out (string-append "ngx_module_srcs=\""
+ srcs "\"~%")))
+ (format out ". auto/module~%")
+ (format out "ngx_addon_name=$ngx_module_name~%"))))))
+ (add-before 'configure 'patch-/bin/sh
+ (lambda _
+ (substitute* "auto/feature"
+ (("/bin/sh") (which "sh")))
+ #t))
+ (replace 'configure
+ ;; This phase is largely copied from the nginx package.
+ (lambda* (#:key outputs #:allow-other-keys)
+ (let ((flags
+ (list ;; A copy of nginx’ flags follows, otherwise we
+ ;; get a binary compatibility error. FIXME: Code
+ ;; duplication is bad.
+ (string-append "--prefix=" (assoc-ref outputs "out"))
+ "--with-http_ssl_module"
+ "--with-http_v2_module"
+ "--with-pcre-jit"
+ "--with-debug"
+ ;; Even when not cross-building, we pass the
+ ;; --crossbuild option to avoid customizing for the
+ ;; kernel version on the build machine.
+ ,(let ((system "Linux") ; uname -s
+ (release "3.2.0") ; uname -r
+ ;; uname -m
+ (machine (match (or (%current-target-system)
+ (%current-system))
+ ("x86_64-linux" "x86_64")
+ ("i686-linux" "i686")
+ ("mips64el-linux" "mips64")
+ ;; Prevent errors when querying
+ ;; this package on unsupported
+ ;; platforms, e.g. when running
+ ;; "guix package --search="
+ (_ "UNSUPPORTED"))))
+ (string-append "--crossbuild="
+ system ":" release ":" machine))
+ ;; The following are the args decribed on
+ ;;
<https://www.nginx.com/blog/compiling-dynamic-modules-nginx-plus>.
+ ;; Enabling --with-compat here and in the nginx package
+ ;; would ensure binary compatibility even when using
+ ;; different configure options from the main nginx
+ ;; package. This is not needed for Guix.
+ ;; "--with-compat"
+ "--add-dynamic-module=.")))
+ (setenv "CC" "gcc")
+ (format #t "environment variable `CC' set to `gcc'~%")
+ (format #t "configure flags: ~s~%" flags)
+ (apply invoke "./configure" flags)
+ #t)))
+ (replace 'install
+ (lambda* (#:key outputs #:allow-other-keys)
+ (let* ((out (assoc-ref outputs "out"))
+ (modules-dir (string-append out "/etc/nginx/modules"))
+ (doc-dir (string-append
+ out
"/share/doc/nginx-accept-language-module")))
+ (mkdir-p modules-dir)
+ (copy-file "objs/ngx_http_accept_language_module.so"
+ (string-append
+ modules-dir
"/ngx_http_accept_language_module.so"))
+ (mkdir-p doc-dir)
+ (copy-file "README.textile"
+ (string-append doc-dir "/README.textile"))
+ #t))))))
+ (home-page
+ "https://www.nginx.com/resources/wiki/modules/accept_language/";)
+ (synopsis "Nginx module for parsing the Accept-Language HTTP header")
+ (description
+ "Nginx module that parses the Accept-Language field in HTTP headers and
+chooses the most suitable locale for the user from the list of locales
+supported at your website.")
+ (license (delete-duplicates
+ (cons license:bsd-2 ;license of nginx-accept-language-module
+ ;; The module’s code is linked statically with nginx,
+ ;; therefore nginx’ other licenses may also apply to its
+ ;; binary:
+ (package-license nginx)))))))
+
(define-public fcgi
(package
(name "fcgi")
--
2.23.0
>From 250ae2011ac1c976508136e9f50cb04e6ab5f23c Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflor...@pelzflorian.de>
Date: Sat, 2 Nov 2019 14:05:30 +0100
Subject: [PATCH 3/3] services: Make it possible to include dynamic modules in
nginx.
* gnu/services/web.scm (<nginx-configuration>): Add modules field.
(nginx-configuration-modules): New field accessor.
(emit-load-module): New procedure.
(default-nginx-config): Add support for the modules field.
* doc/guix.texi (NGINX): Document it.
---
doc/guix.texi | 4 ++++
gnu/services/web.scm | 8 ++++++++
2 files changed, 12 insertions(+)
diff --git a/doc/guix.texi b/doc/guix.texi
index 30b69d8869..898123da2b 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -19770,6 +19770,10 @@ use the size of the processors cache line.
@item @code{server-names-hash-bucket-max-size} (default: @code{#f})
Maximum bucket size for the server names hash tables.
+@item @code{modules} (default: @code{'()})
+List of nginx dynamic modules to load. Should be a list of strings or
+string valued G-expressions.
+
@item @code{extra-content} (default: @code{""})
Extra content for the @code{http} block. Should be string or a string
valued G-expression.
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 899be1c168..896d06eb18 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -9,6 +9,7 @@
;;; Copyright © 2018 Pierre-Antoine Rouby <pierre-antoine.ro...@inria.fr>
;;; Copyright © 2017, 2018, 2019 Christopher Baines <m...@cbaines.net>
;;; Copyright © 2018 Marius Bakke <mba...@fastmail.com>
+;;; Copyright © 2019 Florian Pelz <pelzflor...@pelzflorian.de>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -95,6 +96,7 @@
nginx-configuration-upstream-blocks
nginx-configuration-server-names-hash-bucket-size
nginx-configuration-server-names-hash-bucket-max-size
+ nginx-configuration-modules
nginx-configuration-extra-content
nginx-configuration-file
@@ -522,6 +524,7 @@
(default #f))
(server-names-hash-bucket-max-size
nginx-configuration-server-names-hash-bucket-max-size
(default #f))
+ (modules nginx-configuration-modules (default '()))
(extra-content nginx-configuration-extra-content
(default ""))
(file nginx-configuration-file ;#f | string | file-like
@@ -542,6 +545,9 @@ of index files."
((? string? str) (list str " ")))
names))
+(define (emit-load-module module)
+ (list "load_module " module ";\n"))
+
(define emit-nginx-location-config
(match-lambda
(($ <nginx-location-configuration> uri body)
@@ -615,12 +621,14 @@ of index files."
server-blocks upstream-blocks
server-names-hash-bucket-size
server-names-hash-bucket-max-size
+ modules
extra-content)
(apply mixed-text-file "nginx.conf"
(flatten
"user nginx nginx;\n"
"pid " run-directory "/pid;\n"
"error_log " log-directory "/error.log info;\n"
+ (map emit-load-module modules)
"http {\n"
" client_body_temp_path " run-directory "/client_body_temp;\n"
" proxy_temp_path " run-directory "/proxy_temp;\n"
--
2.23.0
