[RFC/MERGE][bug #269535] python-2.6 compatibility
Martin Pool
mbp at canonical.com
Fri Sep 26 03:10:00 BST 2008
I think this can come in now, I just have some tweaks and questions.
You should also add a news entry when you add it, and also a note about
the api change of renaming message->msg.
This wasn't caught by BB because you didn't have literally '[merge]' in
the subject. And don't be too shy by calling them rfc if it's really
ready to merge.
On Wed, Sep 17, 2008 at 7:49 PM, Vincent Ladeuil <v.ladeuil+lp at free.fr> wrote:
> Hi all,
>
> Python-2.6 final is about to be released (see
> http://www.python.org/dev/peps/pep-0361/).
>
> I made some tests with python-2.6rc1 compiled from lp:python rev
> 40409 on ubuntu hardy.
>
> - __init__ and __new__ doesn't accept parameters anymore. Some
> were easy to fix, some less. Remote transports using multiple
> inheritance for SmartMedium has been slightly reworked to
> *avoid* the multiple inheritance.
I can't believe __init__ cannot take parameters any more. Do you mean
that object.__init__ won't accept any parameters it's given?
> - DiffSymlink.diff_symlink didn't encode the file names used in
> the diff (I'm shooting a bit in the dark here, the fix itself
> may be wrong, but without it python 2.6 produces a u'new' where
> python-2.5 was producing a simple 'new' so the problem may be
> deeper than it appears and require further inspection).
So it looks like this will cause actual failures on trees with symlinks
rather than just test failures? If that's so perhaps we should not say
2.6 is supported yet. If the failure is subtle perhaps we should
specifically guard against it, e.g. by raising an error if we do get a
unicode value back from readlink?
> With this patch the whole test suite passes
> , only issuing:
>
> ,----
> | test_selftest.TestWarningTests.test_/v/home/vila/src/bzr/experimental/py26bzr/bzrlib/tests/test_selftest.py:1636: DeprecationWarning: functions overriding warnings.showwarning() must support the 'line' argument
> | warnings.warn("this is your last warning")
> `----
>
> which I didn't fully investigate yet (first try got me lost (we
> don't override showwarning directly :)).
I'm pretty sure you need to just change _catcher() on about line 1137 of
tests/__init__.py to take a line argument (and possibly do something
with it.)
> - start the discussion about when and how we want to officially
> support python-2.6.
If we can get to a point of it passing all tests and working reasonably
I think we should keep it there. 2.6 is only going to become more
common. We've already heard from someone on OpenSuse(?) that their next
release will have _only_ 2.6.
> === modified file 'bzrlib/errors.py'
> --- bzrlib/errors.py 2008-09-10 17:33:01 +0000
> +++ bzrlib/errors.py 2008-09-13 06:53:41 +0000
> @@ -17,6 +17,8 @@
> """Exceptions for bzr, and reporting of them.
> """
>
> +import sys
> +
>
> from bzrlib import (
> osutils,
> @@ -102,9 +104,6 @@
> fmt = self._get_format_string()
> if fmt:
> d = dict(self.__dict__)
> - # special case: python2.5 puts the 'message' attribute in a
> - # slot, so it isn't seen in __dict__
> - d['message'] = getattr(self, 'message', 'no message')
> s = fmt % d
> # __str__() should always return a 'str' object
> # never a 'unicode' object.
OK, so you're deleting this because you're renaming all the members to
'message'. I'd like to see perhaps at least a comment in this file saying
that name can't be used, to give people a clue, because the behaviour has
varied across interpreters.
> +# sha is deprecated in python2.6 but hashlib is available as of 2.5
> +if sys.version_info < (2, 5):
> + import md5 as _md5
> + md5 = _md5.new
> + import sha as _sha
> + sha = _sha.new
Normally we would call these _mod_md5 I think, but this is ok as the
names are only used here.
> === modified file 'bzrlib/revisionspec.py'
> --- bzrlib/revisionspec.py 2008-07-19 15:00:27 +0000
> +++ bzrlib/revisionspec.py 2008-09-13 06:53:41 +0000
> @@ -139,7 +139,7 @@
>
> def __new__(cls, spec, _internal=False):
> if _internal:
> - return object.__new__(cls, spec, _internal=_internal)
> + return object.__new__(cls)
>
> symbol_versioning.warn('Creating a RevisionSpec directly has'
> ' been deprecated in version 0.11. Use'
>
It seems to me that this __new__ method is only here to catch code
that's trying to construct one directly.
Will this even work correctly if you don't pass through the spec
parameter?
As it's been deprecated for a very long time, why not just remove the
__new__?
> === modified file 'bzrlib/tests/test_http.py'
> --- bzrlib/tests/test_http.py 2008-08-14 02:15:31 +0000
> +++ bzrlib/tests/test_http.py 2008-09-16 12:40:51 +0000
> @@ -449,13 +449,6 @@
> '"GET /foo/bar HTTP/1.1" 200 - "-" "bzr/%s'
> % bzrlib.__version__) > -1)
>
> - def test_get_smart_medium(self):
> - # For HTTP, get_smart_medium should return the transport object.
> - server = self.get_readonly_server()
> - http_transport = self._transport(server.get_url())
> - medium = http_transport.get_smart_medium()
> - self.assertIs(medium, http_transport)
> -
> def test_has_on_bogus_host(self):
> # Get a free address and don't 'accept' on it, so that we
> # can be sure there is no http handler there, but set a
OK, so now you have a separate medium from the transport object this
test would not pass its final assertion. But why not still run it and
check we get _a_ medium? (Or maybe that's covered enough elsewhere?)
> === modified file 'bzrlib/tests/tree_implementations/test_inv.py'
> --- bzrlib/tests/tree_implementations/test_inv.py 2008-06-11 04:20:16 +0000
> +++ bzrlib/tests/tree_implementations/test_inv.py 2008-09-13 06:53:41 +0000
> @@ -77,6 +77,18 @@
>
> def test_symlink_target(self):
> self.requireFeature(SymlinkFeature)
> +
> + import sys
> + from bzrlib.tests import KnownFailure
> + from bzrlib.tests.tree_implementations import _dirstate_tree_from_workingtree
> + if (self.workingtree_to_test_tree is _dirstate_tree_from_workingtree
> + and sys.version_info >= (2, 6)):
> + # Furthermore the problem here is that it interacts badly with a
> + # '\0\n\0'.join(lines) where lines contains *one* unicode string
> + # where all the other strings are not unicode.
> + raise KnownFailure("python-2.6 os.readlink returns unicode path"
> + " if called with unicode path")
> +
Could you please file a bug if there isn't one already, and put the
number in here?
Also the imports, as they're used at least twice, would be better off at
the module scope.
Since this is copy-and-pasted five times, please instead lift it to a
common function that just raises KnownFailure if a tree is not going to
work.
> + def send_http_smart_request(self, bytes):
> + try:
> + # Get back the http_transport hold by the weak reference
> + t = self._http_transport_ref()
> + code, body_filelike = t._post(bytes)
> + if code != 200:
> + raise InvalidHttpResponse(
> + t._remote_path('.bzr/smart'),
> + 'Expected 200 response code, got %r' % (code,))
> + except errors.InvalidHttpResponse, e:
> + raise errors.SmartProtocolError(str(e))
> + return body_filelike
> +
> +
Maybe we should show the status text or part of the body of the
response?
--
Martin <http://launchpad.net/~mbp/>
More information about the bazaar
mailing list