[PATCH] Re: any more sftp fixes?

[John A Meinel]

Matthieu Moy wrote:
> 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).

Yeah, see my follow up, that the old error was completely opaque as to
your problem.

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

Yep. There is also another one in tests/test_sftp.py

But I've caught them. Thanks for the reminder.

John
=:->

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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051130/0905f307/attachment.pgp