Merge lp:~vila/bzr/805809-no-final-path into lp:bzr/2.2

Proposed by Vincent Ladeuil
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merged at revision: 5133
Proposed branch: lp:~vila/bzr/805809-no-final-path
Merge into: lp:bzr/2.2
Diff against target: 109 lines (+75/-2)
3 files modified
NEWS (+3/-0)
bzrlib/merge.py (+8/-2)
bzrlib/tests/test_conflicts.py (+64/-0)
To merge this branch: bzr merge lp:~vila/bzr/805809-no-final-path
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+66868@code.launchpad.net

Commit message

Handle path conflicts involving different root-ids

Description of the change

The path conflicts involving the root-ids are still a bit obscure
but roughly in this case, we don't inject the root path for the
branch we are merging from in the tree transform (and I can't see
why we should).

The fix is minimal as I target 2.2 but I'm not sure it's worth
digging for a better fix for now.

I'll check what will happen when merging up in 2.3 and 2.4 but
some earlier tests showed that the fix is valid for trunk too.

The "bad" news is that this will require a bzr upgrade on jubany
to fix the corresponding failures (31), but I'll care about that
once this patch has landed.

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

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

On 7/5/2011 11:19 AM, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/805809-no-final-path into lp:bzr/2.2.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #805809 in Bazaar: "merge fails with NoFinalPath"
> https://bugs.launchpad.net/bzr/+bug/805809
>
> For more details, see:
> https://code.launchpad.net/~vila/bzr/805809-no-final-path/+merge/66868
>
> The path conflicts involving the root-ids are still a bit obscure
> but roughly in this case, we don't inject the root path for the
> branch we are merging from in the tree transform (and I can't see
> why we should).
>
> The fix is minimal as I target 2.2 but I'm not sure it's worth
> digging for a better fix for now.
>
> I'll check what will happen when merging up in 2.3 and 2.4 but
> some earlier tests showed that the fix is valid for trunk too.
>
> The "bad" news is that this will require a bzr upgrade on jubany
> to fix the corresponding failures (31), but I'll care about that
> once this patch has landed.
>

- - parent_path = fp.get_path(
- - self.tt.trans_id_file_id(other_parent))
+ if other_parent == self.other_tree.get_root_id():
+ # The tree transform don't know about the other
root,
+ # so we special case here to avoid a NoFinalPath
+ # exception
+ parent_path = ''

^- "doesn't know about"

 merge: approve

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

iEYEARECAAYFAk4S3DkACgkQJdeBCYSNAAOUzgCfdnwcNATvOnd40oqy1N4DJrZZ
aZgAoMCGCU8ZfufSZOOTSNPqA0dXjq4q
=b6VC
-----END PGP SIGNATURE-----

review: Approve
Revision history for this message
Vincent Ladeuil (vila) 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 'NEWS'
2--- NEWS 2011-05-17 09:57:58 +0000
3+++ NEWS 2011-07-05 09:34:35 +0000
4@@ -24,6 +24,9 @@
5 doubly-stacked branch.
6 (James Westby, Martin Pool, #715000)
7
8+* Don't crash while merging and encountering obscure path conflicts
9+ involving different root-ids. (Vincent Ladeuil, #805809)
10+
11 Improvements
12 ************
13
14
15=== modified file 'bzrlib/merge.py'
16--- bzrlib/merge.py 2010-07-29 03:56:54 +0000
17+++ bzrlib/merge.py 2011-07-05 09:34:35 +0000
18@@ -1624,8 +1624,14 @@
19 if other_parent is None or other_name is None:
20 other_path = '<deleted>'
21 else:
22- parent_path = fp.get_path(
23- self.tt.trans_id_file_id(other_parent))
24+ if other_parent == self.other_tree.get_root_id():
25+ # The tree transform doesn't know about the other root,
26+ # so we special case here to avoid a NoFinalPath
27+ # exception
28+ parent_path = ''
29+ else:
30+ parent_path = fp.get_path(
31+ self.tt.trans_id_file_id(other_parent))
32 other_path = osutils.pathjoin(parent_path, other_name)
33 c = _mod_conflicts.Conflict.factory(
34 'path conflict', path=this_path,
35
36=== modified file 'bzrlib/tests/test_conflicts.py'
37--- bzrlib/tests/test_conflicts.py 2011-01-14 21:35:52 +0000
38+++ bzrlib/tests/test_conflicts.py 2011-07-05 09:34:35 +0000
39@@ -1049,6 +1049,70 @@
40 """)
41
42
43+class TestNoFinalPath(script.TestCaseWithTransportAndScript):
44+
45+ def test_bug_805809(self):
46+ self.run_script("""
47+$ bzr init trunk
48+Created a standalone tree (format: 2a)
49+$ cd trunk
50+$ echo trunk >file
51+$ bzr add
52+adding file
53+$ bzr commit -m 'create file on trunk'
54+2>Committing to: .../trunk/
55+2>added file
56+2>Committed revision 1.
57+# Create a debian branch based on trunk
58+$ cd ..
59+$ bzr branch trunk -r 1 debian
60+2>Branched 1 revision(s).
61+$ cd debian
62+$ mkdir dir
63+$ bzr add
64+adding dir
65+$ bzr mv file dir
66+file => dir/file
67+$ bzr commit -m 'rename file to dir/file for debian'
68+2>Committing to: .../debian/
69+2>added dir
70+2>renamed file => dir/file
71+2>Committed revision 2.
72+# Create an experimental branch with a new root-id
73+$ cd ..
74+$ bzr init experimental
75+$ cd experimental
76+# merge debian even without a common ancestor
77+$ bzr merge ../debian -r0..2
78+2>+N dir/
79+2>+N dir/file
80+2>All changes applied successfully.
81+$ bzr commit -m 'merging debian into experimental'
82+2>Committing to: .../experimental/
83+2>deleted
84+2>modified dir
85+2>Committed revision 1.
86+# Create an ubuntu branch with yet another root-id
87+$ cd ..
88+$ bzr init ubuntu
89+$ cd ubuntu
90+# Also merge debian
91+$ bzr merge ../debian -r0..2
92+2>+N dir/
93+2>+N dir/file
94+2>All changes applied successfully.
95+$ bzr commit -m 'merging debian'
96+2>Committing to: .../ubuntu/
97+2>deleted
98+2>modified dir
99+2>Committed revision 1.
100+# Now try to merge experimental
101+$ bzr merge ../experimental
102+2>Path conflict: dir / dir
103+2>1 conflicts encountered.
104+""")
105+
106+
107 class TestResolveActionOption(tests.TestCase):
108
109 def setUp(self):

Subscribers

People subscribed via source and target branches