Merge lp:~abudden/bzr/switch_parent_513709 into lp:bzr

[Dr Al]

Set correct parent when using 'switch -b' with bound branches (Fix for Bug #513709).

Work in progress towards fixing Bug 513709.

[Dr Al (abudden)]

The background for this (in addition to the bug report) is I'd really like to see the "changes relative to parent" view when using bound branches with a feature branch model. I think this is the first change that is required, but in Bazaar Explorer's view_workingtree.py there is the comment:

        # Display the delta versus the submit branch iff one is
        # defined and it's local. Unfortunately though, merging a feature
        # branch into trunk sets the merged-from branch as the submit
        # branch which means we see a "silly" submit delta panel for trunk.
        # To work around that, we don't show the panel for bound branches
        # as trunk ought to be bound. We could look for the name
        # "trunk" instead but that sounds even more of a hack. :-(

I'd also like to explore the background to this as this means even with this merge proposal, I can only view the changes if I open the master branch and not if I open the bound branch.

[Vincent Ladeuil (vila)]

Thanks for working on this !

I've simplified the tests a bit (avoiding the code duplication) and put my changes at lp:~vila/bzr/switch_parent_513709 if you agree with them, just merge them in your own branch.

Now for the patch:

9 + bound_branch = self.get_master_branch()

Is confusing, this should be called master since this is what the branch is bound to.

Don't forget to add a news entry in the doc/en/release-notes/bzr-2.4 txt file.

Also, we need you to execute the contributor agreement http://www.canonical.com/contributors to be able to accept your patch.

I'll put this proposal into 'Work In Progress', put it back in 'NeedsReview' when needed.

review: Needs Fixing

[Dr Al (abudden)]

Okay, I've pushed those changes and sent the Canonical Contributors agreement to Martin Pool, <email address hidden> and you (Vincent).

On a note that's not related to this merge proposal, on http://www.canonical.com/contributors, there are two links to the PDF, one in the third paragraph just after "easy to understand" and one under item 1 of "How to submit it by email". The latter link doesn't work.

[Martin Pool (mbp)]

On 6 May 2011 09:21, Dr Al <email address hidden> wrote:
> Okay, I've pushed those changes and sent the Canonical Contributors agreement to Martin Pool, <email address hidden> and you (Vincent).

Got it, thanks.

>
> On a note that's not related to this merge proposal, on http://www.canonical.com/contributors, there are two links to the PDF, one in the third paragraph just after "easy to understand" and one under item 1 of "How to submit it by email".  The latter link doesn't work.

Thanks, bug 778334.

[Vincent Ladeuil (vila)]

> Okay, I've pushed those changes and sent the Canonical Contributors agreement
> to Martin Pool, <email address hidden> and you (Vincent).

Great, thanks !

>
> On a note that's not related to this merge proposal, on
> http://www.canonical.com/contributors, there are two links to the PDF, one in
> the third paragraph just after "easy to understand" and one under item 1 of
> "How to submit it by email". The latter link doesn't work.

Nice catch !

Marting was faster than me and filed bug #778334 :)

I'll land and update the bug page.

review: Approve

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2011-04-27 11:25:28 +0000
+++ bzrlib/branch.py	2011-05-06 07:26:44 +0000
@@ -1271,7 +1271,11 @@
             if repository_policy is not None:
                 repository_policy.configure_branch(result)
             self.copy_content_into(result, revision_id=revision_id)
-            result.set_parent(self.bzrdir.root_transport.base)
+            master_branch = self.get_master_branch()
+            if master_branch is None:
+                result.set_parent(self.bzrdir.root_transport.base)
+            else:
+                result.set_parent(master_branch.bzrdir.root_transport.base)
         finally:
             result.unlock()
         return result

=== modified file 'bzrlib/switch.py'
--- bzrlib/switch.py	2010-05-07 11:50:28 +0000
+++ bzrlib/switch.py	2011-05-06 07:26:44 +0000
@@ -103,6 +103,7 @@
             b.pull(to_branch, overwrite=True,
                 possible_transports=possible_transports)
             b.set_bound_location(to_branch.base)
+            b.set_parent(b.get_master_branch().get_parent())
         else:
             raise errors.BzrCommandError('Cannot switch a branch, '
                 'only a checkout.')

=== modified file 'bzrlib/tests/blackbox/test_branch.py'
--- bzrlib/tests/blackbox/test_branch.py	2011-04-19 04:37:48 +0000
+++ bzrlib/tests/blackbox/test_branch.py	2011-05-06 07:26:44 +0000
@@ -30,8 +30,10 @@
 from bzrlib.tests import (
     fixtures,
     HardlinkFeature,
+    script,
     test_server,
     )
+from bzrlib.tests.blackbox import test_switch
 from bzrlib.tests.test_sftp_transport import TestCaseWithSFTPServer
 from bzrlib.tests.script import run_script
 from bzrlib.urlutils import local_path_to_url, strip_trailing_slash
@@ -516,3 +518,32 @@
             2>The command 'bzr %(command)s' has been deprecated in bzr 2.4. Please use 'bzr branch' instead.
             2>bzr: ERROR: Not a branch...
             """ % locals())
+
+
+class TestBranchParentLocation(test_switch.TestSwitchParentLocationBase):
+
+    def _checkout_and_branch(self, option=''):
+        self.script_runner.run_script(self, '''
+                $ bzr checkout %(option)s repo/trunk checkout
+                $ cd checkout
+                $ bzr branch --switch ../repo/trunk ../repo/branched
+                2>Branched 0 revision(s).
+                2>Tree is up to date at revision 0.
+                2>Switched to branch:...branched...
+                $ cd ..
+                ''' % locals())
+        bound_branch = branch.Branch.open_containing('checkout')[0]
+        master_branch = branch.Branch.open_containing('repo/branched')[0]
+        return (bound_branch, master_branch)
+
+    def test_branch_switch_parent_lightweight(self):
+        """Lightweight checkout using bzr branch --switch."""
+        bb, mb = self._checkout_and_branch(option='--lightweight')
+        self.assertParent('repo/trunk', bb)
+        self.assertParent('repo/trunk', mb)
+
+    def test_branch_switch_parent_heavyweight(self):
+        """Heavyweight checkout using bzr branch --switch."""
+        bb, mb = self._checkout_and_branch()
+        self.assertParent('repo/trunk', bb)
+        self.assertParent('repo/trunk', mb)

=== modified file 'bzrlib/tests/blackbox/test_switch.py'
--- bzrlib/tests/blackbox/test_switch.py	2011-04-15 07:01:22 +0000
+++ bzrlib/tests/blackbox/test_switch.py	2011-05-06 07:26:44 +0000
@@ -20,9 +20,16 @@
 
 import os
 
-from bzrlib import osutils
+from bzrlib import (
+        osutils,
+        urlutils,
+        branch,
+        )
 from bzrlib.workingtree import WorkingTree
-from bzrlib.tests import TestCaseWithTransport
+from bzrlib.tests import (
+        TestCaseWithTransport,
+        script,
+        )
 from bzrlib.directory_service import directories
 
 
@@ -129,7 +136,7 @@
         """Using switch on a heavy checkout should find master sibling
 
         The behaviour of lighweight and heavy checkouts should be
-        consistentwhen using the convenient "switch to sibling" feature
+        consistent when using the convenient "switch to sibling" feature
         Both should switch to a sibling of the branch
         they are bound to, and not a sibling of themself"""
 
@@ -270,3 +277,54 @@
         self.run_bzr('checkout --lightweight a checkout')
         self.run_bzr('switch --directory checkout b')
         self.assertFileEqual('initial\nmore\n', 'checkout/a')
+
+class TestSwitchParentLocationBase(TestCaseWithTransport):
+
+    def setUp(self):
+        """Set up a repository and branch ready for testing."""
+        super(TestSwitchParentLocationBase, self).setUp()
+        self.script_runner = script.ScriptRunner()
+        self.script_runner.run_script(self, '''
+                $ bzr init-repo --no-trees repo
+                Shared repository...
+                Location:
+                  shared repository: repo
+                $ bzr init repo/trunk
+                Created a repository branch...
+                Using shared repository: ...
+                ''')
+
+    def assertParent(self, expected_parent, branch):
+        """Verify that the parent is not None and is set correctly."""
+        actual_parent = branch.get_parent()
+        self.assertIsSameRealPath(urlutils.local_path_to_url(expected_parent),
+                                  branch.get_parent())
+
+
+class TestSwitchParentLocation(TestSwitchParentLocationBase):
+
+    def _checkout_and_switch(self, option=''):
+        self.script_runner.run_script(self, '''
+                $ bzr checkout %(option)s repo/trunk checkout
+                $ cd checkout
+                $ bzr switch --create-branch switched
+                2>Tree is up to date at revision 0.
+                2>Switched to branch:...switched...
+                $ cd ..
+                ''' % locals())
+        bound_branch = branch.Branch.open_containing('checkout')[0]
+        master_branch = branch.Branch.open_containing('repo/switched')[0]
+        return (bound_branch, master_branch)
+
+    def test_switch_parent_lightweight(self):
+        """Lightweight checkout using bzr switch."""
+        bb, mb = self._checkout_and_switch(option='--lightweight')
+        self.assertParent('repo/trunk', bb)
+        self.assertParent('repo/trunk', mb)
+
+    def test_switch_parent_heavyweight(self):
+        """Heavyweight checkout using bzr switch."""
+        bb, mb = self._checkout_and_switch()
+        self.assertParent('repo/trunk', bb)
+        self.assertParent('repo/trunk', mb)
+

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- doc/en/release-notes/bzr-2.4.txt	2011-05-05 15:26:26 +0000
+++ doc/en/release-notes/bzr-2.4.txt	2011-05-06 07:26:44 +0000
@@ -37,6 +37,9 @@
 
 * All Tree types can now be exported as tar.*, zip or directories.
   (Aaron Bentley)
+  
+* Correct parent is now set when using 'switch -b' with bound branches.
+  (A. S. Budden, #513709)
 
 Documentation
 *************