[MERGE] Register netloc protocols at 'register_transport_proto' time

Vincent Ladeuil v.ladeuil+lp at free.fr
Sat Oct 20 07:39:04 BST 2007


>>>>> "john" == John Arbash Meinel <john at arbash-meinel.com> writes:

    john> The attached patch fixes the bug I just mentioned about
    john> missing netloc protocol registrations when sharing
    john> transports.

Nice catch.

So, you found a code path where we try to split an url before
calling get_transport...

But we are still in a grey area, see
https://bugs.edge.launchpad.net/bzr/+bug/122258.

Basically, decorators trick urlparse in a way that rarely causes
problems. I think you run into it because you have switched from
sftp to bzr+ssh.

So I think the right solution is either to make urlparse deal
with decorators or to really filter out the decorators *before*
calling urlparse (easier said than done ;-)

    john> I went ahead and defaulted the parameter to False,
    john> because there were more protocols that should not be
    john> registered. (All of the decorators, and memory, etc.)

Can you elaborate on why you did that ?
register_urlparse_netloc_protocol ensures that the urls will be
parsed correctly (most of the time :-/). I'm afraid we are still
exposed to the underlying problem then.

    john> Also, this fixes some unknown bugs, in that
    john> https+urllib and https+pycurl were never registered.

Doh !

    john> Do we need a test for this? Perhaps just a test that if
    john> you set register_netloc=True it adds it to urlparse's
    john> netloc list? And one that when it isn't set, it
    john> doesn't?

We need more tests to ensure that we correctly parse the urls
when we use decorators.

Anyway, this patch improves the current situation, but I think you
should register unconditionally.

In itself, doing that may have its own problems since we can't
add the same prefix two times, but we have no way to verify that
we don't try to *remove* it accordingly.

On the other hand, _unregister_urlparse_netloc_protocol is
private and targeted at the tests, so I'm not too worried.

     Vincent



More information about the bazaar mailing list