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

Matt Mackall mpm at selenic.com
Thu Jan 31 13:34:48 CST 2013


On Thu, 2013-01-31 at 13:10 -0600, Kevin Bullock wrote:
> On Jan 31, 2013, at 12:57 PM, pierre-yves.david at logilab.fr wrote:
> 
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at 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).

Spotting about half a dozen regressions in a week in this one area has
given me The Fear. I'd rate the odds of having a significant
spotted-shortly-after-release bug here at something like 95%. Having a
safety valve of some sort will help me sleep.

If I take this, I may actually postpone including the docs (which
elevate it from insurance to first-class feature).

> > 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?
> 2. If we have to do it twice, why on earth wouldn't you wrap it in a function?
> 
> 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.
> 
> pacem in terris / мир / शान्ति / ‎‫سَلاَم‬ / 平和
> Kevin R. Bullock
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list