[PATCH 2 of 9] context: factor out exception handling while looking changeid up

[FUJIWARA Katsunori]

At Mon, 30 Mar 2015 13:35:17 +0900,
Yuya Nishihara wrote:
> 
> On Mon, 30 Mar 2015 03:34:24 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1427653751 -32400
> > #      Mon Mar 30 03:29:11 2015 +0900
> > # Node ID 501346a6222d37bf0e841175ecde906d79673044
> > # Parent  ea145f508b3df90b8393db3722a9fc9397c3354c
> > context: factor out exception handling while looking changeid up
> > 
> > This patch factors out exception handling while looking changeid up in
> > "changectx.__init__()", to reuse it in subsequent patches.
> > 
> > This avoids duplication of similar code in subsequent patches.
> > 
> > diff --git a/mercurial/context.py b/mercurial/context.py
> > --- a/mercurial/context.py
> > +++ b/mercurial/context.py
> > @@ -353,6 +353,46 @@ def makememctx(repo, parents, text, user
> >                            date, extra, editor)
> >      return ctx
> >  
> > +def _wraplookup(lookupfunc, repo, changeid, abort):
> > +    """Wrap looking ``changeid`` up in ``repo`` for common exception handling
> > +
> > +    ``lookupfunc`` should be the function taking ``repo`` and
> > +    ``changeid`` arguments.
> > +
> > +    If ``lookupfunc`` raises Filtered*Error, this re-raises
> > +    FilteredRepoLookupError.
> > +
> > +    If ``abort`` is true and ``lookupfunc`` returns false value, this
> > +    raises RepoLookupError.
> > +
> > +    Otherwise, this returns the value returned by ``lookupfunc``.
> > +    """
> > +    try:
> > +        found = lookupfunc(repo, changeid)
> > +    except (error.FilteredIndexError, error.FilteredLookupError,
> > +            error.FilteredRepoLookupError):
> > +        if repo.filtername == 'visible':
> > +            msg = _("hidden revision '%s'") % changeid
> > +            hint = _('use --hidden to access hidden revisions')
> > +            raise error.FilteredRepoLookupError(msg, hint=hint)
> > +        msg = _("filtered revision '%s' (not in '%s' subset)")
> > +        msg %= (changeid, repo.filtername)
> > +        raise error.FilteredRepoLookupError(msg)
> > +    except IndexError:
> > +        found = None
> > +
> > +    if abort and not found:
> > +        try:
> > +            if len(changeid) == 20:
> > +                changeid = hex(changeid)
> > +        except TypeError:
> > +            pass
> > +
> > +        raise error.RepoLookupError(
> > +            _("unknown revision '%s'") % changeid)
> > +
> > +    return found
> 
> _wraplookup() is never called with abort=False in this series. Do you have
> a plan to use it?

At first, I will replace "x in self" and "self.changelog.rev(x)"
ignoring raised exception in localrepo by "findbyhash(abort=False)".

Especially, "node in self" of "commithook()" in "localrepo.commit()"
should be replaced, because:

  - "node in self" implies examination of "repo.dirstate.p1()", and

  - it may show "ignoring unknown working parent" unexpectedly

    "commithook()" is invoked even if transaction is aborted.

    in such case, changelog may be already reloaded from restored one,
    and "repo.dirstate.p1()" may be recognized as invalid.
    
Then, I'll replace other "x in repo" by "findbyhash()" or so for
efficiency: for example, "bookmarks.py" has many such code paths.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp