Merge lp:~jameinel/bzr/preview-pull-760152 into lp:bzr/2.3

Proposed by John A Meinel on 2011-04-20
Status: Merged
Approved by: John A Meinel on 2011-04-20
Approved revision: 4878
Merged at revision: 5641
Proposed branch: lp:~jameinel/bzr/preview-pull-760152
Merge into: lp:bzr/2.3
Diff against target: 81 lines (+25/-3)
3 files modified
bzrlib/builtins.py (+1/-1)
bzrlib/tests/blackbox/test_merge.py (+20/-2)
doc/en/release-notes/bzr-2.3.txt (+4/-0)
To merge this branch: bzr merge lp:~jameinel/bzr/preview-pull-760152
Reviewer Review Type Date Requested Status
Jelmer Vernooij (community) 2011-04-20 Approve on 2011-04-20
Review via email: mp+58460@code.launchpad.net

Commit message

Bug #760152, 'bzr merge --preview --pull' should not change the last-revision (be equivalent to "bzr merge --preview")

Description of the change

This is a small tweak to the merge code.

Basically, if you use "bzr merge --preview" it shows you the diff, without changing the WorkingTree. It turns out that "bzr merge --pull --preview" would apply "--pull" and change the branch's last-revision even though it really should just "--preview"

I'm targeting 2.3, though we could easily backport this as far back as we want. (The initial change is on the 2.1 branch, but the NEWS update is obviously on 2.3)

To post a comment you must log in.
Jelmer Vernooij (jelmer) wrote :

On Wed, 2011-04-20 at 09:54 +0000, John A Meinel wrote:
> John A Meinel has proposed merging lp:~jameinel/bzr/preview-pull-760152 into lp:bzr/2.3.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #760152 in Bazaar: "bzr merge --pull --preview BRANCH does not always honor the --preview option"
> https://bugs.launchpad.net/bzr/+bug/760152
>
> For more details, see:
> https://code.launchpad.net/~jameinel/bzr/preview-pull-760152/+merge/58460
>
> This is a small tweak to the merge code.

   review approve

> I'm targeting 2.3, though we could easily backport this as far back as we want. (The initial change is on the 2.1 branch, but the NEWS update is obviously on 2.3)
Since this was only reported recently I wonder many people will actually
be affected by this and so how useful it would be to backport further
than 2.3. On the other hand, it also doesn't look like a very risky
change.

Cheers,

Jelmer

review: Approve
John A Meinel (jameinel) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/builtins.py'
2--- bzrlib/builtins.py 2011-01-12 01:01:53 +0000
3+++ bzrlib/builtins.py 2011-04-20 09:55:15 +0000
4@@ -3948,7 +3948,7 @@
5 merger.other_rev_id is not None):
6 note('Nothing to do.')
7 return 0
8- if pull:
9+ if pull and not preview:
10 if merger.interesting_files is not None:
11 raise errors.BzrCommandError('Cannot pull individual files')
12 if (merger.base_rev_id == tree.last_revision()):
13
14=== modified file 'bzrlib/tests/blackbox/test_merge.py'
15--- bzrlib/tests/blackbox/test_merge.py 2010-12-02 10:41:05 +0000
16+++ bzrlib/tests/blackbox/test_merge.py 2011-04-20 09:55:15 +0000
17@@ -19,8 +19,11 @@
18 """Black-box tests for bzr merge.
19 """
20
21+import doctest
22 import os
23
24+from testtools import matchers
25+
26 from bzrlib import (
27 branch,
28 branchbuilder,
29@@ -352,13 +355,13 @@
30
31 def pullable_branch(self):
32 tree_a = self.make_branch_and_tree('a')
33- self.build_tree(['a/file'])
34+ self.build_tree_contents([('a/file', 'bar\n')])
35 tree_a.add(['file'])
36 self.id1 = tree_a.commit('commit 1')
37
38 tree_b = self.make_branch_and_tree('b')
39 tree_b.pull(tree_a.branch)
40- file('b/file', 'wb').write('foo')
41+ self.build_tree_contents([('b/file', 'foo\n')])
42 self.id2 = tree_b.commit('commit 2')
43
44 def test_merge_pull(self):
45@@ -369,6 +372,21 @@
46 tree_a = workingtree.WorkingTree.open('.')
47 self.assertEqual([self.id2], tree_a.get_parent_ids())
48
49+ def test_merge_pull_preview(self):
50+ self.pullable_branch()
51+ (out, err) = self.run_bzr('merge --pull --preview -d a b')
52+ self.assertThat(out, matchers.DocTestMatches(
53+"""=== modified file 'file'
54+--- file\t...
55++++ file\t...
56+@@ -1,1 +1,1 @@
57+-bar
58++foo
59+
60+""", doctest.ELLIPSIS | doctest.REPORT_UDIFF))
61+ tree_a = workingtree.WorkingTree.open('a')
62+ self.assertEqual([self.id1], tree_a.get_parent_ids())
63+
64 def test_merge_kind_change(self):
65 tree_a = self.make_branch_and_tree('tree_a')
66 self.build_tree_contents([('tree_a/file', 'content_1')])
67
68=== modified file 'doc/en/release-notes/bzr-2.3.txt'
69--- doc/en/release-notes/bzr-2.3.txt 2011-04-08 08:03:30 +0000
70+++ doc/en/release-notes/bzr-2.3.txt 2011-04-20 09:55:15 +0000
71@@ -49,6 +49,10 @@
72 plugins because it's often too long.
73 (Martin Pool, #716389)
74
75+* ``bzr merge --preview --pull`` should respect the ``--preview`` option
76+ first, and not actually change the branch tip revision.
77+ (John Arbash Meinel, Dennis Duchier, #760152)
78+
79 * ``bzr push`` into a repository (that doesn't have a branch), will no
80 longer copy all revisions in the repository. Only the ones in the
81 ancestry of the source branch, like it does in all other cases.

Subscribers

People subscribed via source and target branches