Bazaar

Merge lp:~lifeless/bzr/bug-375013 into lp:~bzr/bzr/trunk-old

  1. bug-375013
  2. Merge into trunk-old
Proposed by Robert Collins
Status: Merged
Merged at revision: not available
Proposed branch: lp:~lifeless/bzr/bug-375013
Merge into: lp:~bzr/bzr/trunk-old
Diff against target: 309 lines
To merge this branch: bzr merge lp:~lifeless/bzr/bug-375013
Reviewer Review Type Date Requested Status
Vincent Ladeuil Approve
Review via email: mp+9967@code.launchpad.net

This proposal supersedes a proposal from 2009-08-10.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote : Posted in a previous version of this proposal

Fix test_branch_from_trivial_stacked_branch_streaming_acceptance to work
with rich root formats, pending work on bug 375013.

Basically, committing to stacked branches is broken if there is any
actual content, but this test isn't trying to exercise that - and this
prevents changing the default in our test suite to a rich root format.

So I've fixed the test, and that may permit us to reduce the priority of
the bug (as its an existing defect in bzr, for any rich root format).

-Rob

Revision history for this message
Martin Pool (mbp) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Vincent Ladeuil (vila) wrote :

With the clarifications given on IRC, this sounds trivial enough.

review: Approve
Revision history for this message
John A Meinel (jameinel) wrote :

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

Robert Collins wrote:
> Robert Collins has proposed merging lp:~lifeless/bzr/bug-375013 into lp:bzr.
>
>
>
 === modified file 'bzrlib/__init__.py'
- --- bzrlib/__init__.py 2009-07-20 11:27:05 +0000
+++ bzrlib/__init__.py 2009-08-11 03:02:56 +0000
@@ -50,7 +50,7 @@
 # Python version 2.0 is (2, 0, 0, 'final', 0)." Additionally we use a
 # releaselevel of 'dev' for unreleased under-development code.

- -version_info = (1, 18, 0, 'dev', 0)
+version_info = (2, 0, 0, 'dev', 0)

^- So is this official then?

Certainly it seems spurious to bring this in as part of the rest of the
patch.

Especially given that you didn't bump "api_minimum_version" so I don't
see why it is relevant.

But hey, if we are officially doing it, then it doesn't really matter.

Anyway, if we are going to be rejecting commit, then we probably should
open another bug about:

"bzr merge BUNDLE" doesn't honor stacking invariants in stacked
repository, and do a similar "reject if local repository is stacked".

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

iEYEARECAAYFAkqBhUIACgkQJdeBCYSNAANZqQCggLCnR404ukng6cI2Cjn13cMp
sUIAoJJC3NO/V8JkbJqdgmrsIrq1mGZX
=8TWr
-----END PGP SIGNATURE-----

Revision history for this message
John A Meinel (jameinel) wrote :

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

...

> -version_info = (1, 18, 0, 'dev', 0)
> +version_info = (2, 0, 0, 'dev', 0)
>
> ^- So is this official then?
>
> Certainly it seems spurious to bring this in as part of the rest of the
> patch.
>

Please ignore me. It seems it is just MAD race again. I can see that
Martin did, indeed, update to 2.0 on bzr trunk, and probably you just
got that patch before Launchpad did... *sigh*.

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

iEYEARECAAYFAkqBmwYACgkQJdeBCYSNAAOFhwCgsHwF2S/WTdoJ0Jba7kDizPlA
gqEAmgPRGSsCU5bgEIb5Cu7DOjR9RosS
=s4L0
-----END PGP SIGNATURE-----

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-08-11 02:58:23 +0000
3+++ NEWS 2009-08-11 10:35:16 +0000
4@@ -12,6 +12,12 @@
5 Compatibility Breaks
6 ********************
7
8+* Committing directly to a stacked branch from a lightweight checkout will
9+ no longer work. In previous versions this would appear to work but would
10+ generate repositories with insufficient data to create deltas, leading
11+ to later errors when branching or reading from the repository.
12+ (Robert Collins, bug #375013)
13+
14 New Features
15 ************
16
17
18=== modified file 'bzrlib/repository.py'
19--- bzrlib/repository.py 2009-08-06 02:23:37 +0000
20+++ bzrlib/repository.py 2009-08-11 10:35:16 +0000
21@@ -1695,6 +1695,10 @@
22 :param revprops: Optional dictionary of revision properties.
23 :param revision_id: Optional revision id.
24 """
25+ if self._fallback_repositories:
26+ raise errors.BzrError("Cannot commit from a lightweight checkout "
27+ "to a stacked branch. See "
28+ "https://bugs.launchpad.net/bzr/+bug/375013 for details.")
29 result = self._commit_builder_class(self, parents, config,
30 timestamp, timezone, committer, revprops, revision_id)
31 self.start_write_group()
32
33=== modified file 'bzrlib/tests/blackbox/test_branch.py'
34--- bzrlib/tests/blackbox/test_branch.py 2009-08-03 04:19:03 +0000
35+++ bzrlib/tests/blackbox/test_branch.py 2009-08-11 10:35:16 +0000
36@@ -149,29 +149,6 @@
37 class TestBranchStacked(ExternalBase):
38 """Tests for branch --stacked"""
39
40- def check_shallow_branch(self, branch_revid, stacked_on):
41- """Assert that the branch 'newbranch' has been published correctly.
42-
43- :param stacked_on: url of a branch this one is stacked upon.
44- :param branch_revid: a revision id that should be the only
45- revision present in the stacked branch, and it should not be in
46- the reference branch.
47- """
48- new_branch = branch.Branch.open('newbranch')
49- # The branch refers to the mainline
50- self.assertEqual(stacked_on, new_branch.get_stacked_on_url())
51- # and the branch's work was pushed
52- self.assertTrue(new_branch.repository.has_revision(branch_revid))
53- # The newly committed revision shoud be present in the stacked branch,
54- # but not in the stacked-on branch. Because stacking is set up by the
55- # branch object, if we open the stacked branch's repository directly,
56- # bypassing the branch, we see only what's in the stacked repository.
57- stacked_repo = bzrdir.BzrDir.open('newbranch').open_repository()
58- stacked_repo_revisions = set(stacked_repo.all_revision_ids())
59- if len(stacked_repo_revisions) != 1:
60- self.fail("wrong revisions in stacked repository: %r"
61- % (stacked_repo_revisions,))
62-
63 def assertRevisionInRepository(self, repo_path, revid):
64 """Check that a revision is in a repository, disregarding stacking."""
65 repo = bzrdir.BzrDir.open(repo_path).open_repository()
66@@ -198,12 +175,14 @@
67 format='1.9')
68 branch_tree.branch.set_stacked_on_url(trunk_tree.branch.base)
69 # with some work on it
70- branch_tree.commit('moar work plz')
71+ work_tree = trunk_tree.branch.bzrdir.sprout('local').open_workingtree()
72+ work_tree.commit('moar work plz')
73+ work_tree.branch.push(branch_tree.branch)
74 # branching our local branch gives us a new stacked branch pointing at
75 # mainline.
76 out, err = self.run_bzr(['branch', 'branch', 'newbranch'])
77 self.assertEqual('', out)
78- self.assertEqual('Branched 1 revision(s).\n',
79+ self.assertEqual('Branched 2 revision(s).\n',
80 err)
81 # it should have preserved the branch format, and so it should be
82 # capable of supporting stacking, but not actually have a stacked_on
83@@ -222,7 +201,9 @@
84 format='1.9')
85 branch_tree.branch.set_stacked_on_url(trunk_tree.branch.base)
86 # with some work on it
87- branch_revid = branch_tree.commit('moar work plz')
88+ work_tree = trunk_tree.branch.bzrdir.sprout('local').open_workingtree()
89+ branch_revid = work_tree.commit('moar work plz')
90+ work_tree.branch.push(branch_tree.branch)
91 # you can chain branches on from there
92 out, err = self.run_bzr(['branch', 'branch', '--stacked', 'branch2'])
93 self.assertEqual('', out)
94@@ -231,7 +212,8 @@
95 self.assertEqual(branch_tree.branch.base,
96 branch.Branch.open('branch2').get_stacked_on_url())
97 branch2_tree = WorkingTree.open('branch2')
98- branch2_revid = branch2_tree.commit('work on second stacked branch')
99+ branch2_revid = work_tree.commit('work on second stacked branch')
100+ work_tree.branch.push(branch2_tree.branch)
101 self.assertRevisionInRepository('branch2', branch2_revid)
102 self.assertRevisionsInBranchRepository(
103 [trunk_revid, branch_revid, branch2_revid],
104@@ -250,9 +232,8 @@
105 self.assertEqual('Created new stacked branch referring to %s.\n' %
106 trunk_tree.branch.base, err)
107 self.assertRevisionNotInRepository('newbranch', original_revid)
108- new_tree = WorkingTree.open('newbranch')
109- new_revid = new_tree.commit('new work')
110- self.check_shallow_branch(new_revid, trunk_tree.branch.base)
111+ new_branch = branch.Branch.open('newbranch')
112+ self.assertEqual(trunk_tree.branch.base, new_branch.get_stacked_on_url())
113
114 def test_branch_stacked_from_smart_server(self):
115 # We can branch stacking on a smart server
116@@ -329,7 +310,9 @@
117 t.commit(message='commit %d' % count)
118 tree2 = t.branch.bzrdir.sprout('feature', stacked=True
119 ).open_workingtree()
120- tree2.commit('feature change')
121+ local_tree = t.branch.bzrdir.sprout('local-working').open_workingtree()
122+ local_tree.commit('feature change')
123+ local_tree.branch.push(tree2.branch)
124 self.reset_smart_call_log()
125 out, err = self.run_bzr(['branch', self.get_url('feature'),
126 'local-target'])
127
128=== modified file 'bzrlib/tests/per_branch/test_stacking.py'
129--- bzrlib/tests/per_branch/test_stacking.py 2009-08-05 02:30:59 +0000
130+++ bzrlib/tests/per_branch/test_stacking.py 2009-08-11 10:35:16 +0000
131@@ -150,8 +150,8 @@
132 raise TestNotApplicable(e)
133 # stacked repository
134 self.assertRevisionNotInRepository('newbranch', trunk_revid)
135- new_tree = new_dir.open_workingtree()
136- new_branch_revid = new_tree.commit('something local')
137+ tree = new_dir.open_branch().create_checkout('local')
138+ new_branch_revid = tree.commit('something local')
139 self.assertRevisionNotInRepository('mainline', new_branch_revid)
140 self.assertRevisionInRepository('newbranch', new_branch_revid)
141
142@@ -173,8 +173,8 @@
143 new_dir = remote_bzrdir.sprout('newbranch', stacked=True)
144 # stacked repository
145 self.assertRevisionNotInRepository('newbranch', trunk_revid)
146- new_tree = new_dir.open_workingtree()
147- new_branch_revid = new_tree.commit('something local')
148+ tree = new_dir.open_branch().create_checkout('local')
149+ new_branch_revid = tree.commit('something local')
150 self.assertRevisionNotInRepository('mainline', new_branch_revid)
151 self.assertRevisionInRepository('newbranch', new_branch_revid)
152
153@@ -333,7 +333,8 @@
154
155 def test_fetch_copies_from_stacked_on_and_stacked(self):
156 stacked, unstacked = self.prepare_stacked_on_fetch()
157- stacked.commit('second commit', rev_id='rev2')
158+ tree = stacked.branch.create_checkout('local')
159+ tree.commit('second commit', rev_id='rev2')
160 unstacked.fetch(stacked.branch.repository, 'rev2')
161 unstacked.get_revision('rev1')
162 unstacked.get_revision('rev2')
163@@ -355,13 +356,15 @@
164 stack_on.add('a')
165 stack_on.commit('base commit')
166 stacked_dir = stack_on.bzrdir.sprout('stacked', stacked=True)
167- stacked_tree = stacked_dir.open_workingtree()
168+ stacked_branch = stacked_dir.open_branch()
169+ local_tree = stack_on.bzrdir.sprout('local').open_workingtree()
170 for i in range(20):
171 text_lines[0] = 'changed in %d\n' % i
172- self.build_tree_contents([('stacked/a', ''.join(text_lines))])
173- stacked_tree.commit('commit %d' % i)
174- stacked_tree.branch.repository.pack()
175- check.check_dwim(stacked_tree.branch.base, False, True, True)
176+ self.build_tree_contents([('local/a', ''.join(text_lines))])
177+ local_tree.commit('commit %d' % i)
178+ local_tree.branch.push(stacked_branch)
179+ stacked_branch.repository.pack()
180+ check.check_dwim(stacked_branch.base, False, True, True)
181
182 def test_pull_delta_when_stacked(self):
183 if not self.branch_format.supports_stacking():
184@@ -470,7 +473,8 @@
185 except errors.NoWorkingTree:
186 stacked = stacked_dir.open_branch().create_checkout(
187 'stacked-checkout', lightweight=True)
188- stacked.commit('second commit', rev_id='rev2')
189+ tree = stacked.branch.create_checkout('local')
190+ tree.commit('second commit', rev_id='rev2')
191 # Sanity check: stacked's repo should not contain rev1, otherwise this
192 # test isn't testing what it's supposed to.
193 repo = stacked.branch.repository.bzrdir.open_repository()
194
195=== modified file 'bzrlib/tests/per_repository/test_commit_builder.py'
196--- bzrlib/tests/per_repository/test_commit_builder.py 2009-05-06 05:36:28 +0000
197+++ bzrlib/tests/per_repository/test_commit_builder.py 2009-08-11 10:35:16 +0000
198@@ -1267,3 +1267,19 @@
199 self.addCleanup(branch.repository.abort_write_group)
200 self.assertRaises(ValueError, builder.commit,
201 u'Invalid\r\ncommit message\r\n')
202+
203+ def test_stacked_repositories_reject_commit_builder(self):
204+ # As per bug 375013, committing to stacked repositories is currently
205+ # broken, so repositories with fallbacks refuse to hand out a commit
206+ # builder.
207+ repo_basis = self.make_repository('basis')
208+ branch = self.make_branch('local')
209+ repo_local = branch.repository
210+ try:
211+ repo_local.add_fallback_repository(repo_basis)
212+ except errors.UnstackableRepositoryFormat:
213+ raise tests.TestNotApplicable("not a stackable format.")
214+ repo_local.lock_write()
215+ self.addCleanup(repo_local.unlock)
216+ self.assertRaises(errors.BzrError, repo_local.get_commit_builder,
217+ branch, [], branch.get_config())
218
219=== modified file 'bzrlib/tests/per_repository_reference/test_check.py'
220--- bzrlib/tests/per_repository_reference/test_check.py 2009-03-23 14:59:43 +0000
221+++ bzrlib/tests/per_repository_reference/test_check.py 2009-08-11 10:35:16 +0000
222@@ -33,8 +33,9 @@
223 referring = self.make_branch_and_tree('referring')
224 readonly_base = self.readonly_repository('base')
225 referring.branch.repository.add_fallback_repository(readonly_base)
226- self.build_tree_contents([('referring/file', 'change')])
227- rev2_id = referring.commit('two')
228+ local_tree = referring.branch.create_checkout('local')
229+ self.build_tree_contents([('local/file', 'change')])
230+ rev2_id = local_tree.commit('two')
231 check_result = referring.branch.repository.check(
232 referring.branch.repository.all_revision_ids())
233 check_result.report_results(verbose=False)
234
235=== modified file 'bzrlib/tests/test_pack_repository.py'
236--- bzrlib/tests/test_pack_repository.py 2009-07-17 10:38:41 +0000
237+++ bzrlib/tests/test_pack_repository.py 2009-08-11 10:35:16 +0000
238@@ -865,7 +865,8 @@
239 base.commit('foo')
240 referencing = self.make_branch_and_tree('repo', format=self.get_format())
241 referencing.branch.repository.add_fallback_repository(base.branch.repository)
242- referencing.commit('bar')
243+ local_tree = referencing.branch.create_checkout('local')
244+ local_tree.commit('bar')
245 new_instance = referencing.bzrdir.open_repository()
246 new_instance.lock_read()
247 self.addCleanup(new_instance.unlock)
248@@ -884,13 +885,14 @@
249 # and max packs policy - so we are checking the policy is honoured
250 # in the test. But for now 11 commits is not a big deal in a single
251 # test.
252+ local_tree = tree.branch.create_checkout('local')
253 for x in range(9):
254- tree.commit('commit %s' % x)
255+ local_tree.commit('commit %s' % x)
256 # there should be 9 packs:
257 index = self.index_class(trans, 'pack-names', None)
258 self.assertEqual(9, len(list(index.iter_all_entries())))
259 # committing one more should coalesce to 1 of 10.
260- tree.commit('commit triggering pack')
261+ local_tree.commit('commit triggering pack')
262 index = self.index_class(trans, 'pack-names', None)
263 self.assertEqual(1, len(list(index.iter_all_entries())))
264 # packing should not damage data
265@@ -909,7 +911,7 @@
266 # in the obsolete_packs directory.
267 large_pack_name = list(index.iter_all_entries())[0][1][0]
268 # finally, committing again should not touch the large pack.
269- tree.commit('commit not triggering pack')
270+ local_tree.commit('commit not triggering pack')
271 index = self.index_class(trans, 'pack-names', None)
272 self.assertEqual(2, len(list(index.iter_all_entries())))
273 pack_names = [node[1][0] for node in index.iter_all_entries()]
274
275=== modified file 'bzrlib/tests/test_remote.py'
276--- bzrlib/tests/test_remote.py 2009-07-30 04:27:05 +0000
277+++ bzrlib/tests/test_remote.py 2009-08-11 10:35:16 +0000
278@@ -2651,7 +2651,8 @@
279 tree1.commit('rev1', rev_id='rev1')
280 tree2 = tree1.branch.bzrdir.sprout('tree2', stacked=True
281 ).open_workingtree()
282- tree2.commit('local changes make me feel good.')
283+ local_tree = tree2.branch.create_checkout('local')
284+ local_tree.commit('local changes make me feel good.')
285 branch2 = Branch.open(self.get_url('tree2'))
286 branch2.lock_read()
287 self.addCleanup(branch2.unlock)
288@@ -2727,7 +2728,8 @@
289 _, stacked = self.prepare_stacked_remote_branch()
290 tree = stacked.bzrdir.sprout('tree3', stacked=True
291 ).open_workingtree()
292- tree.commit('more local changes are better')
293+ local_tree = tree.branch.create_checkout('local-tree3')
294+ local_tree.commit('more local changes are better')
295 branch = Branch.open(self.get_url('tree3'))
296 branch.lock_read()
297 return None, branch
298@@ -2744,8 +2746,9 @@
299 # stacked upon sources in topological order.
300 rev_ord, expected_revs = self.get_ordered_revs('knit', 'topological')
301 self.assertEqual(expected_revs, rev_ord)
302- # Getting topological sort requires VFS calls still
303- self.assertLength(12, self.hpss_calls)
304+ # Getting topological sort requires VFS calls still - one of which is
305+ # pushing up from the bound branch.
306+ self.assertLength(13, self.hpss_calls)
307
308 def test_stacked_get_stream_groupcompress(self):
309 # Repository._get_source.get_stream() from a stacked repository with