[PATCH 6 of 6] revset: fix a crash with 'roots(wdir())'
yuya at tcha.org
Sat Jul 4 08:29:09 CDT 2015
On Fri, 03 Jul 2015 09:51:31 -0700, Pierre-Yves David wrote:
> On 07/01/2015 06:28 PM, Matt Harbison wrote:
> > On Wed, 01 Jul 2015 09:01:17 -0400, Yuya Nishihara <yuya at tcha.org> wrote:
> >> 3) can we have a function that is as fast as cl.parentrevs(r) and can
> >> process
> >> wdir() ?
> > Something like this in revset.py? It looks like an optimization to not
> > go through context.
> > def _parentrevs(repo, cl, r):
> > if r is not None:
> > return cl.parentrevs(r)
> > else:
> > return [p.rev() for p in repo[r].parents()]
> > The only reason I didn't do it was because with the local changelog
> > caching optimization, I assumed calling a function over and over would
> > be unacceptable overhead.
It could be a generator to minimize the number of repo.changelog calls, but
I agree it would be still slower than the inlined version.
def _iterparentrevs(repo, subset):
cl = repo.changelog
for r in subset:
if r is not None:
yield [p.rev() for p in repo[r].parents()]
> So I think we should have the API not panic when handed and
> 'wdirrev'/'wdirid' (the same way we probably want to detect 'nullrev'/….
> I see possibles path for this:
> 1) Have the low level API support the 'wdirrev' by providing them some
> access to the dirstate data. That would be the "best" as it would ensure
> transparent support for 'wdir' everywhere (mostly). However, this would
> probably requires a massive number of layer violation and mudding.
> 2) Have low level API detect 'wdirrev' case and "ignore" it, issue a
> devel-warn for developer to spot such case. This will provide a middle
> ground between full support (1) and plain crash (what we have now).
I think (1) would be the hard way. My current idea is similar to (2), but
raising exception instead of devel-warning which needs 'ui' object.
cl.node(wdirrev) -> wdirid
cl.rev(wdirnode) -> wdirrev
cl.parentrevs(wdirrev) -> UnsupportedNodeError
Then, cl.parentrevs(r) will be able to fall back to repo[r].parents():
The overhead will be minimal because 'wdir()' will be rarely used.
> 3) As you guys have been working on it more closely than I did, you will
> likely comes with another better solution.
> 4) I got convinced that my worries are groundless.
I'm sure (4) never ever happen. ;)
More information about the Mercurial-devel