( )⚙ D1204 pushrebase: handle pushing phases through separate bundle2 part

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1204

pushrebase: handle pushing phases through separate bundle2 part
ClosedPublic

Authored by stash on Oct 23 2017, 7:08 AM.
Tags
None
Subscribers
durham

Details

Reviewers
durham
Group Reviewers
Restricted Project
Commits
rFBHGXb11d9934b9ce: pushrebase: handle pushing phases through separate bundle2 part
Summary

In 537de0b14030868e3e850ae388b08f88cabc88e8 upstream hg commit phase pushes
started to use separate bundle2 part. Because of that:

  1. pushkey hook is not called when updating phases.
  2. pushkey bundle2 part doesn't process phases anymore, 'phase-heads' part do

it instead.

It caused breakages in pushrebase. This diff fixes it.

Test Plan

Run tests test-pushrebase* and test-treemanifest*

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

stash created this revision.Oct 23 2017, 7:08 AM
Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 23 2017, 7:08 AM
stash edited the summary of this revision. (Show Details)Oct 23 2017, 7:09 AM
durham added a subscriber: durham.Oct 23 2017, 1:21 PM

Was this breaking the tests? Is this the check:phase thing we shipped? If we shipped this bug, why didn't the tests fail? Do we need a new test?

hgext3rd/pushrebase.py
131

Seems like a non-lambda function declaration would be the exact same number of lines, without the funky '/' right? I know you're just keeping with the style above though

durham added inline comments.Oct 23 2017, 1:27 PM
hgext3rd/pushrebase.py
1054

Maybe add a comment about what this is summing? At first glance, making a dict of a sum seems odd.

durham accepted this revision.Oct 23 2017, 1:29 PM

Looks like this is a different issue from the check:phase error. Accepting, though I still think a commented would help in that dict(sum()) spot.

This revision is now accepted and ready to land.Oct 23 2017, 1:29 PM
stash edited the summary of this revision. (Show Details)Oct 23 2017, 1:35 PM
stash updated this revision to Diff 3053.
Closed by commit rFBHGXb11d9934b9ce: pushrebase: handle pushing phases through separate bundle2 part (authored by stash). · Explain WhyOct 25 2017, 8:28 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext3rd/pushrebase.py (37 lines)
Mtests/test-pushrebase.t (1 line)

Diff 3095

hgext3rd/pushrebase.py

discovery, discovery,
encoding, encoding,
error, error,
exchange, exchange,
extensions, extensions,
hg, hg,
manifest, manifest,
obsolete, obsolete,
phases, phases as phasesmod,
pushkey, pushkey,
registrar, registrar,
revsetlang, revsetlang,
scmutil, scmutil,
util, util,
) )
from mercurial.extensions import wrapcommand, wrapfunction, unwrapfunction from mercurial.extensions import wrapcommand, wrapfunction, unwrapfunction
from mercurial.node import nullid, hex, bin from mercurial.node import nullid, hex, bin
origpushkeyhandler = bundle2.parthandlermapping['pushkey'] origpushkeyhandler = bundle2.parthandlermapping['pushkey']
newpushkeyhandler = lambda *args, **kwargs: \ newpushkeyhandler = lambda *args, **kwargs: \
bundle2pushkey(origpushkeyhandler, *args, **kwargs) bundle2pushkey(origpushkeyhandler, *args, **kwargs)
newpushkeyhandler.params = origpushkeyhandler.params newpushkeyhandler.params = origpushkeyhandler.params
bundle2.parthandlermapping['pushkey'] = newpushkeyhandler bundle2.parthandlermapping['pushkey'] = newpushkeyhandler
bundle2.parthandlermapping['b2x:pushkey'] = newpushkeyhandler bundle2.parthandlermapping['b2x:pushkey'] = newpushkeyhandler
origphaseheadshandler = bundle2.parthandlermapping['phase-heads']
newphaseheadshandler = lambda *args, **kwargs: \
durhamUnsubmitted
Not Done

Seems like a non-lambda function declaration would be the exact same number of lines, without the funky '/' right? I know you're just keeping with the style above though

durham: Seems like a non-lambda function declaration would be the exact same number of lines, without…
bundle2phaseheads(origphaseheadshandler, *args, **kwargs)
newphaseheadshandler.params = origphaseheadshandler.params
bundle2.parthandlermapping['phase-heads'] = newphaseheadshandler
wrapfunction(exchange, 'unbundle', unbundle) wrapfunction(exchange, 'unbundle', unbundle)
wrapfunction(hg, '_peerorrepo', _peerorrepo) wrapfunction(hg, '_peerorrepo', _peerorrepo)
def reposetup(ui, repo): def reposetup(ui, repo):
if repo.ui.configbool('pushrebase', 'blocknonpushrebase'): if repo.ui.configbool('pushrebase', 'blocknonpushrebase'):
repo.ui.setconfig('hooks', 'prechangegroup.blocknonpushrebase', repo.ui.setconfig('hooks', 'prechangegroup.blocknonpushrebase',
blocknonpushrebase) blocknonpushrebase)
self._pushrebasereplaces = {} self._pushrebasereplaces = {}
if version == obsolete._fm1version: if version == obsolete._fm1version:
# only support fm1 1:1 replacements for now, record prec -> sucs # only support fm1 1:1 replacements for now, record prec -> sucs
for prec, sucs, flags, meta, date, parents in markers: for prec, sucs, flags, meta, date, parents in markers:
if len(sucs) == 1: if len(sucs) == 1:
self._pushrebasereplaces[prec] = sucs[0] self._pushrebasereplaces[prec] = sucs[0]
return orig(self, transaction, data) return orig(self, transaction, data)
def _phasemove(orig, pushop, nodes, phase=phases.public): def _phasemove(orig, pushop, nodes, phase=phasesmod.public):
"""prevent original changesets from being marked public """prevent original changesets from being marked public
When marking changesets as public, we need to mark the replaced nodes When marking changesets as public, we need to mark the replaced nodes
returned from the server instead. This is done by looking at the new returned from the server instead. This is done by looking at the new
obsmarker we received during "_mergemarkers" and map old nodes to new ones. obsmarker we received during "_mergemarkers" and map old nodes to new ones.
See exchange.push for the order of this and bundle2 pushback: See exchange.push for the order of this and bundle2 pushback:
i.e. the server won't send obsmarkers directly. i.e. the server won't send obsmarkers directly.
For now, we don't have "1" so things are done in a bit hacky way. For now, we don't have "1" so things are done in a bit hacky way.
""" """
# find replacements. note: _pushrebasereplaces could be empty if obsstore # find replacements. note: _pushrebasereplaces could be empty if obsstore
# is not enabled locally. # is not enabled locally.
mapping = getattr(pushop.repo.obsstore, '_pushrebasereplaces', {}) mapping = getattr(pushop.repo.obsstore, '_pushrebasereplaces', {})
nodes = [mapping.get(n, n) for n in nodes] nodes = [mapping.get(n, n) for n in nodes]
if phase == phases.public: if phase == phasesmod.public:
# only allow new nodes to become public # only allow new nodes to become public
allowednodes = set(mapping.values()) allowednodes = set(mapping.values())
nodes = [n for n in nodes if n in allowednodes] nodes = [n for n in nodes if n in allowednodes]
orig(pushop, nodes, phase) orig(pushop, nodes, phase)
@exchange.b2partsgenerator(commonheadsparttype) @exchange.b2partsgenerator(commonheadsparttype)
def commonheadspartgen(pushop, bundler): def commonheadspartgen(pushop, bundler):
if rebaseparttype not in bundle2.bundle2caps(pushop.remote): if rebaseparttype not in bundle2.bundle2caps(pushop.remote):
if e.errno != errno.ENOENT: if e.errno != errno.ENOENT:
raise raise
if bundle: if bundle:
bundle.close() bundle.close()
# Move public phase forward # Move public phase forward
publishing = op.repo.ui.configbool('phases', 'publish', untrusted=True) publishing = op.repo.ui.configbool('phases', 'publish', untrusted=True)
if publishing: if publishing:
phases.advanceboundary(op.repo, tr, phases.public, [added[-1]]) phasesmod.advanceboundary(op.repo, tr, phasesmod.public, [added[-1]])
addfinalhooks(op, tr, hookargs, added) addfinalhooks(op, tr, hookargs, added)
# Send new commits back to the client # Send new commits back to the client
clientobsmarkerversions = [ clientobsmarkerversions = [
int(v) for v in params.get('obsmarkerversions', '').split('\0') if v] int(v) for v in params.get('obsmarkerversions', '').split('\0') if v]
_addpushbackparts(op, replacements, markers, markerdate, _addpushbackparts(op, replacements, markers, markerdate,
clientobsmarkerversions) clientobsmarkerversions)
args = hookargs.copy() args = hookargs.copy()
args['node'] = hex(n) args['node'] = hex(n)
op.repo.hook("incoming", **args) op.repo.hook("incoming", **args)
tr.addpostclose('serverrebase-cg-hooks', tr.addpostclose('serverrebase-cg-hooks',
lambda tr: op.repo._afterlock(runhooks)) lambda tr: op.repo._afterlock(runhooks))
def bundle2pushkey(orig, op, part): def bundle2pushkey(orig, op, part):
# Merges many dicts into one. First it converts them to list of pairs,
# then concatenates them (using sum), and then creates a diff out of them.
replacements = dict(sum([record.items() replacements = dict(sum([record.items()
for record for record
in op.records[rebaseparttype]], in op.records[rebaseparttype]],
[])) []))
namespace = pushkey.decode(part.params['namespace']) namespace = pushkey.decode(part.params['namespace'])
if namespace == 'phases': if namespace == 'phases':
key = pushkey.decode(part.params['key']) key = pushkey.decode(part.params['key'])
# then update the old location to match the real location # then update the old location to match the real location
# #
# TODO: We would prefer to only do this for pushrebase pushes # TODO: We would prefer to only do this for pushrebase pushes
# but that isn't straightforward so we just do it always here. # but that isn't straightforward so we just do it always here.
# This forbids moving bookmarks backwards from clients. # This forbids moving bookmarks backwards from clients.
part.params['old'] = pushkey.encode(hex(serverbin)) part.params['old'] = pushkey.encode(hex(serverbin))
return orig(op, part) return orig(op, part)
def bundle2phaseheads(orig, op, part):
# Merges many dicts into one. First it converts them to list of pairs,
durhamUnsubmitted
Not Done

Maybe add a comment about what this is summing? At first glance, making a dict of a sum seems odd.

durham: Maybe add a comment about what this is summing? At first glance, making a dict of a sum seems…
# then concatenates them (using sum), and then creates a diff out of them.
replacements = dict(sum([record.items()
for record
in op.records[rebaseparttype]],
[]))
decodedphases = phasesmod.binarydecode(part)
replacedphases = []
for phasetype in decodedphases:
replacedphases.append(
[replacements.get(node, node) for node in phasetype])
# Since we've just read the bundle part, then `orig()` won't be able to
# read it again. Let's replace payload stream with new stream of replaced
# nodes.
part._payloadstream = util.chunkbuffer(
[phasesmod.binaryencode(replacedphases)])
return orig(op, part)

tests/test-pushrebase.t

searching for changes searching for changes
prepushrebase hook: HG_BUNDLE2=1 HG_HOOKNAME=prepushrebase HG_HOOKTYPE=prepushrebase HG_HOOK_BUNDLEPATH=* HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_NODE_ONTO=e95be919ac60f0c114075e32a0a4301afabadb60 HG_SOURCE=push (glob) prepushrebase hook: HG_BUNDLE2=1 HG_HOOKNAME=prepushrebase HG_HOOKTYPE=prepushrebase HG_HOOK_BUNDLEPATH=* HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_NODE_ONTO=e95be919ac60f0c114075e32a0a4301afabadb60 HG_SOURCE=push (glob)
prechangegroup hook: HG_BUNDLE2=1 HG_HOOKNAME=prechangegroup HG_HOOKTYPE=prechangegroup HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob) prechangegroup hook: HG_BUNDLE2=1 HG_HOOKNAME=prechangegroup HG_HOOKTYPE=prechangegroup HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob)
pushing 3 changesets: pushing 3 changesets:
4fcee35c508c first 4fcee35c508c first
11be4ca7f3f4 second 11be4ca7f3f4 second
a5e72ac0df88 last a5e72ac0df88 last
pretxnchangegroup hook: HG_BUNDLE2=1 HG_HOOKNAME=pretxnchangegroup HG_HOOKTYPE=pretxnchangegroup HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_PENDING=$TESTTMP/hookserver HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob) pretxnchangegroup hook: HG_BUNDLE2=1 HG_HOOKNAME=pretxnchangegroup HG_HOOKTYPE=pretxnchangegroup HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_PENDING=$TESTTMP/hookserver HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob)
prepushkey hook: HG_BUNDLE2=1 HG_HOOKNAME=prepushkey HG_HOOKTYPE=prepushkey HG_KEY=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_NAMESPACE=phases HG_NEW=0 HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_OLD=1 HG_PENDING=$TESTTMP/hookserver HG_PHASES_MOVED=1 HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob)
prepushkey hook: HG_BUNDLE2=1 HG_HOOKNAME=prepushkey HG_HOOKTYPE=prepushkey HG_KEY=master HG_NAMESPACE=bookmarks HG_NEW=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_OLD=e95be919ac60f0c114075e32a0a4301afabadb60 HG_PENDING=$TESTTMP/hookserver HG_PHASES_MOVED=1 HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob) prepushkey hook: HG_BUNDLE2=1 HG_HOOKNAME=prepushkey HG_HOOKTYPE=prepushkey HG_KEY=master HG_NAMESPACE=bookmarks HG_NEW=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_OLD=e95be919ac60f0c114075e32a0a4301afabadb60 HG_PENDING=$TESTTMP/hookserver HG_PHASES_MOVED=1 HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob)
pretxnclose hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2=1 HG_HOOKNAME=pretxnclose HG_HOOKTYPE=pretxnclose HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_PENDING=$TESTTMP/hookserver HG_PHASES_MOVED=1 HG_SOURCE=push HG_TXNID=TXN:* HG_TXNNAME=push HG_URL=file:$TESTTMP/hookserver (glob) pretxnclose hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2=1 HG_HOOKNAME=pretxnclose HG_HOOKTYPE=pretxnclose HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_PENDING=$TESTTMP/hookserver HG_PHASES_MOVED=1 HG_SOURCE=push HG_TXNID=TXN:* HG_TXNNAME=push HG_URL=file:$TESTTMP/hookserver (glob)
txnclose hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2=1 HG_HOOKNAME=txnclose HG_HOOKTYPE=txnclose HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_PHASES_MOVED=1 HG_SOURCE=push HG_TXNID=TXN:* HG_TXNNAME=push HG_URL=file:$TESTTMP/hookserver (glob) txnclose hook: HG_BOOKMARK_MOVED=1 HG_BUNDLE2=1 HG_HOOKNAME=txnclose HG_HOOKTYPE=txnclose HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_PHASES_MOVED=1 HG_SOURCE=push HG_TXNID=TXN:* HG_TXNNAME=push HG_URL=file:$TESTTMP/hookserver (glob)
changegroup hook: HG_BUNDLE2=1 HG_HOOKNAME=changegroup HG_HOOKTYPE=changegroup HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob) changegroup hook: HG_BUNDLE2=1 HG_HOOKNAME=changegroup HG_HOOKTYPE=changegroup HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_SOURCE=push HG_TXNID=TXN:* HG_URL=file:$TESTTMP/hookserver (glob)
incoming hook: HG_BUNDLE2=1 HG_HOOKNAME=incoming HG_HOOKTYPE=incoming HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_SOURCE=push HG_TXNID=TXN:$ID$ HG_URL=file:$TESTTMP/hookserver incoming hook: HG_BUNDLE2=1 HG_HOOKNAME=incoming HG_HOOKTYPE=incoming HG_NODE=4fcee35c508c1019667f72cae9b843efa8908701 HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_SOURCE=push HG_TXNID=TXN:$ID$ HG_URL=file:$TESTTMP/hookserver
incoming hook: HG_BUNDLE2=1 HG_HOOKNAME=incoming HG_HOOKTYPE=incoming HG_NODE=11be4ca7f3f4a4e66fe9158d0d2b8defc7c32c7c HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_SOURCE=push HG_TXNID=TXN:$ID$ HG_URL=file:$TESTTMP/hookserver incoming hook: HG_BUNDLE2=1 HG_HOOKNAME=incoming HG_HOOKTYPE=incoming HG_NODE=11be4ca7f3f4a4e66fe9158d0d2b8defc7c32c7c HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_SOURCE=push HG_TXNID=TXN:$ID$ HG_URL=file:$TESTTMP/hookserver
incoming hook: HG_BUNDLE2=1 HG_HOOKNAME=incoming HG_HOOKTYPE=incoming HG_NODE=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_SOURCE=push HG_TXNID=TXN:$ID$ HG_URL=file:$TESTTMP/hookserver incoming hook: HG_BUNDLE2=1 HG_HOOKNAME=incoming HG_HOOKTYPE=incoming HG_NODE=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_NODE_LAST=a5e72ac0df8881afef34132987e8ae78d2e6cb13 HG_SOURCE=push HG_TXNID=TXN:$ID$ HG_URL=file:$TESTTMP/hookserver
updating bookmark master updating bookmark master