GNU bug report logs - #28560
[PATCH] web: Add try-files for the nginx-service-type.

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 28560 in the body.
You can then email your comments to 28560 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: Oleg Pykhalov <go.wigust <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Ludovic
 Courtès <ludo <at> gnu.org>, Christopher Baines <mail <at> cbaines.net>
Subject: [PATCH] web: Add try-files for the nginx-service-type.
Date: Sat, 23 Sep 2017 03:38:39 +0300

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

Hello Christopher,

I'm working with Ludovic on adding Cgit service to GuixSD as an
extension of Nginx service.  To do this I follow
https://wiki.archlinux.org/index.php/Cgit#Using_fcgiwrap where try_files
present in example.  What do you think?

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28283#14

[0001-web-Add-try-files-for-the-nginx-service-type.patch (text/x-patch, inline)]

From 16acb7e5298b21a3c40207e1719248eaa5698d50 Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust <at> gmail.com>
Date: Sat, 23 Sep 2017 03:27:49 +0300
Subject: [PATCH] web: Add try-files for the nginx-service-type.

* gnu/services/web.scm (<nginx-server-configuration>): Add try-files.
(emit-nginx-server-config): Add this.
---
 gnu/services/web.scm | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 4aa6fd501..c03b1da5c 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -99,6 +99,8 @@
                        (default '()))
   (index               nginx-server-configuration-index
                        (default (list "index.html")))
+  (try-files           nginx-server-configuration-try-files
+                       (default '()))
   (ssl-certificate     nginx-server-configuration-ssl-certificate
                        (default "/etc/nginx/cert.pem"))
   (ssl-certificate-key nginx-server-configuration-ssl-certificate-key
@@ -179,6 +181,7 @@ of index files."
          (nginx-server-configuration-ssl-certificate-key server))
         (root (nginx-server-configuration-root server))
         (index (nginx-server-configuration-index server))
+        (try-files (nginx-server-configuration-try-files server))
         (server-tokens? (nginx-server-configuration-server-tokens? server))
         (locations (nginx-server-configuration-locations server)))
     (define-syntax-parameter <> (syntax-rules ()))
@@ -207,6 +210,11 @@ of index files."
      (and/l ssl-certificate-key "      ssl_certificate_key " <> ";\n")
      "      root " root ";\n"
      "      index " (config-index-strings index) ";\n"
+     (if (not (nil? (config-index-strings try-files)))
+         (string-append "      try_files "
+                        (config-index-strings try-files)
+                        ";\n")
+         "")
      "      server_tokens " (if server-tokens? "on" "off") ";\n"
      "\n"
      (map emit-nginx-location-config locations)
-- 
2.14.1

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

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: 28560 <at> debbugs.gnu.org
Subject: Re: [bug#28560] [PATCH] web: Add try-files for the nginx-service-type.
Date: Sun, 24 Sep 2017 00:54:03 +0300

Ah, actually I need also to test on real system config before pushed a patch.  Sorry.

--8<---------------cut here---------------start------------->8---
natsu <at> magnolia ~/src/guix$ ./pre-inst-env guix system build ~/dotfiles/guix/system-magnolia.scm 
Backtrace:
In srfi/srfi-1.scm:
   592:29 19 (map1 (#<<service> type: #<service-type mingetty 16f…> …))
   592:29 18 (map1 (#<<service> type: #<service-type mingetty 16f…> …))
   592:29 17 (map1 (#<<service> type: #<service-type console-font…> …))
   592:29 16 (map1 (#<<service> type: #<service-type ntp 1556910>…> …))
   592:29 15 (map1 (#<<service> type: #<service-type dbus 1556a50…> …))
   592:29 14 (map1 (#<<service> type: #<service-type elogind 2d7f…> …))
   592:29 13 (map1 (#<<service> type: #<service-type upower 2d7ff…> …))
   592:29 12 (map1 (#<<service> type: #<service-type avahi 16fe0a…> …))
   592:29 11 (map1 (#<<service> type: #<service-type wpa-supplica…> …))
   592:29 10 (map1 (#<<service> type: #<service-type network-mana…> …))
   592:29  9 (map1 (#<<service> type: #<service-type slim 16fe050…> …))
   592:29  8 (map1 (#<<service> type: #<service-type firewall 2e5…> …))
   592:29  7 (map1 (#<<service> type: #<service-type fcgiwrap 2dd…> …))
   592:17  6 (map1 (#<<service> type: #<service-type nginx 2dd887…> …))
In gnu/services/web.scm:
   317:21  5 (nginx-shepherd-service _)
   308:35  4 (nginx-action . _)
   256:10  3 (default-nginx-config #<package nginx <at> 1.12.1 gnu/packa…> …)
In srfi/srfi-1.scm:
   592:17  2 (map1 (#<<nginx-server-configuration> http-port: 19418…>))
In gnu/services/web.scm:
    214:9  1 (emit-nginx-server-config _)
In unknown file:
           0 (string-append "      try_files " (("$uri" " ") (# #)) #)

ERROR: In procedure string-append:
ERROR: In procedure string-append: Wrong type (expecting string): (("$uri" " ") ("@cgit" " "))
--8<---------------cut here---------------end--------------->8---

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

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: 28560 <at> debbugs.gnu.org
Subject: Re: [bug#28560] [PATCH] web: Add try-files for the nginx-service-type.
Date: Sun, 24 Sep 2017 01:13:12 +0300

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

OK, I hacked little bit.  Here is a snippet of Nginx service in system
declaration and produced Nginx config from system build.

--8<---------------cut here---------------start------------->8---
(define %cgit-configuration-nginx
  (list
   (nginx-server-configuration
    (root cgit)
    (locations
     (list
      (nginx-location-configuration
       (uri "@cgit")
       (body '("fastcgi_param SCRIPT_FILENAME $document_root/lib/cgit/cgit.cgi;"
               "fastcgi_param PATH_INFO $uri;"
               "fastcgi_param QUERY_STRING $args;"
               "fastcgi_param HTTP_HOST $server_name;"
               "fastcgi_pass 127.0.0.1:9000;")))))
    (try-files (list "$uri" "@cgit"))
    (http-port 19418)
    (https-port #f)
    (ssl-certificate #f)
    (ssl-certificate-key #f))))

(operating-system
  …
  (services (cons* …
                   (service nginx-service-type)
                   (service fcgiwrap-service-type)
                   (service cgit-service-type
                            (cgit-configuration
                             (nginx %cgit-configuration-nginx)))
                   …)))
--8<---------------cut here---------------end--------------->8---

/gnu/store/pv4kna4526cijzpd6my69ikdb5qzb93m-nginx.conf
--8<---------------cut here---------------start------------->8---
user nginx nginx;
pid /var/run/nginx/pid;
error_log /var/log/nginx/error.log info;
http {
    client_body_temp_path /var/run/nginx/client_body_temp;
    proxy_temp_path /var/run/nginx/proxy_temp;
    fastcgi_temp_path /var/run/nginx/fastcgi_temp;
    uwsgi_temp_path /var/run/nginx/uwsgi_temp;
    scgi_temp_path /var/run/nginx/scgi_temp;
    access_log /var/log/nginx/access.log;
    include /gnu/store/vyj2vkmdmlpxn3mnj71vz8zc8j30ahkf-nginx-1.12.1/share/nginx/conf/mime.types;

    server {
      listen 19418;
      server_name _ ;
      root /gnu/store/b4zg0sz6z1n32qnwmgmbbwh78i55hr56-cgit-1.1;
      index index.html ;
      try_files $uri @cgit ;
      server_tokens off;

      location @cgit {
        fastcgi_param SCRIPT_FILENAME $document_root/lib/cgit/cgit.cgi;
        fastcgi_param PATH_INFO $uri;
        fastcgi_param QUERY_STRING $args;
        fastcgi_param HTTP_HOST $server_name;
        fastcgi_pass 127.0.0.1:9000;
      }

    }
}
events {}
--8<---------------cut here---------------end--------------->8---

[0001-web-Add-try-files-for-the-nginx-service-type.patch (text/x-patch, attachment)]

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

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: 28560 <at> debbugs.gnu.org
Subject: Re: [bug#28560] [PATCH] web: Add try-files for the nginx-service-type.
Date: Sun, 24 Sep 2017 02:01:21 +0300

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

Fix failed test.
--8<---------------cut here---------------start------------->8---
guix environment guix -- make check-system TESTS=nginx
--8<---------------cut here---------------end--------------->8---

And system build is fine, too.
--8<---------------cut here---------------start------------->8---
./pre-inst-env guix system build ~/dotfiles/guix/system-magnolia.scm
--8<---------------cut here---------------end--------------->8---

[0001-web-Add-try-files-for-the-nginx-service-type.patch (text/x-patch, attachment)]

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

From: Julien Lepiller <julien <at> lepiller.eu>
To: 28560 <at> debbugs.gnu.org
Subject: Re: [bug#28560] [PATCH] web: Add try-files for the nginx-service-type.
Date: Sun, 24 Sep 2017 15:02:43 +0200

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

Le 24 septembre 2017 01:01:21 GMT+02:00, Oleg Pykhalov <go.wigust <at> gmail.com> a écrit :
>Fix failed test.
>--8<---------------cut here---------------start------------->8---
>guix environment guix -- make check-system TESTS=nginx
>--8<---------------cut here---------------end--------------->8---
>
>And system build is fine, too.
>--8<---------------cut here---------------start------------->8---
>./pre-inst-env guix system build ~/dotfiles/guix/system-magnolia.scm
>--8<---------------cut here---------------end--------------->8---

Thank you for the patch! I think it will be perfect after you update doc/guix.texi too.

[Message part 2 (text/html, inline)]

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

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: Julien Lepiller <julien <at> lepiller.eu>
Cc: 28560 <at> debbugs.gnu.org
Subject: Re: [bug#28560] [PATCH] web: Add try-files for the nginx-service-type.
Date: Sun, 24 Sep 2017 21:09:26 +0300

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

Hello Julien,

Julien Lepiller <julien <at> lepiller.eu> writes:

> Thank you for the patch! I think it will be perfect after you update doc/guix.texi too.

Thank you for review.

I rephrase little bit
https://nginx.org/en/docs/http/ngx_http_core_module.html#try_files

Here is a new patch.

[0001-web-Add-try-files-for-the-nginx-service-type.patch (text/x-patch, attachment)]

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

From: Julien Lepiller <julien <at> lepiller.eu>
Cc: 28560 <at> debbugs.gnu.org
Subject: Re: [bug#28560] [PATCH] web: Add try-files for the nginx-service-type.
Date: Tue, 26 Sep 2017 09:53:52 +0200

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

Le 24 septembre 2017 20:09:26 GMT+02:00, Oleg Pykhalov <go.wigust <at> gmail.com> a écrit :
>Hello Julien,
>
>Julien Lepiller <julien <at> lepiller.eu> writes:
>
>> Thank you for the patch! I think it will be perfect after you update
>doc/guix.texi too.
>
>Thank you for review.
>
>I rephrase little bit
>https://nginx.org/en/docs/http/ngx_http_core_module.html#try_files
>
>Here is a new patch.

Hi,

I don't think your rephrasing works, I have some troubles making sense of it, even though I know what it is supposed to do. Maybe:

A list of files whose existence is checked in the specified order. @code{nginx} will use the first file it finds to process the request.

I'm not sure about the last part of your sentence, do we explain what the "current context" is? Is it even relevant? Could you add a sentence to explain the usage of variables here, such as "$uri"? Maybe a small example would be enough.

Thank you!

[Message part 2 (text/html, inline)]

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

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: Julien Lepiller <julien <at> lepiller.eu>
Cc: 28560 <at> debbugs.gnu.org
Subject: Re: [bug#28560] [PATCH] web: Add try-files for the nginx-service-type.
Date: Thu, 28 Sep 2017 20:04:53 +0300

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

Hello Julien,

Julien Lepiller <julien <at> lepiller.eu> writes:

> I'm not sure about the last part of your sentence, do we explain what
> the "current context" is?

No we didn't.  And I don't see any ‘context’ mention in ‘@subsubsection
Web Services’.

> Is it even relevant?

Probably not relevant.

> Could you add a sentence to explain the usage of variables here, such
> as "$uri"? Maybe a small example would be enough.

Sure.

[0001-web-Add-try-files-for-the-nginx-service-type.patch (text/x-patch, inline)]

From 783d1c22dfda8d1a86c0a01683ebe1278e44c3bd Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust <at> gmail.com>
Date: Sat, 23 Sep 2017 03:27:49 +0300
Subject: [PATCH] web: Add try-files for the nginx-service-type.

* gnu/services/web.scm (<nginx-server-configuration>): Add
  nginx-server-configuration-try-files.
(emit-nginx-server-config): Add this.
* doc/guix.texi (Web Services): Add this.
---
 doc/guix.texi        | 7 ++++++-
 gnu/services/web.scm | 6 ++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index dd0a46a63..1b329d255 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -14263,7 +14263,8 @@ blocks, as in this example:
                         (https-port #f)
                         (ssl-certificate #f)
                         (ssl-certificate-key #f)
-                        (root "/srv/http/extra-website"))))
+                        (root "/srv/http/extra-website")
+                        (try-files (list "$uri" "$uri/index.html")))))
 @end example
 @end deffn
 
@@ -14394,6 +14395,10 @@ server block.
 Index files to look for when clients ask for a directory.  If it cannot be found,
 Nginx will send the list of files in the directory.
 
+@item @code{try-files} (default: @code{'()})
+A list of files whose existence is checked in the specified order.
+@code{nginx} will use the first file it finds to process the request.
+
 @item @code{ssl-certificate} (default: @code{"/etc/nginx/cert.pem"})
 Where to find the certificate for secure connections.  Set it to @code{#f} if
 you don't have a certificate or you don't want to use HTTPS.
diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 4aa6fd501..9d713003c 100644
--- a/gnu/services/web.scm
+++ b/gnu/services/web.scm
@@ -99,6 +99,8 @@
                        (default '()))
   (index               nginx-server-configuration-index
                        (default (list "index.html")))
+  (try-files           nginx-server-configuration-try-files
+                       (default '()))
   (ssl-certificate     nginx-server-configuration-ssl-certificate
                        (default "/etc/nginx/cert.pem"))
   (ssl-certificate-key nginx-server-configuration-ssl-certificate-key
@@ -179,6 +181,7 @@ of index files."
          (nginx-server-configuration-ssl-certificate-key server))
         (root (nginx-server-configuration-root server))
         (index (nginx-server-configuration-index server))
+        (try-files (nginx-server-configuration-try-files server))
         (server-tokens? (nginx-server-configuration-server-tokens? server))
         (locations (nginx-server-configuration-locations server)))
     (define-syntax-parameter <> (syntax-rules ()))
@@ -207,6 +210,9 @@ of index files."
      (and/l ssl-certificate-key "      ssl_certificate_key " <> ";\n")
      "      root " root ";\n"
      "      index " (config-index-strings index) ";\n"
+     (if (not (nil? try-files))
+         (and/l (config-index-strings try-files) "      try_files " <> ";\n")
+         "")
      "      server_tokens " (if server-tokens? "on" "off") ";\n"
      "\n"
      (map emit-nginx-location-config locations)
-- 
2.14.1

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

From: Julien Lepiller <julien <at> lepiller.eu>
To: 28560-done <at> debbugs.gnu.org
Subject: Re: [bug#28560] [PATCH] web: Add try-files for the nginx-service-type.
Date: Thu, 28 Sep 2017 20:40:05 +0200

Le Thu, 28 Sep 2017 20:04:53 +0300,
Oleg Pykhalov <go.wigust <at> gmail.com> a écrit :

> Hello Julien,
> 
> Julien Lepiller <julien <at> lepiller.eu> writes:
> 
> > I'm not sure about the last part of your sentence, do we explain
> > what the "current context" is?  
> 
> No we didn't.  And I don't see any ‘context’ mention in
> ‘@subsubsection Web Services’.
> 
> > Is it even relevant?  
> 
> Probably not relevant.
> 
> > Could you add a sentence to explain the usage of variables here,
> > such as "$uri"? Maybe a small example would be enough.  
> 
> Sure.
> 

Thank you, that was perfect!

Pushed as 4d14808af4c01b4fb0a4564584aa68f0e53c4ef4, with a slightly
different commit message.

Previous Next

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