Merge lp:~lifeless/bzr/stacking-policy into lp:~bzr/bzr/trunk-old

[Robert Collins]

[Robert Collins (lifeless)]

Do not stack on the same branch/repository anymore. This was never
supported and would generally result in infinite recursion. Fixes bug
376243.

The approach taken is to raise an error when activating stacking
locations, which will also prevent the infinite recursion reported in
the bug for existing branches. [At the cost of stopping them opening -
but it will be much faster to error].

-Rob

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> Robert Collins has proposed merging lp:~lifeless/bzr/stacking-policy into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> Do not stack on the same branch/repository anymore. This was never
> supported and would generally result in infinite recursion. Fixes bug
> 376243.
>
> The approach taken is to raise an error when activating stacking
> locations, which will also prevent the infinite recursion reported in
> the bug for existing branches. [At the cost of stopping them opening -
> but it will be much faster to error].
>
> -Rob
>

  vote approve

What if we just didn't try to set the fallback and gave a warning,
rather than raising an error?

Regardless, it is better than triggering an endless loop. Though I
wonder if we really want a new exception. I suppose perhaps a server
would want to handle the error separately?

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpASTUACgkQJdeBCYSNAANmFwCgjbTyTtbGSGQ708hqef8J56EY
Ja4An0PEFBVmYYc2UFR0sWupIMS6w0aC
=KbT4
-----END PGP SIGNATURE-----

review: Approve

=== modified file 'NEWS'
--- NEWS	2009-06-22 21:55:37 +0000
+++ NEWS	2009-06-23 03:35:14 +0000
@@ -42,6 +42,7 @@
   ``BZR_PROGRESS_BAR`` is set to ``none``.
   (Martin Pool, #339385)
 
+<<<<<<< TREE
 * Reduced memory consumption during ``bzr commit`` of large files. For
   pre 2a formats, should be down to ~3x the size of a file, and for
   ``--2a`` formats should be down to exactly 2x the size. Related to bug
@@ -54,6 +55,15 @@
 * Unshelve works correctly when multiple zero-length files are present on
   the shelf. (Aaron Bentley, #363444)
 
+=======
+* Stacking will no longer accept requests to stack on the same
+  branch/repository. Existing branches that incorrectly reference the same
+  repository in a stacking configuration will now raise
+  UnstackableLocationError when the branch is opened. This can be fixed by
+  removing the stacking location inside ``.bzr/branch``.
+  (Robert Collins, #376243)
+
+>>>>>>> MERGE-SOURCE
 Internals
 *********
 

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2009-06-17 03:53:51 +0000
+++ bzrlib/branch.py	2009-06-23 03:35:14 +0000
@@ -105,6 +105,8 @@
     def _activate_fallback_location(self, url):
         """Activate the branch/repository from url as a fallback repository."""
         repo = self._get_fallback_repository(url)
+        if repo.has_same_location(self.repository):
+            raise errors.UnstackableLocationError(self.base, url)
         self.repository.add_fallback_repository(repo)
 
     def break_lock(self):

=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	2009-06-17 05:05:28 +0000
+++ bzrlib/errors.py	2009-06-23 03:35:14 +0000
@@ -636,6 +636,16 @@
         self.url = url
 
 
+class UnstackableLocationError(BzrError):
+
+    _fmt = "The branch '%(branch_url)s' cannot be stacked on '%(target_url)s'."
+
+    def __init__(self, branch_url, target_url):
+        BzrError.__init__(self)
+        self.branch_url = branch_url
+        self.target_url = target_url
+
+
 class UnstackableRepositoryFormat(BzrError):
 
     _fmt = ("The repository '%(url)s'(%(format)s) is not a stackable format. "

=== modified file 'bzrlib/tests/branch_implementations/test_stacking.py'
--- bzrlib/tests/branch_implementations/test_stacking.py	2009-06-11 07:43:56 +0000
+++ bzrlib/tests/branch_implementations/test_stacking.py	2009-06-23 03:35:14 +0000
@@ -78,6 +78,34 @@
             return
         self.assertEqual('../target', branch.get_stacked_on_url())
 
+    def test_set_stacked_on_same_branch_raises(self):
+        # Stacking on the same branch silently raises and doesn't execute the
+        # change. Reported in bug 376243.
+        branch = self.make_branch('branch')
+        try:
+            self.assertRaises(errors.UnstackableLocationError,
+                branch.set_stacked_on_url, '../branch')
+        except unstackable_format_errors:
+            # if the set failed, so must the get
+            self.assertRaises(unstackable_format_errors, branch.get_stacked_on_url)
+            return
+        self.assertRaises(errors.NotStacked, branch.get_stacked_on_url)
+
+    def test_set_stacked_on_same_branch_after_being_stacked_raises(self):
+        # Stacking on the same branch silently raises and doesn't execute the
+        # change.
+        branch = self.make_branch('branch')
+        target = self.make_branch('target')
+        try:
+            branch.set_stacked_on_url('../target')
+        except unstackable_format_errors:
+            # if the set failed, so must the get
+            self.assertRaises(unstackable_format_errors, branch.get_stacked_on_url)
+            return
+        self.assertRaises(errors.UnstackableLocationError,
+            branch.set_stacked_on_url, '../branch')
+        self.assertEqual('../target', branch.get_stacked_on_url())
+
     def assertRevisionInRepository(self, repo_path, revid):
         """Check that a revision is in a repository, disregarding stacking."""
         repo = bzrdir.BzrDir.open(repo_path).open_repository()

=== modified file 'bzrlib/tests/bzrdir_implementations/test_push.py'
--- bzrlib/tests/bzrdir_implementations/test_push.py	2009-04-03 14:24:40 +0000
+++ bzrlib/tests/bzrdir_implementations/test_push.py	2009-06-23 03:35:14 +0000
@@ -33,13 +33,13 @@
         tree.commit('one', rev_id='r1')
         return tree
 
-    def test_push_new_branch(self):     
+    def test_push_new_branch(self):
         tree = self.create_simple_tree()
         dir = self.make_repository('dir').bzrdir
         result = dir.push_branch(tree.branch)
         self.assertEquals(tree.branch, result.source_branch)
         self.assertEquals(dir.open_branch().base, result.target_branch.base)
-        self.assertEquals(dir.open_branch().base, 
+        self.assertEquals(dir.open_branch().base,
             tree.branch.get_push_location())
 
     def test_push_new_empty(self):
@@ -47,7 +47,7 @@
         dir = self.make_repository('dir').bzrdir
         result = dir.push_branch(tree.branch)
         self.assertEquals(tree.branch.base, result.source_branch.base)
-        self.assertEquals(dir.open_branch().base, 
+        self.assertEquals(dir.open_branch().base,
             result.target_branch.base)
 
     def test_push_incremental(self):
@@ -58,7 +58,7 @@
         tree.add(['b'])
         tree.commit('two', rev_id='r2')
         result = dir.push_branch(tree.branch)
-        self.assertEquals(tree.last_revision(), 
+        self.assertEquals(tree.last_revision(),
             result.branch_push_result.new_revid)
         self.assertEquals(2, result.branch_push_result.new_revno)
         self.assertEquals(tree.branch.base, result.source_branch.base)

=== modified file 'bzrlib/tests/test_errors.py'
--- bzrlib/tests/test_errors.py	2009-05-01 06:42:30 +0000
+++ bzrlib/tests/test_errors.py	2009-06-23 03:35:14 +0000
@@ -242,6 +242,11 @@
             "You will need to upgrade the branch to permit branch stacking.",
             str(error))
 
+    def test_unstackable_location(self):
+        error = errors.UnstackableLocationError('foo', 'bar')
+        self.assertEqualDiff("The branch 'foo' cannot be stacked on 'bar'.",
+            str(error))
+
     def test_unstackable_repository_format(self):
         format = u'foo'
         url = "/foo"