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

Regression with setuptools 27.2.0 on appveyor with Windows + Python 3.5 + numpy 1.11.1 #790

Closed
vallsv opened this Issue Sep 15, 2016 · 13 comments

Comments

Projects
None yet
3 participants
@vallsv
Contributor

vallsv commented Sep 15, 2016

Hi,

We have a regression on Windows + Python 3.5 + numpy 1.11.1 on our project when compiling with appveyor.

The issue is relative to unquoted path at linking step:
LINK : fatal error LNK1181: cannot open input file 'Files.obj'
See https://ci.appveyor.com/project/ESRF/pyfai/build/0.13.170/job/vrotml5gkymirvad

I try to reproduce the problem with an own Windows machine without success. Then i only have the issue on appveyor + Python 3.5 + numpy 1.11.1

Here is few tests executed on appveyor according to setuptools versions

  • 27.2.0 -> fail
  • 27.1.2 -> ok
  • 27.1.0 -> fail (also fail on python 2.7, 3.4)
  • 27.0.0 -> ok
  • 26.0.0 -> ok (but fail on python 3.4)
  • 25.0.0 -> fail

While i can't reproduce the problem on an own computer it is very difficult to know what's append. But do you have an idea?

BTW, there is a very mysterious message:
No module named 'numpy.distutils._msvccompiler' in numpy.distutils; trying from distutils. Maybe there is a problem on relative/absolute import or something like that? But this message is also displayed on setuptools 27.0.0, the version which works.

@jaraco

This comment has been minimized.

Member

jaraco commented Sep 15, 2016

So I think you're reporting a regression with 27.1.0 from 27.0.0, and here are the differences. Basically, the monkeypatching logic was consolidated in a new monkey module. There were known defects (specifically addressing the patching of MSVC) with v27.1.0 which were fixed in v27.1.1 and v27.1.2. Can you test v27.1.2?

@jaraco

This comment has been minimized.

Member

jaraco commented Sep 15, 2016

I've updated the docs so you can more readily see the history of releases.

@vallsv

This comment has been minimized.

Contributor

vallsv commented Sep 15, 2016

27.1.2 is working.
Thanks for the link to the history.

@vallsv

This comment has been minimized.

Contributor

vallsv commented Sep 15, 2016

Then i guess the problem come from https://github.com/pypa/setuptools/commit/b6f2fee975c570d2beadb9007e6302411f91ab4b, i have no way to test.

But i guess, if msvc14compiler._get_vc_env was already patched by somebody, when you use patch_func, you don't ask to patch msvc14compiler._get_vc_env, but the function which was provided by somebody else to patch msvc14compiler._get_vc_env. Is that right? Very difficult to me to understand the implications :-D

Then maybe there is no other choices than something like that to really patch msvc14compiler attribute:
patch_func(msvc.msvc14_get_vc_env, msvc14compiler, "_get_vc_env")

@jaraco

This comment has been minimized.

Member

jaraco commented Sep 18, 2016

Mentioning b6f2fee to get the link.

@jaraco

This comment has been minimized.

Member

jaraco commented Sep 18, 2016

Well, the issue is certainly with the quoting of args, which looks suspiciously similar to #739. Since you seem to be getting inconsistent results, I'm not sure what's to be done next. Is it possible that the modules are getting patched multiple times?

@vallsv

This comment has been minimized.

Contributor

vallsv commented Sep 19, 2016

I have no idea.
I will try to patch setuptools first and see the change in appveyor context. Maybe that's another problem. It will be much more difficult to check numpy, cause it have to be compiled.

@vallsv

This comment has been minimized.

Contributor

vallsv commented Sep 19, 2016

Then, the proposed API looks to work with our appveyor context (windows python 2.7, 3.4, 3.5).

What do you think about? I also inverted the order or original and replacement

@jaraco

This comment has been minimized.

Member

jaraco commented Sep 21, 2016

I appreciate you working out a technique that works. That's helpful. I'd like to work out what it is about your implementation that fixes the numpy build. I'd like to either have a test in setuptools that captures the expectation that numpy has about the monkeypatching or somehow add an integration test that will capture the actual numpy expectation.

Based on your investigation, can you imagine a unit test that would capture the expectation that your changes meet?

@mingwandroid

This comment has been minimized.

mingwandroid commented Sep 21, 2016

I checked that this patch allows me to build scipy on Windows Python 3.5 and it does. Thanks.

@vallsv

This comment has been minimized.

Contributor

vallsv commented Sep 27, 2016

Sorry, i was very busy this days.

The problem is that part of setuptools and numpy.distutils monkey-patch the same thing. Then i think if you import one or the other first, it can create some changes.

Right now the setuptools code is nice, but using __module__ and __name__ do not allow to monkey-patch the right thing if numpy.distutils was imported before setuptools.

def distutils_function(): pass
def numpy_distutils_function(): pass

distutils_function.__name__
#> 'distutils_function'

# numpy.distutils monkey-patch distutils
distutils_function = numpy_distutils_function

distutils_function.__name__
#> 'numpy_distutils_function'

# Using __name__ and __module__ while point to the numpy function

I guess the problem is somewhere here. There is no way to use __name__ or __module__ safely.

Anyway, i dont really know why the behaviour on my Windows computer and on appveyor is not the same. I dont really reach deeply on that, cause it is laborious.

@jaraco

This comment has been minimized.

Member

jaraco commented Sep 27, 2016

Thanks. That helps characterize the cause. I'll put together a patch and try to somehow capture the numpy expectation.

@jaraco jaraco closed this in 44a6704 Sep 27, 2016

@vallsv

This comment has been minimized.

Contributor

vallsv commented Sep 28, 2016

Hi. I try the last version on my project. It is working. Thanks a lot.

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