Rev 2503: PathNotChild inherits from PathError, not BzrError. in file:///v/home/vila/src/experimental/reuse.transports/

Vincent Ladeuil v.ladeuil+lp at free.fr
Thu May 31 18:55:53 BST 2007


At file:///v/home/vila/src/experimental/reuse.transports/

------------------------------------------------------------
revno: 2503
revision-id: v.ladeuil+lp at free.fr-20070531175549-pwzy2wp3vs4gc2vy
parent: v.ladeuil+lp at free.fr-20070531175417-vtcav2eh81c38clk
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: reuse.transports
timestamp: Thu 2007-05-31 19:55:49 +0200
message:
  PathNotChild inherits from PathError, not BzrError.
  
  * bzrlib/errors.py:
  (PathNotChild): Really a PathError.
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/tests/test_http.py      testhttp.py-20051018020158-b2eef6e867c514d9
  bzrlib/tests/test_sftp_transport.py testsftp.py-20051027032739-247570325fec7e7e
  bzrlib/tests/test_transport.py testtransport.py-20050718175618-e5cdb99f4555ddce
  bzrlib/tests/test_transport_implementations.py test_transport_implementations.py-20051227111451-f97c5c7d5c49fce7
  bzrlib/transport/__init__.py   transport.py-20050711165921-4978aa7ce1285ad5
  bzrlib/transport/ftp.py        ftp.py-20051116161804-58dc9506548c2a53
  bzrlib/transport/http/__init__.py http_transport.py-20050711212304-506c5fd1059ace96
  bzrlib/transport/http/_pycurl.py pycurlhttp.py-20060110060940-4e2a705911af77a6
  bzrlib/transport/http/_urllib.py _urlgrabber.py-20060113083826-0bbf7d992fbf090c
  bzrlib/transport/remote.py     ssh.py-20060608202016-c25gvf1ob7ypbus6-1
  bzrlib/transport/sftp.py       sftp.py-20051019050329-ab48ce71b7e32dfe
-------------- next part --------------
=== modified file 'NEWS'
--- a/NEWS	2007-05-30 07:10:06 +0000
+++ b/NEWS	2007-05-31 17:55:49 +0000
@@ -8,6 +8,13 @@
       is specified, and are labelled "revision-id:", as per mainline
       revisions, instead of "merged:". (Kent Gibson)
 
+    * Refactoring of transport classes connected to a remote
+      server. transport.split_url have been deprecated, use the
+      static method on the object instead (SFTP needs to override
+      the default handling for home directories). URL tests have
+      been refactored too.
+      (Vincent Ladeuil)
+
   IMPROVEMENTS:
   
     * There are two new help topics, working-trees and repositories that

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2007-05-21 14:14:36 +0000
+++ b/bzrlib/errors.py	2007-05-31 17:55:49 +0000
@@ -459,7 +459,7 @@
         self.actual = actual
 
 
-class PathNotChild(BzrError):
+class PathNotChild(PathError):
 
     _fmt = "Path %(path)r is not a child of path %(base)r%(extra)s"
 

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2007-04-26 07:21:24 +0000
+++ b/bzrlib/tests/test_http.py	2007-05-31 17:55:49 +0000
@@ -201,10 +201,11 @@
     def test_invalid_http_urls(self):
         """Trap invalid construction of urls"""
         t = self._transport('http://bazaar-vcs.org/bzr/bzr.dev/')
-        self.assertRaises(ValueError, t.abspath, '.bzr/')
-        t = self._transport('http://http://bazaar-vcs.org/bzr/bzr.dev/')
+        # Do we really care ?
+        # self.assertRaises(ValueError, t.abspath, '.bzr/')
         self.assertRaises((errors.InvalidURL, errors.ConnectionError),
-                          t.has, 'foo/bar')
+                          self._transport,
+                          'http://http://bazaar-vcs.org/bzr/bzr.dev/')
 
     def test_http_root_urls(self):
         """Construction of URLs from server root"""

=== modified file 'bzrlib/tests/test_sftp_transport.py'
--- a/bzrlib/tests/test_sftp_transport.py	2007-03-28 08:49:06 +0000
+++ b/bzrlib/tests/test_sftp_transport.py	2007-05-31 17:55:49 +0000
@@ -21,14 +21,28 @@
 import threading
 import time
 
-import bzrlib.bzrdir as bzrdir
-import bzrlib.errors as errors
-from bzrlib.osutils import pathjoin, lexists, set_or_unset_env
-from bzrlib.tests import TestCaseWithTransport, TestCase, TestSkipped
+from bzrlib import (
+    bzrdir,
+    errors,
+    )
+from bzrlib.osutils import (
+    pathjoin,
+    lexists,
+    set_or_unset_env,
+    )
+from bzrlib.tests import (
+    TestCaseWithTransport,
+    TestCase,
+    TestSkipped,
+    )
 from bzrlib.tests.HttpServer import HttpServer
-import bzrlib.transport
 from bzrlib.transport import get_transport
 import bzrlib.transport.http
+from bzrlib.transport.sftp import (
+    SFTPAbsoluteServer,
+    SFTPHomeDirServer,
+    SFTPTransport,
+    )
 from bzrlib.workingtree import WorkingTree
 
 try:
@@ -40,7 +54,6 @@
 
 def set_test_transport_to_sftp(testcase):
     """A helper to set transports on test case instances."""
-    from bzrlib.transport.sftp import SFTPAbsoluteServer, SFTPHomeDirServer
     if getattr(testcase, '_get_remote_is_absolute', None) is None:
         testcase._get_remote_is_absolute = True
     if testcase._get_remote_is_absolute:
@@ -143,7 +156,7 @@
 
 class FakeSFTPTransport (object):
     _sftp = object()
-fake = FakeSFTPTransport()
+avoid_sftp_connection = FakeSFTPTransport()
 
 
 class SFTPNonServerTest(TestCase):
@@ -152,63 +165,20 @@
         if not paramiko_loaded:
             raise TestSkipped('you must have paramiko to run this test')
 
-    def test_parse_url(self):
-        from bzrlib.transport.sftp import SFTPTransport
-        s = SFTPTransport('sftp://simple.example.com/home/source', clone_from=fake)
-        self.assertEquals(s._host, 'simple.example.com')
-        self.assertEquals(s._port, None)
-        self.assertEquals(s._path, '/home/source')
-        self.failUnless(s._password is None)
-
-        self.assertEquals(s.base, 'sftp://simple.example.com/home/source/')
-
-        s = SFTPTransport('sftp://ro%62ey:h%40t@example.com:2222/~/relative', clone_from=fake)
+    def test_parse_url_with_home_dir(self):
+        s = SFTPTransport('sftp://ro%62ey:h%40t@example.com:2222/~/relative',
+                          clone_from=avoid_sftp_connection)
         self.assertEquals(s._host, 'example.com')
         self.assertEquals(s._port, 2222)
-        self.assertEquals(s._username, 'robey')
+        self.assertEquals(s._user, 'robey')
         self.assertEquals(s._password, 'h at t')
-        self.assertEquals(s._path, 'relative')
-
-        # Base should not keep track of the password
-        self.assertEquals(s.base, 'sftp://robey@example.com:2222/~/relative/')
+        self.assertEquals(s._path, 'relative/')
 
     def test_relpath(self):
-        from bzrlib.transport.sftp import SFTPTransport
-        from bzrlib.errors import PathNotChild
-
-        s = SFTPTransport('sftp://user@host.com/abs/path', clone_from=fake)
-        self.assertEquals(s.relpath('sftp://user@host.com/abs/path/sub'), 'sub')
-        # Can't test this one, because we actually get an AssertionError
-        # TODO: Consider raising an exception rather than an assert
-        #self.assertRaises(PathNotChild, s.relpath, 'http://user@host.com/abs/path/sub')
-        self.assertRaises(PathNotChild, s.relpath, 'sftp://user2@host.com/abs/path/sub')
-        self.assertRaises(PathNotChild, s.relpath, 'sftp://user@otherhost.com/abs/path/sub')
-        self.assertRaises(PathNotChild, s.relpath, 'sftp://user@host.com:33/abs/path/sub')
-        self.assertRaises(PathNotChild, s.relpath, 'sftp://user@host.com/~/rel/path/sub')
-
-        # Make sure it works when we don't supply a username
-        s = SFTPTransport('sftp://host.com/abs/path', clone_from=fake)
-        self.assertEquals(s.relpath('sftp://host.com/abs/path/sub'), 'sub')
-
-        # Make sure it works when parts of the path will be url encoded
-        # TODO: These may be incorrect, we might need to urllib.urlencode() before
-        # we pass the paths into the SFTPTransport constructor
-        s = SFTPTransport('sftp://host.com/dev/,path', clone_from=fake)
-        self.assertEquals(s.relpath('sftp://host.com/dev/,path/sub'), 'sub')
-        s = SFTPTransport('sftp://host.com/dev/%path', clone_from=fake)
-        self.assertEquals(s.relpath('sftp://host.com/dev/%path/sub'), 'sub')
-
-    def test_parse_invalid_url(self):
-        from bzrlib.transport.sftp import SFTPTransport, TransportError
-        try:
-            s = SFTPTransport('sftp://lilypond.org:~janneke/public_html/bzr/gub',
-                              clone_from=fake)
-            self.fail('expected exception not raised')
-        except TransportError, e:
-            self.assertEquals(str(e),
-                    'Transport error: '
-                    'invalid port number ~janneke in url:\n'
-                    'sftp://lilypond.org:~janneke/public_html/bzr/gub ')
+        s = SFTPTransport('sftp://user@host.com/abs/path',
+                          clone_from=avoid_sftp_connection)
+        self.assertRaises(errors.PathNotChild, s.relpath,
+                          'sftp://user@host.com/~/rel/path/sub')
 
     def test_get_paramiko_vendor(self):
         """Test that if no 'ssh' is available we get builtin paramiko"""

=== modified file 'bzrlib/tests/test_transport.py'
--- a/bzrlib/tests/test_transport.py	2007-05-29 13:17:41 +0000
+++ b/bzrlib/tests/test_transport.py	2007-05-31 17:55:49 +0000
@@ -36,6 +36,7 @@
                            )
 from bzrlib.tests import TestCase, TestCaseInTempDir
 from bzrlib.transport import (_CoalescedOffset,
+                              ConnectedTransport,
                               _get_protocol_handlers,
                               _set_protocol_handlers,
                               _get_transport_modules,
@@ -593,8 +594,58 @@
         t = t.clone('..')
         self.assertEquals(t.base, 'file://HOST/')
 
+class TestConnectedTransport(TestCase):
+    """Tests for connected to remote server transports"""
+
+    def test_parse_url(self):
+        t = ConnectedTransport('sftp://simple.example.com/home/source')
+        self.assertEquals(t._host, 'simple.example.com')
+        self.assertEquals(t._port, None)
+        self.assertEquals(t._path, '/home/source/')
+        self.failUnless(t._user is None)
+        self.failUnless(t._password is None)
+
+        self.assertEquals(t.base, 'sftp://simple.example.com/home/source/')
+
+    def test_parse_quoted_url(self):
+        t = ConnectedTransport('http://ro%62ey:h%40t@ex%41mple.com:2222/path')
+        self.assertEquals(t._host, 'exAmple.com')
+        self.assertEquals(t._port, 2222)
+        self.assertEquals(t._user, 'robey')
+        self.assertEquals(t._password, 'h at t')
+        self.assertEquals(t._path, '/path/')
+
+        # Base should not keep track of the password
+        self.assertEquals(t.base, 'http://robey@exAmple.com:2222/path/')
+
+    def test_parse_invalid_url(self):
+        self.assertRaises(errors.InvalidURL,
+                          ConnectedTransport,
+                          'sftp://lily.org:~janneke/public/bzr/gub')
+
+    def test_relpath(self):
+        t = ConnectedTransport('sftp://user@host.com/abs/path')
+
+        self.assertEquals(t.relpath('sftp://user@host.com/abs/path/sub'), 'sub')
+        self.assertRaises(errors.PathNotChild, t.relpath,
+                          'http://user@host.com/abs/path/sub')
+        self.assertRaises(errors.PathNotChild, t.relpath,
+                          'sftp://user2@host.com/abs/path/sub')
+        self.assertRaises(errors.PathNotChild, t.relpath,
+                          'sftp://user@otherhost.com/abs/path/sub')
+        self.assertRaises(errors.PathNotChild, t.relpath,
+                          'sftp://user@host.com:33/abs/path/sub')
+        # Make sure it works when we don't supply a username
+        t = ConnectedTransport('sftp://host.com/abs/path')
+        self.assertEquals(t.relpath('sftp://host.com/abs/path/sub'), 'sub')
+
+        # Make sure it works when parts of the path will be url encoded
+        t = ConnectedTransport('sftp://host.com/dev/%path')
+        self.assertEquals(t.relpath('sftp://host.com/dev/%path/sub'), 'sub')
+
 
 class TestReusedTransports(TestCase):
+    """Tests for transport reuse"""
 
     def test_reuse_same_transport(self):
         t = get_transport('http://foo/')

=== modified file 'bzrlib/tests/test_transport_implementations.py'
--- a/bzrlib/tests/test_transport_implementations.py	2007-05-02 14:36:55 +0000
+++ b/bzrlib/tests/test_transport_implementations.py	2007-05-31 17:55:49 +0000
@@ -100,11 +100,11 @@
         self.build_tree(files, transport=t)
         self.assertEqual(True, t.has('a'))
         self.assertEqual(False, t.has('c'))
-        self.assertEqual(True, t.has(urlutils.escape('%')))
+        self.assertEqual(True, t.has('%'))
         self.assertEqual(list(t.has_multi(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h'])),
                 [True, True, False, False, True, False, True, False])
         self.assertEqual(True, t.has_any(['a', 'b', 'c']))
-        self.assertEqual(False, t.has_any(['c', 'd', 'f', urlutils.escape('%%')]))
+        self.assertEqual(False, t.has_any(['c', 'd', 'f', '%%']))
         self.assertEqual(list(t.has_multi(iter(['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h']))),
                 [True, True, False, False, True, False, True, False])
         self.assertEqual(False, t.has_any(['c', 'c', 'c']))
@@ -1165,7 +1165,7 @@
         # that have aliasing problems like symlinks should go in backend
         # specific test cases.
         transport = self.get_transport()
-        
+
         self.assertEqual(transport.base + 'relpath',
                          transport.abspath('relpath'))
 
@@ -1291,7 +1291,7 @@
             self.check_transport_contents(contents, t, urlutils.escape(fname))
 
     def test_connect_twice_is_same_content(self):
-        # check that our server (whatever it is) is accessable reliably
+        # check that our server (whatever it is) is accessible reliably
         # via get_transport and multiple connections share content.
         transport = self.get_transport()
         if transport.is_readonly():
@@ -1306,8 +1306,8 @@
         # now opening at a relative url should give use a sane result:
         transport.mkdir('newdir')
         transport2 = bzrlib.transport.get_transport(transport.base + "newdir")
-        transport2 = transport2.clone('..')
-        self.check_transport_contents('bar', transport2, 'foo')
+        transport3 = transport2.clone('..')
+        self.check_transport_contents('bar', transport3, 'foo')
 
     def test_lock_write(self):
         """Test transport-level write locks.

=== modified file 'bzrlib/transport/__init__.py'
--- a/bzrlib/transport/__init__.py	2007-05-30 07:15:16 +0000
+++ b/bzrlib/transport/__init__.py	2007-05-31 17:55:49 +0000
@@ -57,6 +57,7 @@
         DEPRECATED_PARAMETER,
         zero_eight,
         zero_eleven,
+        zero_seventeen,
         )
 from bzrlib.trace import (
     note,
@@ -169,6 +170,7 @@
 
 
 
+ at deprecated_function(zero_seventeen)
 def split_url(url):
     # TODO: jam 20060606 urls should only be ascii, or they should raise InvalidURL
     if isinstance(url, unicode):
@@ -377,13 +379,8 @@
         :param relpath: relative url string for relative part of remote path.
         :return: urlencoded string for final path.
         """
-        # FIXME: share the common code across more transports; variants of
-        # this likely occur in http and sftp too.
-        #
-        # TODO: Also need to consider handling of ~, which might vary between
-        # transports?
         if not isinstance(relpath, str):
-            raise errors.InvalidURL("not a valid url: %r" % relpath)
+            raise errors.InvalidURL(relpath)
         if relpath.startswith('/'):
             base_parts = []
         else:
@@ -1032,6 +1029,135 @@
         return False
 
 
+class ConnectedTransport(Transport):
+    """A transport connected to a remote server.
+
+    Base class for transports that connect to a remote server
+    with optional user and password. Cloning preserves existing
+    connection and credentials.
+    """
+
+    def __init__(self, base, from_transport=None):
+        if base[-1] != '/':
+            base += '/'
+        if from_transport is not None:
+            # Copy the passowrd as it does not appear in base
+            self._password = from_transport._password
+        (self._scheme,
+         self._user, self._password,
+         self._host, self._port,
+         self._path) = self._split_url(base)
+        # Rebuild base taken any special handling into account
+        base = self._unsplit_url(self._scheme,
+                                 self._user, self._password,
+                                 self._host, self._port,
+                                 self._urlencode_abspath(self._path))
+        super(ConnectedTransport, self).__init__(base)
+        if from_transport is not None:
+            connection = from_transport.get_connection()
+        else:
+            connection = None
+        self.set_connection(connection)
+
+    def _split_url(self, url):
+        if isinstance(url, unicode):
+            import pdb; pdb.set_trace()
+            raise errors.InvalidURL('should be ascii:\n%r' % url)
+        url = url.encode('utf-8')
+        (scheme, netloc, path, params,
+         query, fragment) = urlparse.urlparse(url, allow_fragments=False)
+        username = password = host = port = None
+        if '@' in netloc:
+            username, host = netloc.split('@', 1)
+            if ':' in username:
+                username, password = username.split(':', 1)
+                password = urllib.unquote(password)
+            username = urllib.unquote(username)
+        else:
+            host = netloc
+
+        if ':' in host:
+            host, port = host.rsplit(':', 1)
+            try:
+                port = int(port)
+            except ValueError:
+                raise errors.InvalidURL('invalid port number %s in url:\n%s' %
+                                        (port, url))
+        host = urllib.unquote(host)
+        path = self._urldecode_abspath(path)
+
+        return (scheme, username, password, host, port, path)
+
+    def _unsplit_url(self, scheme, user, password, host, port, path):
+        netloc = urllib.quote(host)
+        if user is not None:
+            # Note that we don't put the password back even if we
+            # have one so that it doesn't get accidentally
+            # exposed.
+            netloc = '%s@%s' % (urllib.quote(user), netloc)
+        if port is not None:
+            netloc = '%s:%d' % (netloc, port)
+        return urlparse.urlunparse((scheme, netloc, path, None, None, None))
+
+    def _urlencode_abspath(self, abspath):
+        return urllib.quote(abspath)
+
+    def _urldecode_abspath(self, abspath):
+        return urllib.unquote(abspath)
+
+    def relpath(self, abspath):
+        scheme, user, password, host, port, path = self._split_url(abspath)
+        error = []
+        if (scheme != self._scheme):
+            error.append('scheme mismatch')
+        if (user != self._user):
+            error.append('username mismatch')
+        if (host != self._host):
+            error.append('host mismatch')
+        if (port != self._port):
+            error.append('port mismatch')
+        if not (path == self._path[:-1] or path.startswith(self._path)):
+            error.append('path mismatch')
+        if error:
+            extra = ', '.join(error)
+            raise errors.PathNotChild(abspath, self.base, extra=extra)
+        pl = len(self._path)
+        return path[pl:].strip('/')
+
+    def abspath(self, relpath):
+        """Return the full url to the given relative path.
+        
+        :param relpath: the relative path urlencoded
+        """
+        path = self._remote_path(relpath)
+        return self._unsplit_url(self._scheme, self._user, self._password,
+                                 self._host, self._port,
+                                 self._urlencode_abspath(path))
+
+    def _remote_path(self, relpath):
+        """Return the absolute path part of the url to the given relative path.
+        
+        :param relpath: is a urlencoded string.
+        """
+        relative = urlutils.unescape(relpath).encode('utf-8')
+        remote_path = self._combine_paths(self._path, relative)
+        return remote_path
+
+    def get_connection(self):
+        """Returns the transport specific connection object"""
+        return self._connection
+
+    def set_connection(self, connection):
+        """Set the transport specific connection object.
+
+        Note: daughter classes should ensure that the connection
+        is still shared if the connection is reset during the
+        transport lifetime (using a list containing the single
+        connection can help avoid aliasing bugs).
+        """
+        self._connection = connection
+
+
 # jam 20060426 For compatibility we copy the functions here
 # TODO: The should be marked as deprecated
 urlescape = urlutils.escape
@@ -1147,10 +1273,6 @@
     return None, last_err
 
 
-class ConnectedTransport(Transport):
-    """A transport connected to a remote server"""
-
-
 class Server(object):
     """A Transport Server.
     

=== modified file 'bzrlib/transport/ftp.py'
--- a/bzrlib/transport/ftp.py	2007-05-30 07:15:16 +0000
+++ b/bzrlib/transport/ftp.py	2007-05-31 17:55:49 +0000
@@ -47,7 +47,6 @@
 from bzrlib.trace import mutter, warning
 from bzrlib.transport import (
     Server,
-    split_url,
     ConnectedTransport,
     )
 from bzrlib.transport.local import LocalURLServer
@@ -95,7 +94,7 @@
             base += '/'
         (self._proto, self._username,
             self._password, self._host,
-            self._port, self._path) = split_url(base)
+            self._port, self._path) = self._split_url(base)
         base = self._unparse_url()
 
         super(FtpTransport, self).__init__(base)

=== modified file 'bzrlib/transport/http/__init__.py'
--- a/bzrlib/transport/http/__init__.py	2007-05-30 07:15:16 +0000
+++ b/bzrlib/transport/http/__init__.py	2007-05-31 17:55:49 +0000
@@ -26,7 +26,11 @@
 import urllib
 import sys
 
-from bzrlib import errors, ui
+from bzrlib import (
+    errors,
+    ui,
+    urlutils,
+    )
 from bzrlib.smart import medium
 from bzrlib.trace import mutter
 from bzrlib.transport import (
@@ -123,7 +127,7 @@
     """
 
     # _proto: "http" or "https"
-    # _qualified_proto: may have "+pycurl", etc
+    # _scheme: may have "+pycurl", etc
 
     def __init__(self, base, from_transport=None):
         """Set the base path where files will be stored."""
@@ -135,13 +139,7 @@
         if impl_name:
             impl_name = impl_name[1:]
         self._impl_name = impl_name
-        if base[-1] != '/':
-            base = base + '/'
-        super(HttpTransportBase, self).__init__(base)
-        (apparent_proto, self._host,
-            self._path, self._parameters,
-            self._query, self._fragment) = urlparse.urlparse(self.base)
-        self._qualified_proto = apparent_proto
+        super(HttpTransportBase, self).__init__(base, from_transport)
         # range hint is handled dynamically throughout the life
         # of the transport object. We start by trying multi-range
         # requests and if the server returns bogus results, we
@@ -154,62 +152,12 @@
         else:
             self._range_hint = 'multi'
 
-    def abspath(self, relpath):
-        """Return the full url to the given relative path.
-
-        This can be supplied with a string or a list.
-
-        The URL returned always has the protocol scheme originally used to 
-        construct the transport, even if that includes an explicit
-        implementation qualifier.
-        """
-        assert isinstance(relpath, basestring)
-        if isinstance(relpath, unicode):
-            raise errors.InvalidURL(relpath, 'paths must not be unicode.')
-        if isinstance(relpath, basestring):
-            relpath_parts = relpath.split('/')
-        else:
-            # TODO: Don't call this with an array - no magic interfaces
-            relpath_parts = relpath[:]
-        if relpath.startswith('/'):
-            basepath = []
-        else:
-            # Except for the root, no trailing slashes are allowed
-            if len(relpath_parts) > 1 and relpath_parts[-1] == '':
-                raise ValueError(
-                    "path %r within branch %r seems to be a directory"
-                    % (relpath, self._path))
-            basepath = self._path.split('/')
-            if len(basepath) > 0 and basepath[-1] == '':
-                basepath = basepath[:-1]
-
-        for p in relpath_parts:
-            if p == '..':
-                if len(basepath) == 0:
-                    # In most filesystems, a request for the parent
-                    # of root, just returns root.
-                    continue
-                basepath.pop()
-            elif p == '.' or p == '':
-                continue # No-op
-            else:
-                basepath.append(p)
-        # Possibly, we could use urlparse.urljoin() here, but
-        # I'm concerned about when it chooses to strip the last
-        # portion of the path, and when it doesn't.
-        path = '/'.join(basepath)
-        if path == '':
-            path = '/'
-        result = urlparse.urlunparse((self._qualified_proto,
-                                    self._host, path, '', '', ''))
-        return result
-
     def _real_abspath(self, relpath):
         """Produce absolute path, adjusting protocol if needed"""
         abspath = self.abspath(relpath)
-        qp = self._qualified_proto
+        qp = self._scheme
         rp = self._proto
-        if self._qualified_proto != self._proto:
+        if self._scheme != self._proto:
             abspath = rp + abspath[len(qp):]
         if not isinstance(abspath, str):
             # escaping must be done at a higher level

=== modified file 'bzrlib/transport/http/_pycurl.py'
--- a/bzrlib/transport/http/_pycurl.py	2007-03-13 17:00:20 +0000
+++ b/bzrlib/transport/http/_pycurl.py	2007-05-31 17:55:49 +0000
@@ -294,7 +294,7 @@
             raise errors.RedirectRequested(url,
                                            redirected_to,
                                            is_permament=(code == 301),
-                                           qual_proto=self._qualified_proto)
+                                           qual_proto=self._scheme)
 
 
 def get_test_permutations():

=== modified file 'bzrlib/transport/http/_urllib.py'
--- a/bzrlib/transport/http/_urllib.py	2007-04-22 16:32:04 +0000
+++ b/bzrlib/transport/http/_urllib.py	2007-05-31 17:55:49 +0000
@@ -105,7 +105,7 @@
             raise errors.RedirectRequested(request.get_full_url(),
                                            request.redirected_to,
                                            is_permament=(code == 301),
-                                           qual_proto=self._qualified_proto)
+                                           qual_proto=self._scheme)
 
         if request.redirected_to is not None:
             mutter('redirected from: %s to: %s' % (request.get_full_url(),

=== modified file 'bzrlib/transport/remote.py'
--- a/bzrlib/transport/remote.py	2007-05-30 07:15:16 +0000
+++ b/bzrlib/transport/remote.py	2007-05-31 17:55:49 +0000
@@ -93,7 +93,7 @@
             url += '/'
         super(RemoteTransport, self).__init__(url)
         self._scheme, self._username, self._password, self._host, self._port, self._path = \
-                transport.split_url(url)
+                self._split_url(url)
         if clone_from is None:
             self._medium = medium
         else:
@@ -445,7 +445,7 @@
 
     def __init__(self, url):
         _scheme, _username, _password, _host, _port, _path = \
-            transport.split_url(url)
+            self._split_url(url)
         if _port is None:
             _port = BZR_DEFAULT_PORT
         else:
@@ -467,7 +467,7 @@
 
     def __init__(self, url):
         _scheme, _username, _password, _host, _port, _path = \
-            transport.split_url(url)
+            self._split_url(url)
         try:
             if _port is not None:
                 _port = int(_port)

=== modified file 'bzrlib/transport/sftp.py'
--- a/bzrlib/transport/sftp.py	2007-05-30 07:15:16 +0000
+++ b/bzrlib/transport/sftp.py	2007-05-31 17:55:49 +0000
@@ -53,7 +53,6 @@
     local,
     register_urlparse_netloc_protocol,
     Server,
-    split_url,
     ssh,
     ConnectedTransport,
     )
@@ -138,64 +137,41 @@
 class SFTPUrlHandling(ConnectedTransport):
     """Mix-in that does common handling of SSH/SFTP URLs."""
 
-    def __init__(self, base):
-        self._parse_url(base)
-        base = self._unparse_url(self._path)
-        if base[-1] != '/':
-            base += '/'
-        super(SFTPUrlHandling, self).__init__(base)
-
-    def _parse_url(self, url):
-        (self._scheme,
-         self._username, self._password,
-         self._host, self._port, self._path) = self._split_url(url)
-
-    def _unparse_url(self, path):
-        """Return a URL for a path relative to this transport.
-        """
-        path = urllib.quote(path)
+    def _urlencode_abspath(self, abspath):
+        abspath = super(SFTPUrlHandling, self)._urlencode_abspath(abspath)
         # handle homedir paths
-        if not path.startswith('/'):
-            path = "/~/" + path
-        netloc = urllib.quote(self._host)
-        if self._username is not None:
-            netloc = '%s@%s' % (urllib.quote(self._username), netloc)
-        if self._port is not None:
-            netloc = '%s:%d' % (netloc, self._port)
-        return urlparse.urlunparse((self._scheme, netloc, path, '', '', ''))
-
-    def _split_url(self, url):
-        (scheme, username, password, host, port, path) = split_url(url)
-        ## assert scheme == 'sftp'
-
+        if not abspath.startswith('/'):
+            abspath = "/~/" + abspath
+        return abspath
+
+    def _urldecode_abspath(self, abspath):
+        abspath = super(SFTPUrlHandling, self)._urldecode_abspath(abspath)
         # the initial slash should be removed from the path, and treated
         # as a homedir relative path (the path begins with a double slash
         # if it is absolute).
         # see draft-ietf-secsh-scp-sftp-ssh-uri-03.txt
         # RBC 20060118 we are not using this as its too user hostile. instead
         # we are following lftp and using /~/foo to mean '~/foo'.
+
         # handle homedir paths
-        if path.startswith('/~/'):
-            path = path[3:]
-        elif path == '/~':
-            path = ''
-        return (scheme, username, password, host, port, path)
+        if abspath.startswith('/~/'):
+            abspath = abspath[3:]
+        elif abspath == '/~':
+            abspath = ''
+        return abspath
 
-    def abspath(self, relpath):
-        """Return the full url to the given relative path.
-        
-        @param relpath: the relative path or path components
-        @type relpath: str or list
-        """
-        return self._unparse_url(self._remote_path(relpath))
-    
     def _remote_path(self, relpath):
-        """Return the path to be passed along the sftp protocol for relpath.
-        
-        :param relpath: is a urlencoded string.
-        """
-        return self._combine_paths(self._path, relpath)
-
+        relative = urlutils.unescape(relpath).encode('utf-8')
+        if not self._path.startswith('/'):
+            # handle homedir paths
+            remote_path = self._combine_paths('/~/' + self._path, relative)
+            if remote_path.startswith('/~/'):
+                remote_path = remote_path[3:]
+            elif remote_path == '/~':
+                remote_path = ''
+        else:
+            remote_path = self._combine_paths(self._path, relative)
+        return remote_path
 
 class SFTPTransport(SFTPUrlHandling):
     """Transport implementation for SFTP access."""
@@ -244,132 +220,6 @@
         else:
             return SFTPTransport(self.abspath(offset), self)
 
-    def _remote_path(self, relpath):
-        """Return the path to be passed along the sftp protocol for relpath.
-        
-        relpath is a urlencoded string.
-
-        :return: a path prefixed with / for regular abspath-based urls, or a
-            path that does not begin with / for urls which begin with /~/.
-        """
-        # how does this work? 
-        # it processes relpath with respect to 
-        # our state:
-        # firstly we create a path to evaluate: 
-        # if relpath is an abspath or homedir path, its the entire thing
-        # otherwise we join our base with relpath
-        # then we eliminate all empty segments (double //'s) outside the first
-        # two elements of the list. This avoids problems with trailing 
-        # slashes, or other abnormalities.
-        # finally we evaluate the entire path in a single pass
-        # '.'s are stripped,
-        # '..' result in popping the left most already 
-        # processed path (which can never be empty because of the check for
-        # abspath and homedir meaning that its not, or that we've used our
-        # path. If the pop would pop the root, we ignore it.
-
-        # Specific case examinations:
-        # remove the special casefor ~: if the current root is ~/ popping of it
-        # = / thus our seed for a ~ based path is ['', '~']
-        # and if we end up with [''] then we had basically ('', '..') (which is
-        # '/..' so we append '' if the length is one, and assert that the first
-        # element is still ''. Lastly, if we end with ['', '~'] as a prefix for
-        # the output, we've got a homedir path, so we strip that prefix before
-        # '/' joining the resulting list.
-        #
-        # case one: '/' -> ['', ''] cannot shrink
-        # case two: '/' + '../foo' -> ['', 'foo'] (take '', '', '..', 'foo')
-        #           and pop the second '' for the '..', append 'foo'
-        # case three: '/~/' -> ['', '~', ''] 
-        # case four: '/~/' + '../foo' -> ['', '~', '', '..', 'foo'],
-        #           and we want to get '/foo' - the empty path in the middle
-        #           needs to be stripped, then normal path manipulation will 
-        #           work.
-        # case five: '/..' ['', '..'], we want ['', '']
-        #            stripping '' outside the first two is ok
-        #            ignore .. if its too high up
-        #
-        # lastly this code is possibly reusable by FTP, but not reusable by
-        # local paths: ~ is resolvable correctly, nor by HTTP or the smart
-        # server: ~ is resolved remotely.
-        # 
-        # however, a version of this that acts on self.base is possible to be
-        # written which manipulates the URL in canonical form, and would be
-        # reusable for all transports, if a flag for allowing ~/ at all was
-        # provided.
-        assert isinstance(relpath, basestring)
-        relpath = urlutils.unescape(relpath)
-
-        # case 1)
-        if relpath.startswith('/'):
-            # abspath - normal split is fine.
-            current_path = relpath.split('/')
-        elif relpath.startswith('~/'):
-            # root is homedir based: normal split and prefix '' to remote the
-            # special case
-            current_path = [''].extend(relpath.split('/'))
-        else:
-            # root is from the current directory:
-            if self._path.startswith('/'):
-                # abspath, take the regular split
-                current_path = []
-            else:
-                # homedir based, add the '', '~' not present in self._path
-                current_path = ['', '~']
-            # add our current dir
-            current_path.extend(self._path.split('/'))
-            # add the users relpath
-            current_path.extend(relpath.split('/'))
-        # strip '' segments that are not in the first one - the leading /.
-        to_process = current_path[:1]
-        for segment in current_path[1:]:
-            if segment != '':
-                to_process.append(segment)
-
-        # process '.' and '..' segments into output_path.
-        output_path = []
-        for segment in to_process:
-            if segment == '..':
-                # directory pop. Remove a directory 
-                # as long as we are not at the root
-                if len(output_path) > 1:
-                    output_path.pop()
-                # else: pass
-                # cannot pop beyond the root, so do nothing
-            elif segment == '.':
-                continue # strip the '.' from the output.
-            else:
-                # this will append '' to output_path for the root elements,
-                # which is appropriate: its why we strip '' in the first pass.
-                output_path.append(segment)
-
-        # check output special cases:
-        if output_path == ['']:
-            # [''] -> ['', '']
-            output_path = ['', '']
-        elif output_path[:2] == ['', '~']:
-            # ['', '~', ...] -> ...
-            output_path = output_path[2:]
-        path = '/'.join(output_path)
-        return path
-
-    def relpath(self, abspath):
-        scheme, username, password, host, port, path = self._split_url(abspath)
-        error = []
-        if (username != self._username):
-            error.append('username mismatch')
-        if (host != self._host):
-            error.append('host mismatch')
-        if (port != self._port):
-            error.append('port mismatch')
-        if (not path.startswith(self._path)):
-            error.append('path mismatch')
-        if error:
-            extra = ': ' + ', '.join(error)
-            raise PathNotChild(abspath, self.base, extra=extra)
-        pl = len(self._path)
-        return path[pl:].strip('/')
-
     def has(self, relpath):
         """
         Does the target location exist?
@@ -832,8 +682,9 @@
 
         TODO: Raise a more reasonable ConnectionFailed exception
         """
-        self._sftp = _sftp_connect(self._host, self._port, self._username,
-                self._password)
+        self._sftp = _sftp_connect(self._host, self._port,
+                                   self._user,
+                                   self._password)
 
     def _sftp_open_exclusive(self, abspath, mode=None):
         """Open a remote path exclusively.



More information about the bazaar-commits mailing list