Merge lp:~spiv/bzr/annotate-objectnotlocked-496590 into lp:bzr/2.0

[Andrew Bennetts]

[Andrew Bennetts (spiv)]

This is a minimal fix for bug 496590 targetted to 2.0.x. <https://code.edge.launchpad.net/~spiv/bzr/command-cleanup/+merge/16943> has a much more comprehensive fix for pretty much all bugs of this sort in builtins.py, but it's obviously a bit too large to be sensible for 2.0.x.

[Martin Pool (mbp)]

Generally speaking it would be nice to also merge tests to 2.0, for all the usual reasons. But if that would be too hard, or depends on other changes, it's ok like this.

Thanks,

review: Approve

[Andrew Bennetts (spiv)]

Hmm, yes. I suppose I can turn the shell script at <http://launchpadlibrarian.net/37540559/repro.sh> (see comment 3 of bug 496590) into a simple test easily enough, so I'll do that.

=== modified file 'NEWS'
--- NEWS	2010-01-07 08:57:14 +0000
+++ NEWS	2010-01-07 12:37:18 +0000
@@ -20,6 +20,9 @@
 Bug Fixes
 *********
 
+* ``bzr annotate`` on another branch with ``-r branch:...`` no longer
+  fails with an ``ObjectNotLocked`` error.  (Andrew Bennetts, #496590)
+
 * ``bzr export dir`` now requests all file content as a record stream,
   rather than requsting the file content one file-at-a-time. This can make
   exporting over the network significantly faster (54min => 9min in one

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2009-10-29 05:49:32 +0000
+++ bzrlib/builtins.py	2010-01-07 12:37:18 +0000
@@ -4409,21 +4409,25 @@
             branch.lock_read()
         try:
             tree = _get_one_revision_tree('annotate', revision, branch=branch)
-            if wt is not None:
-                file_id = wt.path2id(relpath)
-            else:
-                file_id = tree.path2id(relpath)
-            if file_id is None:
-                raise errors.NotVersionedError(filename)
-            file_version = tree.inventory[file_id].revision
-            if wt is not None and revision is None:
-                # If there is a tree and we're not annotating historical
-                # versions, annotate the working tree's content.
-                annotate_file_tree(wt, file_id, self.outf, long, all,
-                    show_ids=show_ids)
-            else:
-                annotate_file(branch, file_version, file_id, long, all, self.outf,
-                              show_ids=show_ids)
+            tree.lock_read()
+            try:
+                if wt is not None:
+                    file_id = wt.path2id(relpath)
+                else:
+                    file_id = tree.path2id(relpath)
+                if file_id is None:
+                    raise errors.NotVersionedError(filename)
+                file_version = tree.inventory[file_id].revision
+                if wt is not None and revision is None:
+                    # If there is a tree and we're not annotating historical
+                    # versions, annotate the working tree's content.
+                    annotate_file_tree(wt, file_id, self.outf, long, all,
+                        show_ids=show_ids)
+                else:
+                    annotate_file(branch, file_version, file_id, long, all,
+                        self.outf, show_ids=show_ids)
+            finally:
+                tree.unlock()
         finally:
             if wt is not None:
                 wt.unlock()

=== modified file 'bzrlib/tests/blackbox/test_annotate.py'
--- bzrlib/tests/blackbox/test_annotate.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/blackbox/test_annotate.py	2010-01-07 12:37:18 +0000
@@ -29,6 +29,7 @@
 from bzrlib.branch import Branch
 from bzrlib.config import extract_email_address
 from bzrlib.tests import TestCaseWithTransport
+from bzrlib.urlutils import joinpath
 
 
 class TestAnnotate(TestCaseWithTransport):
@@ -153,14 +154,27 @@
 class TestSimpleAnnotate(TestCaseWithTransport):
     """Annotate tests with no complex setup."""
 
-    def _setup_edited_file(self):
+    def _setup_edited_file(self, relpath='.'):
         """Create a tree with a locally edited file."""
-        tree = self.make_branch_and_tree('.')
-        self.build_tree_contents([('file', 'foo\ngam\n')])
+        tree = self.make_branch_and_tree(relpath)
+        file_relpath = joinpath(relpath, 'file')
+        self.build_tree_contents([(file_relpath, 'foo\ngam\n')])
         tree.add('file')
         tree.commit('add file', committer="test@host", rev_id="rev1")
-        self.build_tree_contents([('file', 'foo\nbar\ngam\n')])
+        self.build_tree_contents([(file_relpath, 'foo\nbar\ngam\n')])
         tree.branch.get_config().set_user_option('email', 'current@host2')
+        return tree
+
+    def test_annotate_cmd_revspec_branch(self):
+        tree = self._setup_edited_file('trunk')
+        tree.branch.create_checkout(self.get_url('work'), lightweight=True)
+        os.chdir('work')
+        out, err = self.run_bzr('annotate file -r branch:../trunk')
+        self.assertEqual('', err)
+        self.assertEqual(
+            '1   test@ho | foo\n'
+            '            | gam\n',
+            out)
 
     def test_annotate_edited_file(self):
         tree = self._setup_edited_file()