Merge lp:~doxxx/bzr/534787-pull-lp-in-config into lp:bzr

[Gordon Tyler]

(doxxx) Fixed 'bzr pull' when lp: URL explicitly defined in locations.conf or branch.conf.

This fixes bug 534787 by fixing how get_transport tests whether a base path is actually a URL in its convert_path_to_url function.

It depends on lp:~doxxx/bzr/urlutils-lp-support for fixes in bzrlib.urlutils to handle lp: URLs.

[Martin Pool (mbp)]

That looks pretty nice. (Yet to review the other patch.)

review: Approve

[Andrew Bennetts (spiv)]

 review approve

> This fixes bug 534787 by fixing how get_transport tests whether a base path is
> actually a URL in its convert_path_to_url function.

Looks good. Only one tweak:

[...]
> +* ``bzr pull`` now works when a lp: URL is explicitly defined as the parent
> + or pull location in locations.conf or branch.conf. (Gordon Tyler)

This NEWS entry should include the bug number. I'll fix that and land it (as
the prerequisite branch is now approved).

Thanks!

review: Approve

[Andrew Bennetts (spiv)]

The combined branch (including my tweak to NEWS, <lp:~spiv/bzr/534787-pull-lp-in-config>) is failing tests:

======================================================================
FAIL: bzrlib.tests.test_urlutils.TestUrlToPath.test_is_url
----------------------------------------------------------------------
_StringException: Text attachment: log
------------

------------
Text attachment: traceback
------------
Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/testtools/runtest.py", line 128, in _run_user
    return fn(*args)
  File "/usr/lib/python2.6/dist-packages/testtools/testcase.py", line 368, in _run_test_method
    testMethod()
  File "/home/andrew/warthogs/bzr/534787-pull-lp-in-config/bzrlib/tests/test_urlutils.py", line 211, in test_is_url
    self.assertTrue(urlutils.is_url('C:/foo/bar'))
AssertionError
------------

review: Needs Fixing

[Gordon Tyler (doxxx)]

Sorry, looks like I left a debugging change in, that should be assertFalse.

=== modified file 'NEWS'
--- NEWS	2010-06-02 01:54:25 +0000
+++ NEWS	2010-06-02 01:54:25 +0000
@@ -29,6 +29,9 @@
   arguments on Windows, because bzr script does the same.
   (Alexander Belchenko, #588277)
 
+* ``bzr pull`` now works when a lp: URL is explicitly defined as the parent
+  or pull location in locations.conf or branch.conf. (Gordon Tyler)
+
 Improvements
 ************
 

=== modified file 'bzrlib/tests/test_urlutils.py'
--- bzrlib/tests/test_urlutils.py	2010-06-02 01:54:25 +0000
+++ bzrlib/tests/test_urlutils.py	2010-06-02 01:54:25 +0000
@@ -195,6 +195,20 @@
             dirname('path/to/foo/', exclude_trailing_slash=False))
         self.assertEqual('path/..', dirname('path/../foo'))
         self.assertEqual('../path', dirname('../path/foo'))
+    
+    def test_is_url(self):
+        self.assertTrue(urlutils.is_url('http://foo/bar'))
+        self.assertTrue(urlutils.is_url('bzr+ssh://foo/bar'))
+        self.assertTrue(urlutils.is_url('lp:foo/bar'))
+        self.assertTrue(urlutils.is_url('file:///foo/bar'))
+        self.assertFalse(urlutils.is_url(''))
+        self.assertFalse(urlutils.is_url('foo'))
+        self.assertFalse(urlutils.is_url('foo/bar'))
+        self.assertFalse(urlutils.is_url('/foo'))
+        self.assertFalse(urlutils.is_url('/foo/bar'))
+        self.assertFalse(urlutils.is_url('C:/'))
+        self.assertFalse(urlutils.is_url('C:/foo'))
+        self.assertTrue(urlutils.is_url('C:/foo/bar'))
 
     def test_join(self):
         def test(expected, *args):

=== modified file 'bzrlib/transport/__init__.py'
--- bzrlib/transport/__init__.py	2010-05-30 16:26:11 +0000
+++ bzrlib/transport/__init__.py	2010-06-02 01:54:25 +0000
@@ -1551,9 +1551,6 @@
         return transport
 
 
-# We try to recognize an url lazily (ignoring user, password, etc)
-_urlRE = re.compile(r'^(?P<proto>[^:/\\]+)://(?P<rest>.*)$')
-
 def get_transport(base, possible_transports=None):
     """Open a transport to access a URL or directory.
 
@@ -1572,8 +1569,7 @@
     base = directories.dereference(base)
 
     def convert_path_to_url(base, error_str):
-        m = _urlRE.match(base)
-        if m:
+        if urlutils.is_url(base):
             # This looks like a URL, but we weren't able to
             # instantiate it as such raise an appropriate error
             # FIXME: we have a 'error_str' unused and we use last_err below

=== modified file 'bzrlib/urlutils.py'
--- bzrlib/urlutils.py	2010-06-02 01:54:25 +0000
+++ bzrlib/urlutils.py	2010-06-02 01:54:25 +0000
@@ -104,6 +104,11 @@
     return len(scheme), first_path_slash+m.start('path')
 
 
+def is_url(url):
+    """Tests whether a URL is in actual fact a URL."""
+    return _url_scheme_re.match(url) is not None
+
+
 def join(base, *args):
     """Create a URL by joining sections.