Subject: Re: bug#26302: [website] translations

From: Ludovic Courtès <[email-masked]>
Date: Mon, 04 Nov 2019 18:19:32 +0100

Hi,

"pelzflorian (Florian Pelz)" <[email-masked]> skribis:

> 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.

Good!

>> > +      (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?

You’re right: it’s no longer an issue.

>> > +++ 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 <[email-masked]>
>> > +;;; Copyright © 2016 Tobias Geerinckx-Rice <[email-masked]>
>> > +;;; Copyright © 2017, 2018 Marius Bakke <[email-masked]>
>> 
>> 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.

Agreed.

>> […]
>> 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.

For Guix the general rule is to follow upstream (info "(guix) Package
Naming").

> From b6da504736866bae655e2b4025729345e1ea19b7 Mon Sep 17 00:00:00 2001
> From: Florian Pelz <[email-masked]>
> 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.

I don’t think this bit is necessary: ‘-S’ is documented to return “the
source of the package” and that’s exactly what it does; static
vs. dynamic linking is not a concern at this level, as I see it.

WDYT?

> From 21e6064f42defad1e2d35bbf95a4825fec9e4615 Mon Sep 17 00:00:00 2001
> From: Florian Pelz <[email-masked]>
> 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.

LGTM!

Thanks for taking the time to rewrite the snippet as a build phase.

> From 250ae2011ac1c976508136e9f50cb04e6ab5f23c Mon Sep 17 00:00:00 2001
> From: Florian Pelz <[email-masked]>
> 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.

Then… how does nginx find the module in question, specifically the
‘nginx-accept-language-module’ package?  One has to specify
‘nginx-accept-language-module’ as the nginx package to use, is that
right?  (I had overlooked that before.)

What about adding an example with the ‘accept-language’ module?

Thank you!

Ludo’.