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

Jun Wu quark at fb.com
Tue Oct 25 11:38:58 EDT 2016


Excerpts from Yuya Nishihara's message of 2016-10-25 23:11:02 +0900:
> On Tue, 25 Oct 2016 14:12:08 +0100, Jun Wu wrote:
> > Excerpts from Yuya Nishihara's message of 2016-10-23 15:35:21 +0900:
> > > # 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 abbc5e382e1cb550f6d2ac886dfdb16bd95475ab
> > > # Parent  f180a39d749aeacb72936e629a372623b1f88b8c
> > > changectx: do not include hidden revisions on short node lookup (issue4964)
> > > 
> > > It was changed at dc25ed84bee8, but which seems wrong since we explicitly
> > > filter out hidden nodes by _partialmatch(). This patch makes changectx()
> > > be consistent with the changelog, and detect a hidden short node only if
> > > it has no unique match in filtered changelog.
> > > 
> > > 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)
> > 
> > Currently _partialmatch on unfiltered repo can be much faster than filtered
> > repo. See d0ae5b8f80dc, and fd1bb7c1be78.
> > 
> > I'd suggest get the nodeid from unfiltered repo first, and then test if it
> > is hidden. Alternatively, it may be worthwhile to change _partialmatch to do
> > the test so it's fast for filtered repo as well.
> 
> Do we care the cost to handle ambiguous ids (i.e. failure case) here?
> 
> We could try unfiltered repo first, but we would need to do _partialmatch()
> again if the given changeid is not unique in unfiltered repo, but unique in
> filtered repo. This is a success case and I think success cases are important.

I realized this problem after sending the mail. I'd choose performance for
now (seems martin has similar opinion).

The correct fix would be changing the C radix tree code, which I do want to
touch at some time to support serializing to disk.


More information about the Mercurial-devel mailing list