bug#26302: [website] translations

["pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de>]

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

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 <mhw@netris.org>
> > +;;; Copyright © 2016 Tobias Geerinckx-Rice <me@tobias.gr>
> > +;;; Copyright © 2017, 2018 Marius Bakke <mbakke@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

[-- Attachment #2: 0001-doc-Add-warning-on-the-source-build-option-when-link.patch --]
[-- Type: text/plain, Size: 1203 bytes --]

From b6da504736866bae655e2b4025729345e1ea19b7 Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflorian@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


[-- Attachment #3: 0002-gnu-Add-Nginx-Accept-Language-module.patch --]
[-- Type: text/plain, Size: 9265 bytes --]

From 21e6064f42defad1e2d35bbf95a4825fec9e4615 Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflorian@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.goebel@crazy-compilers.com>
 ;;; Copyright © 2019 Jakob L. Kreuze <zerodaysfordays@sdf.lonestar.org>
+;;; Copyright © 2019 Florian Pelz <pelzflorian@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


[-- Attachment #4: 0003-services-Make-it-possible-to-include-dynamic-modules.patch --]
[-- Type: text/plain, Size: 3373 bytes --]

From 250ae2011ac1c976508136e9f50cb04e6ab5f23c Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflorian@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.rouby@inria.fr>
 ;;; Copyright © 2017, 2018, 2019 Christopher Baines <mail@cbaines.net>
 ;;; Copyright © 2018 Marius Bakke <mbakke@fastmail.com>
+;;; Copyright © 2019 Florian Pelz <pelzflorian@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