[pycrypto] Initial review of Thorsten's Py3k changes

[Dwayne C. Litzenberger]

On Mon, Jan 10, 2011 at 10:18:54AM -0500, Thorsten Behrens wrote:
>Thanks for looking over my commits.

Thanks for tolerating my glacial response time. :)

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

You might want to take a look at a video that tries to explain rebasing.  
Here's one I found:

     http://v.youku.com/v_show/id_XMjE3NDI3ODAw.html

If I were rebasing a lot of changes, making substantial changes as I go, 
I'd probably just create a new branch at origin/master and apply individual 
changes (using git cherry-pick -n $COMMIT_ID) to build up a new series of 
commits.  Then, I would do "git diff oldbranch..newbranch" to see what's 
different between them (hopefully nothing).  Also, I usually re-order the 
commits first, compare the resulting branches using "git diff" to make sure 
I didn't miss anything, and *then* make actual code changes.

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

Hmm.  I was puzzled by what you wrote until I saw this:

     >>> import binascii
     >>> binascii.b2a_hex(b"asdf")
     b'61736466'
     >>>

I would have expected binascii.b2a_hex to return a string, since 
hexadecimal is a human-readable representation of binary data, but 
apparently it doesn't.  Yuck, ok.

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

OK.  It can be done separately later.

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

Ok, I guessed that was the reason, but the commit message just said "add 
support for mpir as alternative to gmp".

-- 
Dwayne C. Litzenberger <dlitz at dlitz.net>
  OpenPGP: 19E1 1FE8 B3CF F273 ED17  4A24 928C EC13 39C2 5CF7