Rev 5570: (spiv) Avoid reopening (and relocking) the same branches/repositories in in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed Dec 15 01:28:34 GMT 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5570 [merge]
revision-id: pqm at pqm.ubuntu.com-20101215012832-s7gz2rnm1n94f4eu
parent: pqm at pqm.ubuntu.com-20101214230051-srsbr9s5e3karbr5
parent: andrew.bennetts at canonical.com-20101215003815-s8qjtrn59o1jke5m
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2010-12-15 01:28:32 +0000
message:
(spiv) Avoid reopening (and relocking) the same branches/repositories in
ControlDir.sprout. (Andrew Bennetts)
modified:
bzrlib/branch.py branch.py-20050309040759-e4baf4e0d046576e
bzrlib/bzrdir.py bzrdir.py-20060131065624-156dfea39c4387cb
bzrlib/controldir.py controldir.py-20100802102926-hvtvh0uae5epuibp-1
bzrlib/remote.py remote.py-20060720103555-yeeg2x51vn0rbtdp-1
bzrlib/tests/blackbox/test_branch.py test_branch.py-20060524161337-noms9gmcwqqrfi8y-1
bzrlib/tests/test_branch.py test_branch.py-20060116013032-97819aa07b8ab3b5
bzrlib/tests/test_bzrdir.py test_bzrdir.py-20060131065654-deba40eef51cf220
bzrlib/tests/test_foreign.py test_foreign.py-20081125004048-ywb901edgp9lluxo-1
doc/en/release-notes/bzr-2.3.txt NEWS-20050323055033-4e00b5db738777ff
doc/en/whats-new/whats-new-in-2.3.txt whatsnewin2.3.txt-20100818072501-x2h25r7jbnknvy30-1
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py 2010-11-18 00:22:24 +0000
+++ b/bzrlib/branch.py 2010-12-14 04:20:17 +0000
@@ -105,6 +105,13 @@
def _activate_fallback_location(self, url):
"""Activate the branch/repository from url as a fallback repository."""
+ for existing_fallback_repo in self.repository._fallback_repositories:
+ if existing_fallback_repo.user_url == url:
+ # This fallback is already configured. This probably only
+ # happens because BzrDir.sprout is a horrible mess. To avoid
+ # confusing _unstack we don't add this a second time.
+ mutter('duplicate activation of fallback %r on %r', url, self)
+ return
repo = self._get_fallback_repository(url)
if repo.has_same_location(self.repository):
raise errors.UnstackableLocationError(self.user_url, url)
@@ -809,7 +816,8 @@
old_repository = self.repository
if len(old_repository._fallback_repositories) != 1:
raise AssertionError("can't cope with fallback repositories "
- "of %r" % (self.repository,))
+ "of %r (fallbacks: %r)" % (old_repository,
+ old_repository._fallback_repositories))
# Open the new repository object.
# Repositories don't offer an interface to remove fallback
# repositories today; take the conceptually simpler option and just
@@ -1266,7 +1274,8 @@
return result
@needs_read_lock
- def sprout(self, to_bzrdir, revision_id=None, repository_policy=None):
+ def sprout(self, to_bzrdir, revision_id=None, repository_policy=None,
+ repository=None):
"""Create a new line of development from the branch, into to_bzrdir.
to_bzrdir controls the branch format.
@@ -1277,7 +1286,7 @@
if (repository_policy is not None and
repository_policy.requires_stacking()):
to_bzrdir._format.require_stacking(_skip_repo=True)
- result = to_bzrdir.create_branch()
+ result = to_bzrdir.create_branch(repository=repository)
result.lock_write()
try:
if repository_policy is not None:
@@ -1634,7 +1643,8 @@
hook(params)
def _initialize_helper(self, a_bzrdir, utf8_files, name=None,
- lock_type='metadir', set_format=True):
+ repository=None, lock_type='metadir',
+ set_format=True):
"""Initialize a branch in a bzrdir, with specified files
:param a_bzrdir: The bzrdir to initialize the branch in
@@ -1674,11 +1684,12 @@
finally:
if lock_taken:
control_files.unlock()
- branch = self.open(a_bzrdir, name, _found=True)
+ branch = self.open(a_bzrdir, name, _found=True,
+ found_repository=repository)
self._run_post_branch_init_hooks(a_bzrdir, name, branch)
return branch
- def initialize(self, a_bzrdir, name=None):
+ def initialize(self, a_bzrdir, name=None, repository=None):
"""Create a branch of this format in a_bzrdir.
:param name: Name of the colocated branch to create.
@@ -1718,7 +1729,8 @@
"""
raise NotImplementedError(self.network_name)
- def open(self, a_bzrdir, name=None, _found=False, ignore_fallbacks=False):
+ def open(self, a_bzrdir, name=None, _found=False, ignore_fallbacks=False,
+ found_repository=None):
"""Return the branch object for a_bzrdir
:param a_bzrdir: A BzrDir that contains a branch.
@@ -2018,8 +2030,11 @@
"""See BranchFormat.get_format_description()."""
return "Branch format 4"
- def initialize(self, a_bzrdir, name=None):
+ def initialize(self, a_bzrdir, name=None, repository=None):
"""Create a branch of this format in a_bzrdir."""
+ if repository is not None:
+ raise NotImplementedError(
+ "initialize(repository=<not None>) on %r" % (self,))
utf8_files = [('revision-history', ''),
('branch-name', ''),
]
@@ -2034,16 +2049,19 @@
"""The network name for this format is the control dirs disk label."""
return self._matchingbzrdir.get_format_string()
- def open(self, a_bzrdir, name=None, _found=False, ignore_fallbacks=False):
+ def open(self, a_bzrdir, name=None, _found=False, ignore_fallbacks=False,
+ found_repository=None):
"""See BranchFormat.open()."""
if not _found:
# we are being called directly and must probe.
raise NotImplementedError
+ if found_repository is None:
+ found_repository = a_bzrdir.open_repository()
return BzrBranch(_format=self,
_control_files=a_bzrdir._control_files,
a_bzrdir=a_bzrdir,
name=name,
- _repository=a_bzrdir.open_repository())
+ _repository=found_repository)
def __str__(self):
return "Bazaar-NG branch format 4"
@@ -2063,7 +2081,8 @@
"""
return self.get_format_string()
- def open(self, a_bzrdir, name=None, _found=False, ignore_fallbacks=False):
+ def open(self, a_bzrdir, name=None, _found=False, ignore_fallbacks=False,
+ found_repository=None):
"""See BranchFormat.open()."""
if not _found:
format = BranchFormat.find_format(a_bzrdir, name=name)
@@ -2074,11 +2093,13 @@
try:
control_files = lockable_files.LockableFiles(transport, 'lock',
lockdir.LockDir)
+ if found_repository is None:
+ found_repository = a_bzrdir.find_repository()
return self._branch_class()(_format=self,
_control_files=control_files,
name=name,
a_bzrdir=a_bzrdir,
- _repository=a_bzrdir.find_repository(),
+ _repository=found_repository,
ignore_fallbacks=ignore_fallbacks)
except errors.NoSuchFile:
raise errors.NotBranchError(path=transport.base, bzrdir=a_bzrdir)
@@ -2116,12 +2137,12 @@
"""See BranchFormat.get_format_description()."""
return "Branch format 5"
- def initialize(self, a_bzrdir, name=None):
+ def initialize(self, a_bzrdir, name=None, repository=None):
"""Create a branch of this format in a_bzrdir."""
utf8_files = [('revision-history', ''),
('branch-name', ''),
]
- return self._initialize_helper(a_bzrdir, utf8_files, name)
+ return self._initialize_helper(a_bzrdir, utf8_files, name, repository)
def supports_tags(self):
return False
@@ -2149,13 +2170,13 @@
"""See BranchFormat.get_format_description()."""
return "Branch format 6"
- def initialize(self, a_bzrdir, name=None):
+ def initialize(self, a_bzrdir, name=None, repository=None):
"""Create a branch of this format in a_bzrdir."""
utf8_files = [('last-revision', '0 null:\n'),
('branch.conf', ''),
('tags', ''),
]
- return self._initialize_helper(a_bzrdir, utf8_files, name)
+ return self._initialize_helper(a_bzrdir, utf8_files, name, repository)
def make_tags(self, branch):
"""See bzrlib.branch.BranchFormat.make_tags()."""
@@ -2179,14 +2200,14 @@
"""See BranchFormat.get_format_description()."""
return "Branch format 8"
- def initialize(self, a_bzrdir, name=None):
+ def initialize(self, a_bzrdir, name=None, repository=None):
"""Create a branch of this format in a_bzrdir."""
utf8_files = [('last-revision', '0 null:\n'),
('branch.conf', ''),
('tags', ''),
('references', '')
]
- return self._initialize_helper(a_bzrdir, utf8_files, name)
+ return self._initialize_helper(a_bzrdir, utf8_files, name, repository)
def __init__(self):
super(BzrBranchFormat8, self).__init__()
@@ -2215,13 +2236,13 @@
This format was introduced in bzr 1.6.
"""
- def initialize(self, a_bzrdir, name=None):
+ def initialize(self, a_bzrdir, name=None, repository=None):
"""Create a branch of this format in a_bzrdir."""
utf8_files = [('last-revision', '0 null:\n'),
('branch.conf', ''),
('tags', ''),
]
- return self._initialize_helper(a_bzrdir, utf8_files, name)
+ return self._initialize_helper(a_bzrdir, utf8_files, name, repository)
def _branch_class(self):
return BzrBranch7
@@ -2269,7 +2290,8 @@
transport = a_bzrdir.get_branch_transport(None, name=name)
location = transport.put_bytes('location', to_branch.base)
- def initialize(self, a_bzrdir, name=None, target_branch=None):
+ def initialize(self, a_bzrdir, name=None, target_branch=None,
+ repository=None):
"""Create a branch of this format in a_bzrdir."""
if target_branch is None:
# this format does not implement branch itself, thus the implicit
@@ -2303,7 +2325,8 @@
return clone
def open(self, a_bzrdir, name=None, _found=False, location=None,
- possible_transports=None, ignore_fallbacks=False):
+ possible_transports=None, ignore_fallbacks=False,
+ found_repository=None):
"""Return the branch that the branch reference in a_bzrdir points at.
:param a_bzrdir: A BzrDir that contains a branch.
=== modified file 'bzrlib/bzrdir.py'
--- a/bzrlib/bzrdir.py 2010-12-07 09:06:39 +0000
+++ b/bzrlib/bzrdir.py 2010-12-14 23:14:44 +0000
@@ -1027,8 +1027,11 @@
tree.clone(result)
return result
- def create_branch(self, name=None):
+ def create_branch(self, name=None, repository=None):
"""See BzrDir.create_branch."""
+ if repository is not None:
+ raise NotImplementedError(
+ "create_branch(repository=<not None>) on %r" % (self,))
return self._format.get_branch_format().initialize(self, name=name)
def destroy_branch(self, name=None):
@@ -1264,9 +1267,10 @@
"""See BzrDir.can_convert_format()."""
return True
- def create_branch(self, name=None):
+ def create_branch(self, name=None, repository=None):
"""See BzrDir.create_branch."""
- return self._format.get_branch_format().initialize(self, name=name)
+ return self._format.get_branch_format().initialize(self, name=name,
+ repository=repository)
def destroy_branch(self, name=None):
"""See BzrDir.create_branch."""
=== modified file 'bzrlib/controldir.py'
--- a/bzrlib/controldir.py 2010-09-10 09:46:15 +0000
+++ b/bzrlib/controldir.py 2010-12-14 00:41:05 +0000
@@ -27,11 +27,10 @@
import textwrap
from bzrlib import (
+ cleanup,
errors,
graph,
- registry,
revision as _mod_revision,
- symbol_versioning,
urlutils,
)
from bzrlib.push import (
@@ -47,6 +46,8 @@
""")
+from bzrlib import registry
+
class ControlComponent(object):
"""Abstract base class for control directory components.
@@ -143,7 +144,7 @@
"""Destroy the repository in this ControlDir."""
raise NotImplementedError(self.destroy_repository)
- def create_branch(self, name=None):
+ def create_branch(self, name=None, repository=None):
"""Create a branch in this ControlDir.
:param name: Name of the colocated branch to create, None for
@@ -364,6 +365,19 @@
:param create_tree_if_local: If true, a working-tree will be created
when working locally.
"""
+ operation = cleanup.OperationWithCleanups(self._sprout)
+ return operation.run(url, revision_id=revision_id,
+ force_new_repo=force_new_repo, recurse=recurse,
+ possible_transports=possible_transports,
+ accelerator_tree=accelerator_tree, hardlink=hardlink,
+ stacked=stacked, source_branch=source_branch,
+ create_tree_if_local=create_tree_if_local)
+
+ def _sprout(self, op, url, revision_id=None, force_new_repo=False,
+ recurse='down', possible_transports=None,
+ accelerator_tree=None, hardlink=False, stacked=False,
+ source_branch=None, create_tree_if_local=True):
+ add_cleanup = op.add_cleanup
target_transport = get_transport(url, possible_transports)
target_transport.ensure_base()
cloning_format = self.cloning_metadir(stacked)
@@ -373,6 +387,7 @@
# even if the origin was stacked
stacked_branch_url = None
if source_branch is not None:
+ add_cleanup(source_branch.lock_read().unlock)
if stacked:
stacked_branch_url = self.root_transport.base
source_repository = source_branch.repository
@@ -388,9 +403,14 @@
source_repository = self.open_repository()
except errors.NoRepositoryPresent:
source_repository = None
+ else:
+ add_cleanup(source_repository.lock_read().unlock)
+ else:
+ add_cleanup(source_branch.lock_read().unlock)
repository_policy = result.determine_repository_policy(
force_new_repo, stacked_branch_url, require_stacking=stacked)
result_repo, is_new_repo = repository_policy.acquire_repository()
+ add_cleanup(result_repo.lock_write().unlock)
is_stacked = stacked or (len(result_repo._fallback_repositories) != 0)
if is_new_repo and revision_id is not None and not is_stacked:
fetch_spec = graph.PendingAncestryResult(
@@ -412,7 +432,8 @@
result_branch = result.create_branch()
else:
result_branch = source_branch.sprout(result,
- revision_id=revision_id, repository_policy=repository_policy)
+ revision_id=revision_id, repository_policy=repository_policy,
+ repository=result_repo)
mutter("created new branch %r" % (result_branch,))
# Create/update the result working tree
@@ -420,7 +441,7 @@
isinstance(target_transport, local.LocalTransport) and
(result_repo is None or result_repo.make_working_trees())):
wt = result.create_workingtree(accelerator_tree=accelerator_tree,
- hardlink=hardlink)
+ hardlink=hardlink, from_branch=result_branch)
wt.lock_write()
try:
if wt.path2id('') is None:
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py 2010-12-02 10:41:05 +0000
+++ b/bzrlib/remote.py 2010-12-14 23:14:44 +0000
@@ -33,6 +33,7 @@
revision as _mod_revision,
static_tuple,
symbol_versioning,
+ urlutils,
)
from bzrlib.branch import BranchReferenceFormat, BranchWriteLockResult
from bzrlib.bzrdir import BzrDir, RemoteBzrDirFormat
@@ -246,14 +247,17 @@
self._ensure_real()
self._real_bzrdir.destroy_repository()
- def create_branch(self, name=None):
+ def create_branch(self, name=None, repository=None):
# as per meta1 formats - just delegate to the format object which may
# be parameterised.
real_branch = self._format.get_branch_format().initialize(self,
- name=name)
+ name=name, repository=repository)
if not isinstance(real_branch, RemoteBranch):
- result = RemoteBranch(self, self.find_repository(), real_branch,
- name=name)
+ if not isinstance(repository, RemoteRepository):
+ raise AssertionError(
+ 'need a RemoteRepository to use with RemoteBranch, got %r'
+ % (repository,))
+ result = RemoteBranch(self, repository, real_branch, name=name)
else:
result = real_branch
# BzrDir.clone_on_transport() uses the result of create_branch but does
@@ -2093,7 +2097,7 @@
name=name)
return result
- def initialize(self, a_bzrdir, name=None):
+ def initialize(self, a_bzrdir, name=None, repository=None):
# 1) get the network name to use.
if self._custom_format:
network_name = self._custom_format.network_name()
@@ -2127,13 +2131,25 @@
# Turn the response into a RemoteRepository object.
format = RemoteBranchFormat(network_name=response[1])
repo_format = response_tuple_to_repo_format(response[3:])
- if response[2] == '':
- repo_bzrdir = a_bzrdir
+ repo_path = response[2]
+ if repository is not None:
+ remote_repo_url = urlutils.join(medium.base, repo_path)
+ url_diff = urlutils.relative_url(repository.user_url,
+ remote_repo_url)
+ if url_diff != '.':
+ raise AssertionError(
+ 'repository.user_url %r does not match URL from server '
+ 'response (%r + %r)'
+ % (repository.user_url, medium.base, repo_path))
+ remote_repo = repository
else:
- repo_bzrdir = RemoteBzrDir(
- a_bzrdir.root_transport.clone(response[2]), a_bzrdir._format,
- a_bzrdir._client)
- remote_repo = RemoteRepository(repo_bzrdir, repo_format)
+ if repo_path == '':
+ repo_bzrdir = a_bzrdir
+ else:
+ repo_bzrdir = RemoteBzrDir(
+ a_bzrdir.root_transport.clone(repo_path), a_bzrdir._format,
+ a_bzrdir._client)
+ remote_repo = RemoteRepository(repo_bzrdir, repo_format)
remote_branch = RemoteBranch(a_bzrdir, remote_repo,
format=format, setup_stacking=False, name=name)
# XXX: We know this is a new branch, so it must have revno 0, revid
=== modified file 'bzrlib/tests/blackbox/test_branch.py'
--- a/bzrlib/tests/blackbox/test_branch.py 2010-12-02 10:41:05 +0000
+++ b/bzrlib/tests/blackbox/test_branch.py 2010-12-14 23:14:44 +0000
@@ -414,7 +414,7 @@
# being too low. If rpc_count increases, more network roundtrips have
# become necessary for this use case. Please do not adjust this number
# upwards without agreement from bzr's network support maintainers.
- self.assertLength(37, self.hpss_calls)
+ self.assertLength(36, self.hpss_calls)
def test_branch_from_trivial_branch_streaming_acceptance(self):
self.setup_smart_server_with_call_log()
=== modified file 'bzrlib/tests/test_branch.py'
--- a/bzrlib/tests/test_branch.py 2010-12-07 09:06:39 +0000
+++ b/bzrlib/tests/test_branch.py 2010-12-14 23:14:44 +0000
@@ -113,7 +113,7 @@
"""See BzrBranchFormat.get_format_string()."""
return "Sample branch format."
- def initialize(self, a_bzrdir, name=None):
+ def initialize(self, a_bzrdir, name=None, repository=None):
"""Format 4 branches cannot be created."""
t = a_bzrdir.get_branch_transport(self, name=name)
t.put_bytes('format', self.get_format_string())
=== modified file 'bzrlib/tests/test_bzrdir.py'
--- a/bzrlib/tests/test_bzrdir.py 2010-10-15 14:21:03 +0000
+++ b/bzrlib/tests/test_bzrdir.py 2010-12-15 00:38:15 +0000
@@ -29,6 +29,7 @@
controldir,
errors,
help_topics,
+ lock,
repository,
osutils,
remote,
@@ -1349,6 +1350,12 @@
def set_parent(self, parent):
self._parent = parent
+ def lock_read(self):
+ return lock.LogicalLockResult(self.unlock)
+
+ def unlock(self):
+ return
+
class TestBzrDirSprout(TestCaseWithMemoryTransport):
=== modified file 'bzrlib/tests/test_foreign.py'
--- a/bzrlib/tests/test_foreign.py 2010-08-02 18:36:31 +0000
+++ b/bzrlib/tests/test_foreign.py 2010-11-30 02:32:37 +0000
@@ -173,17 +173,19 @@
super(DummyForeignVcsBranchFormat, self).__init__()
self._matchingbzrdir = DummyForeignVcsDirFormat()
- def open(self, a_bzrdir, name=None, _found=False):
+ def open(self, a_bzrdir, name=None, _found=False, found_repository=None):
if not _found:
raise NotImplementedError
try:
transport = a_bzrdir.get_branch_transport(None, name=name)
control_files = lockable_files.LockableFiles(transport, 'lock',
lockdir.LockDir)
+ if found_repository is None:
+ found_repository = a_bzrdir.find_repository()
return DummyForeignVcsBranch(_format=self,
_control_files=control_files,
a_bzrdir=a_bzrdir,
- _repository=a_bzrdir.find_repository())
+ _repository=found_repository)
except errors.NoSuchFile:
raise errors.NotBranchError(path=transport.base)
=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- a/doc/en/release-notes/bzr-2.3.txt 2010-12-14 23:00:51 +0000
+++ b/doc/en/release-notes/bzr-2.3.txt 2010-12-14 23:32:28 +0000
@@ -31,6 +31,10 @@
missing inventories. This removes at least one network roundtrip when
pushing to a stacked branch. (Andrew Bennetts)
+* ``ControlDir.sprout`` no longer opens the target repository more than
+ once. This avoids some unnecessary IO, and removes a network roundtrip
+ when doing ``bzr branch`` to a smart server URL. (Andrew Bennetts)
+
* ``bzr resolve`` now accepts ``--take-this`` and ``--take-other`` actions
for text conflicts. This *replace* the whole file with the content
designated by the action. This will *ignore* all differences that would
@@ -53,6 +57,14 @@
.. Changes that may require updates in plugins or other code that uses
bzrlib.
+* ``Branch.sprout``, ``BranchFormat.initalize`` and
+ ``ControlDir.create_branch`` now take an optional ``repository`` keyword
+ argument, and ``BranchFormat.open`` now takes an optional
+ ``found_repository`` keyword argument. These provide the repository
+ object for new branch object to use (for cases when the caller has
+ already opened that repository). Implementations of these APIs will
+ need to be updated to accept these arguments. (Andrew Bennetts)
+
Internals
*********
=== modified file 'doc/en/whats-new/whats-new-in-2.3.txt'
--- a/doc/en/whats-new/whats-new-in-2.3.txt 2010-12-14 09:33:53 +0000
+++ b/doc/en/whats-new/whats-new-in-2.3.txt 2010-12-14 23:32:28 +0000
@@ -69,8 +69,9 @@
* ``bzr send`` uses less memory.
(John Arbash Meinel, #614576)
-* Fetches involving stacked branches and branches with tags are now faster.
- Some redundant network reads were removed. (Andrew Bennetts)
+* Fetches involving stacked branches and branches with tags now do slightly less
+ I/O, and so does branching from an existing branch. This also improves the
+ network performance of these operations. (Andrew Bennetts)
* Inventory entries now consume less memory (on 32-bit Ubuntu file entries
have dropped from 68 bytes to 40, and directory entries from 120 bytes
More information about the bazaar-commits
mailing list