diff mbox series

[bug#49431,maintenance.git] nginx: berlin: Redirect old video URLs for each language.

Message ID 20210706065304.6kqbwqlcf6calkh7@pelzflorian.localdomain
State Accepted
Headers show
Series [bug#49431,maintenance.git] nginx: berlin: Redirect old video URLs for each language. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

pelzflorian (Florian Pelz) July 6, 2021, 6:53 a.m. UTC 
Hi guix,

a week ago Luis nicely redesigned how videos are displayed on the Guix
website
<https://lists.gnu.org/archive/html/guix-patches/2021-07/msg00005.html>.
(Because the old CSS stylesheets are cached by the browser, the
website displayed videos wrongly for a day though.)

This patch adds redirects for the guix website from the old video URLs
like

/LANG/videos/everyday-use-of-gnu-guix,-part-one/index.html
/LANG/videos/everyday-use-of-gnu-guix,-part-one/
/LANG/videos/everyday-use-of-gnu-guix,-part-one

to the new

/LANG/videos/2020/everyday-use-of-gnu-guix-part-one/

but maybe the patch is too ugly because that’s code duplication to
have three added redirections for each video.  Note that my old
videos page actually linked to
</LANG/videos/everyday-use-of-gnu-guix,-part-one/index.html> and not
</LANG/videos/everyday-use-of-gnu-guix,-part-one/> as usual.

Shall I push it as-is to guix/maintenance.git,

or do you think it would be better to rewrite the redirect procedure
to return a list to redirect every URL with suffixes

""
"/"
"/index.html"

and change guix.gnu.org-locations accordingly to flatten the location
list?

The patch was tested on a VM of berlin.scm with some services, file
systems and SSL removed and I’m confident it does what it should.

Regards,
Florian
From d52a189332aa014f9da88fa1ce1e38ffc107fa3b Mon Sep 17 00:00:00 2001
From: Florian Pelz <pelzflorian@pelzflorian.de>
Date: Sun, 4 Jul 2021 14:46:39 +0200
Subject: [PATCH] nginx: berlin: Redirect old video URLs for each language.

* hydra/nginx/berlin.scm (languages-to-accept): New list.  Move here
the languages list from ...
(%extra-content) ... here.  Use languages from 'languages-to-accept'.
(guix.gnu.org-redirect-locations-for-lang): New procedure.  Add
new video redirections.
(guix.gnu.org-redirect-locations): New list.  Move here the
old redirections.  Update old video redirections.
(guix.gnu.org-other-locations): New list.  Move here the other nginx
locations from ...
(guix.gnu.org-locations): ... here.  Reimplement in terms of the above.
---
 hydra/nginx/berlin.scm | 78 ++++++++++++++++++++++++++++++++----------
 1 file changed, 60 insertions(+), 18 deletions(-)

Comments

Luis Felipe July 7, 2021, 5:51 p.m. UTC | #1 
Hi Florian,


On Tuesday, July 6th, 2021 at 6:53 AM, pelzflorian (Florian Pelz) <pelzflorian@pelzflorian.de> wrote:

> This patch adds redirects for the guix website from the old video URLs
>
> like
>
> /LANG/videos/everyday-use-of-gnu-guix,-part-one/index.html
>
> /LANG/videos/everyday-use-of-gnu-guix,-part-one/
>
> /LANG/videos/everyday-use-of-gnu-guix,-part-one
>
> to the new
>
> /LANG/videos/2020/everyday-use-of-gnu-guix-part-one/
>
> but maybe the patch is too ugly because that’s code duplication to
>
> have three added redirections for each video. Note that my old
>
> videos page actually linked to
>
> </LANG/videos/everyday-use-of-gnu-guix,-part-one/index.html> and not
>
> </LANG/videos/everyday-use-of-gnu-guix,-part-one/> as usual.
>
> Shall I push it as-is to guix/maintenance.git,
>
> or do you think it would be better to rewrite the redirect procedure
>
> to return a list to redirect every URL with suffixes
>
> ""
>
> "/"
>
> "/index.html"
>
> and change guix.gnu.org-locations accordingly to flatten the location
>
> list?

I took a look at the patch, but I'm not familiar with the server-side of things, so there's not much I can say except that I trust your judgement on this :)

Thanks for taking care of it.
Ludovic Courtès July 8, 2021, 1:53 p.m. UTC | #2 
Hi,

"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:

> This patch adds redirects for the guix website from the old video URLs
> like
>
> /LANG/videos/everyday-use-of-gnu-guix,-part-one/index.html
> /LANG/videos/everyday-use-of-gnu-guix,-part-one/
> /LANG/videos/everyday-use-of-gnu-guix,-part-one
>
> to the new
>
> /LANG/videos/2020/everyday-use-of-gnu-guix-part-one/

[...]

>>From d52a189332aa014f9da88fa1ce1e38ffc107fa3b Mon Sep 17 00:00:00 2001
> From: Florian Pelz <pelzflorian@pelzflorian.de>
> Date: Sun, 4 Jul 2021 14:46:39 +0200
> Subject: [PATCH] nginx: berlin: Redirect old video URLs for each language.
>
> * hydra/nginx/berlin.scm (languages-to-accept): New list.  Move here
> the languages list from ...
> (%extra-content) ... here.  Use languages from 'languages-to-accept'.
> (guix.gnu.org-redirect-locations-for-lang): New procedure.  Add
> new video redirections.
> (guix.gnu.org-redirect-locations): New list.  Move here the
> old redirections.  Update old video redirections.
> (guix.gnu.org-other-locations): New list.  Move here the other nginx
> locations from ...
> (guix.gnu.org-locations): ... here.  Reimplement in terms of the above.

[...]

>     (redirect "/screenshots/xfce" "/$lang/screenshots/xfce/")
>     (redirect "/security" "/$lang/security/")
>     (redirect "/videos" "/$lang/videos/")
> -   (redirect "/videos/asking-for-help" "/$lang/videos/asking-for-help/")
> -   (redirect "/videos/everyday-use-of-gnu-guix,-part-one" "/$lang/videos/everyday-use-of-gnu-guix,-part-one/")
> -   (redirect "/videos/everyday-use-of-gnu-guix,-part-two" "/$lang/videos/everyday-use-of-gnu-guix,-part-two/")
> -   (redirect "/videos/installation-from-script" "/$lang/videos/installation-from-script/")
> -   (redirect "/videos/packaging,-part-one" "/$lang/videos/packaging,-part-one/")
> -   (redirect "/videos/packaging,-part-two" "/$lang/videos/packaging,-part-two/")
> -   (redirect "/videos/packaging,-part-three" "/$lang/videos/packaging,-part-three/")
> -
> +   (redirect "/videos/asking-for-help" "/$lang/videos/2020/asking-for-help/")
> +   (redirect "/videos/everyday-use-of-gnu-guix,-part-one" "/$lang/videos/2020/everyday-use-of-gnu-guix-part-one/")
> +   (redirect "/videos/everyday-use-of-gnu-guix,-part-two" "/$lang/videos/2020/everyday-use-of-gnu-guix-part-two/")
> +   (redirect "/videos/installation-from-script" "/$lang/videos/2020/installation-from-script/")
> +   (redirect "/videos/packaging,-part-one" "/$lang/videos/2020/packaging-part-one/")
> +   (redirect "/videos/packaging,-part-two" "/$lang/videos/2020/packaging-part-two/")
> +   (redirect "/videos/packaging,-part-three" "/$lang/videos/2020/packaging-part-three/")))

Shouldn’t we also redirect

  /$lang/videos/everyday-use-of-gnu-guix,-part-one/ -> /$lang/videos/2020/everyday-use-of-gnu-guix-part-one/

since the left-hand side here was the right-hand side of the previous
redirects?

Anyway, at first sight it LGTM!

Thanks,
Ludo’.
diff mbox series

Patch

diff --git a/hydra/nginx/berlin.scm b/hydra/nginx/berlin.scm
index 443294e..a8f4690 100644
--- a/hydra/nginx/berlin.scm
+++ b/hydra/nginx/berlin.scm
@@ -188,7 +188,7 @@  PUBLISH-URL."
             (body
              (list "root /var/www/guix;"))))))
 
-(define guix.gnu.org-locations
+(define guix.gnu.org-redirect-locations
   (list
    ;; Short URL for the installation script
    (redirect "/install.sh" "https://git.savannah.gnu.org/cgit/guix.git/plain/etc/guix-install.sh")
@@ -689,14 +689,46 @@  PUBLISH-URL."
    (redirect "/screenshots/xfce" "/$lang/screenshots/xfce/")
    (redirect "/security" "/$lang/security/")
    (redirect "/videos" "/$lang/videos/")
-   (redirect "/videos/asking-for-help" "/$lang/videos/asking-for-help/")
-   (redirect "/videos/everyday-use-of-gnu-guix,-part-one" "/$lang/videos/everyday-use-of-gnu-guix,-part-one/")
-   (redirect "/videos/everyday-use-of-gnu-guix,-part-two" "/$lang/videos/everyday-use-of-gnu-guix,-part-two/")
-   (redirect "/videos/installation-from-script" "/$lang/videos/installation-from-script/")
-   (redirect "/videos/packaging,-part-one" "/$lang/videos/packaging,-part-one/")
-   (redirect "/videos/packaging,-part-two" "/$lang/videos/packaging,-part-two/")
-   (redirect "/videos/packaging,-part-three" "/$lang/videos/packaging,-part-three/")
-
+   (redirect "/videos/asking-for-help" "/$lang/videos/2020/asking-for-help/")
+   (redirect "/videos/everyday-use-of-gnu-guix,-part-one" "/$lang/videos/2020/everyday-use-of-gnu-guix-part-one/")
+   (redirect "/videos/everyday-use-of-gnu-guix,-part-two" "/$lang/videos/2020/everyday-use-of-gnu-guix-part-two/")
+   (redirect "/videos/installation-from-script" "/$lang/videos/2020/installation-from-script/")
+   (redirect "/videos/packaging,-part-one" "/$lang/videos/2020/packaging-part-one/")
+   (redirect "/videos/packaging,-part-two" "/$lang/videos/2020/packaging-part-two/")
+   (redirect "/videos/packaging,-part-three" "/$lang/videos/2020/packaging-part-three/")))
+
+(define (guix.gnu.org-redirect-locations-for-lang lang)
+  (define (redirect-lang old new)
+    (redirect (string-append "/" lang old)
+              (string-append "/" lang new)))
+  (list
+   (redirect-lang "/videos/everyday-use-of-gnu-guix,-part-one" "/videos/2020/everyday-use-of-gnu-guix-part-one/")
+   (redirect-lang "/videos/everyday-use-of-gnu-guix,-part-two" "/videos/2020/everyday-use-of-gnu-guix-part-two/")
+   (redirect-lang "/videos/system-graphical-installer" "/videos/2020/system-graphical-installer/")
+   (redirect-lang "/videos/asking-for-help" "/videos/2020/asking-for-help/")
+   (redirect-lang "/videos/installation-from-script" "/videos/2020/installation-from-script/")
+   (redirect-lang "/videos/packaging,-part-one" "/videos/2020/packaging-part-one/")
+   (redirect-lang "/videos/packaging,-part-two" "/videos/2020/packaging-part-two/")
+   (redirect-lang "/videos/packaging,-part-three" "/videos/2020/packaging-part-three/")
+   (redirect-lang "/videos/everyday-use-of-gnu-guix,-part-one/index.html" "/videos/2020/everyday-use-of-gnu-guix-part-one/")
+   (redirect-lang "/videos/everyday-use-of-gnu-guix,-part-two/index.html" "/videos/2020/everyday-use-of-gnu-guix-part-two/")
+   (redirect-lang "/videos/system-graphical-installer/index.html" "/videos/2020/system-graphical-installer/")
+   (redirect-lang "/videos/asking-for-help/index.html" "/videos/2020/asking-for-help/")
+   (redirect-lang "/videos/installation-from-script/index.html" "/videos/2020/installation-from-script/")
+   (redirect-lang "/videos/packaging,-part-one/index.html" "/videos/2020/packaging-part-one/")
+   (redirect-lang "/videos/packaging,-part-two/index.html" "/videos/2020/packaging-part-two/")
+   (redirect-lang "/videos/packaging,-part-three/index.html" "/videos/2020/packaging-part-three/")
+   (redirect-lang "/videos/everyday-use-of-gnu-guix,-part-one/" "/videos/2020/everyday-use-of-gnu-guix-part-one/")
+   (redirect-lang "/videos/everyday-use-of-gnu-guix,-part-two/" "/videos/2020/everyday-use-of-gnu-guix-part-two/")
+   (redirect-lang "/videos/system-graphical-installer/" "/videos/2020/system-graphical-installer/")
+   (redirect-lang "/videos/asking-for-help/" "/videos/2020/asking-for-help/")
+   (redirect-lang "/videos/installation-from-script/" "/videos/2020/installation-from-script/")
+   (redirect-lang "/videos/packaging,-part-one/" "/videos/2020/packaging-part-one/")
+   (redirect-lang "/videos/packaging,-part-two/" "/videos/2020/packaging-part-two/")
+   (redirect-lang "/videos/packaging,-part-three/" "/videos/2020/packaging-part-three/")))
+
+(define guix.gnu.org-other-locations
+  (list
    (nginx-location-configuration
     (uri "/guix-videos")
     (body (list "alias /srv/videos;")))
@@ -754,6 +786,24 @@  PUBLISH-URL."
     (uri "/.well-known")
     (body (list "root /var/www;")))))
 
+(define languages-to-accept
+  ;; List of languages for redirection; see 'accept-languages' above.
+  '(("en")
+    ("de")
+    ("eo")
+    ("es")
+    ("fr")
+    ("ko")
+    ("ru")
+    ("sk")
+    ("zh-CN" "zh" "zh-Hans" "zh-Hans-CN")))
+
+(define guix.gnu.org-locations
+  (append (append-map guix.gnu.org-redirect-locations-for-lang
+                      (map car languages-to-accept))
+          guix.gnu.org-redirect-locations
+          guix.gnu.org-other-locations))
+
 (define %publish-url "http://localhost:3000")
 
 (define %berlin-servers
@@ -1033,15 +1083,7 @@  synonymous IETF language tags that should be mapped to the same $lang."
    "default_type  application/octet-stream;"
    "sendfile        on;"
 
-   (accept-languages '(("en")
-                       ("de")
-                       ("eo")
-                       ("es")
-                       ("fr")
-                       ("ko")
-                       ("ru")
-                       ("sk")
-                       ("zh-CN" "zh" "zh-Hans" "zh-Hans-CN")))
+   (accept-languages languages-to-accept)
 
    ;; Maximum chunk size to send.  Partly this is a workaround for
    ;; <http://bugs.gnu.org/19939>, but also the nginx docs mention that