This is an archive of the discontinued Mercurial Phabricator instance.

Differential D264

exchange: drop support for lock-based unbundling (BC)
ClosedPublic

Authored by indygreg on Aug 7 2017, 11:23 PM.

Details

Reviewers
martinvonz
Group Reviewers
hg-reviewers
Commits
rHGfda0867cfe03: exchange: drop support for lock-based unbundling (BC)
Summary

Locking over the wire protocol and the "addchangegroup" wire
protocol command has been deprecated since e8c4f3d3df8c, which was
first part of Mercurial 0.9.1.

Support for handling these commands from sshserver was dropped in
9f6e0e7ef828 in 2015, effectively locking out pre 0.9.1 clients
from new servers.

However, client-side code for calling lock and addchangegroup is
still present in exchange.py and the various peer classes to
facilitate pushing to pre 0.9.1 servers.

The lock-based pushing mechanism is extremely brittle. 0.9.1 was
released in July 2006 and I highly doubt anyone is still running
such an ancient version of Mercurial on a server. I'm about to
refactor the peer API and I don't think it is worth keeping
support for this ancient protocol feature. So, this commit removes
client support for the lock-based pushing mechanism. This means
modern clients will no longer be able to push to pre 0.9.1 servers.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Aug 7 2017, 11:23 PM
martinvonz accepted this revision as: martinvonz.Aug 8 2017, 12:38 AM
martinvonz added a subscriber: martinvonz.

Looks great to me. I also can't see this being a problem for anyone.

Closed by commit rHGfda0867cfe03: exchange: drop support for lock-based unbundling (BC) (authored by indygreg). · Explain WhyAug 8 2017, 10:52 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/exchange.py (69 lines)
Mmercurial/httppeer.py (3 lines)
Mmercurial/localrepo.py (3 lines)
Mmercurial/sshpeer.py (44 lines)

Diff 614

mercurial/exchange.py

missing = (set(pushop.repo.requirements) missing = (set(pushop.repo.requirements)
- pushop.remote.local().supported) - pushop.remote.local().supported)
if missing: if missing:
msg = _("required features are not" msg = _("required features are not"
" supported in the destination:" " supported in the destination:"
" %s") % (', '.join(sorted(missing))) " %s") % (', '.join(sorted(missing)))
raise error.Abort(msg) raise error.Abort(msg)
# there are two ways to push to remote repo:
#
# addchangegroup assumes local user can lock remote
# repo (local filesystem, old ssh servers).
#
# unbundle assumes local user cannot lock remote repo (new ssh
# servers, http servers).
if not pushop.remote.canpush(): if not pushop.remote.canpush():
raise error.Abort(_("destination does not support push")) raise error.Abort(_("destination does not support push"))
if not pushop.remote.capable('unbundle'):
raise error.Abort(_('cannot push: destination does not support the '
'unbundle wire protocol command'))
# get local lock as we might write phase data # get local lock as we might write phase data
localwlock = locallock = None localwlock = locallock = None
try: try:
# bundle2 push may receive a reply bundle touching bookmarks or other # bundle2 push may receive a reply bundle touching bookmarks or other
# things requiring the wlock. Take it now to ensure proper ordering. # things requiring the wlock. Take it now to ensure proper ordering.
maypushback = pushop.ui.configbool('experimental', 'bundle2.pushback') maypushback = pushop.ui.configbool('experimental', 'bundle2.pushback')
if (not _forcebundle1(pushop)) and maypushback: if (not _forcebundle1(pushop)) and maypushback:
localwlock = pushop.repo.wlock() localwlock = pushop.repo.wlock()
msg = 'cannot lock source repository: %s\n' % err msg = 'cannot lock source repository: %s\n' % err
pushop.ui.debug(msg) pushop.ui.debug(msg)
try: try:
if pushop.locallocked: if pushop.locallocked:
pushop.trmanager = transactionmanager(pushop.repo, pushop.trmanager = transactionmanager(pushop.repo,
'push-response', 'push-response',
pushop.remote.url()) pushop.remote.url())
pushop.repo.checkpush(pushop) pushop.repo.checkpush(pushop)
lock = None
unbundle = pushop.remote.capable('unbundle')
if not unbundle:
lock = pushop.remote.lock()
try:
_pushdiscovery(pushop) _pushdiscovery(pushop)
if not _forcebundle1(pushop): if not _forcebundle1(pushop):
_pushbundle2(pushop) _pushbundle2(pushop)
_pushchangeset(pushop) _pushchangeset(pushop)
_pushsyncphase(pushop) _pushsyncphase(pushop)
_pushobsolete(pushop) _pushobsolete(pushop)
_pushbookmark(pushop) _pushbookmark(pushop)
finally:
if lock is not None:
lock.release()
if pushop.trmanager: if pushop.trmanager:
pushop.trmanager.close() pushop.trmanager.close()
finally: finally:
if pushop.trmanager: if pushop.trmanager:
pushop.trmanager.release() pushop.trmanager.release()
if locallock is not None: if locallock is not None:
locallock.release() locallock.release()
if localwlock is not None: if localwlock is not None:
def _pushchangeset(pushop): def _pushchangeset(pushop):
"""Make the actual push of changeset bundle to remote repo""" """Make the actual push of changeset bundle to remote repo"""
if 'changesets' in pushop.stepsdone: if 'changesets' in pushop.stepsdone:
return return
pushop.stepsdone.add('changesets') pushop.stepsdone.add('changesets')
if not _pushcheckoutgoing(pushop): if not _pushcheckoutgoing(pushop):
return return
# Should have verified this in push().
assert pushop.remote.capable('unbundle')
pushop.repo.prepushoutgoinghooks(pushop) pushop.repo.prepushoutgoinghooks(pushop)
outgoing = pushop.outgoing outgoing = pushop.outgoing
unbundle = pushop.remote.capable('unbundle')
# TODO: get bundlecaps from remote # TODO: get bundlecaps from remote
bundlecaps = None bundlecaps = None
# create a changegroup from local # create a changegroup from local
if pushop.revs is None and not (outgoing.excluded if pushop.revs is None and not (outgoing.excluded
or pushop.repo.changelog.filteredrevs): or pushop.repo.changelog.filteredrevs):
# push everything, # push everything,
# use the fast path, no race possible on push # use the fast path, no race possible on push
bundler = changegroup.cg1packer(pushop.repo, bundlecaps) bundler = changegroup.cg1packer(pushop.repo, bundlecaps)
cg = changegroup.getsubset(pushop.repo, cg = changegroup.getsubset(pushop.repo,
outgoing, outgoing,
bundler, bundler,
'push', 'push',
fastpath=True) fastpath=True)
else: else:
cg = changegroup.getchangegroup(pushop.repo, 'push', outgoing, cg = changegroup.getchangegroup(pushop.repo, 'push', outgoing,
bundlecaps=bundlecaps) bundlecaps=bundlecaps)
# apply changegroup to remote # apply changegroup to remote
if unbundle:
# local repo finds heads on server, finds out what # local repo finds heads on server, finds out what
# revs it must push. once revs transferred, if server # revs it must push. once revs transferred, if server
# finds it has different heads (someone else won # finds it has different heads (someone else won
# commit/push race), server aborts. # commit/push race), server aborts.
if pushop.force: if pushop.force:
remoteheads = ['force'] remoteheads = ['force']
else: else:
remoteheads = pushop.remoteheads remoteheads = pushop.remoteheads
# ssh: return remote's addchangegroup() # ssh: return remote's addchangegroup()
# http: return remote's addchangegroup() or 0 for error # http: return remote's addchangegroup() or 0 for error
pushop.cgresult = pushop.remote.unbundle(cg, remoteheads, pushop.cgresult = pushop.remote.unbundle(cg, remoteheads,
pushop.repo.url()) pushop.repo.url())
else:
# we return an integer indicating remote head count
# change
pushop.cgresult = pushop.remote.addchangegroup(cg, 'push',
pushop.repo.url())
def _pushsyncphase(pushop): def _pushsyncphase(pushop):
"""synchronise phase information locally and remotely""" """synchronise phase information locally and remotely"""
cheads = pushop.commonheads cheads = pushop.commonheads
# even when we don't push, exchanging phase data is useful # even when we don't push, exchanging phase data is useful
remotephases = pushop.remote.listkeys('phases') remotephases = pushop.remote.listkeys('phases')
if (pushop.ui.configbool('ui', '_usedassubrepo') if (pushop.ui.configbool('ui', '_usedassubrepo')
and remotephases # server supports phases and remotephases # server supports phases

mercurial/httppeer.py

try: try:
self._fetchcaps() self._fetchcaps()
except error.RepoError: except error.RepoError:
self.caps = set() self.caps = set()
self.ui.debug('capabilities: %s\n' % self.ui.debug('capabilities: %s\n' %
(' '.join(self.caps or ['none']))) (' '.join(self.caps or ['none'])))
return self.caps return self.caps
def lock(self):
raise error.Abort(_('operation not supported over http'))
def _callstream(self, cmd, _compressible=False, **args): def _callstream(self, cmd, _compressible=False, **args):
if cmd == 'pushkey': if cmd == 'pushkey':
args['data'] = '' args['data'] = ''
data = args.pop('data', None) data = args.pop('data', None)
headers = args.pop('headers', {}) headers = args.pop('headers', {})
self.ui.debug("sending %s command\n" % cmd) self.ui.debug("sending %s command\n" % cmd)
q = [('cmd', cmd)] q = [('cmd', cmd)]

mercurial/localrepo.py

bundler.addpart(out) bundler.addpart(out)
stream = util.chunkbuffer(bundler.getchunks()) stream = util.chunkbuffer(bundler.getchunks())
b = bundle2.getunbundler(self.ui, stream) b = bundle2.getunbundler(self.ui, stream)
bundle2.processbundle(self._repo, b) bundle2.processbundle(self._repo, b)
raise raise
except error.PushRaced as exc: except error.PushRaced as exc:
raise error.ResponseError(_('push failed:'), str(exc)) raise error.ResponseError(_('push failed:'), str(exc))
def lock(self):
return self._repo.lock()
def pushkey(self, namespace, key, old, new): def pushkey(self, namespace, key, old, new):
return self._repo.pushkey(namespace, key, old, new) return self._repo.pushkey(namespace, key, old, new)
def listkeys(self, namespace): def listkeys(self, namespace):
return self._repo.listkeys(namespace) return self._repo.listkeys(namespace)
def debugwireargs(self, one, two, three=None, four=None, five=None): def debugwireargs(self, one, two, three=None, four=None, five=None):
'''used to test argument passing over the wire''' '''used to test argument passing over the wire'''

mercurial/sshpeer.py

from .i18n import _ from .i18n import _
from . import ( from . import (
error, error,
pycompat, pycompat,
util, util,
wireproto, wireproto,
) )
class remotelock(object):
def __init__(self, repo):
self.repo = repo
def release(self):
self.repo.unlock()
self.repo = None
def __enter__(self):
return self
def __exit__(self, exc_type, exc_val, exc_tb):
if self.repo:
self.release()
def __del__(self):
if self.repo:
self.release()
def _serverquote(s): def _serverquote(s):
if not s: if not s:
return s return s
'''quote a string for the remote shell ... which we assume is sh''' '''quote a string for the remote shell ... which we assume is sh'''
if re.match('[a-zA-Z0-9@%_+=:,./-]*$', s): if re.match('[a-zA-Z0-9@%_+=:,./-]*$', s):
return s return s
return "'%s'" % s.replace("'", "'\\''") return "'%s'" % s.replace("'", "'\\''")
def _send(self, data, flush=False): def _send(self, data, flush=False):
self.pipeo.write("%d\n" % len(data)) self.pipeo.write("%d\n" % len(data))
if data: if data:
self.pipeo.write(data) self.pipeo.write(data)
if flush: if flush:
self.pipeo.flush() self.pipeo.flush()
self.readerr() self.readerr()
def lock(self):
self._call("lock")
return remotelock(self)
def unlock(self):
self._call("unlock")
def addchangegroup(self, cg, source, url, lock=None):
'''Send a changegroup to the remote server. Return an integer
similar to unbundle(). DEPRECATED, since it requires locking the
remote.'''
d = self._call("addchangegroup")
if d:
self._abort(error.RepoError(_("push refused: %s") % d))
for d in iter(lambda: cg.read(4096), ''):
self.pipeo.write(d)
self.readerr()
self.pipeo.flush()
self.readerr()
r = self._recv()
if not r:
return 1
try:
return int(r)
except ValueError:
self._abort(error.ResponseError(_("unexpected response:"), r))
instance = sshpeer instance = sshpeer