Merge lp:~spiv/bzr/reconfigure-unstacked-copies-tagged-revisions-401646 into lp:bzr

[Andrew Bennetts]

Copy tagged revisions as part of 'bzr reconfigure --unstacked' (#401646)

Fixes bug 401646 ('bzr reconfigure --unstacked' should copy tagged revisions), as a follow-on from the fix for bug 309682.

I added a smaller helper function to bzrlib.tests.fixtures. I'm a little uncertain about if it really paid off, but it does result in slightly more concise tests in a few places. I suppose it might be clearer if the canned branch/repo it produces had more obviously named revision IDs than 'rev-1' and 'rev-2'. Maybe 'branch-tip' and 'outside-tip-ancestry'? Not sure.

[Jelmer Vernooij (jelmer)]

I hadn't seen the fixtures used for branch construction yet, that looks very interesting.

review: Approve (code)

[Jelmer Vernooij (jelmer)]

> I hadn't seen the fixtures used for branch construction yet, that looks very
> interesting.
I think the alternative revision ids you suggest would be useful. Perhaps not particularly in this case, but moreso as this pattern gets copied by other people.

[Vincent Ladeuil (vila)]

+1 on meaningful names. I also use '1' instead of 'rev-1' in some log tests as this matches directly with the revnos and provide an different way to check for expected results.

review: Approve

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2011-02-07 04:30:00 +0000
+++ bzrlib/branch.py	2011-02-11 06:11:19 +0000
@@ -880,13 +880,14 @@
                 # XXX: If you unstack a branch while it has a working tree
                 # with a pending merge, the pending-merged revisions will no
                 # longer be present.  You can (probably) revert and remerge.
-                #
-                # XXX: This only fetches up to the tip of the repository; it
-                # doesn't bring across any tags.  That's fairly consistent
-                # with how branch works, but perhaps not ideal.
-                self.repository.fetch(old_repository,
-                    revision_id=self.last_revision(),
-                    find_ghosts=True)
+                try:
+                    tags_to_fetch = set(self.tags.get_reverse_tag_dict())
+                except errors.TagsNotSupported:
+                    tags_to_fetch = set()
+                fetch_spec = _mod_graph.NotInOtherForRevs(self.repository,
+                    old_repository, required_ids=[self.last_revision()],
+                    if_present_ids=tags_to_fetch, find_ghosts=True).execute()
+                self.repository.fetch(old_repository, fetch_spec=fetch_spec)
             finally:
                 old_repository.unlock()
         finally:
 
=== modified file 'bzrlib/tests/blackbox/test_branch.py'
--- bzrlib/tests/blackbox/test_branch.py	2011-01-12 23:33:40 +0000
+++ bzrlib/tests/blackbox/test_branch.py	2011-02-11 06:11:19 +0000
@@ -28,6 +28,7 @@
 from bzrlib.repofmt.knitrepo import RepositoryFormatKnit1
 from bzrlib.tests import TestCaseWithTransport
 from bzrlib.tests import (
+    fixtures,
     HardlinkFeature,
     test_server,
     )
@@ -270,11 +271,8 @@
 
     def test_branch_fetches_all_tags(self):
         builder = self.make_branch_builder('source')
-        builder.build_commit(message="Rev 1", rev_id='rev-1')
-        builder.build_commit(message="Rev 2", rev_id='rev-2')
-        source = builder.get_branch()
+        source = fixtures.build_branch_with_non_ancestral_rev(builder)
         source.tags.set_tag('tag-a', 'rev-2')
-        source.set_last_revision_info(1, 'rev-1')
         # Now source has a tag not in its ancestry.  Make a branch from it.
         self.run_bzr('branch source new-branch')
         new_branch = branch.Branch.open('new-branch')
@@ -466,12 +464,9 @@
     def test_branch_from_branch_with_tags(self):
         self.setup_smart_server_with_call_log()
         builder = self.make_branch_builder('source')
-        builder.build_commit(message="Rev 1", rev_id='rev-1')
-        builder.build_commit(message="Rev 2", rev_id='rev-2')
-        source = builder.get_branch()
+        source = fixtures.build_branch_with_non_ancestral_rev(builder)
         source.tags.set_tag('tag-a', 'rev-2')
         source.tags.set_tag('tag-missing', 'missing-rev')
-        source.set_last_revision_info(1, 'rev-1')
         # Now source has a tag not in its ancestry.  Make a branch from it.
         self.reset_smart_call_log()
         out, err = self.run_bzr(['branch', self.get_url('source'), 'target'])
 
=== modified file 'bzrlib/tests/blackbox/test_pull.py'
--- bzrlib/tests/blackbox/test_pull.py	2011-02-07 04:14:29 +0000
+++ bzrlib/tests/blackbox/test_pull.py	2011-02-11 06:11:19 +0000
@@ -29,7 +29,10 @@
 from bzrlib.branch import Branch
 from bzrlib.directory_service import directories
 from bzrlib.osutils import pathjoin
-from bzrlib.tests import TestCaseWithTransport
+from bzrlib.tests import (
+    fixtures,
+    TestCaseWithTransport,
+    )
 from bzrlib.uncommit import uncommit
 from bzrlib.workingtree import WorkingTree
 
@@ -148,13 +151,9 @@
         """
         # Make a source, sprout a target off it
         builder = self.make_branch_builder('source')
-        builder.build_commit(message="Rev 1", rev_id='rev-1')
-        source = builder.get_branch()
+        source = fixtures.build_branch_with_non_ancestral_rev(builder)
         target_bzrdir = source.bzrdir.sprout('target')
-        # Add a non-ancestry tag to source
-        builder.build_commit(message="Rev 2", rev_id='rev-2')
         source.tags.set_tag('tag-a', 'rev-2')
-        source.set_last_revision_info(1, 'rev-1')
         # Pull from source
         self.run_bzr('pull -d target source')
         target = target_bzrdir.open_branch()
 
=== modified file 'bzrlib/tests/fixtures.py'
--- bzrlib/tests/fixtures.py	2010-06-26 01:07:16 +0000
+++ bzrlib/tests/fixtures.py	2011-02-11 06:11:19 +0000
@@ -97,3 +97,31 @@
     def __exit__(self, exc_type, exc_val, exc_tb):
         self._calls.append('__exit__')
         return False # propogate exceptions.
+
+
+def build_branch_with_non_ancestral_rev(branch_builder):
+    """Builds a branch with a rev not in the ancestry of the tip.
+
+    This is the revision graph::
+
+      rev-2
+        |
+      rev-1
+        |
+      (null)
+
+    The branch tip is 'rev-1'.  'rev-2' is present in the branch's repository,
+    but is not part of rev-1's ancestry.
+
+    :param branch_builder: A BranchBuilder (e.g. from
+        TestCaseWithMemoryTransport.make_branch_builder).
+    :returns: the new branch
+    """
+    # Make a sequence of two commits
+    branch_builder.build_commit(message="Rev 1", rev_id='rev-1')
+    branch_builder.build_commit(message="Rev 2", rev_id='rev-2')
+    # Move the branch tip back to the first commit
+    source = branch_builder.get_branch()
+    source.set_last_revision_info(1, 'rev-1')
+    return source
+
 
=== modified file 'bzrlib/tests/per_branch/test_pull.py'
--- bzrlib/tests/per_branch/test_pull.py	2010-12-01 01:07:39 +0000
+++ bzrlib/tests/per_branch/test_pull.py	2011-02-11 06:11:19 +0000
@@ -25,7 +25,11 @@
     memorytree,
     revision,
     )
-from bzrlib.tests import per_branch, TestNotApplicable
+from bzrlib.tests import (
+    fixtures,
+    per_branch,
+    TestNotApplicable,
+    )
 
 
 class TestPull(per_branch.TestCaseWithBranch):
@@ -144,17 +148,14 @@
             builder = self.make_branch_builder('source')
         except errors.UninitializableFormat:
             raise TestNotApplicable('uninitializeable format')
-        builder.build_commit(message="Rev 1", rev_id='rev-1')
-        source = builder.get_branch()
+        source = fixtures.build_branch_with_non_ancestral_rev(builder)
         target = source.bzrdir.sprout('target').open_branch()
-        # Add a non-ancestry tag to source
-        builder.build_commit(message="Rev 2", rev_id='rev-2')
+        # Add a tag to the source, then pull from source
         try:
             source.tags.set_tag('tag-a', 'rev-2')
         except errors.TagsNotSupported:
             raise TestNotApplicable('format does not support tags.')
-        source.set_last_revision_info(1, 'rev-1')
-        # Pull from source
+        source.tags.set_tag('tag-a', 'rev-2')
         target.pull(source)
         # The tag is present, and so is its revision.
         self.assertEqual('rev-2', target.tags.lookup_tag('tag-a'))
@@ -167,19 +168,15 @@
             builder = self.make_branch_builder('source')
         except errors.UninitializableFormat:
             raise TestNotApplicable('uninitializeable format')
-        builder.build_commit(message="Rev 1", rev_id='rev-1')
-        source = builder.get_branch()
+        source = fixtures.build_branch_with_non_ancestral_rev(builder)
         target = source.bzrdir.sprout('target').open_branch()
-        # Add a non-ancestry tag to source
-        builder.build_commit(message="Rev 2", rev_id='rev-2')
-        try:
-            source.tags.set_tag('tag-a', 'rev-2')
-        except errors.TagsNotSupported:
-            raise TestNotApplicable('format does not support tags.')
-        source.set_last_revision_info(1, 'rev-1')
         # Add a new commit to the ancestry
         builder.build_commit(message="Rev 2 again", rev_id='rev-2-again')
-        # Pull from source
+        # Add a tag to the source, then pull rev-2-again from source
+        try:
+            source.tags.set_tag('tag-a', 'rev-2')
+        except errors.TagsNotSupported:
+            raise TestNotApplicable('format does not support tags.')
         target.pull(source, 'rev-2-again')
         # The tag is present, and so is its revision.
         self.assertEqual('rev-2', target.tags.lookup_tag('tag-a'))
 
=== modified file 'bzrlib/tests/per_branch/test_stacking.py'
--- bzrlib/tests/per_branch/test_stacking.py	2011-01-13 01:02:53 +0000
+++ bzrlib/tests/per_branch/test_stacking.py	2011-02-11 06:11:19 +0000
@@ -23,7 +23,7 @@
     errors,
     )
 from bzrlib.revision import NULL_REVISION
-from bzrlib.tests import TestNotApplicable, transport_util
+from bzrlib.tests import fixtures, TestNotApplicable, transport_util
 from bzrlib.tests.per_branch import TestCaseWithBranch
 
 
@@ -176,17 +176,20 @@
 
     def test_unstack_fetches(self):
         """Removing the stacked-on branch pulls across all data"""
+        try:
+            builder = self.make_branch_builder('trunk')
+        except errors.UninitializableFormat:
+            raise TestNotApplicable('uninitializeable format')
         # We have a mainline
-        trunk_tree = self.make_branch_and_tree('mainline')
-        trunk_revid = trunk_tree.commit('revision on mainline')
-        # and make branch from it which is stacked
+        trunk = fixtures.build_branch_with_non_ancestral_rev(builder)
+        mainline_revid = 'rev-1'
+        # and make branch from it which is stacked (with no tags)
         try:
-            new_dir = trunk_tree.bzrdir.sprout(self.get_url('newbranch'),
-                stacked=True)
+            new_dir = trunk.bzrdir.sprout(self.get_url('newbranch'), stacked=True)
         except unstackable_format_errors, e:
             raise TestNotApplicable(e)
         # stacked repository
-        self.assertRevisionNotInRepository('newbranch', trunk_revid)
+        self.assertRevisionNotInRepository('newbranch', mainline_revid)
         # TODO: we'd like to commit in the stacked repository; that requires
         # some care (maybe a BranchBuilder) if it's remote and has no
         # workingtree
@@ -195,13 +198,21 @@
         # now when we unstack that should implicitly fetch, to make sure that
         # the branch will still work
         new_branch = new_dir.open_branch()
+        try:
+            new_branch.tags.set_tag('tag-a', 'rev-2')
+        except errors.TagsNotSupported:
+            tags_supported = False
+        else:
+            tags_supported = True
         new_branch.set_stacked_on_url(None)
-        self.assertRevisionInRepository('newbranch', trunk_revid)
+        self.assertRevisionInRepository('newbranch', mainline_revid)
         # of course it's still in the mainline
-        self.assertRevisionInRepository('mainline', trunk_revid)
+        self.assertRevisionInRepository('trunk', mainline_revid)
+        if tags_supported:
+            # the tagged revision in trunk is now in newbranch too
+            self.assertRevisionInRepository('newbranch', 'rev-2')
         # and now we're no longer stacked
-        self.assertRaises(errors.NotStacked,
-            new_branch.get_stacked_on_url)
+        self.assertRaises(errors.NotStacked, new_branch.get_stacked_on_url)
 
     def test_unstack_already_locked(self):
         """Removing the stacked-on branch with an already write-locked branch
 
=== modified file 'bzrlib/tests/per_controldir/test_controldir.py'
--- bzrlib/tests/per_controldir/test_controldir.py	2011-02-07 04:14:29 +0000
+++ bzrlib/tests/per_controldir/test_controldir.py	2011-02-11 06:11:19 +0000
@@ -33,6 +33,7 @@
     )
 import bzrlib.revision
 from bzrlib.tests import (
+    fixtures,
     ChrootedTestCase,
     TestNotApplicable,
     TestSkipped,
@@ -676,14 +677,11 @@
         # when sprouting a branch all revisions named in the tags are copied
         # too.
         builder = self.make_branch_builder('source')
-        builder.build_commit(message="Rev 1", rev_id='rev-1')
-        builder.build_commit(message="Rev 2", rev_id='rev-2')
-        source = builder.get_branch()
+        source = fixtures.build_branch_with_non_ancestral_rev(builder)
         try:
             source.tags.set_tag('tag-a', 'rev-2')
         except errors.TagsNotSupported:
             raise TestNotApplicable('Branch format does not support tags.')
-        source.set_last_revision_info(1, 'rev-1')
         # Now source has a tag not in its ancestry.  Sprout its controldir.
         dir = source.bzrdir
         target = dir.sprout(self.get_url('target'))
 
=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- doc/en/release-notes/bzr-2.4.txt	2011-02-10 16:30:54 +0000
+++ doc/en/release-notes/bzr-2.4.txt	2011-02-11 06:11:19 +0000
@@ -68,6 +68,10 @@
   paths, however they may still print junk if not on a UTF-8 terminal.
   (Martin [gz], #707954)
 
+* ``bzr reconfigure --unstacked`` now copies revisions (and their
+  ancestors) named in tags into the unstacked repository, not just the
+  ancestry of the branch's tip.  (Andrew Bennetts, #401646)
+
 * ``bzr serve`` no longer crashes when a server_started hook is installed and
   IPv6 support is available on the system. (Jelmer Vernooij, #293697)