Merge lp:~nmb/bzr/484516-push-revspec-tree into lp:bzr

[Neil Martinsen-Burrell]

Create working tree at specified revision when doing a local push

This change fixes bug 484516 by creating a pushed working tree at the specified revision. I have added a script test based on the reproduction recipe, as well as augmenting bzrlib/tests/test_push.py:test_push_with_revisionspec.

[Jelmer Vernooij (jelmer)]

This seems reasonable to me.

review: Approve

[Marius Kruger (amanica)]

Congrats on tracking this nasty down.
I just have 2 comments that I think can be addressed when merging.

1) I suggest adding a comment at the end of your blackbox test_push_with_revspec
  +++ bzrlib/tests/blackbox/test_push.py 2012-06-10 23:32:32 +0000
  @@ -445,20 +445,22 @@ class TestPush(tests.TestCaseWithTranspo
  + $ bzr st ../other # checking that file is not created (bug #484516)
2) You are adding a trailing space here..
  +++ bzrlib/workingtree.py 2012-06-10 23:32:32 +0000
  @@ -527,21 +527,22 @@ class WorkingTree(bzrlib.mutabletree.Mut
  + merge.transform_tree(tree, <--

review: Approve

[Jelmer Vernooij (jelmer)]

Have you looked at the performance impact of this change? This might have to fetch the full contents of the tree another time.

[Jelmer Vernooij (jelmer)]

It might be worthwile to use self.revision_tree() rather than self.branch.repository.revision_tree().

review: Needs Fixing

[Neil Martinsen-Burrell (nmb)]

> It might be worthwile to use self.revision_tree() rather than
> self.branch.repository.revision_tree().

self.revision_tree() doesn't seem to work the same. That is, it breaks lots of other test cases with e.g.

...
NoSuchRevisionInTree: The revision id {added} is not present in the tree <WorkingTree6 of /private/var/folders/d8/wjzb13qx7ns9yjh46bq9ljs00000gn/T/testbzr-jPjTz6.tmp/bzrlib.tests.blackbox.test_push.TestPushStrictWithChanges.test_push_with_revision(out-of-sync-trees)/work/local>

Resubmitting for review with Marius's suggested change.

[Jelmer Vernooij (jelmer)]

On Mon, Jul 02, 2012 at 05:35:30AM -0000, Neil Martinsen-Burrell wrote:
> > It might be worthwile to use self.revision_tree() rather than
> > self.branch.repository.revision_tree().
>
> self.revision_tree() doesn't seem to work the same. That is, it breaks lots of other test cases with e.g.
>
> ...
> NoSuchRevisionInTree: The revision id {added} is not present in the tree <WorkingTree6 of /private/var/folders/d8/wjzb13qx7ns9yjh46bq9ljs00000gn/T/testbzr-jPjTz6.tmp/bzrlib.tests.blackbox.test_push.TestPushStrictWithChanges.test_push_with_revision(out-of-sync-trees)/work/local>
Perhaps trying WorkingTree.revision_tree() first and if that raises
NoSuchRevision, falling back to Repository.revision_tree() ?

Without using WorkingTree.revision_tree() we're going to be fetching
the entire revision tree from the server in some cases, which is a
major performance regression.

Cheers,

Jelmer

[Neil Martinsen-Burrell (nmb)]

I added a try/except that first tries to look up the revision tree from the working tree and only goes the the branch's repository if the first lookup fails.

[Jelmer Vernooij (jelmer)]

review: Approve

[Jelmer Vernooij (jelmer)]

thanks!

[Martin Packman (gz)]

sent to pqm by email

[Martin Packman (gz)]

Failed on PQM due to needing release notes updating, I'll do that and merge.

=== modified file 'bzrlib/tests/blackbox/test_push.py'
--- bzrlib/tests/blackbox/test_push.py	2012-04-16 11:08:11 +0000
+++ bzrlib/tests/blackbox/test_push.py	2012-07-04 18:04:24 +0000
@@ -452,6 +452,8 @@
         self.assertTrue(repo_to.has_revision('from-1'))
         self.assertFalse(repo_to.has_revision('from-2'))
         self.assertEqual(tree_to.branch.last_revision_info()[1], 'from-1')
+        self.assertFalse(
+            tree_to.changes_from(tree_to.basis_tree()).has_changed())
 
         self.run_bzr_error(
             ['bzr: ERROR: bzr push --revision '
@@ -917,3 +919,28 @@
             2>All changes applied successfully.
             2>Pushed up to revision 1.
             """)
+
+    def test_push_with_revspec(self):
+        self.run_script("""
+            $ bzr init-repo .
+            Shared repository with trees (format: 2a)
+            Location:
+              shared repository: .
+            $ bzr init trunk
+            Created a repository tree (format: 2a)
+            Using shared repository...
+            $ cd trunk
+            $ bzr commit -m 'first rev' --unchanged
+            2>Committing to:...trunk/
+            2>Committed revision 1.
+            $ echo foo > file
+            $ bzr add
+            adding file
+            $ bzr commit -m 'we need some foo'
+            2>Committing to:...trunk/
+            2>added file
+            2>Committed revision 2.
+            $ bzr push -r 1 ../other
+            2>Created new branch.
+            $ bzr st ../other # checking that file is not created (#484516)
+            """)

=== modified file 'bzrlib/workingtree.py'
--- bzrlib/workingtree.py	2012-06-18 11:43:07 +0000
+++ bzrlib/workingtree.py	2012-07-04 18:04:24 +0000
@@ -534,7 +534,12 @@
         else:
             # TODO now merge from tree.last_revision to revision (to preserve
             # user local changes)
-            merge.transform_tree(tree, self)
+            try:
+                other_tree = self.revision_tree(revision_id)
+            except errors.NoSuchRevision:
+                other_tree = self.branch.repository.revision_tree(revision_id)
+
+            merge.transform_tree(tree, other_tree)
             if revision_id == _mod_revision.NULL_REVISION:
                 new_parents = []
             else:

=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- doc/en/release-notes/bzr-2.6.txt	2012-06-26 15:40:44 +0000
+++ doc/en/release-notes/bzr-2.6.txt	2012-07-04 18:04:24 +0000
@@ -44,6 +44,9 @@
 * Implement ``ResponseFile.readline`` and ``ReponseFile.tell``,
   fixing some clones over HTTP. (Jelmer Vernooij, #963769)
 
+* When pushing a specific revision, create the new working tree at
+  that revision. (#484516, Neil Martinsen-Burrell)
+
 Documentation
 *************