Rev 4639: (andrew) Fix 'Revision ... not present' errors when upgrading stacked in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Mon Aug 24 05:18:01 BST 2009
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 4639 [merge]
revision-id: pqm at pqm.ubuntu.com-20090824041800-8hrf00q7l2qzkbv3
parent: pqm at pqm.ubuntu.com-20090824025148-za58q8tujgy8rhs8
parent: andrew.bennetts at canonical.com-20090824030806-u0oexn1t9kpoyrru
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2009-08-24 05:18:00 +0100
message:
(andrew) Fix 'Revision ... not present' errors when upgrading stacked
branches.
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/repository.py rev_storage.py-20051111201905-119e9401e46257e3
bzrlib/tests/per_interrepository/test_fetch.py test_fetch.py-20080425213627-j60cjh782ufm83ry-1
=== modified file 'NEWS'
--- a/NEWS 2009-08-21 03:17:13 +0000
+++ b/NEWS 2009-08-24 03:08:06 +0000
@@ -37,6 +37,10 @@
* Fix IndexError printing CannotBindAddress errors.
(Martin Pool, #286871)
+* Fix "Revision ... not present" errors when upgrading stacked branches,
+ or when doing fetches from a stacked source to a stacked target.
+ (Andrew Bennetts, #399140)
+
Improvements
************
=== modified file 'bzrlib/repository.py'
--- a/bzrlib/repository.py 2009-08-19 18:04:49 +0000
+++ b/bzrlib/repository.py 2009-08-21 00:04:55 +0000
@@ -3732,25 +3732,80 @@
return False
return True
- def _get_delta_for_revision(self, tree, parent_ids, basis_id, cache):
+ def _get_trees(self, revision_ids, cache):
+ possible_trees = []
+ for rev_id in revision_ids:
+ if rev_id in cache:
+ possible_trees.append((rev_id, cache[rev_id]))
+ else:
+ # Not cached, but inventory might be present anyway.
+ try:
+ tree = self.source.revision_tree(rev_id)
+ except errors.NoSuchRevision:
+ # Nope, parent is ghost.
+ pass
+ else:
+ cache[rev_id] = tree
+ possible_trees.append((rev_id, tree))
+ return possible_trees
+
+ def _get_delta_for_revision(self, tree, parent_ids, possible_trees):
"""Get the best delta and base for this revision.
:return: (basis_id, delta)
"""
- possible_trees = [(parent_id, cache[parent_id])
- for parent_id in parent_ids
- if parent_id in cache]
- if len(possible_trees) == 0:
- # There either aren't any parents, or the parents aren't in the
- # cache, so just use the last converted tree
- possible_trees.append((basis_id, cache[basis_id]))
deltas = []
+ # Generate deltas against each tree, to find the shortest.
+ texts_possibly_new_in_tree = set()
for basis_id, basis_tree in possible_trees:
delta = tree.inventory._make_delta(basis_tree.inventory)
+ for old_path, new_path, file_id, new_entry in delta:
+ if new_path is None:
+ # This file_id isn't present in the new rev, so we don't
+ # care about it.
+ continue
+ if not new_path:
+ # Rich roots are handled elsewhere...
+ continue
+ kind = new_entry.kind
+ if kind != 'directory' and kind != 'file':
+ # No text record associated with this inventory entry.
+ continue
+ # This is a directory or file that has changed somehow.
+ texts_possibly_new_in_tree.add((file_id, new_entry.revision))
deltas.append((len(delta), basis_id, delta))
deltas.sort()
return deltas[0][1:]
+ def _fetch_parent_invs_for_stacking(self, parent_map, cache):
+ """Find all parent revisions that are absent, but for which the
+ inventory is present, and copy those inventories.
+
+ This is necessary to preserve correctness when the source is stacked
+ without fallbacks configured. (Note that in cases like upgrade the
+ source may be not have _fallback_repositories even though it is
+ stacked.)
+ """
+ parent_revs = set()
+ for parents in parent_map.values():
+ parent_revs.update(parents)
+ present_parents = self.source.get_parent_map(parent_revs)
+ absent_parents = set(parent_revs).difference(present_parents)
+ parent_invs_keys_for_stacking = self.source.inventories.get_parent_map(
+ (rev_id,) for rev_id in absent_parents)
+ parent_inv_ids = [key[-1] for key in parent_invs_keys_for_stacking]
+ for parent_tree in self.source.revision_trees(parent_inv_ids):
+ current_revision_id = parent_tree.get_revision_id()
+ parents_parents_keys = parent_invs_keys_for_stacking[
+ (current_revision_id,)]
+ parents_parents = [key[-1] for key in parents_parents_keys]
+ basis_id = _mod_revision.NULL_REVISION
+ basis_tree = self.source.revision_tree(basis_id)
+ delta = parent_tree.inventory._make_delta(basis_tree.inventory)
+ self.target.add_inventory_by_delta(
+ basis_id, delta, current_revision_id, parents_parents)
+ cache[current_revision_id] = parent_tree
+
def _fetch_batch(self, revision_ids, basis_id, cache):
"""Fetch across a few revisions.
@@ -3769,29 +3824,54 @@
pending_deltas = []
pending_revisions = []
parent_map = self.source.get_parent_map(revision_ids)
+ self._fetch_parent_invs_for_stacking(parent_map, cache)
for tree in self.source.revision_trees(revision_ids):
+ # Find a inventory delta for this revision.
+ # Find text entries that need to be copied, too.
current_revision_id = tree.get_revision_id()
parent_ids = parent_map.get(current_revision_id, ())
+ parent_trees = self._get_trees(parent_ids, cache)
+ possible_trees = list(parent_trees)
+ if len(possible_trees) == 0:
+ # There either aren't any parents, or the parents are ghosts,
+ # so just use the last converted tree.
+ possible_trees.append((basis_id, cache[basis_id]))
basis_id, delta = self._get_delta_for_revision(tree, parent_ids,
- basis_id, cache)
+ possible_trees)
if self._converting_to_rich_root:
self._revision_id_to_root_id[current_revision_id] = \
tree.get_root_id()
- # Find text entries that need to be copied
+ # Determine which texts are in present in this revision but not in
+ # any of the available parents.
+ texts_possibly_new_in_tree = set()
for old_path, new_path, file_id, entry in delta:
- if new_path is not None:
- if not new_path:
- # This is the root
- if not self.target.supports_rich_root():
- # The target doesn't support rich root, so we don't
- # copy
- continue
- if self._converting_to_rich_root:
- # This can't be copied normally, we have to insert
- # it specially
- root_keys_to_create.add((file_id, entry.revision))
- continue
- text_keys.add((file_id, entry.revision))
+ if new_path is None:
+ # This file_id isn't present in the new rev
+ continue
+ if not new_path:
+ # This is the root
+ if not self.target.supports_rich_root():
+ # The target doesn't support rich root, so we don't
+ # copy
+ continue
+ if self._converting_to_rich_root:
+ # This can't be copied normally, we have to insert
+ # it specially
+ root_keys_to_create.add((file_id, entry.revision))
+ continue
+ kind = entry.kind
+ texts_possibly_new_in_tree.add((file_id, entry.revision))
+ for basis_id, basis_tree in possible_trees:
+ basis_inv = basis_tree.inventory
+ for file_key in list(texts_possibly_new_in_tree):
+ file_id, file_revision = file_key
+ try:
+ entry = basis_inv[file_id]
+ except errors.NoSuchId:
+ continue
+ if entry.revision == file_revision:
+ texts_possibly_new_in_tree.remove(file_key)
+ text_keys.update(texts_possibly_new_in_tree)
revision = self.source.get_revision(current_revision_id)
pending_deltas.append((basis_id, delta,
current_revision_id, revision.parent_ids))
@@ -3833,8 +3913,13 @@
for parent_tree in self.source.revision_trees(parent_map):
current_revision_id = parent_tree.get_revision_id()
parents_parents = parent_map[current_revision_id]
+ possible_trees = self._get_trees(parents_parents, cache)
+ if len(possible_trees) == 0:
+ # There either aren't any parents, or the parents are
+ # ghosts, so just use the last converted tree.
+ possible_trees.append((basis_id, cache[basis_id]))
basis_id, delta = self._get_delta_for_revision(parent_tree,
- parents_parents, basis_id, cache)
+ parents_parents, possible_trees)
self.target.add_inventory_by_delta(
basis_id, delta, current_revision_id, parents_parents)
# insert signatures and revisions
=== modified file 'bzrlib/tests/per_interrepository/test_fetch.py'
--- a/bzrlib/tests/per_interrepository/test_fetch.py 2009-08-14 00:55:42 +0000
+++ b/bzrlib/tests/per_interrepository/test_fetch.py 2009-08-20 06:54:03 +0000
@@ -17,7 +17,6 @@
import sys
-import bzrlib
from bzrlib import (
errors,
inventory,
@@ -266,6 +265,91 @@
self.assertEqual(expected_texts, unstacked_repo.texts.keys())
self.assertCanStreamRevision(unstacked_repo, 'third')
+ def test_fetch_from_stacked_to_stacked_copies_parent_inventories(self):
+ """Fetch from a stacked branch copies inventories for parents of
+ revisions at the stacking boundary.
+
+ Specifically, fetch will copy the parent inventories from the
+ source for which the corresponding revisions are not present. This
+ will happen even when the source repository has no fallbacks configured
+ (as is the case during upgrade).
+ """
+ if not self.repository_format.supports_external_lookups:
+ raise TestNotApplicable("Need stacking support in the source.")
+ if not self.repository_format_to.supports_external_lookups:
+ raise TestNotApplicable("Need stacking support in the target.")
+ builder = self.make_branch_builder('branch')
+ builder.start_series()
+ builder.build_snapshot('base', None, [
+ ('add', ('', 'root-id', 'directory', '')),
+ ('add', ('file', 'file-id', 'file', 'content\n'))])
+ builder.build_snapshot('left', ['base'], [
+ ('modify', ('file-id', 'left content\n'))])
+ builder.build_snapshot('right', ['base'], [
+ ('modify', ('file-id', 'right content\n'))])
+ builder.build_snapshot('merge', ['left', 'right'], [
+ ('modify', ('file-id', 'left and right content\n'))])
+ builder.finish_series()
+ branch = builder.get_branch()
+ repo = self.make_repository('old-trunk')
+ # Make a pair of equivalent trunk repos in the from and to formats.
+ old_trunk = repo.bzrdir.create_branch()
+ old_trunk.repository.fetch(branch.repository, 'left')
+ old_trunk.repository.fetch(branch.repository, 'right')
+ repo = self.make_to_repository('new-trunk')
+ new_trunk = repo.bzrdir.create_branch()
+ new_trunk.repository.fetch(branch.repository, 'left')
+ new_trunk.repository.fetch(branch.repository, 'right')
+ # Make the source; a repo stacked on old_trunk contained just the data
+ # for 'merge'.
+ repo = self.make_repository('old-stacked')
+ old_stacked_branch = repo.bzrdir.create_branch()
+ old_stacked_branch.set_stacked_on_url(old_trunk.base)
+ old_stacked_branch.repository.fetch(branch.repository, 'merge')
+ # Make the target, a repo stacked on new_trunk.
+ repo = self.make_to_repository('new-stacked')
+ new_stacked_branch = repo.bzrdir.create_branch()
+ new_stacked_branch.set_stacked_on_url(new_trunk.base)
+ old_unstacked_repo = old_stacked_branch.bzrdir.open_repository()
+ new_unstacked_repo = new_stacked_branch.bzrdir.open_repository()
+ # Reopen the source and target repos without any fallbacks, and fetch
+ # 'merge'.
+ new_unstacked_repo.fetch(old_unstacked_repo, 'merge')
+ # Now check the results. new_unstacked_repo should contain all the
+ # data necessary to stream 'merge' (i.e. the parent inventories).
+ new_unstacked_repo.lock_read()
+ self.addCleanup(new_unstacked_repo.unlock)
+ self.assertFalse(new_unstacked_repo.has_revision('left'))
+ self.assertFalse(new_unstacked_repo.has_revision('right'))
+ self.assertEqual(
+ set([('left',), ('right',), ('merge',)]),
+ new_unstacked_repo.inventories.keys())
+ # And the basis inventories have been copied correctly
+ new_trunk.lock_read()
+ self.addCleanup(new_trunk.unlock)
+ left_tree, right_tree = new_trunk.repository.revision_trees(
+ ['left', 'right'])
+ new_stacked_branch.lock_read()
+ self.addCleanup(new_stacked_branch.unlock)
+ (stacked_left_tree,
+ stacked_right_tree) = new_stacked_branch.repository.revision_trees(
+ ['left', 'right'])
+ self.assertEqual(left_tree.inventory, stacked_left_tree.inventory)
+ self.assertEqual(right_tree.inventory, stacked_right_tree.inventory)
+ # Finally, it's not enough to see that the basis inventories are
+ # present. The texts introduced in merge (and only those) should be
+ # present, and also generating a stream should succeed without blowing
+ # up.
+ self.assertTrue(new_unstacked_repo.has_revision('merge'))
+ expected_texts = set([('file-id', 'merge')])
+ if new_stacked_branch.repository.texts.get_parent_map([('root-id',
+ 'merge')]):
+ # If a (root-id,merge) text exists, it should be in the stacked
+ # repo.
+ expected_texts.add(('root-id', 'merge'))
+ self.assertEqual(expected_texts, new_unstacked_repo.texts.keys())
+ self.assertCanStreamRevision(new_unstacked_repo, 'merge')
+
def test_fetch_missing_basis_text(self):
"""If fetching a delta, we should die if a basis is not present."""
tree = self.make_branch_and_tree('tree')
More information about the bazaar-commits
mailing list