Matthias Klose wrote:
>
> Matthias Klose added the comment:
>
> some random comments about py3k-20121004-MINGW.patch:
>
> - Modules/_ctypes/libffi_msvc/win32.S
>    Please can you get rid of libffi_msvc and use libffi?
>    afaics, libffi has support for mingw32.

No python still use custom hack for MSVC.  win32.S include this hack and 
could be compiled with gcc.

> - there seem to be chunks which are unrelated to mingw, like:
>
> @@ -830,15 +926,18 @@ class PyBuildExt(build_ext):
>               if have_usable_openssl:
>                   # The _hashlib module wraps optimized implementations
>                   # of hash functions from the OpenSSL library.
> +                # NOTE: _hashlib require only OpenSSL crypto library !
>                   exts.append( Extension('_hashlib', ['_hashopenssl.c'],
>                                          depends = ['hashlib.h'],
>                                          include_dirs = ssl_incs,
>                                          library_dirs = ssl_libs,
> -                                       libraries = ['ssl', 'crypto']) )
> +                                       libraries = ['crypto']) )
>
> please file separate issues.
>
>
> - why setup_info.in. looks like something which could be done with get_config_var.

Yes and no.  get_config_var will use makefile variables (indirectly from 
sysconfigdata).
If you add a make macro(variable) then it will be installed.
Setup info is designed to communicate from configure to setup.py without 
to expose data to installation.

> - why re-reading files in setup.py, and grepping these for config options?

For historical reasons. Before other commits to start to look info some 
generated by autotool files.

>
> I think the patch would benefit from splitting it up into several self-contained chunks.

May be.
I don't know how to group split. If is based on functionality patch 
order will be important as more then one patch will update "near lines" -
for instance pyport.h is difficult to split :(.
cygwinccompiler.py could be rewritten as separate issue, already opened.

> did you try to do builds for windows and linux after this patch was applied?

Title is correct. Hosts include native linux, cygwin, mingw and cross 
linux->mingw, linux->android ( not published ). All from one and the 
same source tree.

Roumen