[MERGE][0.90][Bug 133965] PathNotChild, port mismatch with "bzr info" for bzr:// smartserver
Andrew Bennetts
andrew at canonical.com
Sun Aug 26 08:55:08 BST 2007
John Arbash Meinel wrote:
[...]
> >
> > Is the best way to change all transports to explicitly set the default
> > port? Can the default be dropped from bzr://?
I think so. I've attached a bundle that does this (i.e. sets the default port).
> > I'm not going to have time to complete this tonight, so if someone can
> > come up with a fix I would appreciate it.
[...]
>
>
> Well, it seems like this would be the simple fix:
> === modified file 'bzrlib/transport/remote.py'
> - --- bzrlib/transport/remote.py 2007-07-30 14:36:04 +0000
> +++ bzrlib/transport/remote.py 2007-08-24 14:20:41 +0000
> @@ -443,9 +443,10 @@
>
> def _build_medium(self):
> assert self.base.startswith('bzr://')
> - - if self._port is None:
> - - self._port = BZR_DEFAULT_PORT
> - - return medium.SmartTCPClientMedium(self._host, self._port), None
> + port = self._port
> + if port is None:
> + port = BZR_DEFAULT_PORT
> + return medium.SmartTCPClientMedium(self._host, port), None
>
>
> class RemoteSSHTransport(RemoteTransport):
>
> Basically, just create a local variable to hold the real port, and don't change
> the self._port value.
Here's a more complete fix, with tests.
I don't mind if John's fix lands in 0.90 if it does the job, but if so:
a) it needs tests. You ought to be able to adapt the tests out of my bundle
easily.
b) I think my more complete fix should still land in bzr.dev.
Anyway I've attached my proposed fix for 0.90.
It adds a “DEFAULT_PORT” class variable to ConnectedTransport for subclasses to
override. So far I've only set the default port on RemoteTCPTransport, to be
conservative because 0.90 is so close to being released. Ideally it would be
used by e.g. HttpTransportBase too, so that if anyone ever does this they get a
sane result, rather than a PathNotChild error as they do now:
t = get_transport('http://example.com/')
...
t.relpath('http://example.com:80/')
I had to change ConnectedTransport._split_url and _unsplit_url to be
classmethods rather than staticmethods to make this work, but I think that's a
safe and reasonable change.
Please review!
-Andrew.
-------------- next part --------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: andrew.bennetts at canonical.com-20070826074034-\
# 4iokcajfev9ph615
# target_branch: http://bazaar-vcs.org/bzr/bzr.0.90
# testament_sha1: 034ac83129283572f0ac7fc77e7de0a48fdbbf99
# timestamp: 2007-08-26 17:42:34 +1000
# source_branch: http://people.ubuntu.com/~andrew/bzr/port-mismatch-\
# bug-133965
# base_revision_id: pqm at pqm.ubuntu.com-20070822004557-ipcsdwnro02sbrpd
#
# Begin patch
=== modified file 'NEWS'
--- NEWS 2007-08-21 20:54:11 +0000
+++ NEWS 2007-08-26 07:40:34 +0000
@@ -18,6 +18,12 @@
from the developer document catalog back to the main one.
(Ian Clatworthy, Sabin Iacob, Alexander Belchenko)
+ BUGFIXES:
+
+ * Fix ''bzr info bzr://host/'' and other operations on ''bzr://' URLs with
+ an implicit port. We were incorrectly raising PathNotChild due to
+ inconsistent treatment of the ''_port'' attribute on the Transport object.
+ (Andrew Bennetts, #133965)
bzr 0.90rc1 2007-08-14
======================
=== modified file 'bzrlib/tests/test_transport.py'
--- bzrlib/tests/test_transport.py 2007-07-22 15:44:59 +0000
+++ bzrlib/tests/test_transport.py 2007-08-26 07:40:34 +0000
@@ -54,6 +54,10 @@
from bzrlib.transport.memory import MemoryTransport
from bzrlib.transport.local import (LocalTransport,
EmulatedWin32LocalTransport)
+from bzrlib.transport.remote import (
+ BZR_DEFAULT_PORT,
+ RemoteTCPTransport
+ )
# TODO: Should possibly split transport-specific tests into their own files.
@@ -714,6 +718,50 @@
self.assertIsNot(t1, t2)
+class TestRemoteTCPTransport(TestCase):
+ """Tests for bzr:// transport (RemoteTCPTransport)."""
+
+ def test_relpath_with_implicit_port(self):
+ """Connected transports with the same URL are the same, even if the
+ port is implicit.
+
+ So t.relpath(url) should always be '' if t.base is the same as url, or
+ if the only difference is that one explicitly specifies the default
+ port and the other doesn't specify a port.
+ """
+ t_implicit_port = RemoteTCPTransport('bzr://host.com/')
+ self.assertEquals('', t_implicit_port.relpath('bzr://host.com/'))
+ self.assertEquals('', t_implicit_port.relpath('bzr://host.com:4155/'))
+ t_explicit_port = RemoteTCPTransport('bzr://host.com:4155/')
+ self.assertEquals('', t_explicit_port.relpath('bzr://host.com/'))
+ self.assertEquals('', t_explicit_port.relpath('bzr://host.com:4155/'))
+
+ def test_construct_uses_default_port(self):
+ """If no port is specified, then RemoteTCPTransport uses
+ BZR_DEFAULT_PORT.
+ """
+ t = get_transport('bzr://host.com/')
+ self.assertEquals(BZR_DEFAULT_PORT, t._port)
+
+ def test_url_omits_default_port(self):
+ """If a RemoteTCPTransport uses the default port, then its base URL
+ will omit the port.
+
+ This is like how ":80" is omitted from "http://example.com/".
+ """
+ t = get_transport('bzr://host.com:4155/')
+ self.assertEquals('bzr://host.com/', t.base)
+
+ def test_url_includes_non_default_port(self):
+ """Non-default ports are included in the transport's URL.
+
+ Contrast this to `test_url_omits_default_port`.
+ """
+ t = get_transport('bzr://host.com:666/')
+ self.assertEquals('bzr://host.com:666/', t.base)
+
+
+
def get_test_permutations():
"""Return transport permutations to be used in testing.
=== modified file 'bzrlib/transport/__init__.py'
--- bzrlib/transport/__init__.py 2007-08-09 03:23:04 +0000
+++ bzrlib/transport/__init__.py 2007-08-26 07:40:34 +0000
@@ -1136,8 +1136,13 @@
Host and credentials are available as private attributes, cloning preserves
them and share the underlying, protocol specific, connection.
+
+ :cvar DEFAULT_PORT: The default port for this protocol. This is the port
+ implied by URLs that don't explicitly specify one. e.g. 80 for HTTP.
"""
+ DEFAULT_PORT = None
+
def __init__(self, base, _from_transport=None):
"""Constructor.
@@ -1184,8 +1189,8 @@
else:
return self.__class__(self.abspath(offset), _from_transport=self)
- @staticmethod
- def _split_url(url):
+ @classmethod
+ def _split_url(cls, url):
"""
Extract the server address, the credentials and the path from the url.
@@ -1222,10 +1227,14 @@
host = urllib.unquote(host)
path = urllib.unquote(path)
+ if port is None:
+ # The port isn't explicitly specified, so return the default (if
+ # there is one).
+ port = cls.DEFAULT_PORT
return (scheme, user, password, host, port, path)
- @staticmethod
- def _unsplit_url(scheme, user, password, host, port, path):
+ @classmethod
+ def _unsplit_url(cls, scheme, user, password, host, port, path):
"""
Build the full URL for the given already URL encoded path.
@@ -1252,7 +1261,9 @@
# have one so that it doesn't get accidentally
# exposed.
netloc = '%s@%s' % (urllib.quote(user), netloc)
- if port is not None:
+ if port is not None and port != cls.DEFAULT_PORT:
+ # Include the port in the netloc (unless it's the same as the
+ # default, in which case we omit it as it is redundant).
netloc = '%s:%d' % (netloc, port)
path = urllib.quote(path)
return urlparse.urlunparse((scheme, netloc, path, None, None, None))
=== modified file 'bzrlib/transport/remote.py'
--- bzrlib/transport/remote.py 2007-07-30 14:36:04 +0000
+++ bzrlib/transport/remote.py 2007-08-26 07:40:34 +0000
@@ -441,10 +441,10 @@
SmartTCPClientMedium).
"""
+ DEFAULT_PORT = BZR_DEFAULT_PORT
+
def _build_medium(self):
assert self.base.startswith('bzr://')
- if self._port is None:
- self._port = BZR_DEFAULT_PORT
return medium.SmartTCPClientMedium(self._host, self._port), None
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWZb9qcEABb1fgHRQef///3//
//D////wYAuvj3nsuy8x65XrQAvAt9zFe3u927aWJd2ctsVs8JIpo1DJqp+GRT9JppiJ5NBMKPKb
RlG1PU0PKabSHoygamSnkYI1T0ymh6gGg0GQAADQAAaADU0ZTEmp5Jo009QwTTQNGTTIGQGmRkaA
yAEiIjRMqepmjE0ptNJ6GiPJqBmpiYhoAD1AANqk9VNp6EgGmh6gNHqMmQyZDIA0A0DQDQEkiYIB
MgRiNFT/RTTyNRjVPQ9UA9qjMpoB+qMhMAXxRdJAyp/X258kKWMSsztubUc29G7PQ0k8JU7fOIX2
/e0aelDqODHH8CoL/ne4KB6QXZn6XGjBLamb7ayrO31V2+7YtaGT29m7d3xxzl1KglzsipgN76Uh
+/B74yqlg0XqZCNVo5uKECKgEtC2fl/np37T8/SWDFv9uhDDeBtibbQ2ruxGV9b2t9jZwxGrsnn5
cQrtwcKJIDmrBEwPNaEtr2ISoHOYGrmQHqQ7gjCpacorW5y7V6B2wzAKEvs0PhxKeuZHTUf7ynvj
gI7JI2dC/hYYQxXdszvt3qBS4CiMszqNFMhEBknEhguNZBDGWZg1K2ktIg1Rc4pA5cEF7oYsrrpc
T/MkTF02wjGcFQpS+MDVFaRxQY4u7AVrwtuut1Uwo6Ob2YMIC/AC8llMZERvS5tSQlbBVPI0Lk4T
aqVRGL6g1QtMQL2JoVv43VZO77da/GkRoo5m7hn3I7vuA+16NnFIznj1VRTDDEVaD0BHM7cyfHPy
g1KRRKUihxqvGEnKRBA49DoT4CCAdd3gdN/V4c7m2nsxkL7OcJ3Bh0w8L49VIdjiFUbBOqql5CDy
X2L3nfyJ9gMMXb3jCDUfgahLpa4xjczWDdAQuTvG5yynPtvZq/ySGV7MUNk9BNtGd2SZbiSuHmDC
U8Ehj0o2oQ8nCoYX12w4Gc88vuRDHEdaS8aPVXR3c4DFC9qKrZ3tuflyBSxQwJZTmfHLpVCDQd/u
71A84YrMMkBhbEPWeYulBT/MYJO3ZXSJDDxEiWBjMjSCDeqWOkGoMAf6XpIg8QdsaeNtKCghICER
IYirSMkbNDggLQZYex1tAfcziO1bHY5oaC72acM202YIcanGWsSyMX3hIxi4uLqGI79kSRAyPbbv
7zGOo7DObI/riQLjIkTebAeaLRkRYmxpDWgDUaiy2esuhnSA4bF2SgrKgqdwzIEparBeaCAHx4LI
0nT/EdegLrmupXg6Y8vhixoNmwtLTmK3rp0lHjkyAzCovnTauMeJIOAaDEQ6uNCIYK7iXD+hh2TL
CSRtCEUkN0umYiW0dN44VlKYgxVZQePe7WVjwmxWRi5OAzCaUkoSXHG945C4mUVMCTYXAG3t7s1b
Kj6VD7NsmiEmISBwRHGTkvAMrNNJky/Mn8MCyWWp4+RhPUXwCJxKxbryswNJaVBWW6ylTQTy9i4w
KFRIuKz+NxY/VHmfc+3mGT1ORkpCTHgJNMkYD5j8p2kjPac2RMfdWXvOYoWlhpFpxKGox6FYOuc6
hYGQCpe5QGxGErip5iOvLiJAwLaMXErVYlEwJUwIkChIkPKy2lzK67CDNZfOlE0McNpZKRUk4YSz
qGgXGQxeRGHEyYxiDfyvLyBjWPLggUJjsbBp1Fs5XjyJfaSDMsmqyZo2PHkyY8jpLjtEt3il8NNP
r3Vbew9bAsV7Qmx0PP17zp05QehyTgejgBBuRt8joZB8mPq4I+bLRnthp4kcD4XtCCKkQxsMeH5E
eXnm6CziZlJbSiSwH8ac3NwHign7vKkvgwMwx8jjxl+Pf3QPWBeyeT0xjev6fy00jp7vfvV9jVz1
IPvMX18pVUlgQVz065O/99vZ12zyW5Mrmhqv6kVJP9s5H0mpeqxXkHC5HjvmWkwtdtM/ve0uipB4
TLf8JlvIPsA/Tuy8lQHmg9A4LQb2h/4U+LNANcns20HKBVzaTxQX6Z8rFxtpmSr4fxsnQWUlpGbZ
sqUO719EvbQHw3cgXUG5ibqOpnW3IncYZsDHxtwAukodfZAQ0WdbQfMxcDBsUuF1fUOiRxn1/Qeo
+n2kDdUefyNJHh5j9dpmfED5M9CXn2JaiIbUbP1Nz+743nzykFNECWKBOAWdNmL9eqSQ41E9y5Hc
GbrgxGjl0jKQXM0MtFnGI7Dip9WsqLZo3S3vahzIMQPoMWBRL/IxsMFiq59+8SuXicikvAmpyad8
LlzYGUsger2OYExXiIPwbmMCw1mQ/Qbo85l/TP38Oc8RLUkvUktNKcTzOBoY6eTPMOHHUUL+FHSg
hwL6djt2R5GZRdHWAD3+LdYW6GNfPurO6DMppp6nGwx1m80loUFYTJWFsgYkjogaiETQrpnJCcrv
r0lJJjtFYHY3OGcWyK0Q+LGCA5nQ4hVo8DNirugw8Hbw7eXQhksl1RhAqXQld+Uw4ng3zTvdYAyg
wQERSLpixwKt1buQzGc4oDmH2pmrR/poxsYhhgKwgBgMmOk4jCxnKjz6bufLbfEkGBU6pIYldvW5
EKos2ekCPMz/E0JKZfUopkESdmuOjlHXL+7JQBs2YanDbeWetOCCgzJstdEoyaNwksmTHQbDCDir
jEiFFDkaa7Bx2YGwND1GEFyFhpNIxumszmkgSdFIijEQJkz6CAuwSsK8UrmXoHuDYG4YDx+Gh8NX
6TjWe2tPHd1CMVyardMeuf95Dx37mk/C2SJOdTo5SGB4mEzCwwdGzRKSFeBzTbwawTv1a6/uSVr5
9MnDszPmSXg54J/v9gh/QrDy8zQN4CpuRp66+WFoLhcWl4XOE/a9QGquSm1HB8dkY9QlaQrWIIF4
YGZDYeEFlhFh8TO+NksZWnUJQ+REJnl4ss4lz6/c0xo/TPqEuPLeB7unZ68+wJtTR1MJa0ql9ehd
s0VsBhRClUhc7Mbrzk9lhVSxCovgYaSVQysGnNoR/wMEw5XK3uKwZtHewjo8G/NiM780e68RF+hD
vAo6Q5cZUWEtoNl0I1o8lqddghMDh+wlJWS013GMoB4YwSTWPnSFbWICnC1JO7QgSqSQ6KcwnUcg
qD8Xh1+G0bHP+eoPsMOiewh18ov5XiXU8YAKjWOHR3emQNLCCifqokp663kYZVgHvh2gOKEp9QFP
cgFOIBEdCmhRKWSGk5nw0a8sSU00wNCDNwFYUhcRSi6jJ1iXJKHuZmZmTTKcd8p59LyqGqKWk0OB
nZCgUg3EcXB8QefgWvWO3SgsB7en8Eu/s6GuiXMx35g6qiUeu4DLvevmabbYxjYCNNUnPGfVxnbt
yQza6UBR1fcODWyf3oCNtdwGWAO9QQwUdqMwlRhITJMwyYi5Ody3/J52tRaqwsmqv0utiLMmlvEk
2HR5N4wqNgDbeRwhzBw6JPYF0XWEm+lmo1TpOpYyDQRuZESj1BoZBtgGhJ9Bsp/QyJSZJFkEyXVe
4N1TAmSKXgXyoejM2tO0LeWEYU3EpSxvGl5RzWbTcEtIh1dG2YDq+BrSFUiEEOPNrLaDCuV6UYvP
EyUQUWCYOvQBgooNZ4QtGjW7DA4b7o5cjUz821PcBgfHAvSZqhItGWI7EeWwhfJPgSvAyVqGxZj7
UUPDW5zF8Z6JiaDSVCZSgz+G4zlAVB3mIhD6f3YlQG/GpPqLmHGXREueuIfC/avyk1Q6fIenuy6a
rhLTtdBdBpeCha45Mkvy0lZi3FjDMKvnhuOp1m1gDS5rebXU8O5BCEG1fV95vO5i9FvPWAf4u5Ip
woSEt+1OCA==
More information about the bazaar
mailing list