Patchwork [V3] hgweb: don't attempt to show hidden bookmarks (issue3774)

login
register
mail settings
Submitter Kevin Bullock
Date Jan. 25, 2013, 9:53 p.m.
Message ID <f8a44ed8139d3f070389.1359150813@vpn0-77.vpn.umn.edu>
Download mbox | patch
Permalink /patch/739/
State Superseded
Headers show

Comments

Kevin Bullock - Jan. 25, 2013, 9:53 p.m.
# HG changeset patch
# User Kevin Bullock <kbullock@ringworld.org>
# Date 1359135834 21600
# Branch stable
# Node ID f8a44ed8139d3f07038969845a3ef527d6c0b826
# Parent  075143f60807c96fd051fd1dde5830852e4e6099
hgweb: don't attempt to show hidden bookmarks (issue3774)

localrepository._bookmarks is unfiltered, but hgweb gets a filtered
repo. This fixes the resulting traceback on the 'bookmarks' page.
Greg Ward - Jan. 25, 2013, 10:07 p.m.
On 25 January 2013, Kevin Bullock said:
> # HG changeset patch
> # User Kevin Bullock <kbullock@ringworld.org>
> # Date 1359135834 21600
> # Branch stable
> # Node ID f8a44ed8139d3f07038969845a3ef527d6c0b826
> # Parent  075143f60807c96fd051fd1dde5830852e4e6099
> hgweb: don't attempt to show hidden bookmarks (issue3774)
> diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
> --- a/tests/test-bookmarks-pushpull.t
> +++ b/tests/test-bookmarks-pushpull.t
> @@ -288,6 +288,16 @@ hgweb
>  
>    $ hg serve -p $HGPORT -d --pid-file=../hg.pid -E errors.log
>    $ cat ../hg.pid >> $DAEMON_PIDS
> +
> +issue3774
> +
> +  $ hg phase -fs X
> +  $ "$TESTDIR/get-with-headers.py" --headeronly 127.0.0.1:$HGPORT 'bookmarks'
> +  200 Script output follows
> +  $ sleep 0.1
> +  $ cat errors.log
> +  $ hg phase -d X
> +

Rule 1 of concurrent programming: if your code requires a sleep to
work correctly, it's racy. That applies to tests just as much as to
production code.

Also, I strongly suspect that floating point sleep times are a GNUism.
Portability requires "sleep 1", which will slow the test down
annoyingly, which will cause you to delete the sleep, which should
make the race condition more obvious! ;-)

       Greg
Matt Mackall - Jan. 25, 2013, 10:24 p.m.
On Fri, 2013-01-25 at 17:07 -0500, Greg Ward wrote:
> On 25 January 2013, Kevin Bullock said:
> > # HG changeset patch
> > # User Kevin Bullock <kbullock@ringworld.org>
> > # Date 1359135834 21600
> > # Branch stable
> > # Node ID f8a44ed8139d3f07038969845a3ef527d6c0b826
> > # Parent  075143f60807c96fd051fd1dde5830852e4e6099
> > hgweb: don't attempt to show hidden bookmarks (issue3774)
> > diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
> > --- a/tests/test-bookmarks-pushpull.t
> > +++ b/tests/test-bookmarks-pushpull.t
> > @@ -288,6 +288,16 @@ hgweb
> >  
> >    $ hg serve -p $HGPORT -d --pid-file=../hg.pid -E errors.log
> >    $ cat ../hg.pid >> $DAEMON_PIDS
> > +
> > +issue3774
> > +
> > +  $ hg phase -fs X
> > +  $ "$TESTDIR/get-with-headers.py" --headeronly 127.0.0.1:$HGPORT 'bookmarks'
> > +  200 Script output follows
> > +  $ sleep 0.1
> > +  $ cat errors.log
> > +  $ hg phase -d X
> > +
> 
> Rule 1 of concurrent programming: if your code requires a sleep to
> work correctly, it's racy. That applies to tests just as much as to
> production code.
> 
> Also, I strongly suspect that floating point sleep times are a GNUism.
> Portability requires "sleep 1", which will slow the test down
> annoyingly, which will cause you to delete the sleep, which should
> make the race condition more obvious! ;-)

Indeed. I've spent a bunch of time trying to kill such sleeps in the
test suite. The usual tricks I've used is to manage concurrency with
hooks, wait, and testing for file existence. See here for a bunch of
examples:

$ hg log -r 'keyword(mpm) and keyword(race)' tests

Patch

diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
--- a/mercurial/hgweb/webcommands.py
+++ b/mercurial/hgweb/webcommands.py
@@ -424,7 +424,7 @@  def tags(web, req, tmpl):
                 latestentry=lambda **x: entries(True, True, **x))
 
 def bookmarks(web, req, tmpl):
-    i = web.repo._bookmarks.items()
+    i = [b for b in web.repo._bookmarks.items() if b[1] in web.repo]
     parity = paritygen(web.stripecount)
 
     def entries(latestonly, **map):
diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t
--- a/tests/test-bookmarks-pushpull.t
+++ b/tests/test-bookmarks-pushpull.t
@@ -288,6 +288,16 @@  hgweb
 
   $ hg serve -p $HGPORT -d --pid-file=../hg.pid -E errors.log
   $ cat ../hg.pid >> $DAEMON_PIDS
+
+issue3774
+
+  $ hg phase -fs X
+  $ "$TESTDIR/get-with-headers.py" --headeronly 127.0.0.1:$HGPORT 'bookmarks'
+  200 Script output follows
+  $ sleep 0.1
+  $ cat errors.log
+  $ hg phase -d X
+
   $ cd ../a
 
   $ hg debugpushkey http://localhost:$HGPORT/ namespaces