Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Monkey-patch _msvccompile.gen_lib_option like any other compilators #7925

Merged
merged 1 commit into from
Aug 11, 2016

Conversation

vallsv
Copy link
Contributor

@vallsv vallsv commented Aug 10, 2016

setuptools is now using _msvccompile instead of msvccompile9.

numpy is monkey-patching symmetrically gen_lib_options and spawn function for all compilators. But _msvccompile.gen_lib_options was not monkey-patched while _msvccompile.spawn is already monkey-patched throug the super class ccompiler.spawn.

This patch only symmetrically patch _msvccompile to prevent command line file quoting.

`setuptools` is now using `_msvccompile` instead of `msvccompile9`.

numpy is monkey-patching symmetrically `gen_lib_options` and `spawn` function for all compilators. But `_msvccompile.gen_lib_options` was not monkey-patched while `_msvccompile.spawn` is already monkey-patched throug the super class `ccompiler.spawn`.

This patch only symmetrically patch `_msvccompile` to prevent param file quoting.
@rgommers
Copy link
Member

Despite the weird underscore, _msvccompile is the main (public) compiler class for MSVC 2015 (and up in the future). So this patch looks fine to me.

Can't verify that this actually will get rid of issue gh-7921, so leaving it open for a day or two for comments.

@pitrou ping

@vallsv
Copy link
Contributor Author

vallsv commented Aug 10, 2016

Thanks a lot.
And that's right, this underscore is really confusing.

@pitrou
Copy link
Member

pitrou commented Aug 10, 2016

Can't verify that this actually will get rid of issue gh-7921, so leaving it open for a day or two for comments.

AFAICT, it seems to work (on Win64 + Python 3.5).

@rgommers rgommers merged commit e1f191c into numpy:master Aug 11, 2016
@rgommers
Copy link
Member

Merged, thanks @vallsv @pitrou

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants