[MERGE] bundle commandline cleanup

[John A Meinel]

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

Aaron Bentley wrote:
> Hi all,
> 
> This patch cleans up bundle commandline handling.
> 
> 1. Duplicate 'using saved location' messages are eliminated
> 2. If one or zero revision parameters is specified, a base branch is
> used to pick the base revision.  This base branch may be a saved or
> specified value.
> 3. If two revision parameters are specified, the first is the base
> revision and the second is the target revision.
> 
> Aaron



...

- -        if base_branch is not None:
+        if revision is None or len(revision) < 2:
+            submit_branch = target_branch.get_submit_branch()
+            if base is None:
+                base = submit_branch
+            if base is None:
+                base = target_branch.get_parent()
+            if base is None:
+                raise errors.BzrCommandError("No base branch known or"
+                                             " specified.")
+            elif not base_specified:
+                note('Using saved location: %s' % base)
+            base_branch = Branch.open(base)


^- this should be:

self.outf.write('Using saved location: %s\n'
	% urlutils.urlfordisplay(base, self.outf.encoding))

But it probably was note() before, I'm just letting you know how the new
encoding stuff does it.


+
+            # We don't want to lock the same branch across
+            # 2 different branches
+            if target_branch.base == base_branch.base:
+                base_branch = target_branch
+            if submit_branch is None or remember:
+                if base_specified:
+                    target_branch.set_submit_branch(base_branch.base)
+                elif remember:
+                    raise errors.BzrCommandError('--remember requires a
branch'
+                                                 ' to be specified.')
             target_branch.repository.fetch(base_branch.repository,
- -                                           revision_id=base_revision)
- -            del base_branch
+                                           base_branch.last_revision())
+            base_revision = common_ancestor(base_branch.last_revision(),
+                                            target_revision,
+                                            target_branch.repository)
+


^- So why are you always pulling from 'last_revision()' rather than the
base revision that was specified? Potentially they are not even the same
ancestry.
It is possible the user did:

bzr bundle -r revid:...
And that isn't related to the last_revision()


...


v- I don't see anything in these tests that check the output of
'run_bzr' to make sure it isn't printing 'used saved location' twice.


 class TestBundle(TestCaseInTempDir):

+    def make_trees(self):
+        grandparent_tree =
BzrDir.create_standalone_workingtree('grandparent')
+        grandparent_tree.commit('initial commit', rev_id='revision1')
+        parent_bzrdir = grandparent_tree.bzrdir.sprout('parent')
+        parent_tree = parent_bzrdir.open_workingtree()
+        parent_tree.commit('next commit', rev_id='revision2')
+        branch_tree =
parent_tree.bzrdir.sprout('branch').open_workingtree()
+        branch_tree.commit('last commit', rev_id='revision3')
+
     def test_uses_parent(self):
         """Parent location is used as a basis by default"""
- -
- -        parent_tree = BzrDir.create_standalone_workingtree('parent')
- -        parent_tree.commit('initial commit', rev_id='revision1')
- -        os.chdir('parent')
+        self.make_trees()
+        os.chdir('grandparent')
         errmsg = self.run_bzr('bundle', retcode=3)[1]
         self.assertContainsRe(errmsg, 'No base branch known or specified')
- -        branch_tree =
parent_tree.bzrdir.sprout('../branch').open_workingtree()
- -        branch_tree.commit('next commit', rev_id='revision2')
- -        branch_tree.commit('last commit', rev_id='revision3')
         os.chdir('../branch')
- -        br = read_bundle(StringIO(self.run_bzr('bundle')[0]))
- -        self.assertEqual(br.revisions[0].revision_id, 'revision3')
- -        self.assertEqual(len(br.revisions), 2)
- -        self.assertEqual(br.revisions[1].revision_id, 'revision2')
+        stdout, stderr = self.run_bzr('bundle')
+        self.assertEqual(stderr.count('Using saved location'), 1)
+        br = read_bundle(StringIO(stdout))
+        self.assertRevisions(br, ['revision3'])
+
+    def assertRevisions(self, bi, expected):
+        self.assertEqual([r.revision_id for r in bi.revisions], expected)

     def test_uses_submit(self):
         """Submit location can be used and set"""
- -
- -        submit_tree = BzrDir.create_standalone_workingtree('submit')
- -        submit_tree.commit('initial commit', rev_id='revision1')
- -        parent_tree =
submit_tree.bzrdir.sprout('parent').open_workingtree()
- -        parent_tree.commit('next commit', rev_id='revision2')
- -        branch_tree =
parent_tree.bzrdir.sprout('branch').open_workingtree()
- -        branch_tree.commit('last commit', rev_id='revision3')
+        self.make_trees()
         os.chdir('branch')
         br = read_bundle(StringIO(self.run_bzr('bundle')[0]))
- -        self.assertEqual(br.revisions[0].revision_id, 'revision3')
- -        self.assertEqual(len(br.revisions), 1)
- -        br = read_bundle(StringIO(self.run_bzr('bundle',
'../submit')[0]))
- -        self.assertEqual(len(br.revisions), 2)
+        self.assertRevisions(br, ['revision3'])
+        br = read_bundle(StringIO(self.run_bzr('bundle',
'../grandparent')[0]))
+        self.assertRevisions(br, ['revision3', 'revision2'])
         # submit location should be auto-remembered
         br = read_bundle(StringIO(self.run_bzr('bundle')[0]))
- -        self.assertEqual(len(br.revisions), 2)
+        self.assertRevisions(br, ['revision3', 'revision2'])
         self.run_bzr('bundle', '../parent')
         br = read_bundle(StringIO(self.run_bzr('bundle')[0]))
- -        self.assertEqual(len(br.revisions), 2)
+        self.assertRevisions(br, ['revision3', 'revision2'])
         self.run_bzr('bundle', '../parent', '--remember')
         br = read_bundle(StringIO(self.run_bzr('bundle')[0]))
- -        self.assertEqual(len(br.revisions), 1)
+        self.assertRevisions(br, ['revision3'])
         err = self.run_bzr('bundle', '--remember', retcode=3)[1]
         self.assertContainsRe(err,
                               '--remember requires a branch to be
specified.')
+
+    def test_revision_branch_interaction(self):
+        self.make_trees()
+        os.chdir('branch')
+        bi = read_bundle(StringIO(self.run_bzr('bundle',
'../grandparent')[0]))
+        self.assertRevisions(bi, ['revision3', 'revision2'])
+        out = StringIO(self.run_bzr('bundle', '../grandparent', '-r',
'-2')[0])
+        bi = read_bundle(out)
+        self.assertRevisions(bi, ['revision2'])
+        bi = read_bundle(StringIO(self.run_bzr('bundle', '-r',
'-2..-1')[0]))
+        self.assertRevisions(bi, ['revision3'])
+        self.run_bzr('bundle', '../grandparent', '-r', '-2..-1', retcode=3)


Otherwise, the changes look pretty good.

John
=:->


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

iD8DBQFEoGgGJdeBCYSNAAMRAi4IAJ49IoHIgoVgaKiios5LdSQE4rithQCgrlqu
WvUJvtbV4VidD8HHcW1w6WA=
=Si69
-----END PGP SIGNATURE-----