[MERGE][#325897] bzr missing -v between an empty branch and a non-empty branch raises an IndexError

[Aaron Bentley]

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

bb:tweak

Marius Kruger wrote:
> 2009/2/6 Marius Kruger <amanic at gmail.com <mailto:amanic at gmail.com>>

> === modified file 'NEWS'
> --- NEWS	2009-02-05 10:20:51 +0000
> +++ NEWS	2009-02-06 01:54:25 +0000
> @@ -118,6 +118,9 @@
>        handled.
>        (John Arbash Meinel, Vincent Ladeuil)
>  
> +    * ``bzr missing -v`` between an empty branch and a non-empty branch used
> +      to raise an IndexError.  (Marius Kruger, #325897)
> +
>    DOCUMENTATION:
>  
>      * Improved plugin developer documentation.  (Martin Pool)
> 
> === modified file 'bzrlib/missing.py'
> --- bzrlib/missing.py	2009-01-01 23:21:07 +0000
> +++ bzrlib/missing.py	2009-02-05 23:16:01 +0000
> @@ -38,11 +38,15 @@
>          rev = revision_source.get_revision(rev_id)
>          if verbose:
>              remote_tree = revision_source.revision_tree(rev_id)
> -            parent_rev_id = rev.parent_ids[0]
> -            if last_rev_id == parent_rev_id:
> -                parent_tree = last_tree
> +            if len(rev.parent_ids) == 0:
> +                parent_tree = revision_source.revision_tree(
> +                    _mod_revision.NULL_REVISION)

This will actually cause the null revision tree to be opened twice,
since it's already opened at line 28 or so.

Instead of doing the tree, it would be better to focus on the parent_id:

if len(rev.parent_ids) == 0:
    parent_rev_id = _mod_revision.NULL_REVISION
else:
    parent_rev_id = rev.parent_ids[0]
if last_rev_id == parent_rev_id:
    parent_tree = last_tree
else:
    parent_tree = revision_source.revision_tree(parent_rev_id)

> === modified file 'bzrlib/tests/blackbox/test_missing.py'
> --- bzrlib/tests/blackbox/test_missing.py	2009-01-25 00:55:26 +0000
> +++ bzrlib/tests/blackbox/test_missing.py	2009-02-05 23:16:01 +0000
> @@ -178,6 +178,62 @@
>          self.assertMessages(out, ('a3', 'a4', 'b3', 'b4'),
>              ('a2', 'a5', 'b2', 'b5'))
>  
> +    def test_verbose_empty_branches(self):
> +        a_tree = self.make_branch_and_tree('a')
> +        b_tree = self.make_branch_and_tree('b')
> +
> +        # both empty
> +        os.chdir('a')
> +        out,err = self.run_bzr('missing -v ../b')
> +        self.assertEqual(out, 'Branches are up to date.\n')
> +
> +        # add files to make sure correct deltas are shown
> +        self.build_tree_contents([('file1', 'stuff\n')])
> +        a_tree.add('file1')
> +        a_tree.commit(message='rev1')
> +        self.build_tree_contents([('file2', 'stuff\n')])
> +        a_tree.add('file2')
> +        a_tree.commit(message='rev2')
> +
> +        # other empty
> +        out,err = self.run_bzr('missing -v ../b', retcode=1)
> +        self.assertContainsRe(out, "You have 2 extra revision\(s\):")
> +        self.assertContainsRe(out, """(?s)\
> +revno: 2
> +.*
> +message:
> +  rev2
> +added:
> +  file2""")
> +        self.assertContainsRe(out, """(?s)\
> +revno: 1
> +.*
> +message:
> +  rev1
> +added:
> +  file1
> +""")
> +
> +        # this empty
> +        os.chdir('../b')
> +        out,err = self.run_bzr('missing -v ../a', retcode=1)
> +        self.assertContainsRe(out, "You are missing 2 revision\(s\):")
> +        self.assertContainsRe(out, """(?s)\
> +revno: 2
> +.*
> +message:
> +  rev2
> +added:
> +  file2""")
> +        self.assertContainsRe(out, """(?s)\
> +revno: 1
> +.*
> +message:
> +  rev1
> +added:
> +  file1
> +""")
> +
>      def test_missing_check_last_location(self):
>          # check that last location shown as filepath not file URL
>  

This is testing the wrong thing.  It's nice to test "bzr missing", but
it's crucial to test iter_log_revisions.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkmQjQsACgkQ0F+nu1YWqI2fhgCeJYK4hth+HIE/aS1eM7KP3nNd
7j0AoIHq7ZA4KF/2v0Sw50CqCPHWwrTc
=ePtK
-----END PGP SIGNATURE-----