This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1585

infinitepush: add logic to add a stream level parameter to bundle2
ClosedPublic

Authored by pulkit on Dec 4 2017, 4:20 PM.
Tags
None
Subscribers
None

Details

Reviewers
durham
Group Reviewers
Restricted Project
Commits
rFBHGX736e4b77922e: infinitepush: add logic to add a stream level parameter to bundle2
Summary

The current method to check whether the bundle is a bundle2 push or not
introduces a new bundle2 part and we can only store changegroup using that part.

This patch introduces logic which will add a stream level parameter to bundle2,
reading whose value while unbundling, we can easily decide whether this bundle
is infinitepush bundle or not.
The stream level parameters are read before any of the parts and hence we can be
sured that we get to know whether this push if infinitepush or not before
processing any part.
This will also helps us in storing the whole bundle i.e. changegroup, bookmarks,
obsmarker etc. in the store.
The logic is currently hidden behind a config option.

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

pulkit created this revision.Dec 4 2017, 4:20 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 4 2017, 4:20 PM
durham accepted this revision.Dec 4 2017, 11:24 PM

Couple questions inline, but I'll push it once they're answered.

infinitepush/__init__.py
406

Since we don't use source, outgoing, or opts, I'd leave them as **kwargs and pass them along below as **kwargs

410

Hmm, shouldn't this only be added if the push is actually an infinitepush push? Or will that logic come later?

1247

ui.configbool instead of ui.config?

This revision is now accepted and ready to land.Dec 4 2017, 11:24 PM
pulkit updated this revision to Diff 4117.Dec 5 2017, 9:39 AM
pulkit marked 2 inline comments as done.Dec 5 2017, 9:42 AM
pulkit added inline comments.
infinitepush/__init__.py
410

Yes it should, I wanted to make sure I am heading in right direction so that's why for now I put this behind a config which defaults to False.

Closed by commit rFBHGX736e4b77922e: infinitepush: add logic to add a stream level parameter to bundle2 (authored by pulkit). · Explain WhyDec 5 2017, 12:17 PM
This revision was automatically updated to reflect the committed changes.
pulkit added inline comments.Dec 5 2017, 4:46 PM
infinitepush/__init__.py
968

@durham I see we are doing the same thing here. Does the logic for checking whether this push is an infinitepush or not is one in line 949-963? I am unable to find the bookmark pattern matching thing which you told me.

durham added inline comments.Dec 7 2017, 4:54 PM
infinitepush/__init__.py
968

Hmm, maybe you are right. Maybe it doesn't exist on the server side yet (it exists on the client side in this file in _scratchbranchmatcher). I guess we can just rely on this parameter for the first iteration, then to the automatic detection at a later point.

Revision Contents
Changeset List

Diff 4118

infinitepush/__init__.py

bookcmd[1].append( bookcmd[1].append(
('', 'remote-path', '', ('', 'remote-path', '',
'name of the remote path to list the bookmarks')) 'name of the remote path to list the bookmarks'))
wrapcommand(commands.table, 'pull', _pull) wrapcommand(commands.table, 'pull', _pull)
wrapcommand(commands.table, 'update', _update) wrapcommand(commands.table, 'update', _update)
wrapfunction(discovery, 'checkheads', _checkheads) wrapfunction(discovery, 'checkheads', _checkheads)
wrapfunction(bundle2, '_addpartsfromopts', _addpartsfromopts)
wireproto.wirepeer.listkeyspatterns = listkeyspatterns wireproto.wirepeer.listkeyspatterns = listkeyspatterns
# Move infinitepush part before pushrebase part # Move infinitepush part before pushrebase part
# to avoid generation of both parts. # to avoid generation of both parts.
partorder = exchange.b2partsgenorder partorder = exchange.b2partsgenorder
index = partorder.index('changeset') index = partorder.index('changeset')
if pushrebaseparttype in partorder: if pushrebaseparttype in partorder:
else: else:
return orig(ui, repo, *names, **opts) return orig(ui, repo, *names, **opts)
def _checkheads(orig, pushop): def _checkheads(orig, pushop):
if pushop.ui.configbool(experimental, configscratchpush, False): if pushop.ui.configbool(experimental, configscratchpush, False):
return return
return orig(pushop) return orig(pushop)
def _addpartsfromopts(orig, ui, repo, bundler, *args, **kwargs):
durhamUnsubmitted
Done

Since we don't use source, outgoing, or opts, I'd leave them as **kwargs and pass them along below as **kwargs

durham: Since we don't use source, outgoing, or opts, I'd leave them as `**kwargs` and pass them along…
""" adds a stream level part to bundle2 storing whether this is an
infinitepush bundle or not """
if ui.configbool('infinitepush', 'bundle-stream', False):
bundler.addparam('infinitepush', True)
durhamUnsubmitted
Not Done

Hmm, shouldn't this only be added if the push is actually an infinitepush push? Or will that logic come later?

durham: Hmm, shouldn't this only be added if the push is actually an infinitepush push? Or will that…
pulkitAuthorUnsubmitted
Not Done

Yes it should, I wanted to make sure I am heading in right direction so that's why for now I put this behind a config which defaults to False.

pulkit: Yes it should, I wanted to make sure I am heading in right direction so that's why for now I…
return orig(ui, repo, bundler, *args, **kwargs)
def wireprotolistkeyspatterns(repo, proto, namespace, patterns): def wireprotolistkeyspatterns(repo, proto, namespace, patterns):
patterns = decodelist(patterns) patterns = decodelist(patterns)
d = repo.listkeys(encoding.tolocal(namespace), patterns).iteritems() d = repo.listkeys(encoding.tolocal(namespace), patterns).iteritems()
return pushkey.encodekeys(d) return pushkey.encodekeys(d)
def localrepolistkeys(orig, self, namespace, patterns=None): def localrepolistkeys(orig, self, namespace, patterns=None):
if namespace == 'bookmarks' and patterns: if namespace == 'bookmarks' and patterns:
index = self.bundlestore.index index = self.bundlestore.index
if not pushop.outgoing.missing: if not pushop.outgoing.missing:
pushop.ui.status(_('no changes found\n')) pushop.ui.status(_('no changes found\n'))
pushop.cgresult = 0 pushop.cgresult = 0
return return
# This parameter tells the server that the following bundle is an # This parameter tells the server that the following bundle is an
# infinitepush. This let's it switch the part processing to our infinitepush # infinitepush. This let's it switch the part processing to our infinitepush
# code path. # code path.
bundler.addparam("infinitepush", "True") bundler.addparam("infinitepush", "True")
pulkitAuthorUnsubmitted
Not Done

@durham I see we are doing the same thing here. Does the logic for checking whether this push is an infinitepush or not is one in line 949-963? I am unable to find the bookmark pattern matching thing which you told me.

pulkit: @durham I see we are doing the same thing here. Does the logic for checking whether this push…
durhamUnsubmitted
Not Done

Hmm, maybe you are right. Maybe it doesn't exist on the server side yet (it exists on the client side in this file in _scratchbranchmatcher). I guess we can just rely on this parameter for the first iteration, then to the automatic detection at a later point.

durham: Hmm, maybe you are right. Maybe it doesn't exist on the server side yet (it exists on the…
nonforwardmove = pushop.force or pushop.ui.configbool(experimental, nonforwardmove = pushop.force or pushop.ui.configbool(experimental,
confignonforwardmove) confignonforwardmove)
scratchparts = getscratchbranchparts(pushop.repo, scratchparts = getscratchbranchparts(pushop.repo,
pushop.remote, pushop.remote,
pushop.outgoing, pushop.outgoing,
nonforwardmove, nonforwardmove,
pushop.ui, pushop.ui,
log(scratchbranchparttype, eventtype='failure', log(scratchbranchparttype, eventtype='failure',
elapsedms=(time.time() - parthandlerstart) * 1000, elapsedms=(time.time() - parthandlerstart) * 1000,
errormsg=str(e)) errormsg=str(e))
raise raise
finally: finally:
if bundle: if bundle:
bundle.close() bundle.close()
@bundle2.b2streamparamhandler('infinitepush')
def processinfinitepush(unbundler, param, value):
""" process the bundle2 stream level parameter containing whether this push
is an infinitepush or not. """
if value and unbundler.ui.configbool('infinitepush',
durhamUnsubmitted
Done

ui.configbool instead of ui.config?

durham: ui.configbool instead of ui.config?
'bundle-stream', False):
pass
@bundle2.parthandler(scratchbranchparttype, @bundle2.parthandler(scratchbranchparttype,
('bookmark', 'bookprevnode' 'create', 'force', ('bookmark', 'bookprevnode' 'create', 'force',
'pushbackbookmarks', 'cgversion')) 'pushbackbookmarks', 'cgversion'))
def bundle2scratchbranch(op, part): def bundle2scratchbranch(op, part):
'''unbundle a bundle2 part containing a changegroup to store''' '''unbundle a bundle2 part containing a changegroup to store'''
bundler = bundle2.bundle20(op.repo.ui) bundler = bundle2.bundle20(op.repo.ui)
cgversion = part.params.get('cgversion', '01') cgversion = part.params.get('cgversion', '01')

tests/test-check-config-hg.t

undocumented: fbconduit.backingrepos (list) [[reponame]] undocumented: fbconduit.backingrepos (list) [[reponame]]
undocumented: fbconduit.gitcallsigns (list) undocumented: fbconduit.gitcallsigns (list)
undocumented: fbconduit.host (str) undocumented: fbconduit.host (str)
undocumented: fbconduit.path (str) undocumented: fbconduit.path (str)
undocumented: fbconduit.protocol (str) undocumented: fbconduit.protocol (str)
undocumented: fbconduit.reponame (str) undocumented: fbconduit.reponame (str)
undocumented: fbhistedit.exec_in_user_shell (str) undocumented: fbhistedit.exec_in_user_shell (str)
undocumented: grep.command (str) undocumented: grep.command (str)
undocumented: infinitepush.bundle-stream (bool)
undocumented: morestatus.show (bool) undocumented: morestatus.show (bool)
undocumented: nointerrupt.interactiveonly (bool) [True] undocumented: nointerrupt.interactiveonly (bool) [True]
undocumented: perftweaks.cachenoderevs (bool) [True] undocumented: perftweaks.cachenoderevs (bool) [True]
undocumented: perftweaks.disablebranchcache (bool) undocumented: perftweaks.disablebranchcache (bool)
undocumented: perftweaks.disablecasecheck (bool) undocumented: perftweaks.disablecasecheck (bool)
undocumented: perftweaks.disabletags (bool) undocumented: perftweaks.disabletags (bool)
undocumented: perftweaks.preferdeltas (bool) undocumented: perftweaks.preferdeltas (bool)
undocumented: phabstatus.logpeekahead (int) [30] undocumented: phabstatus.logpeekahead (int) [30]