[merge][1.8][#277481] set -DWIN32 when compiling all pyrex extensions
andrew.bennetts at canonical.com
Wed Oct 8 05:59:23 BST 2008
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.
More information about the bazaar