[BUG?] bzr server hooks dont expose the real local url

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Jun 5 08:28:24 BST 2007


>>>>> "robert" == Robert Collins <robertc at robertcollins.net> writes:

    robert> On Mon, 2007-06-04 at 11:46 +0200, Vincent Ladeuil wrote:
    >> 
    >> I think you're looking for abspath here, but you have used it for
    >> clone (may be wrongly).
    >> 
    >> I think abspath definition is: from this path relative to you
    >> (the transport), give me back something that I can use to present
    >> that path to the user without referring to you.

Vila, you're on crack.

test_transport_implementation.TransportTests.test_clone_to_root
rings a bell ? You've been bitten by this one already.

And what about:

class ConnectedTransport(Transport):
    def clone(self, offset=None):
        if offset is None:
            return self.__class__(self.base, self)
        else:
            return self.__class__(self.abspath(offset), self)

you wrote some days ago ? Doesn't use abspath ? Huh ?

And the care you take in HttpTransportBase to make abspath
returns the http+urllib instead of http prefix while _remote_path
returns http (and make it a good candidate for external_url
implementation) ? Do you imagine systems outside bzrlib knowing
something about +urllib or +pycurl ?

    robert> No, I'm not looking for abspath. The abspath as
    robert> currently defined is used within the bzrlib code to
    robert> get the absolute path of a thing

    robert> within the transport.

*within* indeed, I was confused it seems ;-)

    robert> And clone should definately be going to the abspath,
    robert> I dont think thats a bug at all.

Nope. Definitely. It's even the spec of abspath as described in
the test above.

    robert> Even though one can redefine it, it would then be
    robert> inconsistent with os.path.abspath, and also I think
    robert> in the interests of security we really only want an
    robert> api that won't be easily misunderstood as one for
    robert> regular use.

But coming back to the original proposal:

    robert> for memory: it will raise an error (its not externally visible)
    robert> for chroot: it will return self.server.backing_transport.external_url
    robert> for everything else, its self.base

self.base + relpath except for transports that use decorators (including
aftp). 

I'm less clear for things like bzr+ssh though, should that just be ssh:/ ?

And an error for bzr:/ ?

       Vincent



More information about the bazaar mailing list