Merge lp:~spiv/bzr/checkout-tags-propagation-603395-2.2 into lp:bzr/2.2

Proposed by Andrew Bennetts
Status: Rejected
Rejected by: Andrew Bennetts
Proposed branch: lp:~spiv/bzr/checkout-tags-propagation-603395-2.2
Merge into: lp:bzr/2.2
Diff against target: 423 lines (+272/-16)
8 files modified
NEWS (+13/-0)
bzrlib/branch.py (+1/-1)
bzrlib/builtins.py (+3/-1)
bzrlib/tag.py (+69/-13)
bzrlib/tests/blackbox/test_merge.py (+1/-1)
bzrlib/tests/blackbox/test_tags.py (+12/-0)
bzrlib/tests/per_branch/test_tags.py (+170/-0)
bzrlib/tests/test_tag.py (+3/-0)
To merge this branch: bzr merge lp:~spiv/bzr/checkout-tags-propagation-603395-2.2
Reviewer Review Type Date Requested Status
Vincent Ladeuil Disapprove
John A Meinel Pending
Review via email: mp+40406@code.launchpad.net

This proposal supersedes a proposal from 2010-11-01.

Description of the change

This is part of the fix for bug 603395. It makes BasicTags.merge_to also merge to the master branch, if there is one. This is consistent with e.g. set_tag. bzr-builddeb directly calls that API, and presumably expects merge_to in a checkout branch to update the master as well. Because bzr-builddeb does call this API directly I don't believe <https://code.launchpad.net/~spiv/bzr/tags-commit-propagation-603395-2.2/+merge/39733> (which has already landed) alone is enough to fix the UDD bug. bzr-svn also calls this API, so it is likely affected too. I know John commented on the other proposal that he thought it alone would be sufficient, but I'm unconvinced. (And even if this isn't necessary it still seems like a reasonable change to me, the previous behaviour was too low on the Rusty API usability scale IMO.)

I've dealt with my reservations on the original merge proposal: merge_to now accepts an optional kwarg, ignore_master, that cmd_merge uses to avoid prematurely propagating changes to the master. So I think this avoids any chance of regressing at all on #99137 (bzr revert will still not remove the new tags from the child branch, but that's not a regression). This does mean that external implementations of merge_to should be updated. I've documented the API change in NEWS, and made sure the callsite in bzr that does pass that kwarg falls back to not passing the kwarg. I've tested the fallback manually with bzr-svn (which has an external implementation of merge_to), and it appears to work as intended.

The tests take care to deal with all the permutations, including those that can only be triggered by an unfixed bzr operating on the checkout. See the test comments for details.

To post a comment you must log in.
Revision history for this message
Andrew Bennetts (spiv) wrote : Posted in a previous version of this proposal

I should point out: this does have the perhaps unwanted side-effect that "bzr merge" can cause tags to be added the master even if the next command is "bzr commit". That's really <https://bugs.launchpad.net/bzr/+bug/99137> (tags are "permanently" propagated by merge), and this patch is just making the overall behaviour more consistent. I can see an argument that undoing the accidental limiting of the the effect of bug 99137 might not be acceptable in a stable branch, though.

A compromise would be to add a new optional argument to merge_to (or perhaps a whole new method) that callers can use to explicitly enable/disable propagation. I'm not sure if it would be better to default to enabled (so that plugins like builddeb can automatically benefit), or disabled (to minimise the change in behaviour, and also perhaps to avoid needing to tweak the API layers between cmd_merge and br.tags.merge_to to pass the flag). With 'bzr merge' not propagating we'd then rely on <https://code.launchpad.net/~spiv/bzr/tags-commit-propagation-603395-2.2> to propagate at commit time rather than merge time.

Revision history for this message
John A Meinel (jameinel) wrote : Posted in a previous version of this proposal

38 - dest_dict = to_tags.get_tag_dict()
39 - result, conflicts = self._reconcile_tags(source_dict, dest_dict,
40 - overwrite)
41 - if result != dest_dict:
42 - to_tags._set_tag_dict(result)
43 + master = to_tags.branch.get_master_branch()
44 + try:
45 + if master is not None:
46 + master.lock_write()
47 + conflicts = self._merge_to(to_tags, source_dict, overwrite)
48 + if master is not None:
49 + conflicts += self._merge_to(master.tags, source_dict,
50 + overwrite)
51 + finally:
52 + if master is not None:
53 + master.unlock()

Isn't this better written as:

master = to_tags.branch.get_master_branch()
if master is not None:
    master.lock_write()
    try:
      conflicts = ...
    finally:
      master.unlock()

Note that the way you wrote it, if we fail to take the write lock in the first place (no such permission), we end up still calling unlock.

I think you wrote it this way because you check the conflicts anyway, but the code looks clearer to me if you just add an 'else: conflicts = ...' to the above statement.

I suppose either way it is a bit clumsy, since you want the merge to fail immediately if you can't propagate the tags to the master branch before you try to merge to the local one...

I think the logic is otherwise ok.

review: Needs Fixing
Revision history for this message
Andrew Bennetts (spiv) wrote : Posted in a previous version of this proposal

John A Meinel wrote:
[...]
> Note that the way you wrote it, if we fail to take the write lock in
> the first place (no such permission), we end up still calling unlock.

Good point!

> I think you wrote it this way because you check the conflicts anyway,
> but the code looks clearer to me if you just add an 'else: conflicts =
> ...' to the above statement.
>
> I suppose either way it is a bit clumsy, since you want the merge to
> fail immediately if you can't propagate the tags to the master branch
> before you try to merge to the local one...

Really what I want is to use add_cleanup. It seemed like it was perhaps
overkill, so I didn't do that initially. But this review makes it clear
to me that it really would be worthwhile, despite incurring a larger
diff.

Revision history for this message
Vincent Ladeuil (vila) wrote :

Our policy for stable branches is a strict 'no API changes'.

I realize you have gone very far in ensuring compatibility and that this impacts UDD but this still doesn't justify breaking our policy IMHO.

Are there some other ways to propagate the tags to the master branch without this patch ?

I'm otherwise +1 for landing this on trunk.

review: Disapprove
Revision history for this message
Vincent Ladeuil (vila) wrote :

More thoughts on this as I realize it's always frustrating to have a mp rejected.

My concerns here of course are: what if we break something ? Would that means we'll get into a whack-a-mole frenzy updating bzr and plugins introducing even more problems ?

Instead of focusing on the right fix on trunk...

What are your thoughts here and what do you plan to do next ?

Is there some way we can have some real-world testing here ? May be asking Jelmer for bzr-svn fallouts ? Do we have a clear understanding of *which* plugins are (or could be) impacted there ?

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

I think I'm happy with the change, but I would agree with Vincent's hesitancy to land it in 2.2, vs just landing it in trunk.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2010-11-08 06:10:41 +0000
+++ NEWS 2010-11-09 06:33:49 +0000
@@ -42,6 +42,11 @@
42 root. Including the one that wasn't present in 2.1.42 root. Including the one that wasn't present in 2.1.
43 (Vincent Ladeuil, #646133)43 (Vincent Ladeuil, #646133)
4444
45* The ``branch.tags.merge_to(target_branch)`` API used by plugins such as
46 ``bzr-builddeb`` now propagates changes to the master branch of the
47 target branch (if there is one). This makes it consistent with the
48 other tag APIs. (Andrew Bennetts, #603395)
49
45* Using bzr with `lp:` urls behind an http proxy should work.50* Using bzr with `lp:` urls behind an http proxy should work.
46 (Robert Collins, #558343)51 (Robert Collins, #558343)
4752
@@ -58,6 +63,14 @@
58API Changes63API Changes
59***********64***********
6065
66* The ``merge_to`` method of ``branch.tags`` objects is now expected to
67 take a ``ignore_master=False`` keyword parameter. Plugins that
68 implement this method should add that parameter to their signature.
69 Callsites in bzr itself that pass that flag will still try to work with
70 the old signature (they catch TypeError and retry without the flag), but
71 this support is deprecated and will be removed eventually.
72 (Andrew Bennetts, #603395)
73
61Internals74Internals
62*********75*********
6376
6477
=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py 2010-08-13 07:32:06 +0000
+++ bzrlib/branch.py 2010-11-09 06:33:49 +0000
@@ -3326,7 +3326,7 @@
3326 if isinstance(format, remote.RemoteBranchFormat):3326 if isinstance(format, remote.RemoteBranchFormat):
3327 format._ensure_real()3327 format._ensure_real()
3328 return format._custom_format3328 return format._custom_format
3329 return format 3329 return format
33303330
3331 @needs_write_lock3331 @needs_write_lock
3332 def copy_content_into(self, revision_id=None):3332 def copy_content_into(self, revision_id=None):
33333333
=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py 2010-07-28 07:05:19 +0000
+++ bzrlib/builtins.py 2010-11-09 06:33:49 +0000
@@ -3980,7 +3980,9 @@
3980 if ((remember or tree.branch.get_submit_branch() is None) and3980 if ((remember or tree.branch.get_submit_branch() is None) and
3981 user_location is not None):3981 user_location is not None):
3982 tree.branch.set_submit_branch(other_branch.base)3982 tree.branch.set_submit_branch(other_branch.base)
3983 _merge_tags_if_possible(other_branch, tree.branch)3983 # Merge tags (but don't set them in the master branch yet, the user
3984 # might revert this merge). Commit will propagate them.
3985 _merge_tags_if_possible(other_branch, tree.branch, ignore_master=True)
3984 merger = _mod_merge.Merger.from_revision_ids(pb, tree,3986 merger = _mod_merge.Merger.from_revision_ids(pb, tree,
3985 other_revision_id, base_revision_id, other_branch, base_branch)3987 other_revision_id, base_revision_id, other_branch, base_branch)
3986 if other_path != '':3988 if other_path != '':
39873989
=== modified file 'bzrlib/tag.py'
--- bzrlib/tag.py 2010-03-25 09:39:03 +0000
+++ bzrlib/tag.py 2010-11-09 06:33:49 +0000
@@ -28,7 +28,9 @@
2828
29from bzrlib import (29from bzrlib import (
30 bencode,30 bencode,
31 cleanup,
31 errors,32 errors,
33 symbol_versioning,
32 trace,34 trace,
33 )35 )
3436
@@ -57,7 +59,7 @@
57 lookup_tag = _not_supported59 lookup_tag = _not_supported
58 delete_tag = _not_supported60 delete_tag = _not_supported
5961
60 def merge_to(self, to_tags, overwrite=False):62 def merge_to(self, to_tags, overwrite=False, ignore_master=False):
61 # we never have anything to copy63 # we never have anything to copy
62 pass64 pass
6365
@@ -177,7 +179,7 @@
177 raise ValueError("failed to deserialize tag dictionary %r: %s"179 raise ValueError("failed to deserialize tag dictionary %r: %s"
178 % (tag_content, e))180 % (tag_content, e))
179181
180 def merge_to(self, to_tags, overwrite=False):182 def merge_to(self, to_tags, overwrite=False, ignore_master=False):
181 """Copy tags between repositories if necessary and possible.183 """Copy tags between repositories if necessary and possible.
182184
183 This method has common command-line behaviour about handling185 This method has common command-line behaviour about handling
@@ -188,11 +190,19 @@
188190
189 :param to_tags: Branch to receive these tags191 :param to_tags: Branch to receive these tags
190 :param overwrite: Overwrite conflicting tags in the target branch192 :param overwrite: Overwrite conflicting tags in the target branch
193 :param ignore_master: Do not modify the tags in the target's master
194 branch (if any). Default is false (so the master will be updated).
195 New in bzr 2.2.2 and bzr 2.3.
191196
192 :returns: A list of tags that conflicted, each of which is197 :returns: A list of tags that conflicted, each of which is
193 (tagname, source_target, dest_target), or None if no copying was198 (tagname, source_target, dest_target), or None if no copying was
194 done.199 done.
195 """200 """
201 operation = cleanup.OperationWithCleanups(self._merge_to_operation)
202 return operation.run(to_tags, overwrite, ignore_master)
203
204 def _merge_to_operation(self, operation, to_tags, overwrite, ignore_master):
205 add_cleanup = operation.add_cleanup
196 if self.branch == to_tags.branch:206 if self.branch == to_tags.branch:
197 return207 return
198 if not self.branch.supports_tags():208 if not self.branch.supports_tags():
@@ -203,15 +213,40 @@
203 # no tags in the source, and we don't want to clobber anything213 # no tags in the source, and we don't want to clobber anything
204 # that's in the destination214 # that's in the destination
205 return215 return
206 to_tags.branch.lock_write()216 # We merge_to both master and child individually.
207 try:217 #
208 dest_dict = to_tags.get_tag_dict()218 # It's possible for master and child to have differing sets of
209 result, conflicts = self._reconcile_tags(source_dict, dest_dict,219 # tags, in which case it's possible to have different sets of
210 overwrite)220 # conflicts. We report the union of both conflict sets. In
211 if result != dest_dict:221 # that case it's likely the child and master have accepted
212 to_tags._set_tag_dict(result)222 # different tags from the source, which may be a surprising result, but
213 finally:223 # the best we can do in the circumstances.
214 to_tags.branch.unlock()224 #
225 # Ideally we'd improve this API to report the different conflicts
226 # more clearly to the caller, but we don't want to break plugins
227 # such as bzr-builddeb that use this API.
228 add_cleanup(to_tags.branch.lock_write().unlock)
229 if ignore_master:
230 master = None
231 else:
232 master = to_tags.branch.get_master_branch()
233 if master is not None:
234 add_cleanup(master.lock_write().unlock)
235 conflicts = self._merge_to(to_tags, source_dict, overwrite)
236 if master is not None:
237 conflicts += self._merge_to(master.tags, source_dict,
238 overwrite)
239 # We use set() to remove any duplicate conflicts from the master
240 # branch. We then use list() to keep the behaviour as close to 2.2.1
241 # and earlier as possible, to minimise potential compatibility issues.
242 return list(set(conflicts))
243
244 def _merge_to(self, to_tags, source_dict, overwrite):
245 dest_dict = to_tags.get_tag_dict()
246 result, conflicts = self._reconcile_tags(source_dict, dest_dict,
247 overwrite)
248 if result != dest_dict:
249 to_tags._set_tag_dict(result)
215 return conflicts250 return conflicts
216251
217 def rename_revisions(self, rename_map):252 def rename_revisions(self, rename_map):
@@ -249,6 +284,27 @@
249 return result, conflicts284 return result, conflicts
250285
251286
252def _merge_tags_if_possible(from_branch, to_branch):287def _merge_tags_if_possible(from_branch, to_branch, ignore_master=False):
253 from_branch.tags.merge_to(to_branch.tags)288 # Try hard to support merge_to implementations that don't expect
289 # 'ignore_master' (new in bzr 2.2.2/2.3). First, if the flag isn't set
290 # then we can safely avoid passing ignore_master at all.
291 if not ignore_master:
292 from_branch.tags.merge_to(to_branch.tags)
293 return
294 # If the flag is set, try to pass it, but be ready to catch TypeError.
295 try:
296 from_branch.tags.merge_to(to_branch.tags, ignore_master=ignore_master)
297 except TypeError:
298 # Probably this implementation of 'merge_to' is from a plugin that
299 # doesn't expect the 'ignore_master' keyword argument (e.g. bzr-svn
300 # 1.0.4). There's a small risk that the TypeError is actually caused
301 # by a completely different problem (which is why we don't catch it for
302 # the ignore_master=False case), but even then there's probably no harm
303 # in calling a second time.
304 symbol_versioning.warn(
305 symbol_versioning.deprecated_in((2,2,2)) % (
306 "Tags.merge_to (of %r) that doesn't accept ignore_master kwarg"
307 % (from_branch.tags,),),
308 DeprecationWarning)
309 from_branch.tags.merge_to(to_branch.tags)
254310
255311
=== modified file 'bzrlib/tests/blackbox/test_merge.py'
--- bzrlib/tests/blackbox/test_merge.py 2010-05-20 18:23:17 +0000
+++ bzrlib/tests/blackbox/test_merge.py 2010-11-09 06:33:49 +0000
@@ -23,9 +23,9 @@
2323
24from bzrlib import (24from bzrlib import (
25 branch,25 branch,
26 branchbuilder,
26 bzrdir,27 bzrdir,
27 conflicts,28 conflicts,
28 errors,
29 merge_directive,29 merge_directive,
30 osutils,30 osutils,
31 tests,31 tests,
3232
=== modified file 'bzrlib/tests/blackbox/test_tags.py'
--- bzrlib/tests/blackbox/test_tags.py 2010-11-01 05:21:13 +0000
+++ bzrlib/tests/blackbox/test_tags.py 2010-11-09 06:33:49 +0000
@@ -132,6 +132,18 @@
132 builder.build_commit(message='Commit in fork.', rev_id='fork-1')132 builder.build_commit(message='Commit in fork.', rev_id='fork-1')
133 return fork133 return fork
134134
135 def test_merge_without_commit_does_not_propagate_tags_to_master(self):
136 """'bzr merge' alone does not propagate tags to a master branch.
137
138 (If the user runs 'bzr commit', then that is when the tags from the
139 merge are propagated.)
140 """
141 master, child = self.make_master_and_checkout()
142 fork = self.make_fork(master)
143 fork.tags.set_tag('new-tag', fork.last_revision())
144 self.run_bzr(['merge', '../fork'], working_dir='child')
145 self.assertEqual({}, master.tags.get_tag_dict())
146
135 def test_commit_in_heavyweight_checkout_copies_tags_to_master(self):147 def test_commit_in_heavyweight_checkout_copies_tags_to_master(self):
136 master, child = self.make_master_and_checkout()148 master, child = self.make_master_and_checkout()
137 fork = self.make_fork(master)149 fork = self.make_fork(master)
138150
=== modified file 'bzrlib/tests/per_branch/test_tags.py'
--- bzrlib/tests/per_branch/test_tags.py 2010-03-12 02:14:56 +0000
+++ bzrlib/tests/per_branch/test_tags.py 2010-11-09 06:33:49 +0000
@@ -144,6 +144,176 @@
144 b1.tags.merge_to(b2.tags)144 b1.tags.merge_to(b2.tags)
145145
146146
147class TestTagsMergeToInCheckouts(per_branch.TestCaseWithBranch):
148 """Tests for checkout.branch.tags.merge_to.
149
150 In particular this exercises variations in tag conflicts in the master
151 branch and/or the checkout (child). It may seem strange to have different
152 tags in the child and master, but 'bzr merge' intentionally updates the
153 child and not the master (instead the next 'bzr commit', if the user
154 decides to commit, will update the master). Also, merge_to in bzr < 2.2.2
155 didn't propagate changes to the master, and current bzr versions may find
156 themselves operating on checkouts touched by older bzrs
157
158 So we need to make sure bzr copes gracefully with differing tags in the
159 master versus the child.
160
161 See also <https://bugs.launchpad.net/bzr/+bug/603395>.
162 """
163
164 def setUp(self):
165 super(TestTagsMergeToInCheckouts, self).setUp()
166 branch1 = self.make_branch('tags-probe')
167 if not branch1._format.supports_tags():
168 raise tests.TestSkipped(
169 "format %s doesn't support tags" % branch1._format)
170 branch2 = self.make_branch('bind-probe')
171 try:
172 branch2.bind(branch1)
173 except errors.UpgradeRequired:
174 raise tests.TestNotApplicable(
175 "format %s doesn't support bound branches" % branch2._format)
176
177 def test_merge_to_propagates_tags(self):
178 """merge_to(child) also merges tags to the master."""
179 master = self.make_branch('master')
180 other = self.make_branch('other')
181 other.tags.set_tag('foo', 'rev-1')
182 child = self.make_branch('child')
183 child.bind(master)
184 child.update()
185 other.tags.merge_to(child.tags)
186 self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
187 self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
188
189 def test_ignore_master_disables_tag_propagation(self):
190 """merge_to(child, ignore_master=True) does not merge tags to the
191 master.
192 """
193 master = self.make_branch('master')
194 other = self.make_branch('other')
195 other.tags.set_tag('foo', 'rev-1')
196 child = self.make_branch('child')
197 child.bind(master)
198 child.update()
199 other.tags.merge_to(child.tags, ignore_master=True)
200 self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
201 self.assertRaises(errors.NoSuchTag, master.tags.lookup_tag, 'foo')
202
203 def test_merge_to_overwrite_conflict_in_master(self):
204 """merge_to(child, overwrite=True) overwrites any conflicting tags in
205 the master.
206 """
207 master = self.make_branch('master')
208 other = self.make_branch('other')
209 other.tags.set_tag('foo', 'rev-1')
210 child = self.make_branch('child')
211 child.bind(master)
212 child.update()
213 master.tags.set_tag('foo', 'rev-2')
214 tag_conflicts = other.tags.merge_to(child.tags, overwrite=True)
215 self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
216 self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
217 self.assertLength(0, tag_conflicts)
218
219 def test_merge_to_overwrite_conflict_in_child_and_master(self):
220 """merge_to(child, overwrite=True) overwrites any conflicting tags in
221 both the child and the master.
222 """
223 master = self.make_branch('master')
224 master.tags.set_tag('foo', 'rev-2')
225 other = self.make_branch('other')
226 other.tags.set_tag('foo', 'rev-1')
227 child = self.make_branch('child')
228 child.bind(master)
229 child.update()
230 tag_conflicts = other.tags.merge_to(child.tags, overwrite=True)
231 self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
232 self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
233 self.assertLength(0, tag_conflicts)
234
235 def test_merge_to_conflict_in_child_only(self):
236 """When new_tags.merge_to(child.tags) conflicts with the child but not
237 the master, a conflict is reported and the child receives the new tag.
238 """
239 master = self.make_branch('master')
240 master.tags.set_tag('foo', 'rev-2')
241 other = self.make_branch('other')
242 other.tags.set_tag('foo', 'rev-1')
243 child = self.make_branch('child')
244 child.bind(master)
245 child.update()
246 master.tags.delete_tag('foo')
247 tag_conflicts = other.tags.merge_to(child.tags)
248 # Conflict in child, so it is unchanged.
249 self.assertEquals('rev-2', child.tags.lookup_tag('foo'))
250 # No conflict in the master, so the 'foo' tag equals other's value here.
251 self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
252 # The conflict is reported.
253 self.assertEquals([(u'foo', 'rev-1', 'rev-2')], tag_conflicts)
254
255 def test_merge_to_conflict_in_master_only(self):
256 """When new_tags.merge_to(child.tags) conflicts with the master but not
257 the child, a conflict is reported and the child receives the new tag.
258 """
259 master = self.make_branch('master')
260 other = self.make_branch('other')
261 other.tags.set_tag('foo', 'rev-1')
262 child = self.make_branch('child')
263 child.bind(master)
264 child.update()
265 master.tags.set_tag('foo', 'rev-2')
266 tag_conflicts = other.tags.merge_to(child.tags)
267 # No conflict in the child, so the 'foo' tag equals other's value here.
268 self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
269 # Conflict in master, so it is unchanged.
270 self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
271 # The conflict is reported.
272 self.assertEquals([(u'foo', 'rev-1', 'rev-2')], tag_conflicts)
273
274 def test_merge_to_same_conflict_in_master_and_child(self):
275 """When new_tags.merge_to(child.tags) conflicts the same way with the
276 master and the child a single conflict is reported.
277 """
278 master = self.make_branch('master')
279 master.tags.set_tag('foo', 'rev-2')
280 other = self.make_branch('other')
281 other.tags.set_tag('foo', 'rev-1')
282 child = self.make_branch('child')
283 child.bind(master)
284 child.update()
285 tag_conflicts = other.tags.merge_to(child.tags)
286 # Both master and child conflict, so both stay as rev-2
287 self.assertEquals('rev-2', child.tags.lookup_tag('foo'))
288 self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
289 # The conflict is reported exactly once, even though it occurs in both
290 # master and child.
291 self.assertEquals([(u'foo', 'rev-1', 'rev-2')], tag_conflicts)
292
293 def test_merge_to_different_conflict_in_master_and_child(self):
294 """When new_tags.merge_to(child.tags) conflicts differently in the
295 master and the child both conflicts are reported.
296 """
297 master = self.make_branch('master')
298 master.tags.set_tag('foo', 'rev-2')
299 other = self.make_branch('other')
300 other.tags.set_tag('foo', 'rev-1')
301 child = self.make_branch('child')
302 child.bind(master)
303 child.update()
304 # We use the private method _set_tag_dict because normally bzr tries to
305 # avoid this scenario.
306 child.tags._set_tag_dict({'foo': 'rev-3'})
307 tag_conflicts = other.tags.merge_to(child.tags)
308 # Both master and child conflict, so both stay as they were.
309 self.assertEquals('rev-3', child.tags.lookup_tag('foo'))
310 self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
311 # Both conflicts are reported.
312 self.assertEquals(
313 [(u'foo', 'rev-1', 'rev-2'), (u'foo', 'rev-1', 'rev-3')],
314 sorted(tag_conflicts))
315
316
147class TestUnsupportedTags(per_branch.TestCaseWithBranch):317class TestUnsupportedTags(per_branch.TestCaseWithBranch):
148 """Formats that don't support tags should give reasonable errors."""318 """Formats that don't support tags should give reasonable errors."""
149319
150320
=== modified file 'bzrlib/tests/test_tag.py'
--- bzrlib/tests/test_tag.py 2010-03-25 09:39:03 +0000
+++ bzrlib/tests/test_tag.py 2010-11-09 06:33:49 +0000
@@ -120,6 +120,9 @@
120120
121121
122class TestTagsInCheckouts(TestCaseWithTransport):122class TestTagsInCheckouts(TestCaseWithTransport):
123 """Tests for how tags are synchronised between the master and child branch
124 of a checkout.
125 """
123126
124 def test_update_tag_into_checkout(self):127 def test_update_tag_into_checkout(self):
125 # checkouts are directly connected to the tags of their master branch:128 # checkouts are directly connected to the tags of their master branch:

Subscribers

People subscribed via source and target branches