Merge lp:~mbp/bzr/32669-2.0-symlink-branch into lp:bzr/2.0

[Martin Pool]

This fixes 'bzr add SYMLINK_TO_BRANCH' (bug 32669) to add the symlink, rather than running add in the referent branch. The basic fix is that when you say "open_containing_from_transport", we shouldn't follow a symlink in the last part.

[Martin Pool (mbp)]

I'm not sure this is passing tests yet so not asking for review.

[Vincent Ladeuil (vila)]

@Martin: ping.
I just cut 2.0.6, is this abandoned ?

Unmerged revisions

4758. By Martin Pool on 2010-07-18

open_containing opens the directory containing symlinks, not the target

4757. By Martin Pool on 2010-07-18

Remove typo from test

4756. By Martin Pool on 2010-07-18

Merge transport symlink support

4755. By Martin Pool on 2010-07-17

Add test for bug 32669

=== modified file 'NEWS'
--- NEWS	2010-07-19 08:47:42 +0000
+++ NEWS	2010-07-19 08:47:42 +0000
@@ -18,6 +18,13 @@
   history no longer crash when deleted files are involved.
   (Vincent Ladeuil, John Arbash Meinel, #375898)
 
+* ``bzr add SYMLINK_TO_BRANCH`` and other commands now work on the
+  symlink, not on the branch that it points to.  (Specifically
+  ``BzrDir.open_containing`` if the last component of the path is a
+  symlink, will open the directory containing that symlink rather than the
+  directory that it points to.)
+  (Martin Pool, #32669)
+
 * ``bzr commit SYMLINK`` now works, rather than trying to commit the
   target of the symlink.
   (Martin Pool, John Arbash Meinel, #128562)
 
=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py	2010-02-18 04:45:24 +0000
+++ bzrlib/bzrdir.py	2010-07-19 08:47:42 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006, 2007, 2008, 2009 Canonical Ltd
+# Copyright (C) 2005-2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -28,6 +28,7 @@
 # TODO: Move old formats into a plugin to make this file smaller.
 
 import os
+import stat
 import sys
 
 from bzrlib.lazy_import import lazy_import
@@ -908,26 +909,48 @@
         format, UnknownFormatError or UnsupportedFormatError are raised.
         If there is one, it is returned, along with the unused portion of url.
 
+        If the transport points directly to a symlink, we don't follow it, but
+        rather walk up to the containing directory.  This is so that you can
+        operate on a symlink within one branch that points to another branch.
+        This is applied only on the last component of the name, not every step
+        up.
+
         :return: The BzrDir that contains the path, and a Unicode path
                 for the rest of the URL.
         """
-        # this gets the normalised url back. I.e. '.' -> the full path.
-        url = a_transport.base
-        while True:
-            try:
-                result = BzrDir.open_from_transport(a_transport)
-                return result, urlutils.unescape(a_transport.relpath(url))
+        def go_up(t):
+            try:
+                new_t = t.clone('..')
+                mutter("walk up to %s" % new_t)
+            except errors.InvalidURLJoin:
+                # reached the root, whatever that may be
+                raise errors.NotBranchError(path=t.base)
+            if new_t.base == t.base:
+                # reached the root, whatever that may be
+                raise errors.NotBranchError(path=t.base)
+            return probe(new_t)
+        def probe(t):
+            try:
+                return BzrDir.open_from_transport(t)
             except errors.NotBranchError, e:
-                pass
+                return go_up(t)
+        def initial_probe(t):
             try:
-                new_t = a_transport.clone('..')
-            except errors.InvalidURLJoin:
-                # reached the root, whatever that may be
-                raise errors.NotBranchError(path=url)
-            if new_t.base == a_transport.base:
-                # reached the root, whatever that may be
-                raise errors.NotBranchError(path=url)
-            a_transport = new_t
+                # nb: this depends on 'a'.stat('') mapping to stat('a') but it
+                # does
+                if stat.S_ISLNK(t.stat('').st_mode):
+                    # not going to even try opening this; continue upwards
+                    mutter("found symlink at %r, walking up" % t)
+                    return go_up(t)
+            except errors.TransportNotPossible:
+                pass
+            except errors.NoSuchFile:
+                pass
+            mutter("didn't find symlink at %r" % t)
+            return probe(a_transport)
+        bd = initial_probe(a_transport)
+        return bd, urlutils.unescape(
+            bd.root_transport.relpath(a_transport.base))
 
     def _get_tree_branch(self):
         """Return the branch and tree, if any, for this bzrdir.
 
=== modified file 'bzrlib/tests/blackbox/test_add.py'
--- bzrlib/tests/blackbox/test_add.py	2009-08-10 08:25:05 +0000
+++ bzrlib/tests/blackbox/test_add.py	2010-07-19 08:47:42 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006, 2007, 2009 Canonical Ltd
+# Copyright (C) 2005, 2006, 2007, 2009, 2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -19,7 +19,10 @@
 
 import os
 
-from bzrlib import osutils
+from bzrlib import (
+    osutils,
+    tests,
+    )
 from bzrlib.tests import (
     condition_isinstance,
     split_suite_by_condition,
@@ -225,8 +228,12 @@
         self.run_bzr(['add', u'\u1234?', u'\u1235*'])
         self.assertEquals(self.run_bzr('unknowns')[0], 'cc\n')
 
+
+class TestAddSymlinks(tests.TestCaseWithTransport):
+
+    _test_needs_features = [tests.SymlinkFeature]
+
     def test_add_via_symlink(self):
-        self.requireFeature(SymlinkFeature)
         self.make_branch_and_tree('source')
         self.build_tree(['source/top.txt'])
         os.symlink('source', 'link')
@@ -234,8 +241,23 @@
         self.assertEquals(out, 'adding top.txt\n')
 
     def test_add_symlink_to_abspath(self):
-        self.requireFeature(SymlinkFeature)
         self.make_branch_and_tree('tree')
         os.symlink(osutils.abspath('target'), 'tree/link')
         out = self.run_bzr(['add', 'tree/link'])[0]
         self.assertEquals(out, 'adding link\n')
+
+    def test_smart_add_symlink_to_branch(self):
+        # add of a symlink pointing to another branch should add the symlink,
+        # not the files in the other branch
+        # https://bugs.edge.launchpad.net/bzr/+bug/32669
+        tree1 = self.make_branch_and_tree('tree1')
+        tree2 = self.make_branch_and_tree('tree2')
+        self.build_tree_contents([
+            ('tree1/link@', '../tree2'),
+            ('tree2/file', 'file'),
+            ('tree2/evil@', '../tree1'),
+            ])
+        out, err = self.run_bzr(['add', 'tree1/link'])
+        # if this fails, it's probably because add opened the wrong working
+        # tree, which is tested in test_bzrdir
+        self.assertEquals(out, 'adding link\n')
 
=== modified file 'bzrlib/tests/test_bzrdir.py'
--- bzrlib/tests/test_bzrdir.py	2009-07-10 07:14:02 +0000
+++ bzrlib/tests/test_bzrdir.py	2010-07-19 08:47:42 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006, 2007 Canonical Ltd
+# Copyright (C) 2005, 2006, 2007, 2010 Canonical Ltd
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -30,6 +30,7 @@
     repository,
     osutils,
     remote,
+    tests,
     urlutils,
     win32utils,
     workingtree,
@@ -1325,3 +1326,30 @@
         url = transport.base
         err = self.assertRaises(errors.BzrError, bzrdir.BzrDir.open, url)
         self.assertEqual('fail', err._preformatted_string)
+
+
+class TestSymlinks(TestCaseWithTransport):
+    """Tests for opening bzrdirs named by symlinks.
+
+    See bug 32669 and friends.
+
+    See also tests.blackbox.test_add
+    """
+
+    _test_needs_features = [tests.SymlinkFeature]
+
+    def test_open_named_by_symlink(self):
+        """A symlink within a tree pointing to another tree.
+        
+        Treated primarily as a symlink, not indirected.
+        """
+        tree1 = self.make_branch_and_tree('tree1')
+        tree2 = self.make_branch_and_tree('tree2')
+        self.build_tree_contents([
+            ('tree1/link@', '../tree2'),
+            ])
+        link_url = tree1.abspath('link')
+        control, relpath = bzrdir.BzrDir.open_containing(
+            link_url, [tree1._transport, tree2._transport])
+        self.assertEquals(control.transport.base, tree1.bzrdir.transport.base)
+        self.assertEquals(relpath, 'link')