Bazaar

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

  1. 135234-checkout-relpath
  2. Merge into bzr.dev
Proposed by Martin Pool
Status: Rejected
Rejected by: Martin Pool
Proposed branch: lp:~mbp/bzr/135234-checkout-relpath
Merge into: lp:bzr
Diff against target: 195 lines (+53/-18)
6 files modified
NEWS (+7/-3)
bzrlib/branch.py (+23/-7)
bzrlib/builtins.py (+2/-1)
bzrlib/tests/blackbox/test_bound_branches.py (+2/-0)
bzrlib/tests/blackbox/test_checkout.py (+12/-4)
bzrlib/tests/test_branch.py (+7/-3)
To merge this branch: bzr merge lp:~mbp/bzr/135234-checkout-relpath
Reviewer Review Type Date Requested Status
Ian Clatworthy Approve
Review via email: mp+21343@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

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 updated
5094. By Martin Pool

Fix typo

Revision history for this message
Martin Pool (mbp) wrote :

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 updated
5095. By Martin Pool

merge news

Revision history for this message
Martin Pool (mbp) wrote :

Fed to pqm

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

Revision history for this message
Martin Pool (mbp) wrote :

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:

Revision history for this message
Martin Pool (mbp) wrote :

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.

Revision history for this message
Ian Clatworthy (ian-clatworthy) wrote :

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

merge news

5094. By Martin Pool

Fix typo

5093. By Martin Pool

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

5092. By Martin Pool

Update BranchReference tests to expect relative URLs

5091. By Martin Pool

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

5090. By Martin Pool

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-03-22 13:59:33 +0000
3+++ NEWS 2010-03-24 06:42:27 +0000
4@@ -107,6 +107,9 @@
5 revspec as representing the first revision of the branch.
6 (Vincent Ladeuil, #519862)
7
8+* ``bzr mkdir DIR`` will not create DIR unless DIR's parent is a versioned
9+ directory. (Parth Malwankar, #138600)
10+
11 * ``bzr remove-tree`` can now remove multiple working trees.
12 (Jared Hance, Andrew Bennetts, #253137)
13
14@@ -138,13 +141,14 @@
15 to the Ubuntu Units Policy <https://wiki.ubuntu.com/UnitsPolicy>.
16 (Gordon Tyler, #514399)
17
18+* The master of a bound branch is now stored as a relative URL if
19+ possible, and similarly for lightweight checkouts.
20+ (Martin Pool, #135234)
21+
22 * Support kind markers for socket and fifo filesystem objects. This
23 prevents ``bzr status --short`` from crashing when those files are
24 present. (John Arbash Meinel, #303275)
25
26-* ``bzr mkdir DIR`` will not create DIR unless DIR's parent is a versioned
27- directory. (Parth Malwankar, #138600)
28-
29 * SSH child processes will now ignore SIGQUIT on nix systems so breaking into
30 the debugger won't kill the session.
31 (Martin <gzlist@googlemail.com>, #162502)
32
33=== modified file 'bzrlib/branch.py'
34--- bzrlib/branch.py 2010-03-22 13:59:33 +0000
35+++ bzrlib/branch.py 2010-03-24 06:42:27 +0000
36@@ -2004,7 +2004,8 @@
37 def get_reference(self, a_bzrdir):
38 """See BranchFormat.get_reference()."""
39 transport = a_bzrdir.get_branch_transport(None)
40- return transport.get_bytes('location')
41+ # ignore trailing newline
42+ return transport.get_bytes('location').rstrip()
43
44 def set_reference(self, a_bzrdir, to_branch):
45 """See BranchFormat.set_reference()."""
46@@ -2019,12 +2020,17 @@
47 raise errors.UninitializableFormat(self)
48 mutter('creating branch reference in %s', a_bzrdir.transport.base)
49 branch_transport = a_bzrdir.get_branch_transport(self, name=name)
50+ # NB: Don't add a trailing newline, because bzr prior to 2.2
51+ # can't cope with it
52 branch_transport.put_bytes('location',
53- target_branch.bzrdir.root_transport.base)
54+ urlutils.relative_url(a_bzrdir.root_transport.base,
55+ target_branch.base))
56 branch_transport.put_bytes('format', self.get_format_string())
57 return self.open(
58 a_bzrdir, name, _found=True,
59- possible_transports=[target_branch.bzrdir.root_transport])
60+ possible_transports=[a_bzrdir.root_transport,
61+ branch_transport,
62+ target_branch.bzrdir.root_transport])
63
64 def __init__(self):
65 super(BranchReferenceFormat, self).__init__()
66@@ -2064,8 +2070,9 @@
67 (format, self))
68 if location is None:
69 location = self.get_reference(a_bzrdir)
70+ abs_location = urlutils.join(a_bzrdir.root_transport.base, location)
71 real_bzrdir = bzrdir.BzrDir.open(
72- location, possible_transports=possible_transports)
73+ abs_location, possible_transports=possible_transports)
74 result = real_bzrdir.open_branch(name=name,
75 ignore_fallbacks=ignore_fallbacks)
76 # this changes the behaviour of result.clone to create a new reference
77@@ -2412,8 +2419,14 @@
78 if not bound_loc:
79 return None
80 try:
81- return Branch.open(bound_loc,
82- possible_transports=possible_transports)
83+ # if relative, interpret relative to my location
84+ abs_bound_loc = urlutils.join(self.base, bound_loc)
85+ if possible_transports is None:
86+ possible_transports = []
87+ if self._transport not in possible_transports:
88+ possible_transports += [self._transport]
89+ return Branch.open(abs_bound_loc,
90+ possible_transports=possible_transports)
91 except (errors.NotBranchError, errors.ConnectionError), e:
92 raise errors.BoundBranchConnectionFailure(
93 self, bound_loc, e)
94@@ -2458,7 +2471,10 @@
95 # last_rev is not in the other_last_rev history, AND
96 # other_last_rev is not in our history, and do it without pulling
97 # history around
98- self.set_bound_location(other.base)
99+
100+ # If possible, use a relative URL so that things will work if the
101+ # whole thing is moved, or accessed across a different transport.
102+ self.set_bound_location(urlutils.relative_url(self.base, other.base))
103
104 @needs_write_lock
105 def unbind(self):
106
107=== modified file 'bzrlib/builtins.py'
108--- bzrlib/builtins.py 2010-03-22 13:59:33 +0000
109+++ bzrlib/builtins.py 2010-03-24 06:42:27 +0000
110@@ -4652,10 +4652,11 @@
111 else:
112 raise errors.BzrCommandError('No location supplied '
113 'and no previous location known')
114+ # open to check the other one actually exists
115 b_other = Branch.open(location)
116 try:
117 b.bind(b_other)
118- except errors.DivergedBranches:
119+ except errors.DivergedBranches, e:
120 raise errors.BzrCommandError('These branches have diverged.'
121 ' Try merging, and then bind again.')
122 if b.get_config().has_explicit_nickname():
123
124=== modified file 'bzrlib/tests/blackbox/test_bound_branches.py'
125--- bzrlib/tests/blackbox/test_bound_branches.py 2010-02-04 16:06:36 +0000
126+++ bzrlib/tests/blackbox/test_bound_branches.py 2010-03-24 06:42:27 +0000
127@@ -99,6 +99,8 @@
128
129 d = BzrDir.open('')
130 self.assertNotEqual(None, d.open_branch().get_master_branch())
131+ # the relative URL is remembered: bug 135234
132+ self.assertEqual("../base/", d.open_branch().get_bound_location())
133
134 self.run_bzr('unbind')
135 self.assertEqual(None, d.open_branch().get_master_branch())
136
137=== modified file 'bzrlib/tests/blackbox/test_checkout.py'
138--- bzrlib/tests/blackbox/test_checkout.py 2010-02-17 17:11:16 +0000
139+++ bzrlib/tests/blackbox/test_checkout.py 2010-03-24 06:42:27 +0000
140@@ -52,16 +52,24 @@
141 # if we have a checkout, the branch base should be 'branch'
142 source = bzrdir.BzrDir.open('branch')
143 result = bzrdir.BzrDir.open('checkout')
144- self.assertEqual(source.open_branch().bzrdir.root_transport.base,
145- result.open_branch().get_bound_location())
146+ # the bound location should normally be given relative to the checkout
147+ # <https://launchpad.net/bugs/135234>
148+ self.assertEqual("../branch/",
149+ result.open_branch().get_bound_location())
150
151 def test_checkout_light_makes_checkout(self):
152 self.run_bzr('checkout --lightweight branch checkout')
153 # if we have a checkout, the branch base should be 'branch'
154 source = bzrdir.BzrDir.open('branch')
155 result = bzrdir.BzrDir.open('checkout')
156- self.assertEqual(source.open_branch().bzrdir.root_transport.base,
157- result.open_branch().bzrdir.root_transport.base)
158+ # the bound location should normally be given relative to the checkout
159+ # <https://launchpad.net/bugs/135234>
160+ self.assertEqual("../branch/",
161+ result.get_branch_reference())
162+ # the same branch is actually opened
163+ self.assertEqual(
164+ source.open_branch().bzrdir.root_transport.base,
165+ result.open_branch().bzrdir.root_transport.base)
166
167 def test_checkout_dash_r(self):
168 self.run_bzr('checkout -r -2 branch checkout')
169
170=== modified file 'bzrlib/tests/test_branch.py'
171--- bzrlib/tests/test_branch.py 2010-03-11 13:14:37 +0000
172+++ bzrlib/tests/test_branch.py 2010-03-24 06:42:27 +0000
173@@ -444,15 +444,19 @@
174 self.assertEqual(opened_branch.base, target_branch.base)
175
176 def test_get_reference(self):
177- """For a BranchReference, get_reference should reutrn the location."""
178+ """For a BranchReference, get_reference should return the location."""
179 branch = self.make_branch('target')
180 checkout = branch.create_checkout('checkout', lightweight=True)
181 reference_url = branch.bzrdir.root_transport.abspath('') + '/'
182 # if the api for create_checkout changes to return different checkout types
183 # then this file read will fail.
184- self.assertFileEqual(reference_url, 'checkout/.bzr/branch/location')
185- self.assertEqual(reference_url,
186+ self.assertFileEqual('../target/', 'checkout/.bzr/branch/location')
187+ self.assertEqual('../target/',
188 _mod_branch.BranchReferenceFormat().get_reference(checkout.bzrdir))
189+ # can cope with a trailing newline
190+ self.build_tree_contents([('checkout/.bzr/branch/location', '../foo/\n')])
191+ self.assertEqual('../foo/',
192+ checkout.bzrdir.get_branch_reference())
193
194
195 class TestHooks(tests.TestCase):