Patchwork [STABLE] hgweb: add a `web.view` to control filtering

login
register
mail settings
Submitter Pierre-Yves David
Date Jan. 31, 2013, 6:57 p.m.
Message ID <4a841ef7813f99bc7f09.1359658628@crater2.logilab.fr>
Download mbox | patch
Permalink /patch/774/
State Accepted
Commit 36549fa712da69eabe4b4abbbedeb4546dc7d904
Headers show

Comments

Pierre-Yves David - Jan. 31, 2013, 6:57 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@logilab.fr>
# Date 1359658615 -3600
# Branch stable
# Node ID 4a841ef7813f99bc7f091110e2f0dc8db82dfb86
# Parent  2a1fac3650a5b4d650198604c82ab59969500374
hgweb: add a `web.view` to control filtering

This options add a new `web.view` to control filter level of hgweb.

This option have two purposes:

1) Allow fall back to unfiltered version in case a yet undetected by critical
   bug is found in filtering after 2.5 release

2) People use hgweb as a local repoviewer. When they have secret changesets,
   they wants to use "visible" filter not "served"
Angel Ezquerra - Jan. 31, 2013, 7:26 p.m.
On Jan 31, 2013 7:57 PM, <pierre-yves.david@logilab.fr> wrote:
>
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> # Date 1359658615 -3600
> # Branch stable
> # Node ID 4a841ef7813f99bc7f091110e2f0dc8db82dfb86
> # Parent  2a1fac3650a5b4d650198604c82ab59969500374
> hgweb: add a `web.view` to control filtering
>
> This options add a new `web.view` to control filter level of hgweb.
>
> This option have two purposes:
>
> 1) Allow fall back to unfiltered version in case a yet undetected by
critical
>    bug is found in filtering after 2.5 release
>
> 2) People use hgweb as a local repoviewer. When they have secret
changesets,
>    they wants to use "visible" filter not "served"
>
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1457,5 +1457,11 @@ The full set of options is:
>  ``style``
>      Which template map style to use.
>
>  ``templates``
>      Where to find the HTML templates. Default is install path.
> +
> +``view``
> +    Controls Changesets filter to hgweb. Possible values are ``served``,
> +    ``visible`` and ``all``. Default is ``served``. The ``served``
filter only
> +    shows changesets that can be pulled from the hgweb instance.
The``visible``
> +    filter includes secret changesets but still excludes "hidden" one.

To my non native English speaker ears "served" seemed at first to imply
revisions that _have been_ served, rather than they _can be_ served. This
confused me at first since I thought this mode would not show revisions
that have been pushed to the server. This is nonsense of course but perhaps
another word could be used instead? Perhaps "accessible" or something
similar?

Also, if as you say users use hgweb as a way to view the history
graphically, wouldn't it be necessary to also add a way to switch modes
right from the hgweb interface (this option would set the default view)?
(And to be done after the freeze).

Cheers,

Angel
Pierre-Yves David - Jan. 31, 2013, 7:45 p.m.
On 31 janv. 2013, at 20:26, Angel Ezquerra wrote:

> 
> On Jan 31, 2013 7:57 PM, <pierre-yves.david@logilab.fr> wrote:
> >
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> > # Date 1359658615 -3600
> > # Branch stable
> > # Node ID 4a841ef7813f99bc7f091110e2f0dc8db82dfb86
> > # Parent  2a1fac3650a5b4d650198604c82ab59969500374
> > hgweb: add a `web.view` to control filtering
> >
> > This options add a new `web.view` to control filter level of hgweb.
> >
> > This option have two purposes:
> >
> > 1) Allow fall back to unfiltered version in case a yet undetected by critical
> >    bug is found in filtering after 2.5 release
> >
> > 2) People use hgweb as a local repoviewer. When they have secret changesets,
> >    they wants to use "visible" filter not "served"
> >
> > diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> > --- a/mercurial/help/config.txt
> > +++ b/mercurial/help/config.txt
> > @@ -1457,5 +1457,11 @@ The full set of options is:
> >  ``style``
> >      Which template map style to use.
> >
> >  ``templates``
> >      Where to find the HTML templates. Default is install path.
> > +
> > +``view``
> > +    Controls Changesets filter to hgweb. Possible values are ``served``,
> > +    ``visible`` and ``all``. Default is ``served``. The ``served`` filter only
> > +    shows changesets that can be pulled from the hgweb instance. The``visible``
> > +    filter includes secret changesets but still excludes "hidden" one.
> 
> To my non native English speaker ears "served" seemed at first to imply revisions that _have been_ served, rather than they _can be_ served. This confused me at first since I thought this mode would not show revisions that have been pushed to the server. This is nonsense of course but perhaps another word could be used instead? Perhaps "accessible" or something similar?

For my part I read "served" as "being served". As "served" is the internal name of the filter used everywhere in the code, this is unlikely to changes.

> Also, if as you say users use hgweb as a way to view the history graphically, wouldn't it be necessary to also add a way to switch modes right from the hgweb interface (this option would set the default view)? (And to be done after the freeze).

Having a way to control the mode in the interface is something that'll probably want at some point. But this is a very low priority feature for me.
Pierre-Yves David - Jan. 31, 2013, 7:48 p.m.
On 31 janv. 2013, at 20:10, Kevin Bullock wrote:

> On Jan 31, 2013, at 12:57 PM, pierre-yves.david@logilab.fr wrote:
> 
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
>> # Date 1359658615 -3600
>> # Branch stable
>> # Node ID 4a841ef7813f99bc7f091110e2f0dc8db82dfb86
>> # Parent  2a1fac3650a5b4d650198604c82ab59969500374
>> hgweb: add a `web.view` to control filtering
>> 
>> This options add a new `web.view` to control filter level of hgweb.
>> 
>> This option have two purposes:
>> 
>> 1) Allow fall back to unfiltered version in case a yet undetected by critical
>>  bug is found in filtering after 2.5 release
>> 
>> 2) People use hgweb as a local repoviewer. When they have secret changesets,
>>  they wants to use "visible" filter not "served"
> 
> As far as I can glean, the argument for this patch going in before tomorrow is:
> 
> 2.5 introduces filtering: hgweb now hides secret changesets. This causes a known regression for one use case, and maybe possibly some other as-yet-unknown regressions. So we should provide an option to turn it off.
> 
> I don't buy this argument as regards (1) (unknown regressions); I might buy it as regards (2) (known regression).
> 
>> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
>> --- a/mercurial/help/config.txt
>> +++ b/mercurial/help/config.txt
>> @@ -1457,5 +1457,11 @@ The full set of options is:
>> ``style``
>>    Which template map style to use.
>> 
>> ``templates``
>>    Where to find the HTML templates. Default is install path.
>> +
>> +``view``
>> +    Controls Changesets filter to hgweb. Possible values are ``served``,
>> +    ``visible`` and ``all``. Default is ``served``. The ``served`` filter only
>> +    shows changesets that can be pulled from the hgweb instance. The``visible``
>> +    filter includes secret changesets but still excludes "hidden" one.
>> 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
>> @@ -5,11 +5,11 @@
>> #
>> # This software may be used and distributed according to the terms of the
>> # GNU General Public License version 2 or any later version.
>> 
>> import os
>> -from mercurial import ui, hg, hook, error, encoding, templater, util
>> +from mercurial import ui, hg, hook, error, encoding, templater, util, repoview
>> from common import get_stat, ErrorResponse, permhooks, caching
>> from common import HTTP_OK, HTTP_NOT_MODIFIED, HTTP_BAD_REQUEST
>> from common import HTTP_NOT_FOUND, HTTP_SERVER_ERROR
>> from request import wsgirequest
>> import webcommands, protocol, webutil
>> @@ -57,11 +57,18 @@ class hgweb(object):
>>                u = ui.ui()
>>            self.repo = hg.repository(u, repo)
>>        else:
>>            self.repo = repo
>> 
>> -        self.repo =  self.repo.filtered('served')
>> +        viewconfig = self.config('web', 'view', 'served')
>> +        if viewconfig == 'all':
>> +            self.repo = self.repo.unfiltered()
>> +        elif viewconfig in repoview.filtertable:
>> +            self.repo = self.repo.filtered(viewconfig)
>> +        else:
>> +            self.repo = self.repo.filtered('served')
>> +
>>        self.repo.ui.setconfig('ui', 'report_untrusted', 'off')
>>        self.repo.ui.setconfig('ui', 'nontty', 'true')
>>        hook.redirect(True)
>>        self.mtime = -1
>>        self.size = -1
>> @@ -94,11 +101,17 @@ class hgweb(object):
>>        # rollbacks made less than a second ago
>>        if st.st_mtime != self.mtime or st.st_size != self.size:
>>            self.mtime = st.st_mtime
>>            self.size = st.st_size
>>            self.repo = hg.repository(self.repo.ui, self.repo.root)
>> -            self.repo =  self.repo.filtered('served')
>> +            viewconfig = self.config('web', 'view', 'served')
>> +            if viewconfig == 'all':
>> +                self.repo = self.repo.unfiltered()
>> +            elif viewconfig in repoview.filtertable:
>> +                self.repo = self.repo.filtered(viewconfig)
>> +            else:
>> +                self.repo = self.repo.filtered('served')
> 
> 1. Why do we have to do this twice?

Because there is two place where repo are created

> 2. If we have to do it twice, why on earth wouldn't you wrap it in a function?

Simpler patch for stable. I plan a follow up changeset for default that includes the (conditional) hg.repository call and the filtering switch.

> Before bringing up a code-churn argument, observe that you would be adding less lines of code (and half as much opportunity for a logic error!) if you added a new function.

The diff is bigger but simpler.
Matt Mackall - Feb. 1, 2013, 9:12 p.m.
On Thu, 2013-01-31 at 19:57 +0100, pierre-yves.david@logilab.fr wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@logilab.fr>
> # Date 1359658615 -3600
> # Branch stable
> # Node ID 4a841ef7813f99bc7f091110e2f0dc8db82dfb86
> # Parent  2a1fac3650a5b4d650198604c82ab59969500374
> hgweb: add a `web.view` to control filtering

I've queued a modified version of this using a helper function and
without the docs. As I requested this primarily as insurance against
regressions, I want to wait on promoting it to a first-class documented
feature a little bit. Please resend the doc piece in a couple weeks.

Patch

diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1457,5 +1457,11 @@  The full set of options is:
 ``style``
     Which template map style to use.
 
 ``templates``
     Where to find the HTML templates. Default is install path.
+
+``view``
+    Controls Changesets filter to hgweb. Possible values are ``served``,
+    ``visible`` and ``all``. Default is ``served``. The ``served`` filter only
+    shows changesets that can be pulled from the hgweb instance. The``visible``
+    filter includes secret changesets but still excludes "hidden" one.
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
@@ -5,11 +5,11 @@ 
 #
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
 import os
-from mercurial import ui, hg, hook, error, encoding, templater, util
+from mercurial import ui, hg, hook, error, encoding, templater, util, repoview
 from common import get_stat, ErrorResponse, permhooks, caching
 from common import HTTP_OK, HTTP_NOT_MODIFIED, HTTP_BAD_REQUEST
 from common import HTTP_NOT_FOUND, HTTP_SERVER_ERROR
 from request import wsgirequest
 import webcommands, protocol, webutil
@@ -57,11 +57,18 @@  class hgweb(object):
                 u = ui.ui()
             self.repo = hg.repository(u, repo)
         else:
             self.repo = repo
 
-        self.repo =  self.repo.filtered('served')
+        viewconfig = self.config('web', 'view', 'served')
+        if viewconfig == 'all':
+            self.repo = self.repo.unfiltered()
+        elif viewconfig in repoview.filtertable:
+            self.repo = self.repo.filtered(viewconfig)
+        else:
+            self.repo = self.repo.filtered('served')
+
         self.repo.ui.setconfig('ui', 'report_untrusted', 'off')
         self.repo.ui.setconfig('ui', 'nontty', 'true')
         hook.redirect(True)
         self.mtime = -1
         self.size = -1
@@ -94,11 +101,17 @@  class hgweb(object):
         # rollbacks made less than a second ago
         if st.st_mtime != self.mtime or st.st_size != self.size:
             self.mtime = st.st_mtime
             self.size = st.st_size
             self.repo = hg.repository(self.repo.ui, self.repo.root)
-            self.repo =  self.repo.filtered('served')
+            viewconfig = self.config('web', 'view', 'served')
+            if viewconfig == 'all':
+                self.repo = self.repo.unfiltered()
+            elif viewconfig in repoview.filtertable:
+                self.repo = self.repo.filtered(viewconfig)
+            else:
+                self.repo = self.repo.filtered('served')
             self.maxchanges = int(self.config("web", "maxchanges", 10))
             self.stripecount = int(self.config("web", "stripes", 1))
             self.maxshortchanges = int(self.config("web", "maxshortchanges",
                                                    60))
             self.maxfiles = int(self.config("web", "maxfiles", 10))
diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
--- a/tests/test-obsolete.t
+++ b/tests/test-obsolete.t
@@ -741,10 +741,28 @@  check graph view
 
 check filelog view
 
   $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'log/'`hg id --debug --id`/'babar'
   200 Script output follows
+
+  $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/68'
+  200 Script output follows
+  $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/67'
+  500 Internal Server Error
+  [1]
+
+check that web.view config option:
+
+  $ kill `cat hg.pid`
+  $ cat >> .hg/hgrc << EOF
+  > [web]
+  > view=all
+  > EOF
+  $ wait
+  $ hg serve -n test -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log
+  $ "$TESTDIR/get-with-headers.py" --headeronly localhost:$HGPORT 'rev/67'
+  200 Script output follows
   $ kill `cat hg.pid`
 
 Checking _enable=False warning if obsolete marker exists
 
   $ echo '[extensions]' >> $HGRCPATH