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

[Vincent Ladeuil]

Handle path conflicts involving different root-ids

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.

[John A Meinel (jameinel)]

-----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

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'NEWS'
--- NEWS	2011-05-17 09:57:58 +0000
+++ NEWS	2011-07-05 09:34:35 +0000
@@ -24,6 +24,9 @@
   doubly-stacked branch.
   (James Westby, Martin Pool, #715000)
 
+* Don't crash while merging and encountering obscure path conflicts
+  involving different root-ids. (Vincent Ladeuil, #805809)
+
 Improvements
 ************
 

=== modified file 'bzrlib/merge.py'
--- bzrlib/merge.py	2010-07-29 03:56:54 +0000
+++ bzrlib/merge.py	2011-07-05 09:34:35 +0000
@@ -1624,8 +1624,14 @@
                 if other_parent is None or other_name is None:
                     other_path = '<deleted>'
                 else:
-                    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 doesn't know about the other root,
+                        # so we special case here to avoid a NoFinalPath
+                        # exception
+                        parent_path = ''
+                    else:
+                        parent_path =  fp.get_path(
+                            self.tt.trans_id_file_id(other_parent))
                     other_path = osutils.pathjoin(parent_path, other_name)
                 c = _mod_conflicts.Conflict.factory(
                     'path conflict', path=this_path,

=== modified file 'bzrlib/tests/test_conflicts.py'
--- bzrlib/tests/test_conflicts.py	2011-01-14 21:35:52 +0000
+++ bzrlib/tests/test_conflicts.py	2011-07-05 09:34:35 +0000
@@ -1049,6 +1049,70 @@
 """)
 
 
+class TestNoFinalPath(script.TestCaseWithTransportAndScript):
+
+    def test_bug_805809(self):
+        self.run_script("""
+$ bzr init trunk
+Created a standalone tree (format: 2a)
+$ cd trunk
+$ echo trunk >file
+$ bzr add
+adding file
+$ bzr commit -m 'create file on trunk'
+2>Committing to: .../trunk/
+2>added file
+2>Committed revision 1.
+# Create a debian branch based on trunk
+$ cd ..
+$ bzr branch trunk -r 1 debian
+2>Branched 1 revision(s).
+$ cd debian
+$ mkdir dir
+$ bzr add
+adding dir
+$ bzr mv file dir
+file => dir/file
+$ bzr commit -m 'rename file to dir/file for debian'
+2>Committing to: .../debian/
+2>added dir
+2>renamed file => dir/file
+2>Committed revision 2.
+# Create an experimental branch with a new root-id
+$ cd ..
+$ bzr init experimental
+$ cd experimental
+# merge debian even without a common ancestor
+$ bzr merge ../debian -r0..2
+2>+N  dir/
+2>+N  dir/file
+2>All changes applied successfully.
+$ bzr commit -m 'merging debian into experimental'
+2>Committing to: .../experimental/
+2>deleted 
+2>modified dir
+2>Committed revision 1.
+# Create an ubuntu branch with yet another root-id
+$ cd ..
+$ bzr init ubuntu
+$ cd ubuntu
+# Also merge debian
+$ bzr merge ../debian -r0..2
+2>+N  dir/
+2>+N  dir/file
+2>All changes applied successfully.
+$ bzr commit -m 'merging debian'
+2>Committing to: .../ubuntu/
+2>deleted 
+2>modified dir
+2>Committed revision 1.
+# Now try to merge experimental
+$ bzr merge ../experimental
+2>Path conflict: dir / dir
+2>1 conflicts encountered.
+""")
+
+
 class TestResolveActionOption(tests.TestCase):
 
     def setUp(self):