Rev 5546: (spiv) Tags.merge_to now updates the master branch as well, in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Mon Nov 22 01:39:15 GMT 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5546 [merge]
revision-id: pqm at pqm.ubuntu.com-20101122013913-s3k719bu7brfu64e
parent: pqm at pqm.ubuntu.com-20101119174025-76prtl0dsb9770aj
parent: andrew.bennetts at canonical.com-20101118004357-f2r2zhmahtp7lcob
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2010-11-22 01:39:13 +0000
message:
(spiv) Tags.merge_to now updates the master branch as well,
if any. (#603395) (Andrew Bennetts)
modified:
bzrlib/branch.py branch.py-20050309040759-e4baf4e0d046576e
bzrlib/builtins.py builtins.py-20050830033751-fc01482b9ca23183
bzrlib/tag.py tag.py-20070212110532-91cw79inah2cfozx-1
bzrlib/tests/blackbox/test_merge.py test_merge.py-20060323225809-9bc0459c19917f41
bzrlib/tests/blackbox/test_tags.py test_tags.py-20070116132048-5h4qak2cm22jlb9e-1
bzrlib/tests/per_branch/test_tags.py test_tags.py-20070212110545-w2s799hm2jlbsmg5-1
bzrlib/tests/test_tag.py test_tag.py-20070212110532-91cw79inah2cfozx-2
bzrlib/transport/ssh.py ssh.py-20060824042150-0s9787kng6zv1nwq-1
doc/en/release-notes/bzr-2.3.txt NEWS-20050323055033-4e00b5db738777ff
=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py 2010-11-11 03:53:12 +0000
+++ b/bzrlib/branch.py 2010-11-18 00:22:24 +0000
@@ -3355,7 +3355,7 @@
if isinstance(format, remote.RemoteBranchFormat):
format._ensure_real()
return format._custom_format
- return format
+ return format
@needs_write_lock
def copy_content_into(self, revision_id=None):
=== modified file 'bzrlib/builtins.py'
--- a/bzrlib/builtins.py 2010-11-04 17:48:47 +0000
+++ b/bzrlib/builtins.py 2010-11-18 00:22:24 +0000
@@ -4028,7 +4028,9 @@
if ((remember or tree.branch.get_submit_branch() is None) and
user_location is not None):
tree.branch.set_submit_branch(other_branch.base)
- _merge_tags_if_possible(other_branch, tree.branch)
+ # Merge tags (but don't set them in the master branch yet, the user
+ # might revert this merge). Commit will propagate them.
+ _merge_tags_if_possible(other_branch, tree.branch, ignore_master=True)
merger = _mod_merge.Merger.from_revision_ids(pb, tree,
other_revision_id, base_revision_id, other_branch, base_branch)
if other_path != '':
=== modified file 'bzrlib/tag.py'
--- a/bzrlib/tag.py 2010-03-25 09:39:03 +0000
+++ b/bzrlib/tag.py 2010-11-18 00:30:51 +0000
@@ -28,7 +28,9 @@
from bzrlib import (
bencode,
+ cleanup,
errors,
+ symbol_versioning,
trace,
)
@@ -57,7 +59,7 @@
lookup_tag = _not_supported
delete_tag = _not_supported
- def merge_to(self, to_tags, overwrite=False):
+ def merge_to(self, to_tags, overwrite=False, ignore_master=False):
# we never have anything to copy
pass
@@ -177,7 +179,7 @@
raise ValueError("failed to deserialize tag dictionary %r: %s"
% (tag_content, e))
- def merge_to(self, to_tags, overwrite=False):
+ def merge_to(self, to_tags, overwrite=False, ignore_master=False):
"""Copy tags between repositories if necessary and possible.
This method has common command-line behaviour about handling
@@ -188,11 +190,19 @@
:param to_tags: Branch to receive these tags
:param overwrite: Overwrite conflicting tags in the target branch
+ :param ignore_master: Do not modify the tags in the target's master
+ branch (if any). Default is false (so the master will be updated).
+ New in bzr 2.3.
- :returns: A list of tags that conflicted, each of which is
+ :returns: A set of tags that conflicted, each of which is
(tagname, source_target, dest_target), or None if no copying was
done.
"""
+ operation = cleanup.OperationWithCleanups(self._merge_to_operation)
+ return operation.run(to_tags, overwrite, ignore_master)
+
+ def _merge_to_operation(self, operation, to_tags, overwrite, ignore_master):
+ add_cleanup = operation.add_cleanup
if self.branch == to_tags.branch:
return
if not self.branch.supports_tags():
@@ -203,15 +213,39 @@
# no tags in the source, and we don't want to clobber anything
# that's in the destination
return
- to_tags.branch.lock_write()
- try:
- dest_dict = to_tags.get_tag_dict()
- result, conflicts = self._reconcile_tags(source_dict, dest_dict,
- overwrite)
- if result != dest_dict:
- to_tags._set_tag_dict(result)
- finally:
- to_tags.branch.unlock()
+ # We merge_to both master and child individually.
+ #
+ # It's possible for master and child to have differing sets of
+ # tags, in which case it's possible to have different sets of
+ # conflicts. We report the union of both conflict sets. In
+ # that case it's likely the child and master have accepted
+ # different tags from the source, which may be a surprising result, but
+ # the best we can do in the circumstances.
+ #
+ # Ideally we'd improve this API to report the different conflicts
+ # more clearly to the caller, but we don't want to break plugins
+ # such as bzr-builddeb that use this API.
+ add_cleanup(to_tags.branch.lock_write().unlock)
+ if ignore_master:
+ master = None
+ else:
+ master = to_tags.branch.get_master_branch()
+ if master is not None:
+ add_cleanup(master.lock_write().unlock)
+ conflicts = self._merge_to(to_tags, source_dict, overwrite)
+ if master is not None:
+ conflicts += self._merge_to(master.tags, source_dict,
+ overwrite)
+ # We use set() to remove any duplicate conflicts from the master
+ # branch.
+ return set(conflicts)
+
+ def _merge_to(self, to_tags, source_dict, overwrite):
+ dest_dict = to_tags.get_tag_dict()
+ result, conflicts = self._reconcile_tags(source_dict, dest_dict,
+ overwrite)
+ if result != dest_dict:
+ to_tags._set_tag_dict(result)
return conflicts
def rename_revisions(self, rename_map):
@@ -249,6 +283,27 @@
return result, conflicts
-def _merge_tags_if_possible(from_branch, to_branch):
- from_branch.tags.merge_to(to_branch.tags)
+def _merge_tags_if_possible(from_branch, to_branch, ignore_master=False):
+ # Try hard to support merge_to implementations that don't expect
+ # 'ignore_master' (new in bzr 2.3). First, if the flag isn't set then we
+ # can safely avoid passing ignore_master at all.
+ if not ignore_master:
+ from_branch.tags.merge_to(to_branch.tags)
+ return
+ # If the flag is set, try to pass it, but be ready to catch TypeError.
+ try:
+ from_branch.tags.merge_to(to_branch.tags, ignore_master=ignore_master)
+ except TypeError:
+ # Probably this implementation of 'merge_to' is from a plugin that
+ # doesn't expect the 'ignore_master' keyword argument (e.g. bzr-svn
+ # 1.0.4). There's a small risk that the TypeError is actually caused
+ # by a completely different problem (which is why we don't catch it for
+ # the ignore_master=False case), but even then there's probably no harm
+ # in calling a second time.
+ symbol_versioning.warn(
+ symbol_versioning.deprecated_in((2,3)) % (
+ "Tags.merge_to (of %r) that doesn't accept ignore_master kwarg"
+ % (from_branch.tags,),),
+ DeprecationWarning)
+ from_branch.tags.merge_to(to_branch.tags)
=== modified file 'bzrlib/tests/blackbox/test_merge.py'
--- a/bzrlib/tests/blackbox/test_merge.py 2010-05-20 18:23:17 +0000
+++ b/bzrlib/tests/blackbox/test_merge.py 2010-11-09 04:32:56 +0000
@@ -23,9 +23,9 @@
from bzrlib import (
branch,
+ branchbuilder,
bzrdir,
conflicts,
- errors,
merge_directive,
osutils,
tests,
=== modified file 'bzrlib/tests/blackbox/test_tags.py'
--- a/bzrlib/tests/blackbox/test_tags.py 2010-11-10 02:01:33 +0000
+++ b/bzrlib/tests/blackbox/test_tags.py 2010-11-18 00:22:24 +0000
@@ -132,6 +132,18 @@
builder.build_commit(message='Commit in fork.', rev_id='fork-1')
return fork
+ def test_merge_without_commit_does_not_propagate_tags_to_master(self):
+ """'bzr merge' alone does not propagate tags to a master branch.
+
+ (If the user runs 'bzr commit', then that is when the tags from the
+ merge are propagated.)
+ """
+ master, child = self.make_master_and_checkout()
+ fork = self.make_fork(master)
+ fork.tags.set_tag('new-tag', fork.last_revision())
+ self.run_bzr(['merge', '../fork'], working_dir='child')
+ self.assertEqual({}, master.tags.get_tag_dict())
+
def test_commit_in_heavyweight_checkout_copies_tags_to_master(self):
master, child = self.make_master_and_checkout()
fork = self.make_fork(master)
=== modified file 'bzrlib/tests/per_branch/test_tags.py'
--- a/bzrlib/tests/per_branch/test_tags.py 2010-11-11 03:53:12 +0000
+++ b/bzrlib/tests/per_branch/test_tags.py 2010-11-18 00:43:57 +0000
@@ -96,14 +96,14 @@
# removed when we merge them
b2.tags.set_tag('in-destination', 'revid')
result = b1.tags.merge_to(b2.tags)
- self.assertEquals(result, [])
+ self.assertEquals(list(result), [])
self.assertEquals(b2.tags.lookup_tag('in-destination'), 'revid')
# if there's a conflicting tag, it's reported -- the command line
# interface will say "these tags couldn't be copied"
b1.tags.set_tag('conflicts', 'revid-1')
b2.tags.set_tag('conflicts', 'revid-2')
result = b1.tags.merge_to(b2.tags)
- self.assertEquals(result,
+ self.assertEquals(list(result),
[('conflicts', 'revid-1', 'revid-2')])
# and it keeps the same value
self.assertEquals(b2.tags.lookup_tag('conflicts'), 'revid-2')
@@ -227,6 +227,176 @@
self.assertEqual({'one': 'rev-1-changed'}, b.tags.get_tag_dict())
+class TestTagsMergeToInCheckouts(per_branch.TestCaseWithBranch):
+ """Tests for checkout.branch.tags.merge_to.
+
+ In particular this exercises variations in tag conflicts in the master
+ branch and/or the checkout (child). It may seem strange to have different
+ tags in the child and master, but 'bzr merge' intentionally updates the
+ child and not the master (instead the next 'bzr commit', if the user
+ decides to commit, will update the master). Also, merge_to in bzr < 2.3
+ didn't propagate changes to the master, and current bzr versions may find
+ themselves operating on checkouts touched by older bzrs
+
+ So we need to make sure bzr copes gracefully with differing tags in the
+ master versus the child.
+
+ See also <https://bugs.launchpad.net/bzr/+bug/603395>.
+ """
+
+ def setUp(self):
+ super(TestTagsMergeToInCheckouts, self).setUp()
+ branch1 = self.make_branch('tags-probe')
+ if not branch1._format.supports_tags():
+ raise tests.TestSkipped(
+ "format %s doesn't support tags" % branch1._format)
+ branch2 = self.make_branch('bind-probe')
+ try:
+ branch2.bind(branch1)
+ except errors.UpgradeRequired:
+ raise tests.TestNotApplicable(
+ "format %s doesn't support bound branches" % branch2._format)
+
+ def test_merge_to_propagates_tags(self):
+ """merge_to(child) also merges tags to the master."""
+ master = self.make_branch('master')
+ other = self.make_branch('other')
+ other.tags.set_tag('foo', 'rev-1')
+ child = self.make_branch('child')
+ child.bind(master)
+ child.update()
+ other.tags.merge_to(child.tags)
+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
+ self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
+
+ def test_ignore_master_disables_tag_propagation(self):
+ """merge_to(child, ignore_master=True) does not merge tags to the
+ master.
+ """
+ master = self.make_branch('master')
+ other = self.make_branch('other')
+ other.tags.set_tag('foo', 'rev-1')
+ child = self.make_branch('child')
+ child.bind(master)
+ child.update()
+ other.tags.merge_to(child.tags, ignore_master=True)
+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
+ self.assertRaises(errors.NoSuchTag, master.tags.lookup_tag, 'foo')
+
+ def test_merge_to_overwrite_conflict_in_master(self):
+ """merge_to(child, overwrite=True) overwrites any conflicting tags in
+ the master.
+ """
+ master = self.make_branch('master')
+ other = self.make_branch('other')
+ other.tags.set_tag('foo', 'rev-1')
+ child = self.make_branch('child')
+ child.bind(master)
+ child.update()
+ master.tags.set_tag('foo', 'rev-2')
+ tag_conflicts = other.tags.merge_to(child.tags, overwrite=True)
+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
+ self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
+ self.assertLength(0, tag_conflicts)
+
+ def test_merge_to_overwrite_conflict_in_child_and_master(self):
+ """merge_to(child, overwrite=True) overwrites any conflicting tags in
+ both the child and the master.
+ """
+ master = self.make_branch('master')
+ master.tags.set_tag('foo', 'rev-2')
+ other = self.make_branch('other')
+ other.tags.set_tag('foo', 'rev-1')
+ child = self.make_branch('child')
+ child.bind(master)
+ child.update()
+ tag_conflicts = other.tags.merge_to(child.tags, overwrite=True)
+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
+ self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
+ self.assertLength(0, tag_conflicts)
+
+ def test_merge_to_conflict_in_child_only(self):
+ """When new_tags.merge_to(child.tags) conflicts with the child but not
+ the master, a conflict is reported and the child receives the new tag.
+ """
+ master = self.make_branch('master')
+ master.tags.set_tag('foo', 'rev-2')
+ other = self.make_branch('other')
+ other.tags.set_tag('foo', 'rev-1')
+ child = self.make_branch('child')
+ child.bind(master)
+ child.update()
+ master.tags.delete_tag('foo')
+ tag_conflicts = other.tags.merge_to(child.tags)
+ # Conflict in child, so it is unchanged.
+ self.assertEquals('rev-2', child.tags.lookup_tag('foo'))
+ # No conflict in the master, so the 'foo' tag equals other's value here.
+ self.assertEquals('rev-1', master.tags.lookup_tag('foo'))
+ # The conflict is reported.
+ self.assertEquals([(u'foo', 'rev-1', 'rev-2')], list(tag_conflicts))
+
+ def test_merge_to_conflict_in_master_only(self):
+ """When new_tags.merge_to(child.tags) conflicts with the master but not
+ the child, a conflict is reported and the child receives the new tag.
+ """
+ master = self.make_branch('master')
+ other = self.make_branch('other')
+ other.tags.set_tag('foo', 'rev-1')
+ child = self.make_branch('child')
+ child.bind(master)
+ child.update()
+ master.tags.set_tag('foo', 'rev-2')
+ tag_conflicts = other.tags.merge_to(child.tags)
+ # No conflict in the child, so the 'foo' tag equals other's value here.
+ self.assertEquals('rev-1', child.tags.lookup_tag('foo'))
+ # Conflict in master, so it is unchanged.
+ self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
+ # The conflict is reported.
+ self.assertEquals([(u'foo', 'rev-1', 'rev-2')], list(tag_conflicts))
+
+ def test_merge_to_same_conflict_in_master_and_child(self):
+ """When new_tags.merge_to(child.tags) conflicts the same way with the
+ master and the child a single conflict is reported.
+ """
+ master = self.make_branch('master')
+ master.tags.set_tag('foo', 'rev-2')
+ other = self.make_branch('other')
+ other.tags.set_tag('foo', 'rev-1')
+ child = self.make_branch('child')
+ child.bind(master)
+ child.update()
+ tag_conflicts = other.tags.merge_to(child.tags)
+ # Both master and child conflict, so both stay as rev-2
+ self.assertEquals('rev-2', child.tags.lookup_tag('foo'))
+ self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
+ # The conflict is reported exactly once, even though it occurs in both
+ # master and child.
+ self.assertEquals([(u'foo', 'rev-1', 'rev-2')], list(tag_conflicts))
+
+ def test_merge_to_different_conflict_in_master_and_child(self):
+ """When new_tags.merge_to(child.tags) conflicts differently in the
+ master and the child both conflicts are reported.
+ """
+ master = self.make_branch('master')
+ master.tags.set_tag('foo', 'rev-2')
+ other = self.make_branch('other')
+ other.tags.set_tag('foo', 'rev-1')
+ child = self.make_branch('child')
+ child.bind(master)
+ child.update()
+ # We use the private method _set_tag_dict because normally bzr tries to
+ # avoid this scenario.
+ child.tags._set_tag_dict({'foo': 'rev-3'})
+ tag_conflicts = other.tags.merge_to(child.tags)
+ # Both master and child conflict, so both stay as they were.
+ self.assertEquals('rev-3', child.tags.lookup_tag('foo'))
+ self.assertEquals('rev-2', master.tags.lookup_tag('foo'))
+ # Both conflicts are reported.
+ self.assertEquals(
+ [(u'foo', 'rev-1', 'rev-2'), (u'foo', 'rev-1', 'rev-3')],
+ sorted(tag_conflicts))
+
+
class TestUnsupportedTags(per_branch.TestCaseWithBranch):
"""Formats that don't support tags should give reasonable errors."""
=== modified file 'bzrlib/tests/test_tag.py'
--- a/bzrlib/tests/test_tag.py 2010-03-25 09:39:03 +0000
+++ b/bzrlib/tests/test_tag.py 2010-11-18 00:43:57 +0000
@@ -111,15 +111,18 @@
# conflicting merge
a.tags.set_tag('tag-2', 'z')
conflicts = a.tags.merge_to(b.tags)
- self.assertEqual(conflicts, [('tag-2', 'z', 'y')])
+ self.assertEqual(list(conflicts), [('tag-2', 'z', 'y')])
self.assertEqual('y', b.tags.lookup_tag('tag-2'))
# overwrite conflicts
conflicts = a.tags.merge_to(b.tags, overwrite=True)
- self.assertEqual(conflicts, [])
+ self.assertEqual(list(conflicts), [])
self.assertEqual('z', b.tags.lookup_tag('tag-2'))
class TestTagsInCheckouts(TestCaseWithTransport):
+ """Tests for how tags are synchronised between the master and child branch
+ of a checkout.
+ """
def test_update_tag_into_checkout(self):
# checkouts are directly connected to the tags of their master branch:
=== modified file 'bzrlib/transport/ssh.py'
--- a/bzrlib/transport/ssh.py 2010-10-07 12:45:51 +0000
+++ b/bzrlib/transport/ssh.py 2010-11-18 00:22:24 +0000
@@ -363,13 +363,14 @@
# This platform doesn't support socketpair(), so just use ordinary
# pipes instead.
stdin = stdout = subprocess.PIPE
- sock = None
+ my_sock, subproc_sock = None, None
else:
stdin = stdout = subproc_sock
- sock = my_sock
proc = subprocess.Popen(argv, stdin=stdin, stdout=stdout,
**os_specific_subprocess_params())
- return SSHSubprocessConnection(proc, sock=sock)
+ if subproc_sock is not None:
+ subproc_sock.close()
+ return SSHSubprocessConnection(proc, sock=my_sock)
def connect_sftp(self, username, password, host, port):
try:
@@ -653,25 +654,24 @@
import weakref
_subproc_weakrefs = set()
-def _close_ssh_proc(proc):
+def _close_ssh_proc(proc, sock):
"""Carefully close stdin/stdout and reap the SSH process.
If the pipes are already closed and/or the process has already been
wait()ed on, that's ok, and no error is raised. The goal is to do our best
to clean up (whether or not a clean up was already tried).
"""
- dotted_names = ['stdin.close', 'stdout.close', 'wait']
- for dotted_name in dotted_names:
- attrs = dotted_name.split('.')
- try:
- obj = proc
- for attr in attrs:
- obj = getattr(obj, attr)
- except AttributeError:
- # It's ok for proc.stdin or proc.stdout to be None.
- continue
- try:
- obj()
+ funcs = []
+ for closeable in (proc.stdin, proc.stdout, sock):
+ # We expect that either proc (a subprocess.Popen) will have stdin and
+ # stdout streams to close, or that we will have been passed a socket to
+ # close, with the option not in use being None.
+ if closeable is not None:
+ funcs.append(closeable.close)
+ funcs.append(proc.wait)
+ for func in funcs:
+ try:
+ func()
except OSError:
# It's ok for the pipe to already be closed, or the process to
# already be finished.
@@ -716,7 +716,7 @@
# to avoid leaving processes lingering indefinitely.
def terminate(ref):
_subproc_weakrefs.remove(ref)
- _close_ssh_proc(proc)
+ _close_ssh_proc(proc, sock)
_subproc_weakrefs.add(weakref.ref(self, terminate))
def send(self, data):
@@ -732,7 +732,7 @@
return os.read(self.proc.stdout.fileno(), count)
def close(self):
- _close_ssh_proc(self.proc)
+ _close_ssh_proc(self.proc, self._sock)
def get_sock_or_pipes(self):
if self._sock is not None:
=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- a/doc/en/release-notes/bzr-2.3.txt 2010-11-18 19:38:28 +0000
+++ b/doc/en/release-notes/bzr-2.3.txt 2010-11-22 01:39:13 +0000
@@ -173,6 +173,11 @@
``bzrdir.open_repository()`` to propagate. This is unhelpful at best,
and at worst can trigger infinite loops in callers. (Andrew Bennetts)
+* The ``branch.tags.merge_to(target_branch)`` API used by plugins such as
+ ``bzr-builddeb`` now propagates changes to the master branch of the
+ target branch (if there is one). This makes it consistent with the
+ other tag APIs. (Andrew Bennetts, #603395)
+
* Windows installers no longer requires the Microsoft vcredist to be
installed. (Martin [gz], Gary van der Merwe, #632465)
More information about the bazaar-commits
mailing list