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

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

[Oleg Pykhalov]

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: [PATCH] web: Add try-files for the nginx-service-type. --]
[-- Type: text/x-patch, Size: 2029 bytes --]

From 16acb7e5298b21a3c40207e1719248eaa5698d50 Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@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

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

[Oleg Pykhalov]

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

--8<---------------cut here---------------start------------->8---
natsu@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@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---

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

[Oleg Pykhalov]

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-web-Add-try-files-for-the-nginx-service-type.patch --]
[-- Type: text/x-patch, Size: 1924 bytes --]

From 7177bc6e80c47e728486b4d5e7d61e926f17851a Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@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.
---
 gnu/services/web.scm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gnu/services/web.scm b/gnu/services/web.scm
index 4aa6fd501..aec778801 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,7 @@ of index files."
      (and/l ssl-certificate-key "      ssl_certificate_key " <> ";\n")
      "      root " root ";\n"
      "      index " (config-index-strings index) ";\n"
+     (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

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

[Oleg Pykhalov]

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-web-Add-try-files-for-the-nginx-service-type.patch --]
[-- Type: text/x-patch, Size: 1977 bytes --]

From 20e411b4d3513106de43980afd46a71fea4cf107 Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@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.
---
 gnu/services/web.scm | 6 ++++++
 1 file changed, 6 insertions(+)

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

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

[Julien Lepiller]

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

Le 24 septembre 2017 01:01:21 GMT+02:00, Oleg Pykhalov <go.wigust@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.

[-- Attachment #2: Type: text/html, Size: 948 bytes --]

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

[Oleg Pykhalov]

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

Hello Julien,

Julien Lepiller <julien@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-web-Add-try-files-for-the-nginx-service-type.patch --]
[-- Type: text/x-patch, Size: 2779 bytes --]

From fa5c4df154da2029d1f909c51b742377f55d732e Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@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        | 5 +++++
 gnu/services/web.scm | 6 ++++++
 2 files changed, 11 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 0462a6419..e60ddafde 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -14344,6 +14344,11 @@ 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 to check their existence in the specified order and uses
+the first found file for request processing; the processing is performed
+in the current context.
+
 @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

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

[Julien Lepiller]

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

Le 24 septembre 2017 20:09:26 GMT+02:00, Oleg Pykhalov <go.wigust@gmail.com> a écrit :
>Hello Julien,
>
>Julien Lepiller <julien@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!

[-- Attachment #2: Type: text/html, Size: 1509 bytes --]

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

[Oleg Pykhalov]

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

Hello Julien,

Julien Lepiller <julien@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-web-Add-try-files-for-the-nginx-service-type.patch --]
[-- Type: text/x-patch, Size: 3170 bytes --]

From 783d1c22dfda8d1a86c0a01683ebe1278e44c3bd Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@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

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

[Julien Lepiller]

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

> Hello Julien,
> 
> Julien Lepiller <julien@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.