Patchwork [1,of,7,clfilter,part,1,V3] clfilter: ensure context raise RepoLookupError when the revision is filtered

login
register
mail settings
Submitter Pierre-Yves David
Date Dec. 17, 2012, 4:50 p.m.
Message ID <e954ac5321a9ead1a4e6.1355763010@crater2.logilab.fr>
Download mbox | patch
Permalink /patch/167/
State Accepted
Commit ee3b5fb648c70f5595cca6cd396666dabcf803bb
Headers show

Comments

Pierre-Yves David - Dec. 17, 2012, 4:50 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
# Date 1354489919 -3600
# Node ID e954ac5321a9ead1a4e687385dca7134218ccc83
# Parent  8c9a52492d426741ab24392d49f44a1d4f23613e
clfilter: ensure context raise RepoLookupError when the revision is filtered

Currently the code path of `changectx(filteredrepo, rev)` call
`filteredrepo.changelog.node(rev)`. When `rev` is filtered this raise an
unhandled `IndexError`. This case now raise a `RepoLookupError` as other
error case do.
Sean Farley - Dec. 17, 2012, 4:58 p.m.
On Mon, Dec 17, 2012 at 10:50 AM,  <pierre-yves.david at logilab.fr> wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1354489919 -3600
> # Node ID e954ac5321a9ead1a4e687385dca7134218ccc83
> # Parent  8c9a52492d426741ab24392d49f44a1d4f23613e
> clfilter: ensure context raise RepoLookupError when the revision is filtered
>
> Currently the code path of `changectx(filteredrepo, rev)` call
> `filteredrepo.changelog.node(rev)`. When `rev` is filtered this raise an
> unhandled `IndexError`. This case now raise a `RepoLookupError` as other
> error case do.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -23,10 +23,15 @@ class changectx(object):
>          if changeid == '':
>              changeid = '.'
>          self._repo = repo
>
>          if isinstance(changeid, int):
> +            try:
> +                self._node = repo.changelog.node(changeid)
> +            except IndexError:
> +                raise error.RepoLookupError(
> +                    _("unknown revision '%s'") % changeid)
>              self._rev = changeid
>              self._node = repo.changelog.node(changed)

Not a big deal, but did you mean to leave this duplicated 'self._node
= repo.changelog.node(changed)' after your try/except clause?
Pierre-Yves David - Dec. 17, 2012, 5:07 p.m.
On Mon, Dec 17, 2012 at 10:58:09AM -0600, Sean Farley wrote:
> On Mon, Dec 17, 2012 at 10:50 AM,  <pierre-yves.david at logilab.fr> wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> > # Date 1354489919 -3600
> > # Node ID e954ac5321a9ead1a4e687385dca7134218ccc83
> > # Parent  8c9a52492d426741ab24392d49f44a1d4f23613e
> > clfilter: ensure context raise RepoLookupError when the revision is filtered
> >
> > Currently the code path of `changectx(filteredrepo, rev)` call
> > `filteredrepo.changelog.node(rev)`. When `rev` is filtered this raise an
> > unhandled `IndexError`. This case now raise a `RepoLookupError` as other
> > error case do.
> >
> > diff --git a/mercurial/context.py b/mercurial/context.py
> > --- a/mercurial/context.py
> > +++ b/mercurial/context.py
> > @@ -23,10 +23,15 @@ class changectx(object):
> >          if changeid == '':
> >              changeid = '.'
> >          self._repo = repo
> >
> >          if isinstance(changeid, int):
> > +            try:
> > +                self._node = repo.changelog.node(changeid)
> > +            except IndexError:
> > +                raise error.RepoLookupError(
> > +                    _("unknown revision '%s'") % changeid)
> >              self._rev = changeid
> >              self._node = repo.changelog.node(changed)
> 
> Not a big deal, but did you mean to leave this duplicated 'self._node
> = repo.changelog.node(changed)' after your try/except clause?

Nope, thanks!
Pierre-Yves David - Dec. 17, 2012, 7 p.m.
On Mon, Dec 17, 2012 at 10:58:09AM -0600, Sean Farley wrote:
> On Mon, Dec 17, 2012 at 10:50 AM,  <pierre-yves.david at logilab.fr> wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> > # Date 1354489919 -3600
> > # Node ID e954ac5321a9ead1a4e687385dca7134218ccc83
> > # Parent  8c9a52492d426741ab24392d49f44a1d4f23613e
> > clfilter: ensure context raise RepoLookupError when the revision is filtered
> >
> > Currently the code path of `changectx(filteredrepo, rev)` call
> > `filteredrepo.changelog.node(rev)`. When `rev` is filtered this raise an
> > unhandled `IndexError`. This case now raise a `RepoLookupError` as other
> > error case do.
> >
> > diff --git a/mercurial/context.py b/mercurial/context.py
> > --- a/mercurial/context.py
> > +++ b/mercurial/context.py
> > @@ -23,10 +23,15 @@ class changectx(object):
> >          if changeid == '':
> >              changeid = '.'
> >          self._repo = repo
> >
> >          if isinstance(changeid, int):
> > +            try:
> > +                self._node = repo.changelog.node(changeid)
> > +            except IndexError:
> > +                raise error.RepoLookupError(
> > +                    _("unknown revision '%s'") % changeid)
> >              self._rev = changeid
> >              self._node = repo.changelog.node(changed)
> 
> Not a big deal, but did you mean to leave this duplicated 'self._node
> = repo.changelog.node(changed)' after your try/except clause?

Fixed version can be pulled from

hg pull -r c68a4c3cfe81 http://hg-lab.logilab.org/wip/hg-filter/
Kevin Bullock - Dec. 18, 2012, 5:37 p.m.
On Dec 17, 2012, at 10:50 AM, pierre-yves.david at logilab.fr wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
> # Date 1354489919 -3600
> # Node ID e954ac5321a9ead1a4e687385dca7134218ccc83
> # Parent  8c9a52492d426741ab24392d49f44a1d4f23613e
> clfilter: ensure context raise RepoLookupError when the revision is filtered
> 
> Currently the code path of `changectx(filteredrepo, rev)` call
> `filteredrepo.changelog.node(rev)`. When `rev` is filtered this raise an
> unhandled `IndexError`. This case now raise a `RepoLookupError` as other
> error case do.

Pushed the fixed version with the stray line that Sean pointed out removed.

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock
Idan Kamara - Jan. 10, 2013, 10:56 p.m.
On Mon, Dec 17, 2012 at 6:50 PM, <pierre-yves.david@logilab.fr> wrote:
>
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1354489919 -3600
> # Node ID e954ac5321a9ead1a4e687385dca7134218ccc83
> # Parent  8c9a52492d426741ab24392d49f44a1d4f23613e
> clfilter: ensure context raise RepoLookupError when the revision is
> filtered
>
> Currently the code path of `changectx(filteredrepo, rev)` call
> `filteredrepo.changelog.node(rev)`. When `rev` is filtered this raise an
> unhandled `IndexError`. This case now raise a `RepoLookupError` as other
> error case do.

This isn't sufficient for all the other code paths in here, e.g.
'hg log -r filtered-rev' tracebacks with an IndexError
since it hits this path:

http://selenic.com/repo/hg/file/5db16424142c/mercurial/context.py#l68

It probably applies to some of other calls to changelog
in here.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -23,10 +23,15 @@  class changectx(object):
         if changeid == '':
             changeid = '.'
         self._repo = repo
 
         if isinstance(changeid, int):
+            try:
+                self._node = repo.changelog.node(changeid)
+            except IndexError:
+                raise error.RepoLookupError(
+                    _("unknown revision '%s'") % changeid)
             self._rev = changeid
             self._node = repo.changelog.node(changeid)
             return
         if isinstance(changeid, long):
             changeid = str(changeid)