Rev 2522: Fix merge multiple connections. Test suite *not* passing (sftp in file:///v/home/vila/src/experimental/reuse.transports/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Wed Jun 6 14:52:05 BST 2007
At file:///v/home/vila/src/experimental/reuse.transports/
------------------------------------------------------------
revno: 2522
revision-id: v.ladeuil+lp at free.fr-20070606135202-mqhxcv6z57uce434
parent: v.ladeuil+lp at free.fr-20070606084207-6fa0f02eadtezlrf
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: reuse.transports
timestamp: Wed 2007-06-06 15:52:02 +0200
message:
Fix merge multiple connections. Test suite *not* passing (sftp
refactoring pending but unrelated to merge).
* bzrlib/builtins.py:
(cmd_merge.run): Fix the multiple connections bug by reusing the
tramsport used to check for a bundle and keep all other used
transports in possible_transports.
(_merge_helper): Add a possible_transports parameter for
reuse.
* bzrlib/transport/__init__.py:
(Transport._reuse_for): By default, Transports are not reusable.
(ConnectedTransport._reuse_for): ConnectedTransports are reusable
under certain conditions.
(_urlRE): Fix misleading group name.
(_try_transport_factories): Moved after get_transport (another use
case for moved lines). The do_catching_redirections was
incorrectly inserted between get_transport and
_try_transport_factories.
* bzrlib/tests/test_transport.py:
(TestReusedTransports.test_reuse_same_transport)
(TestReusedTransports.test_don_t_reuse_different_transport): Add
more tests.
* bzrlib/merge.py:
(_get_tree, Merger.set_other): Add a possible_transports parameter
for reuse.
* bzrlib/bzrdir.py:
(BzrDir.open_containing): Add a possible_transports parameter for
reuse.
* bzrlib/branch.py:
(Branch.open_containing): Add a possible_transports parameter for
reuse.
modified:
bzrlib/branch.py branch.py-20050309040759-e4baf4e0d046576e
bzrlib/builtins.py builtins.py-20050830033751-fc01482b9ca23183
bzrlib/bzrdir.py bzrdir.py-20060131065624-156dfea39c4387cb
bzrlib/merge.py merge.py-20050513021216-953b65a438527106
bzrlib/tests/TransportUtil.py transportutil.py-20070525113600-5v2igk89s8fensom-1
bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
bzrlib/transport/__init__.py transport.py-20050711165921-4978aa7ce1285ad5
bzrlib/transport/ftp.py ftp.py-20051116161804-58dc9506548c2a53
-------------- next part --------------
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py 2007-06-05 15:52:12 +0000
+++ b/bzrlib/branch.py 2007-06-06 13:52:02 +0000
@@ -140,7 +140,7 @@
return control.open_branch(_unsupported)
@staticmethod
- def open_containing(url):
+ def open_containing(url, possible_transports=None):
"""Open an existing branch which contains url.
This probes for a branch at url, and searches upwards from there.
@@ -151,7 +151,8 @@
format, UnknownFormatError or UnsupportedFormatError are raised.
If there is one, it is returned, along with the unused portion of url.
"""
- control, relpath = bzrdir.BzrDir.open_containing(url)
+ control, relpath = bzrdir.BzrDir.open_containing(url,
+ possible_transports)
return control.open_branch(), relpath
@staticmethod
=== modified file 'bzrlib/builtins.py'
--- a/bzrlib/builtins.py 2007-06-05 15:52:12 +0000
+++ b/bzrlib/builtins.py 2007-06-06 13:52:02 +0000
@@ -2646,7 +2646,6 @@
directory=None,
):
from bzrlib.tag import _merge_tags_if_possible
- other_revision_id = None
if merge_type is None:
merge_type = _mod_merge.Merge3Merger
@@ -2662,20 +2661,35 @@
change_reporter = delta._ChangeReporter(
unversioned_filter=tree.is_ignored)
+ other_transport = None
+ other_revision_id = None
+ possible_transports = []
+ # The user may provide a bundle or branch as 'branch' We first try to
+ # identify a bundle, if it's not, we try to preserve connection used by
+ # the transport to access the branch.
if branch is not None:
- try:
- mergeable = bundle.read_mergeable_from_url(
- branch)
- except errors.NotABundle:
- pass # Continue on considering this url a Branch
- else:
- if revision is not None:
- raise errors.BzrCommandError(
- 'Cannot use -r with merge directives or bundles')
- other_revision_id = mergeable.install_revisions(
- tree.branch.repository)
- revision = [RevisionSpec.from_string(
- 'revid:' + other_revision_id)]
+ url = urlutils.normalize_url(branch)
+ url, filename = urlutils.split(url, exclude_trailing_slash=False)
+ other_transport = transport.get_transport(url)
+ if filename:
+ try:
+ read_bundle = bundle.read_mergeable_from_transport
+ # There may be redirections but we ignore the intermediate
+ # and final transports used
+ mergeable, t = read_bundle(other_transport, filename)
+ except errors.NotABundle:
+ # Continue on considering this url a Branch but adjust the
+ # other_transport
+ other_transport = other_transport.clone(filename)
+ else:
+ if revision is not None:
+ raise errors.BzrCommandError('Cannot use -r with merge'
+ ' directives or bundles')
+ other_revision_id = mergeable.install_revisions(
+ tree.branch.repository)
+ revision = [RevisionSpec.from_string(
+ 'revid:' + other_revision_id)]
+ possible_transports.append(other_transport)
if revision is None \
or len(revision) < 1 or revision[0].needs_branch():
@@ -2688,7 +2702,8 @@
else:
base = [None, None]
other = [branch, -1]
- other_branch, path = Branch.open_containing(branch)
+ other_branch, path = Branch.open_containing(branch,
+ possible_transports)
else:
if uncommitted:
raise errors.BzrCommandError('Cannot use --uncommitted and'
@@ -2697,11 +2712,13 @@
if len(revision) == 1:
base = [None, None]
if other_revision_id is not None:
+ # We merge from a bundle
other_branch = None
path = ""
other = None
else:
- other_branch, path = Branch.open_containing(branch)
+ other_branch, path = Branch.open_containing(
+ branch, possible_transports)
revno = revision[0].in_history(other_branch).revno
other = [branch, revno]
else:
@@ -2709,9 +2726,11 @@
if None in revision:
raise errors.BzrCommandError(
"Merge doesn't permit empty revision specifier.")
- base_branch, path = Branch.open_containing(branch)
+ base_branch, path = Branch.open_containing(branch,
+ possible_transports)
branch1 = revision[1].get_branch() or branch
- other_branch, path1 = Branch.open_containing(branch1)
+ other_branch, path1 = Branch.open_containing(
+ branch1, possible_transports)
if revision[0].get_branch() is not None:
# then path was obtained from it, and is None.
path = path1
@@ -2719,6 +2738,7 @@
base = [branch, revision[0].in_history(base_branch).revno]
other = [branch1, revision[1].in_history(other_branch).revno]
+ # Remember where we merge from
if ((tree.branch.get_parent() is None or remember) and
other_branch is not None):
tree.branch.set_parent(other_branch.base)
@@ -2736,7 +2756,8 @@
try:
try:
conflict_count = _merge_helper(
- other, base, other_rev_id=other_revision_id,
+ other, base, possible_transports,
+ other_rev_id=other_revision_id,
check_clean=(not force),
merge_type=merge_type,
reprocess=reprocess,
@@ -3682,7 +3703,7 @@
# command-line interpretation helper for merge-related commands
-def _merge_helper(other_revision, base_revision,
+def _merge_helper(other_revision, base_revision, possible_transports=None,
check_clean=True, ignore_zero=False,
this_dir=None, backup_files=False,
merge_type=None,
@@ -3749,7 +3770,7 @@
if other_rev_id is not None:
merger.set_other_revision(other_rev_id, this_tree.branch)
else:
- merger.set_other(other_revision)
+ merger.set_other(other_revision, possible_transports)
merger.pp.next_phase()
merger.set_base(base_revision)
if merger.base_rev_id == merger.other_rev_id:
=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py 2007-05-04 10:33:56 +0000
+++ b/bzrlib/bzrdir.py 2007-06-06 13:52:02 +0000
@@ -588,13 +588,14 @@
raise NotImplementedError(self.open_branch)
@staticmethod
- def open_containing(url):
+ def open_containing(url, possible_transports=None):
"""Open an existing branch which contains url.
:param url: url to search from.
See open_containing_from_transport for more detail.
"""
- return BzrDir.open_containing_from_transport(get_transport(url))
+ transport = get_transport(url, possible_transports)
+ return BzrDir.open_containing_from_transport(transport)
@staticmethod
def open_containing_from_transport(a_transport):
=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py 2007-03-21 01:34:41 +0000
+++ b/bzrlib/merge.py 2007-06-06 13:52:02 +0000
@@ -52,13 +52,13 @@
# TODO: Report back as changes are merged in
-def _get_tree(treespec, local_branch=None):
+def _get_tree(treespec, local_branch=None, possible_transports=None):
from bzrlib import workingtree
location, revno = treespec
if revno is None:
tree = workingtree.WorkingTree.open_containing(location)[0]
return tree.branch, tree
- branch = Branch.open_containing(location)[0]
+ branch = Branch.open_containing(location, possible_transports)[0]
if revno == -1:
revision_id = branch.last_revision()
else:
@@ -206,7 +206,7 @@
return
self.this_tree.add_parent_tree((self.other_rev_id, self.other_tree))
- def set_other(self, other_revision):
+ def set_other(self, other_revision, possible_transports=None):
"""Set the revision and tree to merge from.
This sets the other_tree, other_rev_id, other_basis attributes.
@@ -214,7 +214,8 @@
:param other_revision: The [path, revision] list to merge from.
"""
self.other_branch, self.other_tree = _get_tree(other_revision,
- self.this_branch)
+ self.this_branch,
+ possible_transports)
if other_revision[1] == -1:
self.other_rev_id = self.other_branch.last_revision()
if self.other_rev_id is None:
=== modified file 'bzrlib/tests/TransportUtil.py'
--- a/bzrlib/tests/TransportUtil.py 2007-06-06 08:42:07 +0000
+++ b/bzrlib/tests/TransportUtil.py 2007-06-06 13:52:02 +0000
@@ -81,5 +81,8 @@
self.connections = []
def set_connection_hook(self, transport, connection, credentials):
+ # Note: uncomment the following line and use 'bt' under pdb, here is
+ # the extra connection.
+ # import pdb; pdb.set_trace()
self.connections.append(connection)
=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py 2007-06-03 12:58:49 +0000
+++ b/bzrlib/tests/test_transport.py 2007-06-06 13:52:02 +0000
@@ -685,13 +685,22 @@
"""Tests for transport reuse"""
def test_reuse_same_transport(self):
- t = get_transport('http://foo/')
- t2 = get_transport('http://foo/', possible_transports=[t])
- self.assertIs(t, t2)
+ t1 = get_transport('http://foo/')
+ t2 = get_transport('http://foo/', possible_transports=[t1])
+ self.assertIs(t1, t2)
+
+ # Also check that final '/' are handled correctly
+ t3 = get_transport('http://foo/path/')
+ t4 = get_transport('http://foo/path', possible_transports=[t3])
+ self.assertIs(t3, t4)
+
+ t3 = get_transport('http://foo/path')
+ t4 = get_transport('http://foo/path/', possible_transports=[t3])
+ self.assertIs(t3, t4)
def test_don_t_reuse_different_transport(self):
- t = get_transport('http://foo/')
- t2 = get_transport('http://bar/', possible_transports=[t])
+ t = get_transport('http://foo/path')
+ t2 = get_transport('http://bar/path', possible_transports=[t])
self.assertIsNot(t, t2)
=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py 2007-06-03 15:52:19 +0000
+++ b/bzrlib/transport/__init__.py 2007-06-06 13:52:02 +0000
@@ -1028,6 +1028,8 @@
# several questions about the transport.
return False
+ def _reuse_for(self, other_base):
+ return None
class ConnectedTransport(Transport):
"""A transport connected to a remote server.
@@ -1242,18 +1244,62 @@
(connection, credentials) = self._connection[0]
return self._connection[0][1]
+ def _reuse_for(self, other_base):
+ """Returns a transport sharing the same connection if possible.
+
+ Note: we share the connection if the expected credentials are the
+ same: (host, port, user). Some protocols may disagree and redefine the
+ criteria in daughter classes.
+
+ Note: we don't compare the passwords here because other_base may have
+ been obtained from an existing transport.base which do not mention the
+ password.
+
+ :param other_base: the URL we want to share the connection with.
+
+ :return: A new transport or None if the connection cannot be shared.
+ """
+ (scheme, user, password, host, port, path) = self._split_url(other_base)
+ transport = None
+ # Don't compare passwords, they may be absent from other_base
+ if (scheme,
+ user,
+ host, port) == (self._scheme,
+ self._user,
+ self._host, self._port):
+ if not path.endswith('/'):
+ # This normally occurs at __init__ time, but it's easier to do
+ # it now to avoid positives (creating two transports for the
+ # same base).
+ path += '/'
+ if self._path == path:
+ # shortcut, it's really the same transport
+ return self
+ # We don't call clone here because the intent is different: we
+ # build a new transport on a different base (which may be totally
+ # unrelated) but we share the connection.
+ transport = self.__class__(other_base, self)
+ return transport
+
+
# jam 20060426 For compatibility we copy the functions here
# TODO: The should be marked as deprecated
urlescape = urlutils.escape
urlunescape = urlutils.unescape
-_urlRE = re.compile(r'^(?P<proto>[^:/\\]+)://(?P<path>.*)$')
+# We try to recognize an url lazily (ignoring user, password, etc)
+_urlRE = re.compile(r'^(?P<proto>[^:/\\]+)://(?P<rest>.*)$')
def get_transport(base, possible_transports=None):
"""Open a transport to access a URL or directory.
:param base: either a URL or a directory name.
- :param transports: optional reusable transports list.
+
+ :param transports: optional reusable transports list. If not None, created
+ transports will be added to the list.
+
+ :return: A new transport optionally sharing its connection with one of
+ possible_transports.
"""
if base is None:
base = '.'
@@ -1281,28 +1327,47 @@
transport = None
if possible_transports:
for t in possible_transports:
- if t.base == base:
- transport = t
- break
- if transport is None:
- for proto, factory_list in transport_list_registry.iteritems():
- if proto is not None and base.startswith(proto):
- transport, last_err = _try_transport_factories(base,
- factory_list)
- if transport:
- break
- if transport is None:
- # We tried all the different protocols, now try one last
- # time as a local protocol
- base = convert_path_to_url(base, 'Unsupported protocol: %s')
-
- # The default handler is the filesystem handler, stored
- # as protocol None
- factory_list = transport_list_registry.get(None)
- transport, last_err = _try_transport_factories(base, factory_list)
+ t_same_connection = t._reuse_for(base)
+ if t_same_connection is not None:
+ # Add only new transports
+ if t_same_connection not in possible_transports:
+ possible_transports.append(t_same_connection)
+ return t_same_connection
+
+ for proto, factory_list in transport_list_registry.iteritems():
+ if proto is not None and base.startswith(proto):
+ transport, last_err = _try_transport_factories(base, factory_list)
+ if transport:
+ if possible_transports:
+ assert transport not in possible_transports
+ possible_transports.append(transport)
+ return transport
+
+ # We tried all the different protocols, now try one last
+ # time as a local protocol
+ base = convert_path_to_url(base, 'Unsupported protocol: %s')
+
+ # The default handler is the filesystem handler, stored
+ # as protocol None
+ factory_list = transport_list_registry.get(None)
+ transport, last_err = _try_transport_factories(base, factory_list)
+
return transport
+def _try_transport_factories(base, factory_list):
+ last_err = None
+ for factory in factory_list:
+ try:
+ return factory.get_obj()(base), None
+ except errors.DependencyNotPresent, e:
+ mutter("failed to instantiate transport %r for %r: %r" %
+ (factory, base, e))
+ last_err = e
+ continue
+ return None, last_err
+
+
def do_catching_redirections(action, transport, redirected):
"""Execute an action with given transport catching redirections.
@@ -1345,19 +1410,6 @@
raise errors.TooManyRedirections
-def _try_transport_factories(base, factory_list):
- last_err = None
- for factory in factory_list:
- try:
- return factory.get_obj()(base), None
- except errors.DependencyNotPresent, e:
- mutter("failed to instantiate transport %r for %r: %r" %
- (factory, base, e))
- last_err = e
- continue
- return None, last_err
-
-
class Server(object):
"""A Transport Server.
=== modified file 'bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py 2007-06-03 13:01:58 +0000
+++ b/bzrlib/transport/ftp.py 2007-06-06 13:52:02 +0000
@@ -575,11 +575,11 @@
"""
try:
asyncore.loop(*args, **kwargs)
- # FIXME: If we reach that point, we should raise an
- # exception explaining that the 'count' parameter in
- # setUp is too low or testers may wonder why their
- # test just sits there waiting for a server that is
- # already dead.
+ # FIXME: If we reach that point, we should raise an exception
+ # explaining that the 'count' parameter in setUp is too low or
+ # testers may wonder why their test just sits there waiting for a
+ # server that is already dead. Note that if the tester waits too
+ # long under pdb the server will also die.
except select.error, e:
if e.args[0] != errno.EBADF:
raise
More information about the bazaar-commits
mailing list