Patchwork hgweb: allow web.templates to be a list

login
register
mail settings
Submitter via Mercurial-devel
Date July 8, 2016, 11:07 p.m.
Message ID <CABxrLkintz0JQo00NHeKQUfS9vmMfsHsCitXtZLMPc=WAuBVFQ@mail.gmail.com>
Download mbox | patch
Permalink /patch/15781/
State Not Applicable
Headers show

Comments

via Mercurial-devel - July 8, 2016, 11:07 p.m.
# HG changeset patch
# User Ross Light <light@google.com>
# Date 1468018129 25200
#      Fri Jul 08 15:48:49 2016 -0700
# Node ID 0611009f75ac6e0c661207927ed9606b83b56d28
# Parent  b4d117cee636be8a566f56e84d4b351a736a1299
hgweb: allow web.templates to be a list

templater already supports searching multiple directories, but the config value
was read in as one string. This allows the web.templates setting to use this
functionality.

         # It is shared across requests. The app will replace the object
Anton Shestakov - July 10, 2016, 1:24 p.m.
10.07.2016, 02:26, "Ross Light via Mercurial-devel" <mercurial-devel@mercurial-scm.org>:
> # HG changeset patch
> # User Ross Light <light@google.com>
> # Date 1468018129 25200
> # Fri Jul 08 15:48:49 2016 -0700
> # Node ID 0611009f75ac6e0c661207927ed9606b83b56d28
> # Parent b4d117cee636be8a566f56e84d4b351a736a1299
> hgweb: allow web.templates to be a list
>
> templater already supports searching multiple directories, but the config value
> was read in as one string. This allows the web.templates setting to use this
> functionality.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -2093,8 +2093,8 @@
>      Example: ``monoblue``.
>
>  ``templates``
> - Where to find the HTML templates. The default path to the HTML templates
> - can be obtained from ``hg debuginstall``.
> + List of directories to search for the HTML templates. The default path
> + to the HTML templates can be obtained from ``hg debuginstall``.
>
>  ``websub``
>  ----------
> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
> --- a/mercurial/hgweb/hgweb_mod.py
> +++ b/mercurial/hgweb/hgweb_mod.py
> @@ -99,7 +99,7 @@
>          # we use untrusted=False to prevent a repo owner from using
>          # web.templates in .hg/hgrc to get access to any file readable
>          # by the user running the CGI script
> - self.templatepath = self.config('web', 'templates', untrusted=False)
> + self.templatepath = self.configlist('web', 'templates',
> untrusted=False)

It's hard to believe how many tests this broke.

Also the patch seems to be line-wrapped on 80 symbols, which means it fails to import cleanly. It may be your MUA doing this, in which case try a different one or set up patchbomb extension (I certainly know my current MUA does silly things to whitespace as you can see above, sorry about that).

And seems like the line above will go over 79 characters in length, which means it now doesn't conform to our coding style.

Please refer to 9 and 10 in the submission checklist on https://www.mercurial-scm.org/wiki/ContributingChanges
Yuya Nishihara - July 11, 2016, 12:26 p.m.
On Fri, 8 Jul 2016 16:07:11 -0700, Ross Light via Mercurial-devel wrote:
> # HG changeset patch
> # User Ross Light <light@google.com>
> # Date 1468018129 25200
> #      Fri Jul 08 15:48:49 2016 -0700
> # Node ID 0611009f75ac6e0c661207927ed9606b83b56d28
> # Parent  b4d117cee636be8a566f56e84d4b351a736a1299
> hgweb: allow web.templates to be a list

> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
> --- a/mercurial/hgweb/hgweb_mod.py
> +++ b/mercurial/hgweb/hgweb_mod.py
> @@ -99,7 +99,7 @@
>          # we use untrusted=False to prevent a repo owner from using
>          # web.templates in .hg/hgrc to get access to any file readable
>          # by the user running the CGI script
> -        self.templatepath = self.config('web', 'templates', untrusted=False)
> +        self.templatepath = self.configlist('web', 'templates',
> untrusted=False)

This is a behavior change because web.templates may contain spaces. Windows
people are likely to put spaces in a filename.
Augie Fackler - July 11, 2016, 12:30 p.m.
On Mon, Jul 11, 2016 at 8:26 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Fri, 8 Jul 2016 16:07:11 -0700, Ross Light via Mercurial-devel wrote:
>> # HG changeset patch
>> # User Ross Light <light@google.com>
>> # Date 1468018129 25200
>> #      Fri Jul 08 15:48:49 2016 -0700
>> # Node ID 0611009f75ac6e0c661207927ed9606b83b56d28
>> # Parent  b4d117cee636be8a566f56e84d4b351a736a1299
>> hgweb: allow web.templates to be a list
>
>> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
>> --- a/mercurial/hgweb/hgweb_mod.py
>> +++ b/mercurial/hgweb/hgweb_mod.py
>> @@ -99,7 +99,7 @@
>>          # we use untrusted=False to prevent a repo owner from using
>>          # web.templates in .hg/hgrc to get access to any file readable
>>          # by the user running the CGI script
>> -        self.templatepath = self.config('web', 'templates', untrusted=False)
>> +        self.templatepath = self.configlist('web', 'templates',
>> untrusted=False)
>
> This is a behavior change because web.templates may contain spaces. Windows
> people are likely to put spaces in a filename.

Would it be worth adding a web.extratemplatedirs that is a list for this reason?

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Yuya Nishihara - July 11, 2016, 2:35 p.m.
On Mon, 11 Jul 2016 08:30:23 -0400, Augie Fackler wrote:
> On Mon, Jul 11, 2016 at 8:26 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> > On Fri, 8 Jul 2016 16:07:11 -0700, Ross Light via Mercurial-devel wrote:  
> >> # HG changeset patch
> >> # User Ross Light <light@google.com>
> >> # Date 1468018129 25200
> >> #      Fri Jul 08 15:48:49 2016 -0700
> >> # Node ID 0611009f75ac6e0c661207927ed9606b83b56d28
> >> # Parent  b4d117cee636be8a566f56e84d4b351a736a1299
> >> hgweb: allow web.templates to be a list  
> >  
> >> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
> >> --- a/mercurial/hgweb/hgweb_mod.py
> >> +++ b/mercurial/hgweb/hgweb_mod.py
> >> @@ -99,7 +99,7 @@
> >>          # we use untrusted=False to prevent a repo owner from using
> >>          # web.templates in .hg/hgrc to get access to any file readable
> >>          # by the user running the CGI script
> >> -        self.templatepath = self.config('web', 'templates', untrusted=False)
> >> +        self.templatepath = self.configlist('web', 'templates',
> >> untrusted=False)  
> >
> > This is a behavior change because web.templates may contain spaces. Windows
> > people are likely to put spaces in a filename.  
> 
> Would it be worth adding a web.extratemplatedirs that is a list for this reason?

or introduce new config and deprecate web.templates?

IIRC, last time we did it for [paths] at 8cbb59124e67, I got a few bug reports
to TortoiseHg, which said a path containing space no longer works.
Anton Shestakov - July 12, 2016, 5:10 p.m.
11.07.2016, 01:01, "Ross Light" <light@google.com>:
> I used patchbomb for the revised patch, let me know if you have any further difficulties.

Do you know what happened to that patch? I don't see it here: https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-July/date.html (or in my inbox).
Anton Shestakov - July 12, 2016, 5:21 p.m.
13.07.2016, 01:11, "Anton Shestakov" <engored@ya.ru>:
> 11.07.2016, 01:01, "Ross Light" <light@google.com>:
>>  I used patchbomb for the revised patch, let me know if you have any further difficulties.
>
> Do you know what happened to that patch? I don't see it here: https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-July/date.html (or in my inbox).

Oh, this happened:

Jul 10 12:54:09 2016 (3930) DMARC lookup for light@google.com (_dmarc.google.com) found p=reject in _dmarc.google.com. = v=DMARC1; p=reject; rua=mailto:mailauth-reports@google.com

(from IRC)

Incidentally, https://www.mercurial-scm.org/wiki/IRC is a good place to get feedback faster (and probably learn what other people from google.com do to send patches).
Anton Shestakov - July 13, 2016, 3:52 p.m.
13.07.2016, 02:23, "Ross Light" <light@google.com>:
> # HG changeset patch
> # User Ross Light <light@google.com>
> # Date 1468018129 25200
> # Fri Jul 08 15:48:49 2016 -0700
> # Node ID f93fdaa22399c1dcaa82034a73c04cafcf45bb4e
> # Parent 1b38cfde9530331d8d5767aa09a0de7d90931845
> hgweb: allow web.templates to be a list
>
> templater already supports searching multiple directories, but the config value
> was read in as one string. This allows the web.templates setting to use this
> functionality.
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -2093,8 +2093,8 @@
>      Example: ``monoblue``.
>
>  ``templates``
> - Where to find the HTML templates. The default path to the HTML templates
> - can be obtained from ``hg debuginstall``.
> + List of directories to search for the HTML templates. The default path
> + to the HTML templates can be obtained from ``hg debuginstall``.
>
>  ``websub``
>  ----------
> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
> --- a/mercurial/hgweb/hgweb_mod.py
> +++ b/mercurial/hgweb/hgweb_mod.py
> @@ -99,7 +99,8 @@
>          # we use untrusted=False to prevent a repo owner from using
>          # web.templates in .hg/hgrc to get access to any file readable
>          # by the user running the CGI script
> - self.templatepath = self.config('web', 'templates', untrusted=False)
> + self.templatepath = self.configlist('web', 'templates',
> + untrusted=False) or None
>
>          # This object is more expensive to build than simple config values.
>          # It is shared across requests. The app will replace the object

The patch applies and passes tests, which is good. Unfortunately, only I could see it, because it again didn't make it to mercurial-devel thanks to google.com's DMARC. I heard that "using an app key" works, whatever that means.

Please resend this (whenever you have time) so that other people can see it and chime in.

Also, don't forget to use --flag V2 (would be V3 for me) for hg email so it's clear which version supersedes which.
Pierre-Yves David - July 15, 2016, 12:47 a.m.
On 07/11/2016 04:35 PM, Yuya Nishihara wrote:
> On Mon, 11 Jul 2016 08:30:23 -0400, Augie Fackler wrote:
>> On Mon, Jul 11, 2016 at 8:26 AM, Yuya Nishihara <yuya@tcha.org> wrote:
>>> On Fri, 8 Jul 2016 16:07:11 -0700, Ross Light via Mercurial-devel wrote:  
>>>> # HG changeset patch
>>>> # User Ross Light <light@google.com>
>>>> # Date 1468018129 25200
>>>> #      Fri Jul 08 15:48:49 2016 -0700
>>>> # Node ID 0611009f75ac6e0c661207927ed9606b83b56d28
>>>> # Parent  b4d117cee636be8a566f56e84d4b351a736a1299
>>>> hgweb: allow web.templates to be a list  
>>>  
>>>> diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
>>>> --- a/mercurial/hgweb/hgweb_mod.py
>>>> +++ b/mercurial/hgweb/hgweb_mod.py
>>>> @@ -99,7 +99,7 @@
>>>>          # we use untrusted=False to prevent a repo owner from using
>>>>          # web.templates in .hg/hgrc to get access to any file readable
>>>>          # by the user running the CGI script
>>>> -        self.templatepath = self.config('web', 'templates', untrusted=False)
>>>> +        self.templatepath = self.configlist('web', 'templates',
>>>> untrusted=False)  
>>>
>>> This is a behavior change because web.templates may contain spaces. Windows
>>> people are likely to put spaces in a filename.  
>>
>> Would it be worth adding a web.extratemplatedirs that is a list for this reason?
> 
> or introduce new config and deprecate web.templates?
> 
> IIRC, last time we did it for [paths] at 8cbb59124e67, I got a few bug reports
> to TortoiseHg, which said a path containing space no longer works.

+1. I totally expect people to have used whatever list separator we use
for some directory name.

Patch

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -2093,8 +2093,8 @@ 
     Example: ``monoblue``.

 ``templates``
-    Where to find the HTML templates. The default path to the HTML templates
-    can be obtained from ``hg debuginstall``.
+    List of directories to search for the HTML templates. The default path
+    to the HTML templates can be obtained from ``hg debuginstall``.

 ``websub``
 ----------
diff --git a/mercurial/hgweb/hgweb_mod.py b/mercurial/hgweb/hgweb_mod.py
--- a/mercurial/hgweb/hgweb_mod.py
+++ b/mercurial/hgweb/hgweb_mod.py
@@ -99,7 +99,7 @@ 
         # we use untrusted=False to prevent a repo owner from using
         # web.templates in .hg/hgrc to get access to any file readable
         # by the user running the CGI script
-        self.templatepath = self.config('web', 'templates', untrusted=False)
+        self.templatepath = self.configlist('web', 'templates',
untrusted=False)

         # This object is more expensive to build than simple config values.