[merge][1.8][#277481] set -DWIN32 when compiling all pyrex extensions

Andrew Bennetts andrew.bennetts at canonical.com
Wed Oct 8 05:59:23 BST 2008


bb:tweak

The basic idea makes sense, but there's something funny...

Martin Pool wrote:
[...]
> === modified file 'setup.py'
> --- setup.py	2008-10-02 15:36:12 +0000
> +++ setup.py	2008-10-08 01:16:05 +0000
> @@ -201,7 +201,7 @@
>  unavailable_files = []
>  
>  
> -def add_pyrex_extension(module_name, **kwargs):
> +def add_pyrex_extension(module_name, define_macros=[], **kwargs):
>      """Add a pyrex module to build.
>  
>      This will use Pyrex to auto-generate the .c file if it is available.
> @@ -217,26 +217,28 @@
>      path = module_name.replace('.', '/')
>      pyrex_name = path + '.pyx'
>      c_name = path + '.c'
> +    if sys.platform == 'win32':
> +        # pyrex uses the macro WIN32 to detect the platform, even though it should
> +        # be using something like _WIN32 or MS_WINDOWS, oh well, we can give it the
> +        # right value.
> +        define_macros.append(('WIN32', None))

Mutating a default argument is likely to cause bugs.  If nothing else it means
define_macros will have ('WIN32', None) appended to it several times.  It's
probably harmless with the current setup.py, but I wouldn't bet on this always
being the case.

Why does add_pyrex_extension even allow callers to adjust define_macros?  We
don't seem to use that capability with this patch.

-Andrew.




More information about the bazaar mailing list