Patchwork [RFC] hgweb: replace some linkrev() with introrev() in hope issue4506 goes away

login
register
mail settings
Submitter Anton Shestakov
Date Feb. 16, 2015, 8:18 a.m.
Message ID <43dee1e7b5a6726697cc.1424074684@neuro>
Download mbox | patch
Permalink /patch/7803/
State Changes Requested
Headers show

Comments

Anton Shestakov - Feb. 16, 2015, 8:18 a.m.
# HG changeset patch
# User Anton Shestakov <engored@ya.ru>
# Date 1424074464 -28800
#      Mon Feb 16 16:14:24 2015 +0800
# Node ID 43dee1e7b5a6726697cc5cbdc20f77852ff10f0a
# Parent  0cfff4c42c9a146a09877cefee5a839cd70c6a2f
hgweb: replace some linkrev() with introrev() in hope issue4506 goes away

The issue is titled "filtered revision 'XXX' (not in 'served' subset)" and that
is the error message you sometimes get when trying to look at a file (/file or
/annotate) in hgweb. For example:

http://hg.intevation.org/mercurial/crew/file/90cf454edd70/mercurial/cmdutil.py

There's a test case in the issue, it involves rebase + evolve.

Looks like replacing ctx.linkrev() with ctx.introrev() fixes the problem (as
marmoute expected in one of the comments). There were no tests for something
like this, since they would probably require evolve extension. Which worries me
a bit, since on the other hand, this patch could break something that also
doesn't have tests. I'd like to tell more about the patch, but since I know
nothing about linkrev bugs, I'd rather listen.
Matt Harbison - Feb. 17, 2015, 1:35 a.m.
On Mon, 16 Feb 2015 03:18:04 -0500, Anton Shestakov <engored@ya.ru> wrote:

> # HG changeset patch
> # User Anton Shestakov <engored@ya.ru>
> # Date 1424074464 -28800
> #      Mon Feb 16 16:14:24 2015 +0800
> # Node ID 43dee1e7b5a6726697cc5cbdc20f77852ff10f0a
> # Parent  0cfff4c42c9a146a09877cefee5a839cd70c6a2f
> hgweb: replace some linkrev() with introrev() in hope issue4506 goes away
>
> The issue is titled "filtered revision 'XXX' (not in 'served' subset)"  
> and that
> is the error message you sometimes get when trying to look at a file  
> (/file or
> /annotate) in hgweb. For example:
>
> http://hg.intevation.org/mercurial/crew/file/90cf454edd70/mercurial/cmdutil.py
>
> There's a test case in the issue, it involves rebase + evolve.
>
> Looks like replacing ctx.linkrev() with ctx.introrev() fixes the problem  
> (as
> marmoute expected in one of the comments). There were no tests for  
> something
> like this, since they would probably require evolve extension. Which  
> worries me
> a bit, since on the other hand, this patch could break something that  
> also
> doesn't have tests. I'd like to tell more about the patch, but since I  
> know
> nothing about linkrev bugs, I'd rather listen.

I know nothing about linkrev bugs either, but you should be able to  
formulate the test using 'debugobsolete' without using evolve.  See  
test-obsolete.t for how to get the full 40 character id for the command,  
based on the commit comment.  (Be careful though- I spent a lot of time  
debugging cryptic errors that turned out to be debugging notes coming out  
before the hash, because getid() uses the --debug flag.  It probably would  
be better to use 'log -T "{node}\n"' or something similar.)

--Matt
Augie Fackler - Feb. 17, 2015, 4:28 p.m.
On Mon, Feb 16, 2015 at 04:18:04PM +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <engored@ya.ru>
> # Date 1424074464 -28800
> #      Mon Feb 16 16:14:24 2015 +0800
> # Node ID 43dee1e7b5a6726697cc5cbdc20f77852ff10f0a
> # Parent  0cfff4c42c9a146a09877cefee5a839cd70c6a2f
> hgweb: replace some linkrev() with introrev() in hope issue4506 goes away

LGTM, I guess, but I'd like a test. marmoute, please handle this one.

>
> The issue is titled "filtered revision 'XXX' (not in 'served' subset)" and that
> is the error message you sometimes get when trying to look at a file (/file or
> /annotate) in hgweb. For example:
>
> http://hg.intevation.org/mercurial/crew/file/90cf454edd70/mercurial/cmdutil.py
>
> There's a test case in the issue, it involves rebase + evolve.
>
> Looks like replacing ctx.linkrev() with ctx.introrev() fixes the problem (as
> marmoute expected in one of the comments). There were no tests for something
> like this, since they would probably require evolve extension. Which worries me
> a bit, since on the other hand, this patch could break something that also
> doesn't have tests. I'd like to tell more about the patch, but since I know
> nothing about linkrev bugs, I'd rather listen.
>
> diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
> --- a/mercurial/hgweb/webutil.py
> +++ b/mercurial/hgweb/webutil.py
> @@ -139,8 +139,8 @@ def _siblings(siblings=[], hiderev=None)
>
>  def parents(ctx, hide=None):
>      if (isinstance(ctx, context.basefilectx) and
> -        ctx.changectx().rev() != ctx.linkrev()):
> -        return _siblings([ctx._repo[ctx.linkrev()]], hide)
> +        ctx.changectx().rev() != ctx.introrev()):
> +        return _siblings([ctx._repo[ctx.introrev()]], hide)
>      return _siblings(ctx.parents(), hide)
>
>  def children(ctx, hide=None):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Feb. 18, 2015, 11:38 a.m.
On 02/17/2015 04:28 PM, Augie Fackler wrote:
> On Mon, Feb 16, 2015 at 04:18:04PM +0800, Anton Shestakov wrote:
>> # HG changeset patch
>> # User Anton Shestakov <engored@ya.ru>
>> # Date 1424074464 -28800
>> #      Mon Feb 16 16:14:24 2015 +0800
>> # Node ID 43dee1e7b5a6726697cc5cbdc20f77852ff10f0a
>> # Parent  0cfff4c42c9a146a09877cefee5a839cd70c6a2f
>> hgweb: replace some linkrev() with introrev() in hope issue4506 goes away
>
> LGTM, I guess, but I'd like a test. marmoute, please handle this one.

The general directoin seems good to me. But this needs a test.

You can easily create obsolescence markers using the `hg debugobsolete` 
function. See `hg test-obsolete.t` for examples (or other tests file)

Thanks for looking into this.

Patch

diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
--- a/mercurial/hgweb/webutil.py
+++ b/mercurial/hgweb/webutil.py
@@ -139,8 +139,8 @@  def _siblings(siblings=[], hiderev=None)
 
 def parents(ctx, hide=None):
     if (isinstance(ctx, context.basefilectx) and
-        ctx.changectx().rev() != ctx.linkrev()):
-        return _siblings([ctx._repo[ctx.linkrev()]], hide)
+        ctx.changectx().rev() != ctx.introrev()):
+        return _siblings([ctx._repo[ctx.introrev()]], hide)
     return _siblings(ctx.parents(), hide)
 
 def children(ctx, hide=None):