The try statement at the end of hashlib.py is some of the worst python
code I've had the mispleasure of reading for a long time.

Secondly, it seems flawed in function as well as form.
__get_builtin_constructor can throw an ImportError, which seems
erroneously caught in the except (why on earth does the except span more
than the "import _hashlib"? That's bad style for precisely this reason.)
This will cause an error in the import of hashlib when there are
possibly many working hash functions already imported. Changing the:

try:
exec funcName + ' = __get_builtin_constructor(funcName)'
except ValueError:
pass

to catch ImportError as well solves the problem for me.

Can you propose an actual test case and a patch?

If you're missing any hash algorithm from openssl (say sha224) and the
corresponding _sha224 module, hashlib will fail to import and no hash
algorithms will be available.

Additionally, if no (or only some) openssl algorithms are available, the
error branch expects every hash algorithm to be present in it's own
module, if even one is missing, hashlib will fail to import.

Producing a test case for these without mock objects is difficult at
best. I leave that to the python team, if they desire to do so.

Here is a rewritten hashlib.py, without that ugly mess of code and
without the above two bugs. It passes all tests.

In addition, I would replace
exec funcName + ' = __get_hash(funcName)'
with
globals()[funcName] = __get_hash(funcName)

Yes, I prefer:

globals()[funcName] = __get_hash(funcName)

The exec was used in the original code, I'm not aware of the style of
the python stdlib programmers, so I tried to be as true to what I found
as possible.

I just wanted to blunt my words in the original post, I don't want
Gregory Smith to view it as a personal attack, I was just frustrated in
having to debug the hashlib code. I'm sure even Gregory would admit it's
not code he's proud of (probably done under a tight deadline.) No doubt
Gregory normally does high quality work.

That code was indeed a mess. I've incorporated most suggestions from
your cleaned up version (and fixed a bug in it) in trunk r74479.

Have you ever seen __get_builtin_constructor fail in practice? I can
imagine that packing up a stripped down python interpreter with only a
subset of the hash extension modules would be the only time this would
likely be encountered.

I've changed your code to log an error and continue rather than silently
continue when an expected hash function is not available. That error
log may be overkill, perhaps we should just leave catching this problem
up to the unittests.

I have indeed seen __get_builtin_constructor fail in practice, in the
python build in the Ubuntu repository if IIRC. I seem to recall the
problem was that python was using a version of openssl that didn't
include all of hash algorithms, probably because a python library or the
process hosting python also had a dependency on libssl and had it loaded
already.

But there is also the issue that Python is a language with many
implementations now, and in fact, it's unlikely that CPython will remain
the defacto implementation forever. Having a more robust hashlib.py that
can handle missing algorithm implementations makes a lot of sense (.NET
for example has no implementation of some of the more esoteric SHA
hashes.) I remember that in the early days of IronPython, hashlib.py had
to be completely replaced to be used.