GNU bug report logs - #71586
[PATCH] services: web: Improve nginx formatting for extra-content

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71586 in the body.
You can then email your comments to 71586 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: guix-patches <at> gnu.org
Cc: Richard Sent <richard <at> freakingpenguin.com>
Subject: [PATCH] services: web: Improve nginx formatting for extra-content
Date: Sat, 15 Jun 2024 23:57:12 -0400

* gnu/services/web (default-nginx-config): When extra-content is a list, add 4
space indentation and a newline to every line. If it's a string, continue
inserting it directly. This makes the list serialization behavior more
consistent with other services.

Change-Id: Iec8614ba3cfc37292a566197e8d39b352b04846a
---
 gnu/services/web.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 406117c457..094b1e4b45 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -771,7 +771,12 @@ (define (default-nginx-config config)
            "\n"
            (map emit-nginx-upstream-config upstream-blocks)
            (map emit-nginx-server-config server-blocks)
-           extra-content
+           (match extra-content
+             ((? string? extra-content) extra-content)
+             ((? list? extra-content)
+              (map (lambda (line)
+                     (simple-format #f "    ~A\n" line))
+                   extra-content)))
            "\n}\n"))))
 
 (define %nginx-accounts

base-commit: 612e4dd98f7d1d015e405af9d029bede3fe3c280
-- 
2.45.1

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tomas Volf <~@wolfsden.cz>
To: Richard Sent <richard <at> freakingpenguin.com>
Cc: guix-patches <at> gnu.org
Subject: Re: [PATCH] services: web: Improve nginx formatting for extra-content
Date: Wed, 19 Jun 2024 19:55:17 +0200

[Message part 1 (text/plain, inline)]

Hello,

On 2024-06-15 23:57:12 -0400, Richard Sent wrote:
> * gnu/services/web (default-nginx-config): When extra-content is a list, add 4
> space indentation and a newline to every line. If it's a string, continue
> inserting it directly. This makes the list serialization behavior more
> consistent with other services.
>
> Change-Id: Iec8614ba3cfc37292a566197e8d39b352b04846a
> ---
>  gnu/services/web.scm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/services/web.scm b/gnu/services/web.scm
> index 406117c457..094b1e4b45 100644
> --- a/gnu/services/web.scm
> +++ b/gnu/services/web.scm
> @@ -771,7 +771,12 @@ (define (default-nginx-config config)
>             "\n"
>             (map emit-nginx-upstream-config upstream-blocks)
>             (map emit-nginx-server-config server-blocks)
> -           extra-content
> +           (match extra-content
> +             ((? string? extra-content) extra-content)
> +             ((? list? extra-content)
> +              (map (lambda (line)
> +                     (simple-format #f "    ~A\n" line))

The format here sadly prevents producing `include' directives with link to the
store, since any file-like object will be just serialized as its representation:

  `("foo"
    "bar"
    ("aa" "bb")
    ,(plain-file "foo" "baz"))

Will end up as:

    foo
    bar
    (aa bb)
    #<<plain-file> name: "foo" content: "baz" references: ()>

Neither the list nor the plain-file are as expected.  What about doing just
this:

    (match extra-content
      ((? string? extra-content) extra-content)
      ((? list? extra-content)
       (map (lambda (line)
              `("    " ,line "\n"))
            extra-content)))

This produces the expected (in my opinion) output:

    foo
    bar
    aabb
    /gnu/store/7pz4iczjvgvn9ikpp1ip50bk6vi4sk7b-foo

Meaning you can now do things like

    (extra-content `(("include " ,%some-file.conf ";")))

And it will work as expected.  It already works like this for raw-content in the
nginx-server-configuration.

Thoughts?

> +                   extra-content)))
>             "\n}\n"))))
>
>  (define %nginx-accounts
>
> base-commit: 612e4dd98f7d1d015e405af9d029bede3fe3c280
> --
> 2.45.1
>
>

Have a nice day,
Tomas Volf

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[signature.asc (application/pgp-signature, inline)]

Message #11 received at 71586 <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: 71586 <at> debbugs.gnu.org
Cc: Richard Sent <richard <at> freakingpenguin.com>, ~@wolfsden.cz
Subject: [PATCH v2] services: web: Improve nginx formatting for extra-content
Date: Wed, 19 Jun 2024 14:49:19 -0400

* gnu/services/web (default-nginx-config): When extra-content is a list, add 4
space indentation and a newline to every line. If it's a string, continue
inserting it directly. This makes the list serialization behavior more
consistent with other services.

Change-Id: Iec8614ba3cfc37292a566197e8d39b352b04846a
---

Updated according to feedback. I agree that file-like objects should
be lowered into strings like they normally are and not serialized in
their package form.

emit-nginx-upstream-config has a similar issue.

 gnu/services/web.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 406117c457..0572af1310 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -771,7 +771,12 @@ (define (default-nginx-config config)
            "\n"
            (map emit-nginx-upstream-config upstream-blocks)
            (map emit-nginx-server-config server-blocks)
-           extra-content
+           (match extra-content
+             ((? string? extra-content) extra-content)
+             ((? list? extra-content)
+              (map (lambda (line)
+                     `("    " ,line "\n"))
+                   extra-content)))
            "\n}\n"))))
 
 (define %nginx-accounts

base-commit: e32e3d0a03dc17c4c54a91aad053c9036998b601
-- 
2.45.1

Message #14 received at 71586 <at> debbugs.gnu.org (full text, mbox):

From: Richard Sent <richard <at> freakingpenguin.com>
To: 71586 <at> debbugs.gnu.org
Cc: ~@wolfsden.cz, Richard Sent <richard <at> freakingpenguin.com>
Subject: [PATCH v3] services: web: Improve nginx formatting for extra-content
Date: Thu, 20 Jun 2024 12:42:03 -0400

When extra-content is a list, add 4 space indentation and a newline to every
line. If it's a string, continue inserting it directly. This makes the list
serialization behavior more consistent with other services.

* gnu/services/web (default-nginx-config): Support lists.
* doc/guix.texi (Web Services) [nginx-configuration]: Document it.

Change-Id: Iec8614ba3cfc37292a566197e8d39b352b04846a
---
 doc/guix.texi        | 13 +++++++++++--
 gnu/services/web.scm |  9 ++++++++-
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 0102fd0fad..48fc457627 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -31907,8 +31907,17 @@ Web Services
 @end lisp
 
 @item @code{extra-content} (default: @code{""})
-Extra content for the @code{http} block.  Should be string or a string
-valued G-expression.
+Additional content to be appended to the @code{http} block.  Can either
+be a value that can be lowered into a string or a list of such values.
+In the former case, it is inserted directly.  In the latter, it is
+prefixed with indentation and suffixed with a newline.  Nested lists are
+flattened into one line.
+
+@lisp
+(extra-content "include /etc/nginx/custom-config.conf;")
+(extra-content `("include /etc/nginx/custom-config.conf;"
+                 ("include " ,%custom-config.conf ";")))
+@end lisp
 
 @end table
 @end deftp
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 406117c457..ee3499e5cd 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -771,7 +771,14 @@ (define (default-nginx-config config)
            "\n"
            (map emit-nginx-upstream-config upstream-blocks)
            (map emit-nginx-server-config server-blocks)
-           extra-content
+           (match extra-content
+             ((? list? extra-content)
+              (map (lambda (line)
+                     `("    " ,line "\n"))
+                   extra-content))
+             ;; XXX: For compatibility strings and gexp's are inserted
+             ;; directly.
+             (_ extra-content))
            "\n}\n"))))
 
 (define %nginx-accounts

base-commit: e32e3d0a03dc17c4c54a91aad053c9036998b601
-- 
2.45.1

Message #17 received at 71586 <at> debbugs.gnu.org (full text, mbox):

From: Tomas Volf <~@wolfsden.cz>
To: Richard Sent <richard <at> freakingpenguin.com>
Cc: 71586 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] services: web: Improve nginx formatting for
 extra-content
Date: Thu, 20 Jun 2024 23:29:06 +0200

[Message part 1 (text/plain, inline)]

On 2024-06-20 12:42:03 -0400, Richard Sent wrote:
> When extra-content is a list, add 4 space indentation and a newline to every
> line. If it's a string, continue inserting it directly. This makes the list
> serialization behavior more consistent with other services.
>
> * gnu/services/web (default-nginx-config): Support lists.
> * doc/guix.texi (Web Services) [nginx-configuration]: Document it.
>
> Change-Id: Iec8614ba3cfc37292a566197e8d39b352b04846a

Reviewed-by: Tomas Volf <~@wolfsden.cz>

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[signature.asc (application/pgp-signature, inline)]

Message #22 received at 71586-done <at> debbugs.gnu.org (full text, mbox):

From: Christopher Baines <mail <at> cbaines.net>
To: Richard Sent <richard <at> freakingpenguin.com>
Cc: 71586-done <at> debbugs.gnu.org
Subject: Re: [bug#71586] [PATCH v3] services: web: Improve nginx formatting
 for extra-content
Date: Thu, 27 Jun 2024 11:33:56 +0100

[Message part 1 (text/plain, inline)]

Richard Sent <richard <at> freakingpenguin.com> writes:

> When extra-content is a list, add 4 space indentation and a newline to every
> line. If it's a string, continue inserting it directly. This makes the list
> serialization behavior more consistent with other services.
>
> * gnu/services/web (default-nginx-config): Support lists.
> * doc/guix.texi (Web Services) [nginx-configuration]: Document it.
>
> Change-Id: Iec8614ba3cfc37292a566197e8d39b352b04846a
> ---
>  doc/guix.texi        | 13 +++++++++++--
>  gnu/services/web.scm |  9 ++++++++-
>  2 files changed, 19 insertions(+), 3 deletions(-)

Thanks for the patch, I forgot to close the issue yesterday but I pushed
this to master as da3e71f472c2335f56c3d07758715a514fdb3a9d.

Chris

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.