Merge lp:~mbp/bzr/135234-checkout-relpath into lp:bzr

[Martin Pool]

(mbp) store a relative URL to the master branch of checkouts

This fixes bug 135234 by:

- storing relative URLs to the master branch for lightweight and heavyweight checkouts
- coping with a trailing newline in the location file, in case it's manually edited

[Ian Clatworthy (ian-clatworthy)]

3 cheers for this patch! Some minor things:

* there's a bit of junk in NEWS - "* Cope"
* builtins.py has ", e" added to an except clause and the exception isn't used anywhere. (I'm fine with leaving that as is though - it's harmless.)

review: Approve

[lp:~mbp/bzr/135234-checkout-relpath]

5094. By Martin Pool on 2010-03-22

Fix typo

[Martin Pool (mbp)]

Fed to pqm

star-merge http://bazaar.launchpad.net/~mbp/bzr/135234-checkout-relpath http://bazaar.launchpad.net/~bzr-pqm/bzr/bzr.dev

[lp:~mbp/bzr/135234-checkout-relpath]

5095. By Martin Pool on 2010-03-24

merge news

[Martin Pool (mbp)]

Fed to pqm

star-merge http://bazaar.launchpad.net/~mbp/bzr/135234-checkout-relpath http://bazaar.launchpad.net/~bzr-pqm/bzr/bzr.dev

[Martin Pool (mbp)]

fails in an info test:

 File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/blackbox/test_info.py", line 1210, in assertCheckoutStatusOutput
   ), out)
AssertionError: texts not equal:
 Lightweight checkout (format: 2a)
 Location:
   light checkout root: tree/lightcheckout
         checkout root: tree/checkout
- checkout of branch: repo/branch
+ checkout of branch: ../../repo/branch/
? ++++++ +

 Format:

[Martin Pool (mbp)]

Aaron raises a question in https://bugs.edge.launchpad.net/bzr/+bug/135234/comments/4 as to whether it's really ideal to store relative paths all the time.

We do as a general principle store relative paths when possible, partly because they're more likely to work when the same directories are addressed across different transports.

[Ian Clatworthy (ian-clatworthy)]

I think the frequency of relative paths is increasing as more people use Neil's colo plugin and the various workspace models in Explorer. We could detect those cases pretty easily I suspect, e.g. we want relative paths in at least these cases:

1. If the branch path starts with the checkout path
2. If the checkout path starts with the repository path.

Unmerged revisions

5095. By Martin Pool on 2010-03-24

merge news

5094. By Martin Pool on 2010-03-22

Fix typo

5093. By Martin Pool on 2010-03-15

Cope with a trailing newline in a .bzr/branch/location file

5092. By Martin Pool on 2010-03-15

Update BranchReference tests to expect relative URLs

5091. By Martin Pool on 2010-03-15

Branch references (used for lightweight checkouts) also store a relative url.

5090. By Martin Pool on 2010-03-15

The master of a bound branch is now stored as a relative URL if possible.

=== modified file 'NEWS'
--- NEWS	2010-03-22 13:59:33 +0000
+++ NEWS	2010-03-24 06:42:27 +0000
@@ -107,6 +107,9 @@
   revspec as representing the first revision of the branch.
   (Vincent Ladeuil, #519862)
 
+* ``bzr mkdir DIR`` will not create DIR unless DIR's parent is a versioned
+   directory. (Parth Malwankar, #138600)
+
 * ``bzr remove-tree`` can now remove multiple working trees.
   (Jared Hance, Andrew Bennetts, #253137)
 
@@ -138,13 +141,14 @@
   to the Ubuntu Units Policy <https://wiki.ubuntu.com/UnitsPolicy>.
   (Gordon Tyler, #514399)
 
+* The master of a bound branch is now stored as a relative URL if
+  possible, and similarly for lightweight checkouts.
+  (Martin Pool, #135234)
+
 * Support kind markers for socket and fifo filesystem objects. This
   prevents ``bzr status --short`` from crashing when those files are
   present.  (John Arbash Meinel, #303275)
 
-* ``bzr mkdir DIR`` will not create DIR unless DIR's parent is a versioned
-   directory. (Parth Malwankar, #138600)
-
 * SSH child processes will now ignore SIGQUIT on nix systems so breaking into
   the debugger won't kill the session.
   (Martin <[e-mail]>, #162502)
 
=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2010-03-22 13:59:33 +0000
+++ bzrlib/branch.py	2010-03-24 06:42:27 +0000
@@ -2004,7 +2004,8 @@
     def get_reference(self, a_bzrdir):
         """See BranchFormat.get_reference()."""
         transport = a_bzrdir.get_branch_transport(None)
-        return transport.get_bytes('location')
+        # ignore trailing newline
+        return transport.get_bytes('location').rstrip()
 
     def set_reference(self, a_bzrdir, to_branch):
         """See BranchFormat.set_reference()."""
@@ -2019,12 +2020,17 @@
             raise errors.UninitializableFormat(self)
         mutter('creating branch reference in %s', a_bzrdir.transport.base)
         branch_transport = a_bzrdir.get_branch_transport(self, name=name)
+        # NB: Don't add a trailing newline, because bzr prior to 2.2
+        # can't cope with it
         branch_transport.put_bytes('location',
-            target_branch.bzrdir.root_transport.base)
+            urlutils.relative_url(a_bzrdir.root_transport.base,
+                target_branch.base))
         branch_transport.put_bytes('format', self.get_format_string())
         return self.open(
             a_bzrdir, name, _found=True,
-            possible_transports=[target_branch.bzrdir.root_transport])
+            possible_transports=[a_bzrdir.root_transport,
+                branch_transport,
+                target_branch.bzrdir.root_transport])
 
     def __init__(self):
         super(BranchReferenceFormat, self).__init__()
@@ -2064,8 +2070,9 @@
                     (format, self))
         if location is None:
             location = self.get_reference(a_bzrdir)
+        abs_location = urlutils.join(a_bzrdir.root_transport.base, location)
         real_bzrdir = bzrdir.BzrDir.open(
-            location, possible_transports=possible_transports)
+            abs_location, possible_transports=possible_transports)
         result = real_bzrdir.open_branch(name=name, 
             ignore_fallbacks=ignore_fallbacks)
         # this changes the behaviour of result.clone to create a new reference
@@ -2412,8 +2419,14 @@
         if not bound_loc:
             return None
         try:
-            return Branch.open(bound_loc,
-                               possible_transports=possible_transports)
+            # if relative, interpret relative to my location
+            abs_bound_loc = urlutils.join(self.base, bound_loc)
+            if possible_transports is None:
+                possible_transports = []
+            if self._transport not in possible_transports:
+                possible_transports += [self._transport]
+            return Branch.open(abs_bound_loc,
+                possible_transports=possible_transports)
         except (errors.NotBranchError, errors.ConnectionError), e:
             raise errors.BoundBranchConnectionFailure(
                     self, bound_loc, e)
@@ -2458,7 +2471,10 @@
         # last_rev is not in the other_last_rev history, AND
         # other_last_rev is not in our history, and do it without pulling
         # history around
-        self.set_bound_location(other.base)
+
+        # If possible, use a relative URL so that things will work if the
+        # whole thing is moved, or accessed across a different transport.
+        self.set_bound_location(urlutils.relative_url(self.base, other.base))
 
     @needs_write_lock
     def unbind(self):
 
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2010-03-22 13:59:33 +0000
+++ bzrlib/builtins.py	2010-03-24 06:42:27 +0000
@@ -4652,10 +4652,11 @@
                     else:
                         raise errors.BzrCommandError('No location supplied '
                             'and no previous location known')
+        # open to check the other one actually exists
         b_other = Branch.open(location)
         try:
             b.bind(b_other)
-        except errors.DivergedBranches:
+        except errors.DivergedBranches, e:
             raise errors.BzrCommandError('These branches have diverged.'
                                          ' Try merging, and then bind again.')
         if b.get_config().has_explicit_nickname():
 
=== modified file 'bzrlib/tests/blackbox/test_bound_branches.py'
--- bzrlib/tests/blackbox/test_bound_branches.py	2010-02-04 16:06:36 +0000
+++ bzrlib/tests/blackbox/test_bound_branches.py	2010-03-24 06:42:27 +0000
@@ -99,6 +99,8 @@
 
         d = BzrDir.open('')
         self.assertNotEqual(None, d.open_branch().get_master_branch())
+        # the relative URL is remembered: bug 135234
+        self.assertEqual("../base/", d.open_branch().get_bound_location())
 
         self.run_bzr('unbind')
         self.assertEqual(None, d.open_branch().get_master_branch())
 
=== modified file 'bzrlib/tests/blackbox/test_checkout.py'
--- bzrlib/tests/blackbox/test_checkout.py	2010-02-17 17:11:16 +0000
+++ bzrlib/tests/blackbox/test_checkout.py	2010-03-24 06:42:27 +0000
@@ -52,16 +52,24 @@
         # if we have a checkout, the branch base should be 'branch'
         source = bzrdir.BzrDir.open('branch')
         result = bzrdir.BzrDir.open('checkout')
-        self.assertEqual(source.open_branch().bzrdir.root_transport.base,
-                         result.open_branch().get_bound_location())
+        # the bound location should normally be given relative to the checkout
+        # <https://launchpad.net/bugs/135234>
+        self.assertEqual("../branch/",
+            result.open_branch().get_bound_location())
 
     def test_checkout_light_makes_checkout(self):
         self.run_bzr('checkout --lightweight branch checkout')
         # if we have a checkout, the branch base should be 'branch'
         source = bzrdir.BzrDir.open('branch')
         result = bzrdir.BzrDir.open('checkout')
-        self.assertEqual(source.open_branch().bzrdir.root_transport.base,
-                         result.open_branch().bzrdir.root_transport.base)
+        # the bound location should normally be given relative to the checkout
+        # <https://launchpad.net/bugs/135234>
+        self.assertEqual("../branch/",
+            result.get_branch_reference())
+        # the same branch is actually opened
+        self.assertEqual(
+            source.open_branch().bzrdir.root_transport.base,
+            result.open_branch().bzrdir.root_transport.base)
 
     def test_checkout_dash_r(self):
         self.run_bzr('checkout -r -2 branch checkout')
 
=== modified file 'bzrlib/tests/test_branch.py'
--- bzrlib/tests/test_branch.py	2010-03-11 13:14:37 +0000
+++ bzrlib/tests/test_branch.py	2010-03-24 06:42:27 +0000
@@ -444,15 +444,19 @@
         self.assertEqual(opened_branch.base, target_branch.base)
 
     def test_get_reference(self):
-        """For a BranchReference, get_reference should reutrn the location."""
+        """For a BranchReference, get_reference should return the location."""
         branch = self.make_branch('target')
         checkout = branch.create_checkout('checkout', lightweight=True)
         reference_url = branch.bzrdir.root_transport.abspath('') + '/'
         # if the api for create_checkout changes to return different checkout types
         # then this file read will fail.
-        self.assertFileEqual(reference_url, 'checkout/.bzr/branch/location')
-        self.assertEqual(reference_url,
+        self.assertFileEqual('../target/', 'checkout/.bzr/branch/location')
+        self.assertEqual('../target/',
             _mod_branch.BranchReferenceFormat().get_reference(checkout.bzrdir))
+        # can cope with a trailing newline
+        self.build_tree_contents([('checkout/.bzr/branch/location', '../foo/\n')])
+        self.assertEqual('../foo/', 
+            checkout.bzrdir.get_branch_reference())
 
 
 class TestHooks(tests.TestCase):