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.

5115. By Andrew Bennetts

Remove inaccurate test comment.

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.

Unmerged revisions

5115. By Andrew Bennetts

Remove inaccurate test comment.

5114. By Andrew Bennetts

Add unit test for ignore_master=True, and improve other test comments.

5113. By Andrew Bennetts

Cosmetic tweak to deprecation.

5112. By Andrew Bennetts

Move bound_branch.tags.merge_to tests to per_branch.

5111. By Andrew Bennetts

Move new test to bb.test_tag, to avoid repeating test infrastructure.

5110. By Andrew Bennetts

Add blackbox test to demonstrate that 'merge' alone still does not propagate tags to the master.

5109. By Andrew Bennetts

Merge lp:bzr/2.2.

5108. By Andrew Bennetts

Don't propagate tags to the master branch during cmd_merge.

5107. By Andrew Bennetts

Use add_cleanup for simpler and more correct unlocking.

5106. By Andrew Bennetts

More tests, more description of test intent, and specify exactly what happens with duplicate vs. different conflicts.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-11-08 06:10:41 +0000
3+++ NEWS 2010-11-09 06:33:49 +0000
4@@ -42,6 +42,11 @@
5 root. Including the one that wasn't present in 2.1.
6 (Vincent Ladeuil, #646133)
7
8+* The ``branch.tags.merge_to(target_branch)`` API used by plugins such as
9+ ``bzr-builddeb`` now propagates changes to the master branch of the
10+ target branch (if there is one). This makes it consistent with the
11+ other tag APIs. (Andrew Bennetts, #603395)
12+
13 * Using bzr with `lp:` urls behind an http proxy should work.
14 (Robert Collins, #558343)
15
16@@ -58,6 +63,14 @@
17 API Changes
18 ***********
19
20+* The ``merge_to`` method of ``branch.tags`` objects is now expected to
21+ take a ``ignore_master=False`` keyword parameter. Plugins that
22+ implement this method should add that parameter to their signature.
23+ Callsites in bzr itself that pass that flag will still try to work with
24+ the old signature (they catch TypeError and retry without the flag), but
25+ this support is deprecated and will be removed eventually.
26+ (Andrew Bennetts, #603395)
27+
28 Internals
29 *********
30
31
32=== modified file 'bzrlib/branch.py'
33--- bzrlib/branch.py 2010-08-13 07:32:06 +0000
34+++ bzrlib/branch.py 2010-11-09 06:33:49 +0000
35@@ -3326,7 +3326,7 @@
36 if isinstance(format, remote.RemoteBranchFormat):
37 format._ensure_real()
38 return format._custom_format
39- return format
40+ return format
41
42 @needs_write_lock
43 def copy_content_into(self, revision_id=None):
44
45=== modified file 'bzrlib/builtins.py'
46--- bzrlib/builtins.py 2010-07-28 07:05:19 +0000
47+++ bzrlib/builtins.py 2010-11-09 06:33:49 +0000
48@@ -3980,7 +3980,9 @@
49 if ((remember or tree.branch.get_submit_branch() is None) and
50 user_location is not None):
51 tree.branch.set_submit_branch(other_branch.base)
52- _merge_tags_if_possible(other_branch, tree.branch)
53+ # Merge tags (but don't set them in the master branch yet, the user
54+ # might revert this merge). Commit will propagate them.
55+ _merge_tags_if_possible(other_branch, tree.branch, ignore_master=True)
56 merger = _mod_merge.Merger.from_revision_ids(pb, tree,
57 other_revision_id, base_revision_id, other_branch, base_branch)
58 if other_path != '':
59
60=== modified file 'bzrlib/tag.py'
61--- bzrlib/tag.py 2010-03-25 09:39:03 +0000
62+++ bzrlib/tag.py 2010-11-09 06:33:49 +0000
63@@ -28,7 +28,9 @@
64
65 from bzrlib import (
66 bencode,
67+ cleanup,
68 errors,
69+ symbol_versioning,
70 trace,
71 )
72
73@@ -57,7 +59,7 @@
74 lookup_tag = _not_supported
75 delete_tag = _not_supported
76
77- def merge_to(self, to_tags, overwrite=False):
78+ def merge_to(self, to_tags, overwrite=False, ignore_master=False):
79 # we never have anything to copy
80 pass
81
82@@ -177,7 +179,7 @@
83 raise ValueError("failed to deserialize tag dictionary %r: %s"
84 % (tag_content, e))
85
86- def merge_to(self, to_tags, overwrite=False):
87+ def merge_to(self, to_tags, overwrite=False, ignore_master=False):
88 """Copy tags between repositories if necessary and possible.
89
90 This method has common command-line behaviour about handling
91@@ -188,11 +190,19 @@
92
93 :param to_tags: Branch to receive these tags
94 :param overwrite: Overwrite conflicting tags in the target branch
95+ :param ignore_master: Do not modify the tags in the target's master
96+ branch (if any). Default is false (so the master will be updated).
97+ New in bzr 2.2.2 and bzr 2.3.
98
99 :returns: A list of tags that conflicted, each of which is
100 (tagname, source_target, dest_target), or None if no copying was
101 done.
102 """
103+ operation = cleanup.OperationWithCleanups(self._merge_to_operation)
104+ return operation.run(to_tags, overwrite, ignore_master)
105+
106+ def _merge_to_operation(self, operation, to_tags, overwrite, ignore_master):
107+ add_cleanup = operation.add_cleanup
108 if self.branch == to_tags.branch:
109 return
110 if not self.branch.supports_tags():
111@@ -203,15 +213,40 @@
112 # no tags in the source, and we don't want to clobber anything
113 # that's in the destination
114 return
115- to_tags.branch.lock_write()
116- try:
117- dest_dict = to_tags.get_tag_dict()
118- result, conflicts = self._reconcile_tags(source_dict, dest_dict,
119- overwrite)
120- if result != dest_dict:
121- to_tags._set_tag_dict(result)
122- finally:
123- to_tags.branch.unlock()
124+ # We merge_to both master and child individually.
125+ #
126+ # It's possible for master and child to have differing sets of
127+ # tags, in which case it's possible to have different sets of
128+ # conflicts. We report the union of both conflict sets. In
129+ # that case it's likely the child and master have accepted
130+ # different tags from the source, which may be a surprising result, but
131+ # the best we can do in the circumstances.
132+ #
133+ # Ideally we'd improve this API to report the different conflicts
134+ # more clearly to the caller, but we don't want to break plugins
135+ # such as bzr-builddeb that use this API.
136+ add_cleanup(to_tags.branch.lock_write().unlock)
137+ if ignore_master:
138+ master = None
139+ else:
140+ master = to_tags.branch.get_master_branch()
141+ if master is not None:
142+ add_cleanup(master.lock_write().unlock)
143+ conflicts = self._merge_to(to_tags, source_dict, overwrite)
144+ if master is not None:
145+ conflicts += self._merge_to(master.tags, source_dict,
146+ overwrite)
147+ # We use set() to remove any duplicate conflicts from the master
148+ # branch. We then use list() to keep the behaviour as close to 2.2.1
149+ # and earlier as possible, to minimise potential compatibility issues.
150+ return list(set(conflicts))
151+
152+ def _merge_to(self, to_tags, source_dict, overwrite):
153+ dest_dict = to_tags.get_tag_dict()
154+ result, conflicts = self._reconcile_tags(source_dict, dest_dict,
155+ overwrite)
156+ if result != dest_dict:
157+ to_tags._set_tag_dict(result)
158 return conflicts
159
160 def rename_revisions(self, rename_map):
161@@ -249,6 +284,27 @@
162 return result, conflicts
163
164
165-def _merge_tags_if_possible(from_branch, to_branch):
166- from_branch.tags.merge_to(to_branch.tags)
167+def _merge_tags_if_possible(from_branch, to_branch, ignore_master=False):
168+ # Try hard to support merge_to implementations that don't expect
169+ # 'ignore_master' (new in bzr 2.2.2/2.3). First, if the flag isn't set
170+ # then we can safely avoid passing ignore_master at all.
171+ if not ignore_master:
172+ from_branch.tags.merge_to(to_branch.tags)
173+ return
174+ # If the flag is set, try to pass it, but be ready to catch TypeError.
175+ try:
176+ from_branch.tags.merge_to(to_branch.tags, ignore_master=ignore_master)
177+ except TypeError:
178+ # Probably this implementation of 'merge_to' is from a plugin that
179+ # doesn't expect the 'ignore_master' keyword argument (e.g. bzr-svn
180+ # 1.0.4). There's a small risk that the TypeError is actually caused
181+ # by a completely different problem (which is why we don't catch it for
182+ # the ignore_master=False case), but even then there's probably no harm
183+ # in calling a second time.
184+ symbol_versioning.warn(
185+ symbol_versioning.deprecated_in((2,2,2)) % (
186+ "Tags.merge_to (of %r) that doesn't accept ignore_master kwarg"
187+ % (from_branch.tags,),),
188+ DeprecationWarning)
189+ from_branch.tags.merge_to(to_branch.tags)
190
191
192=== modified file 'bzrlib/tests/blackbox/test_merge.py'
193--- bzrlib/tests/blackbox/test_merge.py 2010-05-20 18:23:17 +0000
194+++ bzrlib/tests/blackbox/test_merge.py 2010-11-09 06:33:49 +0000
195@@ -23,9 +23,9 @@
196
197 from bzrlib import (
198 branch,
199+ branchbuilder,
200 bzrdir,
201 conflicts,
202- errors,
203 merge_directive,
204 osutils,
205 tests,
206
207=== modified file 'bzrlib/tests/blackbox/test_tags.py'
208--- bzrlib/tests/blackbox/test_tags.py 2010-11-01 05:21:13 +0000
209+++ bzrlib/tests/blackbox/test_tags.py 2010-11-09 06:33:49 +0000
210@@ -132,6 +132,18 @@
211 builder.build_commit(message='Commit in fork.', rev_id='fork-1')
212 return fork
213
214+ def test_merge_without_commit_does_not_propagate_tags_to_master(self):
215+ """'bzr merge' alone does not propagate tags to a master branch.
216+
217+ (If the user runs 'bzr commit', then that is when the tags from the
218+ merge are propagated.)
219+ """
220+ master, child = self.make_master_and_checkout()
221+ fork = self.make_fork(master)
222+ fork.tags.set_tag('new-tag', fork.last_revision())
223+ self.run_bzr(['merge', '../fork'], working_dir='child')
224+ self.assertEqual({}, master.tags.get_tag_dict())
225+
226 def test_commit_in_heavyweight_checkout_copies_tags_to_master(self):
227 master, child = self.make_master_and_checkout()
228 fork = self.make_fork(master)
229
230=== modified file 'bzrlib/tests/per_branch/test_tags.py'
231--- bzrlib/tests/per_branch/test_tags.py 2010-03-12 02:14:56 +0000
232+++ bzrlib/tests/per_branch/test_tags.py 2010-11-09 06:33:49 +0000
233@@ -144,6 +144,176 @@
234 b1.tags.merge_to(b2.tags)
235
236
237+class TestTagsMergeToInCheckouts(per_branch.TestCaseWithBranch):
238+ """Tests for checkout.branch.tags.merge_to.
239+
240+ In particular this exercises variations in tag conflicts in the master
241+ branch and/or the checkout (child). It may seem strange to have different
242+ tags in the child and master, but 'bzr merge' intentionally updates the
243+ child and not the master (instead the next 'bzr commit', if the user
244+ decides to commit, will update the master). Also, merge_to in bzr < 2.2.2
245+ didn't propagate changes to the master, and current bzr versions may find
246+ themselves operating on checkouts touched by older bzrs
247+
248+ So we need to make sure bzr copes gracefully with differing tags in the
249+ master versus the child.
250+
251+ See also <https://bugs.launchpad.net/bzr/+bug/603395>.
252+ """
253+
254+ def setUp(self):
255+ super(TestTagsMergeToInCheckouts, self).setUp()
256+ branch1 = self.make_branch('tags-probe')
257+ if not branch1._format.supports_tags():
258+ raise tests.TestSkipped(
259+ "format %s doesn't support tags" % branch1._format)
260+ branch2 = self.make_branch('bind-probe')
261+ try:
262+ branch2.bind(branch1)
263+ except errors.UpgradeRequired:
264+ raise tests.TestNotApplicable(
265+ "format %s doesn't support bound branches" % branch2._format)
266+
267+ def test_merge_to_propagates_tags(self):
268+ """merge_to(child) also merges tags to the master."""
269+ master = self.make_branch('master')
270+ other = self.make_branch('other')
271+ other.tags.set_tag('foo', 'rev-1')
272+ child = self.make_branch('child')
273+ child.bind(master)
274+ child.update()
275+ other.tags.merge_to(child.tags)
276+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
277+ self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
278+
279+ def test_ignore_master_disables_tag_propagation(self):
280+ """merge_to(child, ignore_master=True) does not merge tags to the
281+ master.
282+ """
283+ master = self.make_branch('master')
284+ other = self.make_branch('other')
285+ other.tags.set_tag('foo', 'rev-1')
286+ child = self.make_branch('child')
287+ child.bind(master)
288+ child.update()
289+ other.tags.merge_to(child.tags, ignore_master=True)
290+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
291+ self.assertRaises(errors.NoSuchTag, master.tags.lookup_tag, 'foo')
292+
293+ def test_merge_to_overwrite_conflict_in_master(self):
294+ """merge_to(child, overwrite=True) overwrites any conflicting tags in
295+ the master.
296+ """
297+ master = self.make_branch('master')
298+ other = self.make_branch('other')
299+ other.tags.set_tag('foo', 'rev-1')
300+ child = self.make_branch('child')
301+ child.bind(master)
302+ child.update()
303+ master.tags.set_tag('foo', 'rev-2')
304+ tag_conflicts = other.tags.merge_to(child.tags, overwrite=True)
305+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
306+ self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
307+ self.assertLength(0, tag_conflicts)
308+
309+ def test_merge_to_overwrite_conflict_in_child_and_master(self):
310+ """merge_to(child, overwrite=True) overwrites any conflicting tags in
311+ both the child and the master.
312+ """
313+ master = self.make_branch('master')
314+ master.tags.set_tag('foo', 'rev-2')
315+ other = self.make_branch('other')
316+ other.tags.set_tag('foo', 'rev-1')
317+ child = self.make_branch('child')
318+ child.bind(master)
319+ child.update()
320+ tag_conflicts = other.tags.merge_to(child.tags, overwrite=True)
321+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
322+ self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
323+ self.assertLength(0, tag_conflicts)
324+
325+ def test_merge_to_conflict_in_child_only(self):
326+ """When new_tags.merge_to(child.tags) conflicts with the child but not
327+ the master, a conflict is reported and the child receives the new tag.
328+ """
329+ master = self.make_branch('master')
330+ master.tags.set_tag('foo', 'rev-2')
331+ other = self.make_branch('other')
332+ other.tags.set_tag('foo', 'rev-1')
333+ child = self.make_branch('child')
334+ child.bind(master)
335+ child.update()
336+ master.tags.delete_tag('foo')
337+ tag_conflicts = other.tags.merge_to(child.tags)
338+ # Conflict in child, so it is unchanged.
339+ self.assertEquals('rev-2', child.tags.lookup_tag('foo'))
340+ # No conflict in the master, so the 'foo' tag equals other's value here.
341+ self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
342+ # The conflict is reported.
343+ self.assertEquals([(u'foo', 'rev-1', 'rev-2')], tag_conflicts)
344+
345+ def test_merge_to_conflict_in_master_only(self):
346+ """When new_tags.merge_to(child.tags) conflicts with the master but not
347+ the child, a conflict is reported and the child receives the new tag.
348+ """
349+ master = self.make_branch('master')
350+ other = self.make_branch('other')
351+ other.tags.set_tag('foo', 'rev-1')
352+ child = self.make_branch('child')
353+ child.bind(master)
354+ child.update()
355+ master.tags.set_tag('foo', 'rev-2')
356+ tag_conflicts = other.tags.merge_to(child.tags)
357+ # No conflict in the child, so the 'foo' tag equals other's value here.
358+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
359+ # Conflict in master, so it is unchanged.
360+ self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
361+ # The conflict is reported.
362+ self.assertEquals([(u'foo', 'rev-1', 'rev-2')], tag_conflicts)
363+
364+ def test_merge_to_same_conflict_in_master_and_child(self):
365+ """When new_tags.merge_to(child.tags) conflicts the same way with the
366+ master and the child a single conflict is reported.
367+ """
368+ master = self.make_branch('master')
369+ master.tags.set_tag('foo', 'rev-2')
370+ other = self.make_branch('other')
371+ other.tags.set_tag('foo', 'rev-1')
372+ child = self.make_branch('child')
373+ child.bind(master)
374+ child.update()
375+ tag_conflicts = other.tags.merge_to(child.tags)
376+ # Both master and child conflict, so both stay as rev-2
377+ self.assertEquals('rev-2', child.tags.lookup_tag('foo'))
378+ self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
379+ # The conflict is reported exactly once, even though it occurs in both
380+ # master and child.
381+ self.assertEquals([(u'foo', 'rev-1', 'rev-2')], tag_conflicts)
382+
383+ def test_merge_to_different_conflict_in_master_and_child(self):
384+ """When new_tags.merge_to(child.tags) conflicts differently in the
385+ master and the child both conflicts are reported.
386+ """
387+ master = self.make_branch('master')
388+ master.tags.set_tag('foo', 'rev-2')
389+ other = self.make_branch('other')
390+ other.tags.set_tag('foo', 'rev-1')
391+ child = self.make_branch('child')
392+ child.bind(master)
393+ child.update()
394+ # We use the private method _set_tag_dict because normally bzr tries to
395+ # avoid this scenario.
396+ child.tags._set_tag_dict({'foo': 'rev-3'})
397+ tag_conflicts = other.tags.merge_to(child.tags)
398+ # Both master and child conflict, so both stay as they were.
399+ self.assertEquals('rev-3', child.tags.lookup_tag('foo'))
400+ self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
401+ # Both conflicts are reported.
402+ self.assertEquals(
403+ [(u'foo', 'rev-1', 'rev-2'), (u'foo', 'rev-1', 'rev-3')],
404+ sorted(tag_conflicts))
405+
406+
407 class TestUnsupportedTags(per_branch.TestCaseWithBranch):
408 """Formats that don't support tags should give reasonable errors."""
409
410
411=== modified file 'bzrlib/tests/test_tag.py'
412--- bzrlib/tests/test_tag.py 2010-03-25 09:39:03 +0000
413+++ bzrlib/tests/test_tag.py 2010-11-09 06:33:49 +0000
414@@ -120,6 +120,9 @@
415
416
417 class TestTagsInCheckouts(TestCaseWithTransport):
418+ """Tests for how tags are synchronised between the master and child branch
419+ of a checkout.
420+ """
421
422 def test_update_tag_into_checkout(self):
423 # checkouts are directly connected to the tags of their master branch:

Subscribers

People subscribed via source and target branches