md5 and sha module will be deprecated in python 2.6

[therve]

We have to use the hashlib module available since python 2.5.

Note that I saw this because stdio tests are failing on python trunk. I'm not sure it should be sensitive to warnings emitted...

[Jean-Paul Calderone]

The stdio tests should definitely be changed to not break as a result of this deprecation.

[David Reid]

You should fall back to importing sha and md5 if the hashlib import fails

Something like

try:
    from hashlib import md5, sha1 as sha
except ImportError:
    from md5 import md5
    from sha import sha

[Jean-Paul Calderone]

<itamar> instead, why not have the twisted.python.compat module install a fake hashlib module on Python versions that don't provide it and then use hashlib everywhere?

[devinus]

try:
    import hashlib
except ImportError:
    import md5
    import sha


    class hashlib(object):
        """Mimic a small subset of the Python 2.5 hashlib module."""
        __module__ = 'hashlib'

        @classmethod
        def new(cls, name, string=''):
            if name in ('MD5', 'md5'):
                return md5.new(string)
            elif name in ('SHA1', 'sha1'):
                return sha.new(string)
            raise ValueError, "unsupported hash type"

        @classmethod
        def md5(cls, string=''):
            return md5.new(string)

        @classmethod
        def sha1(cls, string=''):
            return sha.new(string)


    sys.modules['hashlib'] = hashlib

I had all of the files using the old md5 and sha modules converted over, but some were failing the test suite, so I'll need more time.

[Jean-Paul Calderone]

There's <​http://code.krypto.org/python/hashlib/>. I wonder if we can somehow make use of this.

[Wilfredo Sánchez Vega]

Attached hashlib.patch got rid of thw warnings for me and all tests passed.

[Jean-Paul Calderone]

22:14 < wsanchez> How would .compat install the fake module?  I can spend the time to clean up all the other files, just not sure what you had in mind
22:14 < wsanchez> We can do import twisted.python.hashlib which imports the right thing; that'd be easy
22:15 < exarkun> I think that would be fine.
22:15 < exarkun> Preferable to stuffing fakes into sys.modules, in fact.
22:15 < wsanchez> Or from twisted.python.compat import md5, sha1, but that seems a little less nice
22:15 < exarkun> Having twisted.python.hashlib.md5 and twisted.python.hashlib.sha1 sounds good.
22:16 < wsanchez> Yeah, I wouldn't want to monkey with sys.modules; that seems like looking for some kind of sneaky pain later
22:16 < wsanchez> OK I'll do that.

[Wilfredo Sánchez Vega]

Also replacing 'sha1 as sha' imports to simply 'sha1'. Makes searching for uses much easier and will make it easier to switch to directly using Python's hashlib in the future.

[Wilfredo Sánchez Vega]

Tests pass with the v2 patch on my OS X 10.5 box with Py2.5.

[Wilfredo Sánchez Vega]

(In [25340]) Branch for #2763: md5 and sha module will be deprecated in python 2.6

[Wilfredo Sánchez Vega]

(In [25341]) Add a twisted.python.hashlib module which imports hashlib or (if that fails) md5 and sha, which are deprecated in Python 2.6. Fixes #2763.

[Jean-Paul Calderone]

There are some test failures on Windows.

[Jean-Paul Calderone]

Actually, I suppose that was premature. There seem to be mad failures on most platforms. Maybe all. And the test suite is taking ages.

[Wilfredo Sánchez Vega]

Failures appear limited to conch:

  File "c:\twistedbuildbot24\WXP32-full2.4-scmikes-select\Twisted\twisted\conch\ssh\transport.py", line 1226, in _getCipher
    counter=_Counter(iv, mod.block_size))
exceptions.TypeError: 'module' object is not callable

And so far only Windows has completed (Ubuntu lost the builder). It's going…very…slowly…

[Wilfredo Sánchez Vega]

Whoops. r25343 should address that.

[Wilfredo Sánchez Vega]

OK, all tests are passing now:

​http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/hashlib-2763

[therve]

Note that tests passing, but you're not using hashlib... "import hashlib" in twisted.python.hashlib tries to make it import itself. You have to use __import__, or rename the module in twisted.python.

[Wilfredo Sánchez Vega]

OK:

__all__ = ["md5", "sha1"]

try:
    hashlib = __import__("hashlib")
    md5  = hashlib.md5
    sha1 = hashlib.sha1
except ImportError:
    from md5 import md5
    from sha import sha as sha1

[Jean-Paul Calderone]

1. twisted/python/hashlib.py
   1. no copyright statement
   2. no test-case-name
   3. no module docstring
   4. no unit tests
   5. only the __import__ should be protected by the try/except ImportError. The two name bindings should happen outside.
   6. __all__ is cool; hashlib should probably be _hashlib anyway, though, because sometimes people don't pay attention.
2. twisted/conch/scripts/ckeygen.py
   1. copyright statement should be updated
   2. the module has some other random junk that might as well be cleaned up:
      1. module docstring is commented out
      2. there's still a cvs id marker which should be deleted
      3. imports appear in the wrong order
      4. printFingerprint uses deprecated apis
   3. the module has no unit test coverage whatsoever; any code being changed must have tests
3. twisted/cred/util.py
   1. no test-case-name
   2. no unit tests
   3. actually, this module is entirely unused in twisted. Should be deprecated, I think.
4. twisted/words/protocols/oscar.py
   1. no unit tests
   2. copyright date should be updated
   3. old __future__ import should be deleted
5. twisted/words/protocols/msn.py
   1. update copyright date
   2. some trailing whitespace should be stripped
   3. handle_CHL is untested
6. twisted/mail/maildir.py
   1. copyright date needs updating
   2. module docstring should be reformatted to follow coding standard
   3. StringListMailbox has no unit tests

I suspect there's more minor points and untested changes, but I'm exhausted.

[Jean-Paul Calderone]

(In [25354]) address review points

except for StringListMailbox unit tests

refs #2763

[Wilfredo Sánchez Vega]

Many of the review points are completely unrelated to this ticket and should be on a different branch, but whatever. For example, missing unit tests for StringListMailbox has nothing to do with hashlib, and wasn't introduced by this fix. While general code cleanup is nice and all, one fix shouldn't be blocked by unrelated fixes.

r25401 updates the copyright on every file changed on that branch as needed.

[Wilfredo Sánchez Vega]

#3547 on StringListMailbox

[Jean-Paul Calderone]

1. Since Glyph merged #1376, the _fingerprint function in twisted/conch/scripts/ckeygen.py can be replaced with Key.fingerprint (the branch needs to be merged forward for this).
2. There's an extraneously added raise to the except suite in printFingerprint which should be removed.
3. twisted/conch/test/test_ckeygen.py's copyright date should just say 2008, since it didn't exist in any of those prior years.
4. in twisted/conch/test/test_ckeygen.py, there should be another blank line (for a total of two) between KeyGenTests.setUp and KeyGenTests.test_printFingerprint.
5. in twisted/conch/ssh/transport.py, the commented out checks for the digest_size attribute are now doubly obsolete and should be deleted.
6. twisted/news/database.py mixes up imports a bit and can lose the nested_scopes future import
7. twisted/python/otp.py has no test coverage, lots of trailing whitespace, and other crummyness
8. twisted/web/monitor.py has no test coverage; the module can't even be imported.
9. The change to twisted/web2/server.py seems unrelated
10. twisted/words/protocols/oscar.py should have its copyright date updated and the unused imports (time, nested_scopes) removed.
11. twisted/words/test/test_msn.py has an unused import, sys, which should be removed

A bunch of these points are for code that I wrote after my last review anyway, so I'll merge the branch forward and address them.

[Jean-Paul Calderone]

(In [25426]) Branching to 'hashlib-2763-2'

[Jean-Paul Calderone]

(In [25428]) address points 1-5

refs #2763

[Jean-Paul Calderone]

(In [25429]) address point 6

refs #2763

[Jean-Paul Calderone]

(In [25430]) deprecate twisted.python.otp

I tried writing tests for it first, but it's either broken or not well documented enough for me to figure out how to use. In any case, there are other OTP libraries for Python, and there is nothing specific or beneficial to Twisted about this module. It also hasn't really been touched since it was added seven years ago.

No tests for this deprecation because I don't know how to test module level warnings.

refs #2763

[Jean-Paul Calderone]

(In [25431]) delete twisted.web.monitor (addresses review point 8)

This module has been unimportable for 4 years. Someone noticed, and even cared enough to file a ticket, but not enough to write any unit tests. So, removing. This isn't a backward incompatible change because the behavior is the same before and after: the module cannot be imported.

refs #2763

[Jean-Paul Calderone]

(In [25432]) cleanup imports, addresses review points 10 and 11

refs #2763

[Jean-Paul Calderone]

Took care of all the points except for 9. I'm not really sure what's up with that.

[Wilfredo Sánchez Vega]

Reverted twisted/web2/server.py. No clue how that change got onto that branch. :-/

[Jean-Paul Calderone]

Anybody can review this (and having someone other than me give it a look would be great).

[Michael Hudson-Doyle]

Mostly this is a monster yawn of course. I have some small problems with the new tests you've written, and another comment:

1. twisted.words.test.test_msn.NotificationTests.test_challenge Reading this test, I have no idea how to tell if it is correct or not. Given the nature of what this is testing, this is perhaps inevitable, but maybe you could compute the result rather than including it literally, and include a reference to why this is the right thing to do?

2. twisted.test.test_newcred.DeprecatedUtilTests.test_respond Same complaint really. Also, I think it would result in clearer failures if you put the assertion about the length of warnings before the assertions about the contents of the list.

3. Hasn't this branch missed the chance to be included in Twisted 8.2 by now? If so, the deprecation messages need to be changed.

Finally, has this branch been tested on Python 2.6?

[Jean-Paul Calderone]

(In [25448]) Branching to 'hashlib-2763-3'

[Jean-Paul Calderone]

(In [25450]) try to clarify the assertions made in these hashy unit tests

refs #2763

[Jean-Paul Calderone]

(In [25451]) change 8.2 deprecations to 8.3 deprecations

refs #2763

[Jean-Paul Calderone]

Points 1-3 addressed in the previous two commits. The branch has been tested on Python 2.6:

​http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/hashlib-2763 (search for py2.6). The -3 branch is building now. Results at:

​http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/hashlib-2763-3

[Michael Hudson-Doyle]

I'm still not completely convinced by test_respond, but I don't think I care. Land it.

[Jean-Paul Calderone]

(In [25457]) Merge hashlib-2763-3

Author: wsanchez, exarkun Reviewer: exarkun, mwhudson Fixes: #2763

Replace uses of md5 and sha modules in Twisted with use of a new twisted.python.hashlib module which transparently uses the new hashlib standard library module if it is available or falls back to md5 and sha if not.