[PATCH maintenance.git] nginx: berlin: Redirect old video URLs for each language.

  • Done
  • quality assurance status badge
Details
3 participants
  • Ludovic Courtès
  • Luis Felipe
  • pelzflorian (Florian Pelz)
Owner
unassigned
Submitted by
pelzflorian (Florian Pelz)
Severity
normal

Debbugs page

pelzflorian (Florian Pelz) wrote 4 years ago
(address . guix-patches@gnu.org)(name . Luis Felipe)(address . luis.felipe.la@protonmail.com)
20210706065304.6kqbwqlcf6calkh7@pelzflorian.localdomain
Hi guix,

a week ago Luis nicely redesigned how videos are displayed on the Guix
website
(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
Luis Felipe wrote 4 years ago
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)(address . guix-patches@gnu.org)
hAneyXwK4xvLI1Ru5-hseApZGByUZzjVYxydzD-ogm2CaXhA-fJvfdOws3woIjcN5spXTAQZ3wJRFdu6w0IWMpScnZywQ1mW7pyAV8GQYeY=@protonmail.com
Hi Florian,


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

Toggle quote (40 lines)
> 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 wrote 4 years ago
Re: bug#49431: [PATCH maintenance.git] nginx: berlin: Redirect old video URLs for each language.
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)
87a6mwlxo3.fsf@gnu.org
Hi,

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

Toggle quote (11 lines)
> 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/

[...]

Toggle quote (16 lines)
>>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.

[...]

Toggle quote (19 lines)
> (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’.
pelzflorian (Florian Pelz) wrote 4 years ago
(name . Ludovic Courtès)(address . ludo@gnu.org)
20210709070715.235m7qs3gicmms4w@pelzflorian.localdomain
Thank you two for your opinion, Luis and Ludo.

On Thu, Jul 08, 2021 at 03:53:16PM +0200, Ludovic Courtès wrote:
Toggle quote (7 lines)
> 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?

Each valid $lang was handled further down, but my patch was confusing,
especially when somebody wants to add more redirections. Find
attached a revised patch with per-language redirection in only one
place, namely new procedure
`guix.gnu.org-redirects-for-each-language'.

Do I need to add a copyright header? I hereby license/declare my

In the previous patch I had missed some cases without /LANG like

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

Redirecting was unnecessary for

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

which already gets taken care of by

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

thanks to nginx location normalization.

Shall I push and then wait until someone guix system reconfigures
berlin?

Regards,
Florian
pelzflorian (Florian Pelz) wrote 4 years ago
Re: [bug#49431] [PATCH maintenance.git] nginx: berlin: Redirect old video URLs for each language.
(name . Ludovic Courtès)(address . ludo@gnu.org)
20210709193844.dk4e7q5nanqe7iz7@pelzflorian.localdomain
On Fri, Jul 09, 2021 at 09:15:04AM +0200, pelzflorian (Florian Pelz) wrote:
Toggle quote (56 lines)
> From: Florian Pelz <pelzflorian@pelzflorian.de>
> > (guix.gnu.org-redirects-for-each-language): New procedure. Add
> > new video redirections. […]
> > +(define (guix.gnu.org-redirects-for-each-language)
> > + ;; These old URL request paths existed in many forms; without /LANG
> > + ;; in front and with /LANG in front for each language. Redirect
> > + ;; each of them.
> > + (define redirections
> > + (list
> > + (cons "/videos/everyday-use-of-gnu-guix,-part-one" "/videos/2020/everyday-use-of-gnu-guix-part-one/")
> > + (cons "/videos/everyday-use-of-gnu-guix,-part-two" "/videos/2020/everyday-use-of-gnu-guix-part-two/")
> > + (cons "/videos/system-graphical-installer" "/videos/2020/system-graphical-installer/")
> > + (cons "/videos/asking-for-help" "/videos/2020/asking-for-help/")
> > + (cons "/videos/installation-from-script" "/videos/2020/installation-from-script/")
> > + (cons "/videos/packaging,-part-one" "/videos/2020/packaging-part-one/")
> > + (cons "/videos/packaging,-part-two" "/videos/2020/packaging-part-two/")
> > + (cons "/videos/packaging,-part-three" "/videos/2020/packaging-part-three/")))
> > +
> > + (define (redirect-directory old new)
> > + ;; Match nginx' behavior that request URLs with suffix "", "/"
> > + ;; "/index.html" lead to the same file. The suffix "/" is not taken
> > + ;; care of here because it already gets normalized by nginx location
> > + ;; handling. The URLs in 'guix.gnu.org-redirect-locations' do not
> > + ;; need this treatment, because they get an /index.html suffix
> > + ;; through rewriting.
> > + (let ((old-with-slashes-trimmed (string-trim-right old #\/)))
> > + (list
> > + (redirect old-with-slashes-trimmed new)
> > + (redirect (string-append old-with-slashes-trimmed "/index.html") new))))
> > +
> > + (define (guix.gnu.org-redirect-locations-for-lang lang)
> > + (define (redirect-lang old new)
> > + (redirect-directory (string-append "/" lang old)
> > + (string-append "/" lang new)))
> > + (append-map redirect-lang (map car redirections) (map cdr redirections)))
> > +
> > + (append
> > + ;; Now all needed redirections are:
> > + ;;
> > + ;; 1) those without /LANG/ in front get redirected to /$lang/
> > + (append-map redirect-directory
> > + (map car redirections) ;old URLs without /LANG
> > + ;; new URLs with /$lang prepended:
> > + (map (compose (lambda (new-without-lang)
> > + (string-append "/$lang" new-without-lang))
> > + cdr)
> > + redirections))
> > + ;; 2) those with /LANG/ in front get redirected to the same /LANG/
> > + (append-map guix.gnu.org-redirect-locations-for-lang
> > + (map car languages-to-accept))))
> Do I need to add a copyright header? I hereby license/declare my
> patch CC0 <https://creativecommons.org/publicdomain/zero/1.0/>.
> […]
> Shall I push and then wait until someone guix system reconfigures
> berlin?

Sorry for again asking such general questions. A better question
might be, should 'guix.gnu.org-redirects-for-each-language' go to a
separate Guile module with a copyright header (if yes, what could it
be named?), or remain in hydra/nginx/berlin.scm despite more complex
code, without licensing information?

Regards,
Florian
Ludovic Courtès wrote 4 years ago
Re: bug#49431: [PATCH maintenance.git] nginx: berlin: Redirect old video URLs for each language.
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)
87v95iea6e.fsf@gnu.org
Hi,

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

Toggle quote (14 lines)
> On Thu, Jul 08, 2021 at 03:53:16PM +0200, Ludovic Courtès wrote:
>> 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?
>
> Each valid $lang was handled further down, but my patch was confusing,
> especially when somebody wants to add more redirections. Find
> attached a revised patch with per-language redirection in only one
> place, namely new procedure
> `guix.gnu.org-redirects-for-each-language'.

OK.

Toggle quote (3 lines)
> Do I need to add a copyright header? I hereby license/declare my
> patch CC0 <https://creativecommons.org/publicdomain/zero/1.0/>.

Currently there’s no explicit license on this file,
hydra/nginx/berlin.scm.

There’s very little at stake for the sake of transparency, it might be
best to email guix-sysadmins so people who contribute to this file (and
to hydra/berlin.scm) agree on the license. The default license in the
project would be GPLv3+, but these config files are “special”.

Toggle quote (3 lines)
> Shall I push and then wait until someone guix system reconfigures
> berlin?

Yes, please!

Thanks,
Ludo’.
Ludovic Courtès wrote 4 years ago
Re: [bug#49431] [PATCH maintenance.git] nginx: berlin: Redirect old video URLs for each language.
(name . pelzflorian (Florian Pelz))(address . pelzflorian@pelzflorian.de)
87a6mue160.fsf@gnu.org
Hi,

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

Toggle quote (6 lines)
> Sorry for again asking such general questions. A better question
> might be, should 'guix.gnu.org-redirects-for-each-language' go to a
> separate Guile module with a copyright header (if yes, what could it
> be named?), or remain in hydra/nginx/berlin.scm despite more complex
> code, without licensing information?

Let’s keep it in hydra/nginx/berlin.scm for now, and split in several
files later if we feel it’s becoming too messy. (As for copyright, it’s
not even clear to me that a list of redirects is copyrightable per se
since it there’s only one way to do it.)

Ludo’.
pelzflorian (Florian Pelz) wrote 4 years ago
Re: bug#49431: [PATCH maintenance.git] nginx: berlin: Redirect old video URLs for each language.
(name . Ludovic Courtès)(address . ludo@gnu.org)
20210710192832.wwvgw3zc24ufhbju@pelzflorian.localdomain
On Sat, Jul 10, 2021 at 12:27:05PM +0200, Ludovic Courtès wrote:
Toggle quote (12 lines)
> "pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> skribis:
> > Do I need to add a copyright header? I hereby license/declare my
> > patch CC0 <https://creativecommons.org/publicdomain/zero/1.0/>.
>
> Currently there’s no explicit license on this file,
> hydra/nginx/berlin.scm.
>
> There’s very little at stake for the sake of transparency, it might be
> best to email guix-sysadmins so people who contribute to this file (and
> to hydra/berlin.scm) agree on the license. The default license in the
> project would be GPLv3+, but these config files are “special”.

I will send a mail with a patch there. Thank you.


Toggle quote (5 lines)
> > Shall I push and then wait until someone guix system reconfigures
> > berlin?
>
> Yes, please!

Finally pushed as 2d6dc5e01aa32a01b345ba834e32bbf723e67077. Old video
URLs will be redirected after guix system reconfigure.

Regards,
Florian
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 49431@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 49431
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help