[pycrypto] Initial review of Thorsten's Py3k changes

[Thorsten Behrens]

Thanks for looking over my commits.

I have been trying to rebase and am getting nowhere fast. There are a 
lot of merge conflicts, and I end up with something that may or may not 
actually be the current state of the repository.

> Therefore, I have a few recommendations:
>
> - Don't change the test vector data if you don't need to.  Changing
>     hundreds of strings like 'd6a141a7ec3c38dfbd61' to strings like
>     b('d6a141a7ec3c36dfbd61') is unnecessary (since they need to be
>     hex-decoded somewhere anyway), and it makes it difficult to tell by
>     inspection that the test vectors weren't modified.  (How many of you
>     noticed that I changed the 8 to a 6 in that example?)
This was done for a reason. The tests were failing because b'something' 
does not equal 'something', and
because several functions expected bytes, but got str.

As far as I could make out, this was the best way forward. The 
alternative may have been to write things like

assertEqual(b(x),y) and adding b() to various function calls throughout 
the test suite.

instead of leaving the asserts and function calls (largely) the way they 
were.

Adding b() to asserts and function calls struck me as rather opaque. I 
thought it more consistent to change the way the test vectors are 
presented instead.

I get that this makes review hard. I didn't think of that aspect at the 
time.

I'm not sure what the best way forward is for this part of the changes. 
I still think it's cleaner to change the literals than the way asserts 
and functions are called. b(s) in Py3k returns s.encode("latin-1"). 
Compare and contrast these two:

input = b'abcdef00'
expected = b'abcdef00'

x = somefunction(input)
assertEqual(x,expected)

to

input = 'abcdef00'
expected = 'cdefab00'

x = somefunction(input.encode("latin-1"))
assertEqual(x,expected.encode("latin-1"))

If you were to write native Py3k code, you'd choose the former over the 
latter. I tried to get as close to that as I could. I don't quite have 
b'something', since I can't do that and remain compatible with Python 
2.x. But the spirit of it is intact: I am changing the way the literal 
is presented, instead of working with a string literal and changing it 
to bytes whenever I use it.


> - Run your tests with python -tt to ensure consistent use of whitespace.  I
>     haven't been doing this, so the current master doesn't work, but that was
>     a mistake and I will be doing it from now on.
>
Okay.

>       - The commands "git rebase -i", "git cherry-pick -n", "git reset", and
>         "git add -p" are your friends.
I think I need some git schooling. I am not getting anywhere fast :/.

> - References to things like RC5 or IDEA, which have been removed from
>     PyCrypto, can be removed.
I didn't touch those since they may have been there for a reason.
> - If adding additional/alternative dependencies like MPIR, include *why*
>     that's being done in the commit message and/or in the documentation.
Hmm, I thought I did. This was done so _fastmath.c would work on 
Windows. GMP is actively hostile to compilation on Windows. MPIR is a 
GMP fork that is friendly to compilation on Windows.

Yours
Thorsten