[PATCH 1 of 4] histedit: add a method to cleanup nodes safely
Jun Wu
quark at fb.com
Fri Mar 17 22:25:18 EDT 2017
Excerpts from Yuya Nishihara's message of 2017-03-18 11:13:36 +0900:
> On Fri, 17 Mar 2017 08:37:14 -0700, Jun Wu wrote:
> > Excerpts from Yuya Nishihara's message of 2017-03-17 22:32:45 +0900:
> > > On Mon, 13 Mar 2017 22:36:16 -0700, Jun Wu wrote:
> > > > # HG changeset patch
> > > > # User Jun Wu <quark at fb.com>
> > > > # Date 1489464645 25200
> > > > # Mon Mar 13 21:10:45 2017 -0700
> > > > # Node ID b4cf155f7a41ebf314407000f6948716ae0a64e2
> > > > # Parent 3d3109339b57341b333c1112beb41dd281fa944a
> > > > # Available At https://bitbucket.org/quark-zju/hg-draft
> > > > # hg pull https://bitbucket.org/quark-zju/hg-draft -r b4cf155f7a41
> > > > histedit: add a method to cleanup nodes safely
> > >
> > > The series looks good to me, but one nit.
> > >
> > > > diff --git a/hgext/histedit.py b/hgext/histedit.py
> > > > --- a/hgext/histedit.py
> > > > +++ b/hgext/histedit.py
> > > > @@ -1582,4 +1582,29 @@ def cleanupnode(ui, repo, name, nodes):
> > > > repair.strip(ui, repo, c)
> > > >
> > > > +def safecleanupnode(ui, repo, name, nodes):
> > > > + """strip or obsolete nodes
> > > > +
> > > > + nodes could be either a set or dict which maps to replacements.
> > > > + nodes could be unknown (outside the repo).
> > > > + """
> > > > + supportsmarkers = obsolete.isenabled(repo, obsolete.createmarkersopt)
> > > > + if supportsmarkers:
> > > > + if util.safehasattr(nodes, 'get'):
> > > > + # nodes is a dict-like mapping
> > > > + # use unfiltered repo for successors in case they are hidden
> > > > + urepo = repo.unfiltered()
> > > > + marker = lambda x: (repo[x], (urepo[n] for n in nodes.get(x, ())))
> > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >
> > > This isn't a tuple but a generator. Is that by intention?
> >
> > Yes. Since obsolete.createmarkers only requires an "iterable", I think a
> > generator may make the space complexity from O(N) to O(1). Another reason is
> > the code fits in one line.
>
> Well, it works as 'sucs' is consumed only once, but the doc doesn't say
Ah, I thought they could be both be generators.
> 'sucs' can be an iterable. And it seems to me that each item of
> 'relations' is designed as immutable.
Seems it will expand sucs:
nsucs = tuple(s.node() for s in sucs)
And "for rel in relations" is only once. So both "relations" and "sucs" can
be generators.
> "<relations> must be an iterable of (<old>, (<new>, ...)[,{metadata}])
> tuple."
I think generators are iterable.
Anyway, I think this is fine as is. Or I could send a V2. Or I could fix
obsolete.py documentation to make it clear that generators are fine.
More information about the Mercurial-devel
mailing list