[RFC/MERGE][bug #269535] python-2.6 compatibility

Vincent Ladeuil v.ladeuil+lp at free.fr
Mon Sep 29 09:48:15 BST 2008


>>>>> "martin" == Martin Pool <mbp at canonical.com> writes:

    martin> I think this can come in now, I just have some tweaks and questions.
    martin> You should also add a news entry when you add it, and also a note about
    martin> the api change of renaming message->msg.

    martin> This wasn't caught by BB because you didn't have literally '[merge]' in
    martin> the subject.  And don't be too shy by calling them rfc if it's really
    martin> ready to merge.

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

    martin> I can't believe __init__ cannot take parameters any more.  Do you mean
    martin> that object.__init__ won't accept any parameters it's given?

Only for object of course, sorry for the confusion.

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

    martin> So it looks like this will cause actual failures on
    martin> trees with symlinks rather than just test failures?

I'm now convinced that the root cause is deeper. I'll leave that
untouched for now since there are other symlinks bugs and this
test is just showing another aspect.

And no, this will not cause failures because the test fails while
comparing strings in the output, but those strings are really
comments (the target of the symlink was reported as 'new', it's
now reported as u'new', but in both cases the quotes are here so
I think nobody use those names.

    martin> If that's so perhaps we should not say 2.6 is
    martin> supported yet.  If the failure is subtle perhaps we
    martin> should specifically guard against it, e.g. by raising
    martin> an error if we do get a unicode value back from
    martin> readlink?

I'd prefer looking at all the unicode symlink bugs already known
if you're ok with that.

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

    martin> I'm pretty sure you need to just change _catcher() on about line 1137 of
    martin> tests/__init__.py to take a line argument (and possibly do something
    martin> with it.)

Done. (Congrats for the diagnosis).

    >> - start the discussion about when and how we want to officially
    >> support python-2.6.

    martin> If we can get to a point of it passing all tests and
    martin> working reasonably I think we should keep it there.
    martin> 2.6 is only going to become more common.  We've
    martin> already heard from someone on OpenSuse(?) that their
    martin> next release will have _only_ 2.6.

Yeah, but they also mentioned other packages failing so they may
revise their choice. Anyway, I'll try to automate something to
catch regressions as soon as possible.

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

    martin> OK, so you're deleting this because you're renaming all the members to
    martin> 'message'.

'msg' yes.

    martin> I'd like to see perhaps at least a comment in this
    martin> file saying that name can't be used, to give people a
    martin> clue, because the behaviour has varied across
    martin> interpreters.

Done.

    >> +# 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

    martin> Normally we would call these _mod_md5 I think, but
    martin> this is ok as the names are only used here.

No, you're right, I'll change that.

    >> === 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'
    >> 

    martin> It seems to me that this __new__ method is only here to catch code
    martin> that's trying to construct one directly.

    martin> Will this even work correctly if you don't pass through the spec
    martin> parameter?

    martin> As it's been deprecated for a very long time, why not just remove the
    martin> __new__?

Sounds like a simple way to fix bugs, I'll do that :)

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

    martin> OK, so now you have a separate medium from the
    martin> transport object this test would not pass its final
    martin> assertion.  But why not still run it and check we get
    martin> _a_ medium?  (Or maybe that's covered enough
    martin> elsewhere?)

Yes, it *was* special cased for http, that's not needed anymore.

    >> === 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")
    >> +

    martin> Could you please file a bug if there isn't one
    martin> already, and put the number in here?

These ones are not needed anymore (and that's far better :).

On the other hand, there is now a new deprecation warning:

~/py26/bin/python ./bzr selftest --no-plugins -s bt.test_multiparent.TestMultiVersionedFile.test_filenames -v
testing: /v/home/vila/src/bzr/experimental/bzr-py26-compat/bzr
   /v/home/vila/src/bzr/experimental/bzr-py26-compat/bzrlib (1.8dev python2.6rc2)

running 1 tests...
/home/vila/py26/lib/python2.6/gzip.py:313: DeprecationWarning: struct integer overflow masking is deprecated
  write32u(self.fileobj, self.crc)
   OK                 439ms

But since this is internal to python, I think they will fix it
before 2.6final.

      Vincent



More information about the bazaar mailing list