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

WIP: CI: try building with setuptools 50.0 #17209

Closed
wants to merge 1 commit into from

Conversation

rgommers
Copy link
Member

@rgommers rgommers commented Sep 1, 2020

Reverts gh-16993

See also scipy/scipy#12798

@rgommers rgommers added the 31 - Third-party binaries Install/import issues other than Anaconda-specific label Sep 1, 2020
@rgommers
Copy link
Member Author

rgommers commented Sep 1, 2020

That failed with a fairly obscure error on TravisCI:

/ccompiler_opt_cache_clib.py

  error: Command "gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -g -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -g -fstack-protector --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Werror=vla -Werror=nonnull -Werror=pointer-arith -Werror=implicit-function-declaration -Wlogical-op -Wno-sign-compare -fPIC -Ibuild/src.linux-x86_64-3.8/numpy/core/src/common -Inumpy/core/include -Ibuild/src.linux-x86_64-3.8/numpy/core/include/numpy -Inumpy/core/src/common -Inumpy/core/src -Inumpy/core -Inumpy/core/src/npymath -Inumpy/core/src/multiarray -Inumpy/core/src/umath -Inumpy/core/src/npysort -I/home/travis/build/numpy/numpy/builds/venv/include -I/opt/python/3.8.1/include/python3.8 -Ibuild/src.linux-x86_64-3.8/numpy/core/src/common -Ibuild/src.linux-x86_64-3.8/numpy/core/src/npymath -c build/src.linux-x86_64-3.8/numpy/core/src/npysort/mergesort.c -o build/temp.linux-x86_64-3.8/build/src.linux-x86_64-3.8/numpy/core/src/npysort/mergesort.o -MMD -MF build/temp.linux-x86_64-3.8/build/src.linux-x86_64-3.8/numpy/core/src/npysort/mergesort.o.d -msse -msse2 -msse3" failed with exit status 1

  Building wheel for numpy (PEP 517): finished with status 'error'

That was still without trying to force setuptools to use the distutils from stdlib though, so that's the next try.

EDIT: the failure on Windows happens earlier:

TypeError: CCompiler_spawn() got an unexpected keyword argument 'env'

@rgommers
Copy link
Member Author

rgommers commented Sep 1, 2020

TravisCI error, before the actual build even gets started, from distutils import sysconfig simply fails:

++python -c 'from distutils import sysconfig;     print (sysconfig.get_config_var('\''CFLAGS'\''))'
Traceback (most recent call last):
  File "/home/travis/build/numpy/numpy/builds/venv/lib/python3.8/site-packages/setuptools/_distutils/sysconfig.py", line 460, in _init_posix
    _temp = __import__(name, globals(), locals(), ['build_time_vars'], 0)
ModuleNotFoundError: No module named '_sysconfigdata_d_linux_x86_64-linux-gnu'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/travis/build/numpy/numpy/builds/venv/lib/python3.8/site-packages/setuptools/_distutils/sysconfig.py", line 573, in get_config_var
    return get_config_vars().get(name)
  File "/home/travis/build/numpy/numpy/builds/venv/lib/python3.8/site-packages/setuptools/_distutils/sysconfig.py", line 504, in get_config_vars
    func()
  File "/home/travis/build/numpy/numpy/builds/venv/lib/python3.8/site-packages/setuptools/_distutils/sysconfig.py", line 463, in _init_posix
    _temp = __import__(
ModuleNotFoundError: No module named '_sysconfigdata'
+sysflags=
The command "./tools/travis-test.sh" exited with 1.

@rgommers
Copy link
Member Author

rgommers commented Sep 1, 2020

The bigger issue here is the interaction between distutils, numpy.distutils and setuptools.

Old situation, for setuptools <50.0:

  1. numpy.distutils extends and monkeypatches distutils.
  2. the import order in NumPy and SciPy setup.py is:
import setuptools  # unused, just to let setuptools do its monkeypatching dance first
from numpy.distutils.core import setup

This situation worked reasonably well, because:

  • distutils moved slowly
  • larger distutils changes typically showed up in pre-releases of new Python versions, or in bugfix releases (say 3.7.7) that vendors like Anaconda qualify before they ship it, so we could adjust numpy.distutils to those changess
  • setuptools didn't do all that much that was relevant build-wise

New situation, for setuptools 50.0:

(default situation, no env var set)

  1. setuptools replaces distutils with its vendored setuptools._distutils, even if plain distuils is imported first
  2. numpy.distutils is unchanged, so it still extends and monkeypatches distutils - which is now setuptools._distutils

So numpy.distutils finds itself monkeypatching setuptools code all of sudden, and that
setuptools code includes patches from Python 3.9-dev that are therefore now released into the wild with new setuptools releases without any alpha/beta/QA trajectory like we had before.

The releasing new patches quickly without testing will still be an issue for numpy.distutils probably, even if the SETUPTOOLS_USE_DISTUTILS="local" behaviour gets reverted for the time being (which seems likely, since there's a ton of other issues right now).

What now?

Longer-term I don't think it's feasible for numpy.distutils to work as it does today and extend setuptools; the release cycle mismatch will be too much of a problem. I'm not quite sure what the best solution for that is though. Options could include:

  • for every NumPy release, add a hard setuptools <= current_setuptools_version constraint (NumPy releases get supported for 2.5 years though, so that'll mean pretty old setuptools versions for building, e.g., wheels of packages that rely on NumPy - example: scikit-learn needs to build with NumPy 1.13.3 right now, so it would get setuptools 36.5)
  • make numpy.distutils a standalone package on top of setuptools (then at least release cycle can be matched, but situation remains unstable)
  • integrate numpy.distutils into setuptools (more testing will then be done before setuptools releases, but is it a good idea knowledge and work load sharing wise?)

@sverzakov
Copy link

I see, "scikit-learn needs to build with NumPy 1.13.3 right now" seems like a real issue. So it is not only numpy needs to re-release all minor versions, but other packages should switch to them too.

Standalone numpy.distutils and numpy.distutils as a part of setuptools solutions would not be needed, if setuptools followed the semantics of stdlib distutils. But I guess it defeats the purpose of "setuptools distutils adoption"...

The whole issues demonstrates that there is an implicit dependency of numpy on setuptools (when they are installed), which should be made explicit (and it happens in all your 3 proposals).

I have two more "solutions", although I do not know if they are practical:

  1. do not use setutools in numpy at all (and loose all hot fixes)
  2. base all numpy instillation process on setuptools, not distutils (and have either version limitation or chain of re-releases)

Another problem: it looks like for Python <= 3.9 it is still possible to survive with "SETUPTOOLS_USE_DISTUTILS=stdlib" or older setuptools versions, but what if changes in pypa/disutils (e.g., spawn with env param) will be added to stdlib distutils?

@rgommers
Copy link
Member Author

rgommers commented Sep 2, 2020

So it is not only numpy needs to re-release all minor versions, but other packages should switch to them too.

We're not doing that, that's a nonstarter.

Standalone numpy.distutils and numpy.distutils as a part of setuptools solutions would not be needed, if setuptools followed the semantics of stdlib distutils. But I guess it defeats the purpose of "setuptools distutils adoption"...

Exactly. That's simply a "do nothing" option. And setuptools does have valid reasons for moving forward the way they're doing now.

The whole issues demonstrates that there is an implicit dependency of numpy on setuptools (when they are installed), which should be made explicit

It is explicit already, see for example pyproject.toml.

do not use setutools in numpy at all (and loose all hot fixes)

That's the situation we had until fairly recently, but it's not maintainable given what pip and setuptools are doing.

base all numpy instillation process on setuptools, not distutils (and have either version limitation or chain of re-releases)

That's the same option I had, my first one with the version pin.

@eric-wieser
Copy link
Member

It is explicit already, see for example pyproject.toml.

That's only explicit as a build-time dependency. Don't downstream users of numpy.distutils also end up with an implicit runtime dependency on setuptools?

@rgommers
Copy link
Member Author

rgommers commented Sep 2, 2020

Don't downstream users of numpy.distutils also end up with an implicit runtime dependency on setuptools?

I don't see why you'd ever need setuptools at runtime. Can you explain what makes you think that?

@eric-wieser
Copy link
Member

The scenario I'm thinking of is a downstream package that puts numpy as a build dependency, so that it can use numpy.distutils. It will not work correctly unless that downstream package also adds setuptools as a build dependency; therefore, we have an implicit dependency that falls to our users to make explicit. My mental model is that numpy has two parts:

  • numpy.distutils - this has a runtime dependency on setuptools
  • numpy,* - this has a build-time dependency on numpy.distutils (and therefore transitively, a build-time dependency on setuptools)

@rgommers
Copy link
Member Author

rgommers commented Sep 2, 2020

Oh okay, yes that makes sense. I wouldn't call that "runtime" though - it's "build-time for a downstream package". If you'd want to do anything at runtime, like playing with the new Python interface to CPU architecture SIMD support, or using get_info, that should still work without setuptools installed.

@rgommers
Copy link
Member Author

rgommers commented Sep 2, 2020

To make that clearer, both NumPy and SciPy:

  • have setuptools as an explicit build dependency in pyproject.toml
  • do not have setuptools in install_requires

That's how it should stay. The rest is just semantics.

@rgommers
Copy link
Member Author

Not needing this anymore, we're staying with the 0.49 pin. So closing this PR.

@rgommers rgommers closed this Sep 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
31 - Third-party binaries Install/import issues other than Anaconda-specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants