Rev 2836: No longer propagate index differences automatically (Robert Collins) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Thu Sep 20 03:40:56 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2836
revision-id: pqm at pqm.ubuntu.com-20070920024052-y2l7r5o00zrpnr73
parent: pqm at pqm.ubuntu.com-20070920005743-h4qqzyty2apbhjta
parent: ian.clatworthy at internode.on.net-20070920011847-mxya10w42px03fw2
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2007-09-20 03:40:52 +0100
message:
  No longer propagate index differences automatically (Robert Collins)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/knit.py                 knit.py-20051212171256-f056ac8f0fbe1bd9
  bzrlib/tests/interversionedfile_implementations/test_join.py test_join.py-20060302012326-9b5e9b0f0a03fedc
  bzrlib/tests/test_versionedfile.py test_versionedfile.py-20060222045249-db45c9ed14a1c2e5
  bzrlib/tests/test_weave.py     testknit.py-20050627023648-9833cc5562ffb785
  bzrlib/versionedfile.py        versionedfile.py-20060222045106-5039c71ee3b65490
  bzrlib/weave.py                knit.py-20050627021749-759c29984154256b
    ------------------------------------------------------------
    revno: 2835.1.1
    merged: ian.clatworthy at internode.on.net-20070920011847-mxya10w42px03fw2
    parent: pqm at pqm.ubuntu.com-20070920005743-h4qqzyty2apbhjta
    parent: robertc at robertcollins.net-20070919051414-2tgjqteg7k3ps4h0
    committer: Ian Clatworthy <ian.clatworthy at internode.on.net>
    branch nick: ianc-integration
    timestamp: Thu 2007-09-20 11:18:47 +1000
    message:
      No longer propagate index differences automatically (Robert Collins)
    ------------------------------------------------------------
    revno: 2825.4.1
    merged: robertc at robertcollins.net-20070919051414-2tgjqteg7k3ps4h0
    parent: pqm at pqm.ubuntu.com-20070917005035-cshdkpzbj63id1uw
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: knit-index-propogation
    timestamp: Wed 2007-09-19 15:14:14 +1000
    message:
      * ``pull``, ``merge`` and ``push`` will no longer silently correct some
        repository index errors that occured as a result of the Weave disk format.
        Instead the ``reconcile`` command needs to be run to correct those
        problems if they exist (and it has been able to fix most such problems
        since bzr 0.8). Some new problems have been identified during this release
        and you should run ``bzr check`` once on every repository to see if you
        need to reconcile. If you cannot ``pull`` or ``merge`` from a remote
        repository due to mismatched parent errors - a symptom of index errors -
        you should simply take a full copy of that remote repository to a clean
        directory outside any local repositories, then run reconcile on it, and
        finally pull from it locally. (And naturally email the repositories owner
        to ask them to upgrade and run reconcile).
        (Robert Collins)
      
      * ``VersionedFile.fix_parents`` has been removed as a harmful API.
        ``VersionedFile.join`` will no longer accept different parents on either
        side of a join - it will either ignore them, or error, depending on the
        implementation. See notes when upgrading for more information.
        (Robert Collins)
=== modified file 'NEWS'
--- a/NEWS	2007-09-19 09:22:24 +0000
+++ b/NEWS	2007-09-20 01:18:47 +0000
@@ -9,6 +9,20 @@
 
   NOTES WHEN UPGRADING:
 
+   * ``pull``, ``merge`` and ``push`` will no longer silently correct some
+     repository index errors that occured as a result of the Weave disk format.
+     Instead the ``reconcile`` command needs to be run to correct those
+     problems if they exist (and it has been able to fix most such problems
+     since bzr 0.8). Some new problems have been identified during this release
+     and you should run ``bzr check`` once on every repository to see if you
+     need to reconcile. If you cannot ``pull`` or ``merge`` from a remote
+     repository due to mismatched parent errors - a symptom of index errors -
+     you should simply take a full copy of that remote repository to a clean
+     directory outside any local repositories, then run reconcile on it, and
+     finally pull from it locally. (And naturally email the repositories owner
+     to ask them to upgrade and run reconcile).
+     (Robert Collins)
+
   FEATURES:
 
    * New ``reconfigure`` command (Aaron Bentley)
@@ -66,6 +80,12 @@
      inventories without such data, pass working=True to write_inventory.
      (Robert Collins)
 
+   * ``VersionedFile.fix_parents`` has been removed as a harmful API.
+     ``VersionedFile.join`` will no longer accept different parents on either
+     side of a join - it will either ignore them, or error, depending on the
+     implementation. See notes when upgrading for more information.
+     (Robert Collins)
+
   INTERNALS:
 
    * New method on xml serialisers, write_inventory_to_lines, which matches the

=== modified file 'bzrlib/knit.py'
--- a/bzrlib/knit.py	2007-09-12 04:21:51 +0000
+++ b/bzrlib/knit.py	2007-09-19 05:14:14 +0000
@@ -534,21 +534,6 @@
         return KnitVersionedFile(name, transport, factory=self.factory,
                                  delta=self.delta, create=True)
     
-    def _fix_parents(self, version_id, new_parents):
-        """Fix the parents list for version.
-        
-        This is done by appending a new version to the index
-        with identical data except for the parents list.
-        the parents list must be a superset of the current
-        list.
-        """
-        current_values = self._index._cache[version_id]
-        assert set(current_values[4]).difference(set(new_parents)) == set()
-        self._index.add_version(version_id,
-                                current_values[1],
-                                (None, current_values[2], current_values[3]),
-                                new_parents)
-
     def get_data_stream(self, required_versions):
         """Get a data stream for the specified versions.
 
@@ -2201,31 +2186,13 @@
     
             self.source_ancestry = set(self.source.get_ancestry(version_ids))
             this_versions = set(self.target._index.get_versions())
+            # XXX: For efficiency we should not look at the whole index,
+            #      we only need to consider the referenced revisions - they
+            #      must all be present, or the method must be full-text.
+            #      TODO, RBC 20070919
             needed_versions = self.source_ancestry - this_versions
-            cross_check_versions = self.source_ancestry.intersection(this_versions)
-            mismatched_versions = set()
-            for version in cross_check_versions:
-                # scan to include needed parents.
-                n1 = set(self.target.get_parents_with_ghosts(version))
-                n2 = set(self.source.get_parents_with_ghosts(version))
-                if n1 != n2:
-                    # FIXME TEST this check for cycles being introduced works
-                    # the logic is we have a cycle if in our graph we are an
-                    # ancestor of any of the n2 revisions.
-                    for parent in n2:
-                        if parent in n1:
-                            # safe
-                            continue
-                        else:
-                            parent_ancestors = self.source.get_ancestry(parent)
-                            if version in parent_ancestors:
-                                raise errors.GraphCycleError([parent, version])
-                    # ensure this parent will be available later.
-                    new_parents = n2.difference(n1)
-                    needed_versions.update(new_parents.difference(this_versions))
-                    mismatched_versions.add(version)
     
-            if not needed_versions and not mismatched_versions:
+            if not needed_versions:
                 return 0
             full_list = topo_sort(self.source.get_graph())
     
@@ -2268,15 +2235,6 @@
                 raw_records.append((version_id, options, parents, len(raw_data)))
                 raw_datum.append(raw_data)
             self.target._add_raw_records(raw_records, ''.join(raw_datum))
-
-            for version in mismatched_versions:
-                # FIXME RBC 20060309 is this needed?
-                n1 = set(self.target.get_parents_with_ghosts(version))
-                n2 = set(self.source.get_parents_with_ghosts(version))
-                # write a combined record to our history preserving the current 
-                # parents as first in the list
-                new_parents = self.target.get_parents_with_ghosts(version) + list(n2.difference(n1))
-                self.target.fix_parents(version, new_parents)
             return count
         finally:
             pb.finished()
@@ -2317,31 +2275,8 @@
             self.source_ancestry = set(self.source.get_ancestry(version_ids))
             this_versions = set(self.target._index.get_versions())
             needed_versions = self.source_ancestry - this_versions
-            cross_check_versions = self.source_ancestry.intersection(this_versions)
-            mismatched_versions = set()
-            for version in cross_check_versions:
-                # scan to include needed parents.
-                n1 = set(self.target.get_parents_with_ghosts(version))
-                n2 = set(self.source.get_parents(version))
-                # if all of n2's parents are in n1, then its fine.
-                if n2.difference(n1):
-                    # FIXME TEST this check for cycles being introduced works
-                    # the logic is we have a cycle if in our graph we are an
-                    # ancestor of any of the n2 revisions.
-                    for parent in n2:
-                        if parent in n1:
-                            # safe
-                            continue
-                        else:
-                            parent_ancestors = self.source.get_ancestry(parent)
-                            if version in parent_ancestors:
-                                raise errors.GraphCycleError([parent, version])
-                    # ensure this parent will be available later.
-                    new_parents = n2.difference(n1)
-                    needed_versions.update(new_parents.difference(this_versions))
-                    mismatched_versions.add(version)
     
-            if not needed_versions and not mismatched_versions:
+            if not needed_versions:
                 return 0
             full_list = topo_sort(self.source.get_graph())
     
@@ -2361,15 +2296,6 @@
                 self.target.add_lines(
                     version_id, parents, self.source.get_lines(version_id))
                 count = count + 1
-
-            for version in mismatched_versions:
-                # FIXME RBC 20060309 is this needed?
-                n1 = set(self.target.get_parents_with_ghosts(version))
-                n2 = set(self.source.get_parents(version))
-                # write a combined record to our history preserving the current 
-                # parents as first in the list
-                new_parents = self.target.get_parents_with_ghosts(version) + list(n2.difference(n1))
-                self.target.fix_parents(version, new_parents)
             return count
         finally:
             pb.finished()

=== modified file 'bzrlib/tests/interversionedfile_implementations/test_join.py'
--- a/bzrlib/tests/interversionedfile_implementations/test_join.py	2007-07-19 06:34:09 +0000
+++ b/bzrlib/tests/interversionedfile_implementations/test_join.py	2007-09-19 05:14:14 +0000
@@ -54,14 +54,6 @@
         self.assertRaises(errors.RevisionNotPresent,
             f2.join, f1, version_ids=['r3'])
 
-        #f3 = self.get_file('1')
-        #f3.add_lines('r0', ['a\n', 'b\n'], [])
-        #f3.add_lines('r1', ['c\n', 'b\n'], ['r0'])
-        #f4 = self.get_file('2')
-        #f4.join(f3, ['r0'])
-        #self.assertTrue(f4.has_version('r0'))
-        #self.assertFalse(f4.has_version('r1'))
-
     def test_gets_expected_inter_worker(self):
         source = self.get_source()
         target = self.get_target()
@@ -87,25 +79,25 @@
         self.assertTrue(target.has_version('namedleft'))
         self.assertTrue(target.has_version('namedright'))
 
-    def test_join_add_parents(self):
-        """Join inserting new parents into existing versions
-        
-        The new version must have the right parent list and must identify
-        lines originating in another parent.
-        """
+    def test_join_different_parents_existing_version(self):
+        """This may either ignore or error."""
         w1 = self.get_target('w1')
         w2 = self.get_source('w2')
         w1.add_lines('v-1', [], ['line 1\n'])
         w2.add_lines('v-2', [], ['line 2\n'])
         w1.add_lines('v-3', ['v-1'], ['line 1\n'])
         w2.add_lines('v-3', ['v-2'], ['line 1\n'])
-        w1.join(w2)
+        try:
+            w1.join(w2)
+        except errors.WeaveParentMismatch:
+            # Acceptable behaviour:
+            return
         self.assertEqual(sorted(w1.versions()),
                          'v-1 v-2 v-3'.split())
         self.assertEqualDiff(w1.get_text('v-3'),
                 'line 1\n')
         self.assertEqual(sorted(w1.get_parents('v-3')),
-                ['v-1', 'v-2'])
+                ['v-1'])
         ann = list(w1.annotate('v-3'))
         self.assertEqual(len(ann), 1)
         self.assertEqual(ann[0][0], 'v-1')
@@ -148,36 +140,34 @@
         t.join(s)
         self.assertEqual(['base'], t.get_parents('text'))
 
-    def test_join_with_ghosts_merges_parents(self):
-        """Join combined parent lists"""
-        wa = self.build_weave1()
-        wb = self.get_target()
-        wb.add_lines('x1', [], ['line from x1\n'])
-        wb.add_lines('v1', [], ['hello\n'])
-        wb.add_lines('v2', ['v1', 'x1'], ['hello\n', 'world\n'])
-        wa.join(wb)
-        self.assertEqual(['v1','x1'], wa.get_parents('v2'))
-
     def test_join_with_ghosts(self):
         """Join that inserts parents of an existing revision.
 
-        This can happen when merging from another branch who
-        knows about revisions the destination does not.  In 
-        this test the second weave knows of an additional parent of 
-        v2.  Any revisions which are in common still have to have the 
-        same text.
+        This can happen when merging from another branch who knows about
+        revisions the destination does not, and the destinations index is
+        incorrect because it was or is using a ghost-unaware format to
+        represent the index. In this test the second weave knows of an
+        additional parent of v2.
+        
+        However v2 must not be changed because we consider indexes immutable:
+        instead a check or reconcile operation locally should pickup that v2 is
+        wrong and regenerate the index at a later time. So either this errors,
+        or leaves v2 unaltered.
         """
         w1 = self.build_weave1()
         wb = self.get_target()
         wb.add_lines('x1', [], ['line from x1\n'])
         wb.add_lines('v1', [], ['hello\n'])
         wb.add_lines('v2', ['v1', 'x1'], ['hello\n', 'world\n'])
-        w1.join(wb)
-        eq = self.assertEquals
-        eq(sorted(w1.versions()), ['v1', 'v2', 'v3', 'x1',])
-        eq(w1.get_text('x1'), 'line from x1\n')
-        eq(w1.get_lines('v2'), ['hello\n', 'world\n'])
-        eq(w1.get_parents('v2'), ['v1', 'x1'])
+        try:
+            w1.join(wb)
+        except errors.WeaveParentMismatch:
+            # Acceptable behaviour:
+            return
+        self.assertEqual(['v1', 'v2', 'v3', 'x1',], sorted(w1.versions()))
+        self.assertEqual('line from x1\n', w1.get_text('x1'))
+        self.assertEqual(['hello\n', 'world\n'], w1.get_lines('v2'))
+        self.assertEqual(['v1'], w1.get_parents('v2'))
 
     def test_join_with_ignore_missing_versions(self):
         # test that ignore_missing=True makes a listed but absent version id
@@ -205,38 +195,6 @@
         for version, parents in pattern:
             w.add_lines(version, parents, [])
         return w
-
-    def test_join_reorder(self):
-        """Reweave requiring reordering of versions.
-
-        Weaves must be stored such that parents come before children.  When
-        reweaving, we may add new parents to some children, but it is required
-        that there must be *some* valid order that can be found, otherwise the
-        ancestries are contradictory.  (For the specific case of inserting
-        ghost revisions there will be no disagreement, only partial knowledge
-        of the history.)
-
-        Note that the weaves are only partially ordered: when there are two
-        versions where neither is an ancestor of the other the order in which
-        they occur is unconstrained.  When we join those versions into
-        another weave, they may become more constrained and it may be
-        necessary to change their order.
-
-        One simple case of this is 
-
-        w1: (c[], a[], b[a])
-        w2: (b[], c[b], a[])
-        
-        We need to recognize that the final weave must show the ordering
-        a[], b[a], c[b].  The version that must be first in the result is 
-        not first in either of the input weaves.
-        """
-        w1 = self.build_target_weave('1', ('c', []), ('a', []), ('b', ['a']))
-        w2 = self.build_source_weave('2', ('b', []), ('c', ['b']), ('a', []))
-        w1.join(w2)
-        self.assertEqual([], w1.get_parents('a'))
-        self.assertEqual(['a'], w1.get_parents('b'))
-        self.assertEqual(['b'], w1.get_parents('c'))
         
     def test_joining_ghosts(self):
         # some versioned file formats allow lines to be added with parent

=== modified file 'bzrlib/tests/test_versionedfile.py'
--- a/bzrlib/tests/test_versionedfile.py	2007-09-12 04:21:51 +0000
+++ b/bzrlib/tests/test_versionedfile.py	2007-09-19 05:14:14 +0000
@@ -351,7 +351,6 @@
         f.transaction_finished()
         self.assertRaises(errors.OutSideTransaction, f.add_lines, '', [], [])
         self.assertRaises(errors.OutSideTransaction, f.add_lines_with_ghosts, '', [], [])
-        self.assertRaises(errors.OutSideTransaction, f.fix_parents, '', [])
         self.assertRaises(errors.OutSideTransaction, f.join, '')
         self.assertRaises(errors.OutSideTransaction, f.clone_text, 'base', 'bar', ['foo'])
         
@@ -598,43 +597,6 @@
         self.assertTrue(lines['child\n'] > 0)
         self.assertTrue(lines['otherchild\n'] > 0)
 
-    def test_fix_parents(self):
-        # some versioned files allow incorrect parents to be corrected after
-        # insertion - this may not fix ancestry..
-        # if they do not supported, they just do not implement it.
-        # we test this as an interface test to ensure that those that *do*
-        # implementent it get it right.
-        vf = self.get_file()
-        vf.add_lines('notbase', [], [])
-        vf.add_lines('base', [], [])
-        try:
-            vf.fix_parents('notbase', ['base'])
-        except NotImplementedError:
-            return
-        self.assertEqual(['base'], vf.get_parents('notbase'))
-        # open again, check it stuck.
-        vf = self.get_file()
-        self.assertEqual(['base'], vf.get_parents('notbase'))
-
-    def test_fix_parents_with_ghosts(self):
-        # when fixing parents, ghosts that are listed should not be ghosts
-        # anymore.
-        vf = self.get_file()
-
-        try:
-            vf.add_lines_with_ghosts('notbase', ['base', 'stillghost'], [])
-        except NotImplementedError:
-            return
-        vf.add_lines('base', [], [])
-        vf.fix_parents('notbase', ['base', 'stillghost'])
-        self.assertEqual(['base'], vf.get_parents('notbase'))
-        # open again, check it stuck.
-        vf = self.get_file()
-        self.assertEqual(['base'], vf.get_parents('notbase'))
-        # and check the ghosts
-        self.assertEqual(['base', 'stillghost'],
-                         vf.get_parents_with_ghosts('notbase'))
-
     def test_add_lines_with_ghosts(self):
         # some versioned file formats allow lines to be added with parent
         # information that is > than that in the format. Formats that do
@@ -726,7 +688,6 @@
                           'base',
                           [],
                           [])
-        self.assertRaises(errors.ReadOnlyError, vf.fix_parents, 'base', [])
         self.assertRaises(errors.ReadOnlyError, vf.join, 'base')
         self.assertRaises(errors.ReadOnlyError, vf.clone_text, 'base', 'bar', ['foo'])
     

=== modified file 'bzrlib/tests/test_weave.py'
--- a/bzrlib/tests/test_weave.py	2007-09-03 02:58:58 +0000
+++ b/bzrlib/tests/test_weave.py	2007-09-19 05:14:14 +0000
@@ -707,16 +707,6 @@
         eq(wa.get_lines('b1'),
            ['hello\n', 'pale blue\n', 'world\n'])
 
-    def test_join_parent_disagreement(self):
-        #join reconciles differening parents into a union.
-        wa = Weave()
-        wb = Weave()
-        wa.add_lines('v1', [], ['hello\n'])
-        wb.add_lines('v0', [], [])
-        wb.add_lines('v1', ['v0'], ['hello\n'])
-        wa.join(wb)
-        self.assertEqual(['v0'], wa.get_parents('v1'))
-
     def test_join_text_disagreement(self):
         """Cannot join weaves with different texts for a version."""
         wa = Weave()

=== modified file 'bzrlib/versionedfile.py'
--- a/bzrlib/versionedfile.py	2007-09-12 04:21:51 +0000
+++ b/bzrlib/versionedfile.py	2007-09-19 05:14:14 +0000
@@ -209,23 +209,6 @@
         """
         raise NotImplementedError(self.create_empty)
 
-    def fix_parents(self, version_id, new_parents):
-        """Fix the parents list for version.
-        
-        This is done by appending a new version to the index
-        with identical data except for the parents list.
-        the parents list must be a superset of the current
-        list.
-        """
-        version_id = osutils.safe_revision_id(version_id)
-        new_parents = [osutils.safe_revision_id(p) for p in new_parents]
-        self._check_write_ok()
-        return self._fix_parents(version_id, new_parents)
-
-    def _fix_parents(self, version_id, new_parents):
-        """Helper for fix_parents."""
-        raise NotImplementedError(self.fix_parents)
-
     def get_format_signature(self):
         """Get a text description of the data encoding in this file.
         

=== modified file 'bzrlib/weave.py'
--- a/bzrlib/weave.py	2007-09-12 04:21:51 +0000
+++ b/bzrlib/weave.py	2007-09-19 05:14:14 +0000
@@ -1202,10 +1202,7 @@
         if self.target.versions() == [] and version_ids is None:
             self.target._copy_weave_content(self.source)
             return
-        try:
-            self.target._join(self.source, pb, msg, version_ids, ignore_missing)
-        except errors.WeaveParentMismatch:
-            self.target._reweave(self.source, pb, msg)
+        self.target._join(self.source, pb, msg, version_ids, ignore_missing)
 
 
 InterVersionedFile.register_optimiser(InterWeave)




More information about the bazaar-commits mailing list