[PATCH 3 of 3 STABLE V2] changectx: do not include hidden revisions on short node lookup (issue4964)

[Yuya Nishihara]

On Fri, 28 Oct 2016 10:31:40 +0200, Pierre-Yves David wrote:
> On 10/26/2016 03:17 PM, Yuya Nishihara wrote:
> > On Tue, 25 Oct 2016 23:17:14 +0900, Yuya Nishihara wrote:
> >> # HG changeset patch
> >> # User Yuya Nishihara <yuya at tcha.org>
> >> # Date 1477199774 -32400
> >> #      Sun Oct 23 14:16:14 2016 +0900
> >> # Branch stable
> >> # Node ID 242b7a856495179795ee5662f298029c4b492563
> >> # Parent  ecbce2fe4dea116c925a2fecd1b7b50df0a62589
> >> changectx: do not include hidden revisions on short node lookup (issue4964)
> >>
> >> It was changed at dc25ed84bee8, but which seems wrong since we filtered out
> >> hidden nodes by _partialmatch() before that change. This patch makes
> >> changectx() be consistent with the filtered changelog, and detect a hidden
> >> short node only if it has no unique match in the filtered changelog.
> >>
> >> Though we've made shortest(node) use unfiltered changelog, I think this is
> >> a separate issue worth fixing.
> >>
> >> diff --git a/mercurial/context.py b/mercurial/context.py
> >> --- a/mercurial/context.py
> >> +++ b/mercurial/context.py
> >> @@ -481,11 +481,16 @@ class changectx(basectx):
> >>              except error.RepoLookupError:
> >>                  pass
> >>
> >> -            self._node = repo.unfiltered().changelog._partialmatch(changeid)
> >> +            self._node = repo.changelog._partialmatch(changeid)
> >>              if self._node is not None:
> >>                  self._rev = repo.changelog.rev(self._node)
> >>                  return
> >>
> >> +            # lookup hidden node to provide a better error indication
> >> +            n = repo.unfiltered().changelog._partialmatch(changeid)
> >> +            if n is not None:
> >> +                repo.changelog.rev(n)  # must raise FilteredLookupError
> >
> > Please feel free to drop this, the last patch, if the performance is important.
> 
> The behavior update seems good, however, you are mentionning performance
> impact but it is unclear of the extend of it. Can you mention any number?

I haven't measure it, but the number would be similar to the patch 1. If an
ambiguous identifier is specified, and if hidden revisions exist, new code
would be about 10-100x (a few milliseconds) slower.

The question is whether we should care the cost needed only when an ambiguous
identifier is given.

https://www.mercurial-scm.org/pipermail/mercurial-devel/2016-October/089657.html