Rev 4146: Explicitly prevent fetching while the target repository is in a write group. in http://people.ubuntu.com/~robertc/baz2.0/pending/repository.refresh_data
Robert Collins
robertc at robertcollins.net
Mon Mar 16 03:34:10 GMT 2009
At http://people.ubuntu.com/~robertc/baz2.0/pending/repository.refresh_data
------------------------------------------------------------
revno: 4146
revision-id: robertc at robertcollins.net-20090316033405-wpq3do6shludp1bh
parent: pqm at pqm.ubuntu.com-20090316024046-58qc87pfdgu2ugok
committer: Robert Collins <robertc at robertcollins.net>
branch nick: repository.refresh_data
timestamp: Mon 2009-03-16 14:34:05 +1100
message:
Explicitly prevent fetching while the target repository is in a write group.
=== modified file 'NEWS'
--- a/NEWS 2009-03-16 01:56:18 +0000
+++ b/NEWS 2009-03-16 03:34:05 +0000
@@ -43,7 +43,12 @@
* Fixed incorrect "Source format does not support stacking" warning
when pushing to a smart server. (Andrew Bennetts, #334114)
-
+
+ * It is no longer possible to fetch between repositories while the
+ target repository is in a write group. This prevents race conditions
+ that prevent the use of RPC's to perform fetch, and thus allows
+ optimising more operations. (Robert Collins, Andrew Bennetts)
+
* The GNU Changelog formatter is slightly improved in the case where
the delta is empty, and now correctly claims not to support tags.
(Andrea Bolognani)
=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py 2009-03-12 08:50:04 +0000
+++ b/bzrlib/remote.py 2009-03-16 03:34:05 +0000
@@ -643,6 +643,14 @@
"""Ensure that there is a _real_repository set.
Used before calls to self._real_repository.
+
+ Note that _ensure_real causes many roundtrips to the server which are
+ not desirable, and prevents the use of smart one-roundtrip RPC's to
+ perform complex operations (such as accessing parent data, streaming
+ revisions etc). Adding calls to _ensure_real should only be done when
+ bringing up new functionality, adding fallbacks for smart methods that
+ require a fallback path, and never to replace an existing smart method
+ invocation. If in doubt chat to the bzr network team.
"""
if self._real_repository is None:
self.bzrdir._ensure_real()
@@ -1108,24 +1116,27 @@
def fetch(self, source, revision_id=None, pb=None, find_ghosts=False,
fetch_spec=None):
+ # No base implementation to use as RemoteRepository is not a subclass
+ # of Repository; so this is a copy of Repository.fetch().
if fetch_spec is not None and revision_id is not None:
raise AssertionError(
"fetch_spec and revision_id are mutually exclusive.")
- # Not delegated to _real_repository so that InterRepository.get has a
- # chance to find an InterRepository specialised for RemoteRepository.
+ if self.is_in_write_group():
+ raise errors.BzrError("May not fetch while in a write group.")
+ # fast path same-url fetch operations
if self.has_same_location(source) and fetch_spec is None:
# check that last_revision is in 'from' and then return a
# no-operation.
if (revision_id is not None and
- not revision.is_null(revision_id)):
+ not _mod_revision.is_null(revision_id)):
self.get_revision(revision_id)
return 0, []
- inter = repository.InterRepository.get(source, self)
- try:
- return inter.fetch(revision_id=revision_id, pb=pb,
- find_ghosts=find_ghosts, fetch_spec=fetch_spec)
- except NotImplementedError:
- raise errors.IncompatibleRepositories(source, self)
+ # if there is no specific appropriate InterRepository, this will get
+ # the InterRepository base class, which raises an
+ # IncompatibleRepositories when asked to fetch.
+ inter = InterRepository.get(source, self)
+ return inter.fetch(revision_id=revision_id, pb=pb,
+ find_ghosts=find_ghosts, fetch_spec=fetch_spec)
def create_bundle(self, target, base, fileobj, format=None):
self._ensure_real()
=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py 2009-03-13 05:05:50 +0000
+++ b/bzrlib/repository.py 2009-03-16 03:34:05 +0000
@@ -1103,6 +1103,10 @@
If revision_id is None and fetch_spec is None, then all content is
copied.
+ fetch() may not be used when the repository is in a write group -
+ either finish the current write group before using fetch, or use
+ fetch before starting the write group.
+
:param find_ghosts: Find and copy revisions in the source that are
ghosts in the target (and not reachable directly by walking out to
the first-present revision in target from revision_id).
@@ -1117,6 +1121,8 @@
if fetch_spec is not None and revision_id is not None:
raise AssertionError(
"fetch_spec and revision_id are mutually exclusive.")
+ if self.is_in_write_group():
+ raise errors.BzrError("May not fetch while in a write group.")
# fast path same-url fetch operations
if self.has_same_location(source) and fetch_spec is None:
# check that last_revision is in 'from' and then return a
=== modified file 'bzrlib/tests/per_repository/test_fetch.py'
--- a/bzrlib/tests/per_repository/test_fetch.py 2009-03-11 07:55:14 +0000
+++ b/bzrlib/tests/per_repository/test_fetch.py 2009-03-16 03:34:05 +0000
@@ -45,6 +45,17 @@
repo.fetch(tree_a.branch.repository,
revision_id=None)
+ def test_fetch_fails_in_write_group(self):
+ # fetch() manages a write group itself, fetching within one isn't safe.
+ repo = self.make_repository('a')
+ repo.lock_write()
+ self.addCleanup(repo.unlock)
+ repo.start_write_group()
+ self.addCleanup(repo.abort_write_group)
+ # Don't need a specific class - not expecting flow control based on
+ # this.
+ self.assertRaises(errors.BzrError, repo.fetch, repo)
+
def test_fetch_to_knit3(self):
# create a repository of the sort we are testing.
tree_a = self.make_branch_and_tree('a')
More information about the bazaar-commits
mailing list