[PATCH] Re: any more sftp fixes?

[Matthieu Moy]

John A Meinel <john at arbash-meinel.com> writes:

> Matthieu Moy wrote:
>> John A Meinel <john at arbash-meinel.com> writes:
>> 
>>> The changes are available here:
>>> http://bzr.arbash-meinel.com/branches/bzr/sftp-fix/
>> 
>> Still doesn't work for me.
>
> Thanks for the bug report.
> Maybe I missed it the first time around, but what is your specific use
> case which is failing?

Couldn't be simpler:

$ ~/dev/sftp-fix/bzr push sftp://localhost//tmp/yyy
Password: 
bzr: ERROR: path u'sftp://localhost//tmp/yyy' is not under base URL 'sftp://localhost/%2Ftmp'

> What about having it be more:
>
> 'path %r is not under base URL %r: %s' % (abspath, self.base, messages)
>
> Where messages would then contain:
> 'username mismatch', 'port mismatch', 'host mismatch', 'path mismatch', etc.

Would be better than my proposal, but the problem I had was that the
message did _not_ show the port number, at all (see above).

>> Moreover, in relpath(self, abspath) too, abspath does not necessarily
>> contain a port number, so _split_url might return None for the port.
>> The patch also fixes this by setting the port to 22 in _split_url()
>> instead of _parse_url(). That seems to be sufficient for me.
>
> Well, I'm just thought about the fact that because we are using the
> installed ssh, ~/.ssh/config can certainly change the default port
> number. So I'm thinking it should actually stay as None, rather than
> forcing it to port 22. I'll change it back, and then can you let me know
> if that also fixes your problem.

It's probably better to leave it to None, yes, but then, you have to
remove all mentions of 22 in your code.

The following patch works for me:

--- a/home/moy/dev/sftp-fix/bzrlib/transport/sftp.py
+++ b/home/moy/dev/sftp-fix/bzrlib/transport/sftp.py
@@ -573,7 +573,7 @@
         netloc = urllib.quote(self._host)
         if self._username is not None:
             netloc = '%s@%s' % (urllib.quote(self._username), netloc)
-        if self._port not in (None, 22):
+        if self._port != None:
             netloc = '%s:%d' % (netloc, self._port)
 
         return urlparse.urlunparse(('sftp', netloc, path, '', '',
         ''))
@@ -616,8 +616,6 @@
     def _parse_url(self, url):
         (self._username, self._password,
          self._host, self._port, self._path) = self._split_url(url)
-        if self._port is None:
-            self._port = 22
 
     def _sftp_connect(self):
         """Connect to the remote sftp server.

-- 
Matthieu