Rev 4325: When adding rich root data follow the standard revision graph rules, so it does not create 'inconstent parents'. in http://people.ubuntu.com/~robertc/baz2.0/pending/bug-368921
Robert Collins
robertc at robertcollins.net
Tue May 5 04:14:12 BST 2009
At http://people.ubuntu.com/~robertc/baz2.0/pending/bug-368921
------------------------------------------------------------
revno: 4325
revision-id: robertc at robertcollins.net-20090505031358-m42cf0a8ad1gez0s
parent: pqm at pqm.ubuntu.com-20090502005030-s11zizqqgrf3s3k2
committer: Robert Collins <robertc at robertcollins.net>
branch nick: bug-368921
timestamp: Tue 2009-05-05 13:13:58 +1000
message:
When adding rich root data follow the standard revision graph rules, so it does not create 'inconstent parents'.
=== modified file 'NEWS'
--- a/NEWS 2009-04-30 22:40:42 +0000
+++ b/NEWS 2009-05-05 03:13:58 +0000
@@ -64,6 +64,10 @@
* ssh usernames (defined in ~/.ssh/config) are honoured for bzr+ssh connections.
(Vincent Ladeuil, #367726)
+* Upgrading to, or fetching into a 'rich-root' format will now correctly
+ set the Aroot data the same way that reconcile does.
+ (Robert Collins, #368921)
+
Documentation
*************
=== modified file 'bzrlib/branchbuilder.py'
--- a/bzrlib/branchbuilder.py 2009-04-18 20:41:29 +0000
+++ b/bzrlib/branchbuilder.py 2009-05-05 03:13:58 +0000
@@ -111,17 +111,23 @@
reporter=reporter,
**kwargs)
- def _move_branch_pointer(self, new_revision_id):
+ def _move_branch_pointer(self, new_revision_id,
+ allow_leftmost_as_ghost=False):
"""Point self._branch to a different revision id."""
self._branch.lock_write()
try:
# We don't seem to have a simple set_last_revision(), so we
# implement it here.
cur_revno, cur_revision_id = self._branch.last_revision_info()
- g = self._branch.repository.get_graph()
- new_revno = g.find_distance_to_null(new_revision_id,
- [(cur_revision_id, cur_revno)])
- self._branch.set_last_revision_info(new_revno, new_revision_id)
+ try:
+ g = self._branch.repository.get_graph()
+ new_revno = g.find_distance_to_null(new_revision_id,
+ [(cur_revision_id, cur_revno)])
+ self._branch.set_last_revision_info(new_revno, new_revision_id)
+ except errors.GhostRevisionsHaveNoRevno:
+ if not allow_leftmost_as_ghost:
+ raise
+ new_revno = 1
finally:
self._branch.unlock()
if self._tree is not None:
@@ -155,7 +161,7 @@
self._tree = None
def build_snapshot(self, revision_id, parent_ids, actions,
- message=None, timestamp=None):
+ message=None, timestamp=None, allow_leftmost_as_ghost=False):
"""Build a commit, shaped in a specific way.
:param revision_id: The handle for the new commit, can be None
@@ -170,12 +176,15 @@
commit message will be written.
:param timestamp: If non-None, set the timestamp of the commit to this
value.
+ :param allow_leftmost_as_ghost: True if the leftmost parent should be
+ permitted to be a ghost.
:return: The revision_id of the new commit
"""
if parent_ids is not None:
base_id = parent_ids[0]
if base_id != self._branch.last_revision():
- self._move_branch_pointer(base_id)
+ self._move_branch_pointer(base_id,
+ allow_leftmost_as_ghost=allow_leftmost_as_ghost)
if self._tree is not None:
tree = self._tree
@@ -184,7 +193,8 @@
tree.lock_write()
try:
if parent_ids is not None:
- tree.set_parent_ids(parent_ids)
+ tree.set_parent_ids(parent_ids,
+ allow_leftmost_as_ghost=allow_leftmost_as_ghost)
# Unfortunately, MemoryTree.add(directory) just creates an
# inventory entry. And the only public function to create a
# directory is MemoryTree.mkdir() which creates the directory, but
=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py 2009-05-01 07:59:02 +0000
+++ b/bzrlib/repository.py 2009-05-05 03:13:58 +0000
@@ -3557,7 +3557,7 @@
"""Get the parent keys for a given root id."""
root_id, rev_id = root_key
# Include direct parents of the revision, but only if they used
- # the same root_id.
+ # the same root_id and are heads.
parent_keys = []
for parent_id in parent_map[rev_id]:
if parent_id == _mod_revision.NULL_REVISION:
@@ -3577,9 +3577,32 @@
self._revision_id_to_root_id[parent_id] = None
else:
parent_root_id = self._revision_id_to_root_id[parent_id]
- if root_id == parent_root_id or parent_root_id is None:
- parent_keys.append((root_id, parent_id))
- return tuple(parent_keys)
+ if root_id == parent_root_id:
+ # With stacking we _might_ want to refer to a non-local
+ # revision, but this code path only applies when we have the
+ # full content available, so ghosts really are ghosts, not just
+ # the edge of local data.
+ parent_keys.append((parent_id,))
+ else:
+ # root_id may be in the parent anyway.
+ try:
+ tree = self.source.revision_tree(parent_id)
+ except errors.NoSuchRevision:
+ # ghost, can't refer to it.
+ pass
+ else:
+ try:
+ parent_keys.append((tree.inventory[root_id].revision,))
+ except errors.NoSuchId:
+ # not in the tree
+ pass
+ g = graph.Graph(self.source.revisions)
+ heads = g.heads(parent_keys)
+ selected_keys = []
+ for key in parent_keys:
+ if key in heads and key not in selected_keys:
+ selected_keys.append(key)
+ return tuple([(root_id,)+ key for key in selected_keys])
def _new_root_data_stream(self, root_keys_to_create, parent_map):
for root_key in root_keys_to_create:
=== modified file 'bzrlib/tests/per_repository/test_fetch.py'
--- a/bzrlib/tests/per_repository/test_fetch.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/per_repository/test_fetch.py 2009-05-05 03:13:58 +0000
@@ -20,8 +20,10 @@
bzrdir,
errors,
gpg,
+ remote,
repository,
)
+from bzrlib.inventory import ROOT_ID
from bzrlib.tests import TestSkipped
from bzrlib.tests.per_repository import TestCaseWithRepository
from bzrlib.transport import get_transport
@@ -99,6 +101,137 @@
rev2_tree = knit3_repo.revision_tree('rev2')
self.assertEqual('rev1', rev2_tree.inventory.root.revision)
+ def do_test_fetch_to_rich_root_sets_parents_correctly(self, result,
+ snapshots, root_id=ROOT_ID, allow_lefthand_ghost=False):
+ """Assert that result is the parents of 'tip' after fetching snapshots.
+
+ This helper constructs a 1.9 format source, and a test-format target
+ and fetches the result of building snapshots in the source, then
+ asserts that the parents of tip are result.
+
+ :param result: A parents list for the inventories.get_parent_map call.
+ :param snapshots: An iterable of snapshot parameters for
+ BranchBuilder.build_snapshot.
+ '"""
+ # This overlaps slightly with the tests for commit builder about graph
+ # consistency.
+ # Cases:
+ repo = self.make_repository('target')
+ if isinstance(repo, remote.RemoteRepository):
+ remote_format = True
+ if not repo._format.rich_root_data and not remote_format:
+ return # not relevant
+ builder = self.make_branch_builder('source', format='1.9')
+ builder.start_series()
+ for revision_id, parent_ids, actions in snapshots:
+ builder.build_snapshot(revision_id, parent_ids, actions,
+ allow_leftmost_as_ghost=allow_lefthand_ghost)
+ builder.finish_series()
+ source = builder.get_branch()
+ if remote_format and not repo._format.rich_root_data:
+ # use a manual rich root format to ensure the code path is tested.
+ repo = self.make_repository('remote-target',
+ format='1.9-rich-root')
+ repo.lock_write()
+ self.addCleanup(repo.unlock)
+ repo.fetch(source.repository)
+ self.assertEqual(result,
+ repo.texts.get_parent_map([(root_id, 'tip')])[(root_id, 'tip')])
+
+ def test_fetch_to_rich_root_set_parent_no_parents(self):
+ # No parents rev -> No parents
+ self.do_test_fetch_to_rich_root_sets_parents_correctly((),
+ [('tip', None, [('add', ('', ROOT_ID, 'directory', ''))]),
+ ])
+
+ def test_fetch_to_rich_root_set_parent_1_parent(self):
+ # 1 parent rev -> 1 parent
+ self.do_test_fetch_to_rich_root_sets_parents_correctly(
+ ((ROOT_ID, 'base'),),
+ [('base', None, [('add', ('', ROOT_ID, 'directory', ''))]),
+ ('tip', None, []),
+ ])
+
+ def test_fetch_to_rich_root_set_parent_1_ghost_parent(self):
+ # 1 ghost parent -> No parents
+ self.do_test_fetch_to_rich_root_sets_parents_correctly((),
+ [('tip', ['ghost'], [('add', ('', ROOT_ID, 'directory', ''))]),
+ ], allow_lefthand_ghost=True)
+
+ def test_fetch_to_rich_root_set_parent_2_head_parents(self):
+ # 2 parents both heads -> 2 parents
+ self.do_test_fetch_to_rich_root_sets_parents_correctly(
+ ((ROOT_ID, 'left'), (ROOT_ID, 'right')),
+ [('base', None, [('add', ('', ROOT_ID, 'directory', ''))]),
+ ('left', None, []),
+ ('right', ['base'], []),
+ ('tip', ['left', 'right'], []),
+ ])
+
+ def test_fetch_to_rich_root_set_parent_2_parents_1_head(self):
+ # 2 parents one head -> 1 parent
+ self.do_test_fetch_to_rich_root_sets_parents_correctly(
+ ((ROOT_ID, 'right'),),
+ [('left', None, [('add', ('', ROOT_ID, 'directory', ''))]),
+ ('right', None, []),
+ ('tip', ['left', 'right'], []),
+ ])
+
+ def test_fetch_to_rich_root_set_parent_1_parent_different_id_gone(self):
+ # 1 parent different fileid, ours missing -> no parents
+ self.do_test_fetch_to_rich_root_sets_parents_correctly(
+ (),
+ [('base', None, [('add', ('', ROOT_ID, 'directory', ''))]),
+ ('tip', None, [('unversion', ROOT_ID),
+ ('add', ('', 'my-root', 'directory', '')),
+ ]),
+ ], root_id='my-root')
+
+ def test_fetch_to_rich_root_set_parent_1_parent_different_id_moved(self):
+ # 1 parent different fileid, ours moved -> 1 parent
+ # (and that parent honours the changing revid of the other location)
+ self.do_test_fetch_to_rich_root_sets_parents_correctly(
+ (('my-root', 'origin'),),
+ [('origin', None, [('add', ('', ROOT_ID, 'directory', '')),
+ ('add', ('child', 'my-root', 'directory', ''))]),
+ ('base', None, []),
+ ('tip', None, [('unversion', 'my-root'),
+ ('unversion', ROOT_ID),
+ ('add', ('', 'my-root', 'directory', '')),
+ ]),
+ ], root_id='my-root')
+
+ def test_fetch_to_rich_root_set_parent_2_parent_1_different_id_gone(self):
+ # 2 parents, 1 different fileid, our second missing -> 1 parent
+ self.do_test_fetch_to_rich_root_sets_parents_correctly(
+ (('my-root', 'right'),),
+ [('base', None, [('add', ('', ROOT_ID, 'directory', ''))]),
+ ('right', None, [('unversion', ROOT_ID),
+ ('add', ('', 'my-root', 'directory', ''))]),
+ ('tip', ['base', 'right'], [('unversion', ROOT_ID),
+ ('add', ('', 'my-root', 'directory', '')),
+ ]),
+ ], root_id='my-root')
+
+ def test_fetch_to_rich_root_set_parent_2_parent_2_different_id_moved(self):
+ # 2 parents, 1 different fileid, our second moved -> 2 parent
+ # (and that parent honours the changing revid of the other location)
+ self.do_test_fetch_to_rich_root_sets_parents_correctly(
+ (('my-root', 'right'),),
+ # 'my-root' at 'child'.
+ [('origin', None, [('add', ('', ROOT_ID, 'directory', '')),
+ ('add', ('child', 'my-root', 'directory', ''))]),
+ ('base', None, []),
+ # 'my-root' at root
+ ('right', None, [('unversion', 'my-root'),
+ ('unversion', ROOT_ID),
+ ('add', ('', 'my-root', 'directory', ''))]),
+ ('tip', ['base', 'right'], [('unversion', 'my-root'),
+ ('unversion', ROOT_ID),
+ ('add', ('', 'my-root', 'directory', '')),
+ ]),
+ ], root_id='my-root')
+
def test_fetch_all_from_self(self):
tree = self.make_branch_and_tree('.')
rev_id = tree.commit('one')
=== modified file 'bzrlib/tests/test_branchbuilder.py'
--- a/bzrlib/tests/test_branchbuilder.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_branchbuilder.py 2009-05-05 03:13:58 +0000
@@ -326,3 +326,18 @@
builder.finish_series()
self.assertIs(None, builder._tree)
self.assertFalse(builder._branch.is_locked())
+
+ def test_ghost_mainline_history(self):
+ builder = BranchBuilder(self.get_transport().clone('foo'))
+ builder.start_series()
+ try:
+ builder.build_snapshot('tip', ['ghost'],
+ [('add', ('', 'ROOT_ID', 'directory', ''))],
+ allow_leftmost_as_ghost=True)
+ finally:
+ builder.finish_series()
+ b = builder.get_branch()
+ b.lock_read()
+ self.addCleanup(b.unlock)
+ self.assertEqual(('ghost',),
+ b.repository.get_graph().get_parent_map(['tip'])['tip'])
More information about the bazaar-commits
mailing list