This is an archive of the discontinued Mercurial Phabricator instance.

Differential D429

fbamend: precursor -> predecessor
ClosedPublic

Authored by ryanmce on Aug 17 2017, 1:29 PM.

Details

Reviewers
simonfar
simpkins
Group Reviewers
Restricted Project
Commits
rFBHGXaf24a7fe2383: fbamend: precursor -> predecessor
Summary

Get in line with upstream name change and eliminate develwarns.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Branch
default
Lint
Lint OK
Unit
Unit Test Errors

Event Timeline

ryanmce created this revision.Aug 17 2017, 1:29 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 17 2017, 1:29 PM
simonfar accepted this revision.Aug 17 2017, 3:19 PM
simonfar added a subscriber: simonfar.

Just nits, some of them probably old.

hgext3rd/fbamend/revsets.py
41

This comment looks wrong - but I think it might be right.

52

Can you reinsert the s in the function name?

58

This comment is wrong.

hgext3rd/fbamend/unamend.py
23

This looks odd, too.

This revision is now accepted and ready to land.Aug 17 2017, 3:19 PM
simpkins accepted this revision.Aug 17 2017, 6:46 PM
simpkins added a subscriber: simpkins.
simpkins added inline comments.
hgext3rd/fbamend/revsets.py
55

I believe it should work to just say

@revsetpredicate('precursors(set)')
@revsetpredicate('predecessors(set)')
def predecessors(repo, subset, x):
    # ...

The current code is fine too, though, if you prefer that.

quark added a subscriber: quark.Aug 17 2017, 7:37 PM
quark added inline comments.
hgext3rd/fbamend/revsets.py
38–50

This could be removed since the upstream has successors implemented (and is actually allsuccessors which is more useful).

ryanmce added inline comments.Aug 18 2017, 5:37 AM
hgext3rd/fbamend/revsets.py
38–50

Oh cool, I'll remove it then.

55

I'll give that a shot, it looks preferable to me.

58

I didn't actually change it apart from the search-replace; it was wrong originally but I guess I can fix it up since I'm here :-)

hgext3rd/fbamend/unamend.py
23

I agree.

ryanmce marked 6 inline comments as done.Aug 18 2017, 5:54 AM
ryanmce added inline comments.
hgext3rd/fbamend/revsets.py
52

I've deleted it per @quark

ryanmce marked 3 inline comments as done.Aug 18 2017, 6:05 AM
ryanmce updated this revision to Diff 1061.

per suggestions

Closed by commit rFBHGXaf24a7fe2383: fbamend: precursor -> predecessor (authored by ryanmce). · Explain WhyAug 18 2017, 6:18 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 1031

hgext3rd/fbamend/revsets.py

nodemap = cl.nodemap nodemap = cl.nodemap
resultrevs = set(torev(n) resultrevs = set(torev(n)
for n in f(tonode(r) for r in revs) for n in f(tonode(r) for r in revs)
if n in nodemap) if n in nodemap)
s = smartset.baseset(resultrevs - set(revs) - repo.changelog.filteredrevs) s = smartset.baseset(resultrevs - set(revs) - repo.changelog.filteredrevs)
s.sort() s.sort()
return subset & s return subset & s
@revsetpredicate('successors(set)') @revsetpredicate('successors(set)')
def successors(repo, subset, x): def successors(repo, subset, x):
"""Immediate successors for given set""" """Immediate successors for given set"""
# getsuccessors: lookup node by precursor # getsuccessors: lookup node by predecessor
simonfarUnsubmitted
Not Done

This comment looks wrong - but I think it might be right.

simonfar: This comment looks wrong - but I think it might be right.
getsuccessors = repo.obsstore.successors.get getsuccessors = repo.obsstore.successors.get
def f(nodes): def f(nodes):
for n in nodes: for n in nodes:
for m in getsuccessors(n, ()): for m in getsuccessors(n, ()):
# m[0]: precursor, m[1]: successors # m[0]: predecessor, m[1]: successors
for n in m[1]: for n in m[1]:
yield n yield n
return _calculateset(repo, subset, x, f) return _calculateset(repo, subset, x, f)
quarkUnsubmitted
Done

This could be removed since the upstream has successors implemented (and is actually allsuccessors which is more useful).

quark: This could be removed since the upstream has `successors` implemented (and is actually…
ryanmceAuthorUnsubmitted
Done

Oh cool, I'll remove it then.

ryanmce: Oh cool, I'll remove it then.
@revsetpredicate('precursors(set)') @revsetpredicate('precursors(set)')
def precursors(repo, subset, x): def precursor(repo, subset, x):
simonfarUnsubmitted
Done

Can you reinsert the s in the function name?

simonfar: Can you reinsert the s in the function name?
ryanmceAuthorUnsubmitted
Done

I've deleted it per @quark

ryanmce: I've deleted it per @quark
"""Immediate precursors for given set""" return predecessors(repo, subset, x)
# getsuccessors: lookup node by precursor
getprecursors = repo.obsstore.precursors.get @revsetpredicate('predecessors(set)')
simpkinsUnsubmitted
Done

I believe it should work to just say

@revsetpredicate('precursors(set)')
@revsetpredicate('predecessors(set)')
def predecessors(repo, subset, x):
    # ...

The current code is fine too, though, if you prefer that.

simpkins: I believe it should work to just say @revsetpredicate('precursors(set)') @revsetpredicate…
ryanmceAuthorUnsubmitted
Done

I'll give that a shot, it looks preferable to me.

ryanmce: I'll give that a shot, it looks preferable to me.
def predecessors(repo, subset, x):
"""Immediate predecessors for given set"""
# getsuccessors: lookup node by predecessor
simonfarUnsubmitted
Not Done

This comment is wrong.

simonfar: This comment is wrong.
ryanmceAuthorUnsubmitted
Done

I didn't actually change it apart from the search-replace; it was wrong originally but I guess I can fix it up since I'm here :-)

ryanmce: I didn't actually change it apart from the search-replace; it was wrong originally but I guess…
getpredecessors = repo.obsstore.predecessors.get
def f(nodes): def f(nodes):
for n in nodes: for n in nodes:
for m in getprecursors(n, ()): for m in getpredecessors(n, ()):
# m[0]: precursor, m[1]: successors # m[0]: predecessor, m[1]: successors
yield m[0] yield m[0]
return _calculateset(repo, subset, x, f) return _calculateset(repo, subset, x, f)
@revsetpredicate('allsuccessors(set)') @revsetpredicate('allsuccessors(set)')
def allsuccessors(repo, subset, x): def allsuccessors(repo, subset, x):
"""All changesets which are successors for given set, recursively""" """All changesets which are successors for given set, recursively"""
f = lambda nodes: obsutil.allsuccessors(repo.obsstore, nodes) f = lambda nodes: obsutil.allsuccessors(repo.obsstore, nodes)
return _calculateset(repo, subset, x, f) return _calculateset(repo, subset, x, f)
@revsetpredicate('allprecursors(set)') @revsetpredicate('allprecursors(set)')
def allprecursors(repo, subset, x): def allprecursors(repo, subset, x):
"""All changesets which are precursors for given set, recursively""" return allpredecessors(repo, subset, x)
f = lambda nodes: obsutil.allprecursors(repo.obsstore, nodes)
@revsetpredicate('allpredecessors(set)')
def allpredecessors(repo, subset, x):
"""All changesets which are predecessors for given set, recursively"""
f = lambda nodes: obsutil.allpredecessors(repo.obsstore, nodes)
return _calculateset(repo, subset, x, f) return _calculateset(repo, subset, x, f)

hgext3rd/fbamend/unamend.py

obsutil, obsutil,
registrar, registrar,
) )
from mercurial.i18n import _ from mercurial.i18n import _
cmdtable = {} cmdtable = {}
command = registrar.command(cmdtable) command = registrar.command(cmdtable)
def precursormarkers(ctx): def predecessormarkers(ctx):
"""obsolete marker marking this changeset as a successors""" """obsolete marker marking this changeset as a successors"""
simonfarUnsubmitted
Done

This looks odd, too.

simonfar: This looks odd, too.
ryanmceAuthorUnsubmitted
Not Done

I agree.

ryanmce: I agree.
for data in ctx.repo().obsstore.precursors.get(ctx.node(), ()): for data in ctx.repo().obsstore.predecessors.get(ctx.node(), ()):
yield obsutil.marker(ctx.repo(), data) yield obsutil.marker(ctx.repo(), data)
@command('^unamend', []) @command('^unamend', [])
def unamend(ui, repo, **opts): def unamend(ui, repo, **opts):
"""undo the amend operation on a current changeset """undo the amend operation on a current changeset
This command will roll back to the previous version of a changeset, This command will roll back to the previous version of a changeset,
leaving working directory in state in which it was before running leaving working directory in state in which it was before running
`hg amend` (e.g. files modified as part of an amend will be `hg amend` (e.g. files modified as part of an amend will be
marked as modified `hg status`)""" marked as modified `hg status`)"""
try: try:
extensions.find('inhibit') extensions.find('inhibit')
except KeyError: except KeyError:
hint = _("please add inhibit to the list of enabled extensions") hint = _("please add inhibit to the list of enabled extensions")
e = _("unamend requires inhibit extension to be enabled") e = _("unamend requires inhibit extension to be enabled")
raise error.Abort(e, hint=hint) raise error.Abort(e, hint=hint)
unfi = repo.unfiltered() unfi = repo.unfiltered()
# identify the commit from which to unamend # identify the commit from which to unamend
curctx = repo['.'] curctx = repo['.']
# identify the commit to which to unamend # identify the commit to which to unamend
markers = list(precursormarkers(curctx)) markers = list(predecessormarkers(curctx))
if len(markers) != 1: if len(markers) != 1:
e = _("changeset must have one precursor, found %i precursors") e = _("changeset must have one predecessor, found %i predecessors")
raise error.Abort(e % len(markers)) raise error.Abort(e % len(markers))
prednode = markers[0].prednode() prednode = markers[0].prednode()
precctx = unfi[prednode] precctx = unfi[prednode]
if curctx.children(): if curctx.children():
raise error.Abort(_("cannot unamend in the middle of a stack")) raise error.Abort(_("cannot unamend in the middle of a stack"))

hgext3rd/inhibit.py

error, error,
extensions, extensions,
obsolete, obsolete,
util, util,
) )
def _obsoletedrevs(repo): def _obsoletedrevs(repo):
"""Redefine "obsolete()" revset. Previously, X is obsoleted if X appears as """Redefine "obsolete()" revset. Previously, X is obsoleted if X appears as
a precursor in a marker. Now, X is obsoleted if X is a precursor in marker a predecessor in a marker. Now, X is obsoleted if X is a predecessor in
M1, *and* is not a successor in marker M2 where M2.date >= M1.date. marker M1, *and* is not a successor in marker M2 where M2.date >= M1.date.
This allows undo to return to old hashes, and is correct as long as This allows undo to return to old hashes, and is correct as long as
obsmarker is not exchanged. obsmarker is not exchanged.
""" """
getnode = repo.changelog.node getnode = repo.changelog.node
markersbysuccessor = repo.obsstore.precursors.get markersbysuccessor = repo.obsstore.predecessors.get
markersbyprecursor = repo.obsstore.successors.get markersbypredecessor = repo.obsstore.successors.get
result = set() result = set()
for r in obsolete._mutablerevs(repo): for r in obsolete._mutablerevs(repo):
n = getnode(r) n = getnode(r)
m1s = markersbyprecursor(n) m1s = markersbypredecessor(n)
m2s = markersbysuccessor(n) m2s = markersbysuccessor(n)
if m1s: if m1s:
if m2s: if m2s:
# marker: (prec, [succ], flag, meta, (date, timezone), parent) # marker: (prec, [succ], flag, meta, (date, timezone), parent)
d1 = max(m[4][0] for m in m1s) d1 = max(m[4][0] for m in m1s)
d2 = max(m[4][0] for m in m2s) d2 = max(m[4][0] for m in m2s)
if d2 < d1: if d2 < d1:
result.add(r) result.add(r)
else: else:
result.add(r) result.add(r)
return result return result
def _obsstorecreate(orig, self, tr, prec, succs=(), flag=0, parents=None, def _obsstorecreate(orig, self, tr, prec, succs=(), flag=0, parents=None,
date=None, metadata=None, ui=None): date=None, metadata=None, ui=None):
# make "prec in succs" in-marker cycle check a no-op # make "prec in succs" in-marker cycle check a no-op
succs = _nocontainslist(succs) succs = _nocontainslist(succs)
# we need to resolve default date # we need to resolve default date
if date is None: if date is None:
if ui is not None: if ui is not None:
date = ui.configdate('devel', 'default-date') date = ui.configdate('devel', 'default-date')
if date is None: if date is None:
date = util.makedate() date = util.makedate()
# if prec is a successor of an existing marker, make default date bigger so # if prec is a successor of an existing marker, make default date bigger so
# the old marker won't revive the precursor accidentally. This helps tests # the old marker won't revive the predecessor accidentally. This helps tests
# where date are always (0, 0) # where date are always (0, 0)
markers = self.precursors.get(prec) markers = self.predecessors.get(prec)
if markers: if markers:
maxdate = max(m[4] for m in markers) maxdate = max(m[4] for m in markers)
maxdate = (maxdate[0] + 1, maxdate[1]) maxdate = (maxdate[0] + 1, maxdate[1])
if maxdate > date: if maxdate > date:
date = maxdate date = maxdate
return orig(self, tr, prec, succs, flag, parents, date, metadata, ui) return orig(self, tr, prec, succs, flag, parents, date, metadata, ui)
class _nocontainslist(list): class _nocontainslist(list):
# like "list", but __contains__ returns False, used by _obsstorecreate # like "list", but __contains__ returns False, used by _obsstorecreate
def __contains__(self, x): def __contains__(self, x):
return False return False
def _createmarkers(orig, repo, rels, *args, **kwargs): def _createmarkers(orig, repo, rels, *args, **kwargs):
# make precursor context unfiltered so parents() won't raise # make predecessor context unfiltered so parents() won't raise
unfi = repo.unfiltered() unfi = repo.unfiltered()
rels = [list(r) for r in rels] # make mutable rels = [list(r) for r in rels] # make mutable
for r in rels: for r in rels:
try: try:
r[0] = unfi[r[0].node()] r[0] = unfi[r[0].node()]
except error.RepoLookupError: except error.RepoLookupError:
# node could be unknown in current repo # node could be unknown in current repo
pass pass
# NOTE: we should avoid doing this but just change templates to only show a # NOTE: we should avoid doing this but just change templates to only show a
# subset of troubles we care about. # subset of troubles we care about.
revsets['divergent'] = revsets['bumped'] = lambda repo: frozenset() revsets['divergent'] = revsets['bumped'] = lambda repo: frozenset()
# make obsstore.create not complain about in-marker cycles, since we want # make obsstore.create not complain about in-marker cycles, since we want
# to write X -> X to revive X. # to write X -> X to revive X.
extensions.wrapfunction(obsolete.obsstore, 'create', _obsstorecreate) extensions.wrapfunction(obsolete.obsstore, 'create', _obsstorecreate)
# make createmarkers use unfiltered precursor ctx, workarounds an issue # make createmarkers use unfiltered predecessor ctx, workarounds an issue
# that prec.parents() may raise FilteredIndexError. # that prec.parents() may raise FilteredIndexError.
# NOTE: should be fixed upstream once hash-preserving obsstore is a thing. # NOTE: should be fixed upstream once hash-preserving obsstore is a thing.
extensions.wrapfunction(obsolete, 'createmarkers', _createmarkers) extensions.wrapfunction(obsolete, 'createmarkers', _createmarkers)