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

Fix setuptools bootstrapping now that dependencies are no longer vendored #3198

Merged
merged 4 commits into from Feb 21, 2017

Conversation

adamjstewart
Copy link
Member

@tgamblin @svenevs This fixes the bug I introduced in #3195.

Based on the CHANGES.rst that comes with setuptools:

v34.0.0
-------

* #581: Instead of vendoring the growing list of
  dependencies that Setuptools requires to function,
  Setuptools now requires these dependencies just like
  any other project. Unlike other projects, however,
  Setuptools cannot rely on ``setup_requires`` to
  demand the dependencies it needs to install because
  its own machinery would be necessary to pull those
  dependencies if not present (a bootstrapping problem).
  As a result, Setuptools no longer supports self upgrade or
  installation in the general case. Instead, users are
  directed to use pip to install and upgrade using the
  ``wheel`` distributions of setuptools.

  Users are welcome to contrive other means to install
  or upgrade Setuptools using other means, such as
  pre-installing the Setuptools dependencies with pip
  or a bespoke bootstrap tool, but such usage is not
  recommended and is not supported.

  As discovered in #940, not all versions of pip will
  successfully install Setuptools from its pre-built
  wheel. If you encounter issues with "No module named
  six" or "No module named packaging", especially
  following a line "Running setup.py egg_info for package
  setuptools", then your pip is not new enough.

  There's an additional issue in pip where setuptools
  is upgraded concurrently with other source packages,
  described in pip #4253. The proposed workaround is to
  always upgrade Setuptools first prior to upgrading
  other packages that would upgrade Setuptools.

it looks like setuptools no longer supports any installation method aside from pip. All of its dependencies use setuptools, making it very difficult to build from source. I had to hack a couple packages to provide a fallback (distutils.core) so that they could build without setuptools. I'll see if I can get these patches merged upstream.

@svenevs
Copy link
Member

svenevs commented Feb 21, 2017

Yeah I think this is the best thing spack can do right now, I just finished reading through their 581 discussion and my understanding is they are effectively assuming that people will use get-pip.py to install it cleanly or something. Which, given the already confused spack && pip relationship, is probably the last thing you'd want to do here.

It seems like this issue / change may bring about even more changes to many python packages. AKA it's worth remembering that, if for some spack package py-X, things are breaking, it could be this change is bubbling up somehow. I think that depends a lot on how the requirements for the spack package were written, but doesn't this change mean that any py-X that currently lists py-setuptools as a package could be removed, and just upstream it all so that all py-* are using this py-setuptools?

That would probably be very painful to achieve, but that was my understanding of what this change actually means -- setuptools is now expected to be installed independently, never as a requirement.

Very unrelated, that discussion mentions that build dependencies may very well be coming to pip!

@adamjstewart
Copy link
Member Author

That would probably be very painful to achieve, but that was my understanding of what this change actually means -- setuptools is now expected to be installed independently, never as a requirement.

No no, this PR just removes setuptools as a dependency for its own dependencies. We do something similar for curl and openssl. For those packages, and all of their dependencies, we force the use of http instead of https. That way, if you don't have a working curl/openssl installation, you can still download the dependencies and build them. If we used https there, this wouldn't be possible.

In no way would I suggest wholesale removing setuptools dependencies.

@adamjstewart
Copy link
Member Author

It looks like someone beat me to it for appdirs:
ActiveState/appdirs#84

@tgamblin
Copy link
Member

The Python guys sure are trying really hard to make themselves impossible to package. This approach seems like a reasonable way to do things. Although it's kind of annoying that the Python guys assume some kind of circular bootstrapping is going to happen here.

It would be nice to delegate to pip more, but I don't think that makes a lot of sense for a few reasons:

  1. We need to be able to mirror things to airgap networks.
  2. We need compiler hooks for a lot of packages (AFAIK pip doesn't really let me set that stuff).

Wheels are also problematic per (2).

@svenevs: I don't think we can remove setuptools, because we still have to use setup.py to install things.

@svenevs
Copy link
Member

svenevs commented Feb 21, 2017

In no way would I suggest wholesale removing setuptools dependencies.

Like I said, I could be misunderstanding, but it seemed like that's the direction things are heading.

Aka instead of py-setuptools being available as a dependent package for spack, you ALWAYS force that dependency.

@tgamblin
Copy link
Member

@adamjstewart: do all of the current setuptools dependencies function ok with distutils.core's setup routine right now? Will that change one day? 😱

@tgamblin
Copy link
Member

@svenevs:

instead of py-setuptools being available as a dependent package for spack, you ALWAYS force that dependency.

I don't think we can do that for all the py-* packages. Some of them don't even use setup.py to install -- they use makefiles and I think we want to maintain that versatility...

@adamjstewart
Copy link
Member Author

And someone beat me to it for pyparsing too:
https://sourceforge.net/p/pyparsing/patches/10/

Glad other people are aware of the problem.

Btw, I didn't even notice that six depends on pyparsing as a runtime dependency until the setuptools installation crashed, so hopefully there aren't any other random dependencies we are missing.

@adamjstewart
Copy link
Member Author

@tgamblin All of the packages I just converted to use distutils.core work, but I have to assume that not all packages that use setuptools could instead use distutils.core. Otherwise what would be the point of setuptools 😆

@svenevs
Copy link
Member

svenevs commented Feb 21, 2017

@tgamblin

they use makefiles

Oh dear. I mean I'm the very first person to completely ditch Windows support, but that seems really bad!

Anywho, I was more suggesting that instead of people having to specify py-setuptools as a build dependency, you simply always have that. If py-X has a setup.py, whoever is writing py-X for spack damn well better make sure py-setuptools is being asked for. But, if they do not, then if the original developers of X had foresight into setuptools not being available and included the patch above with the fallback to distutils, there is potential room for error.

This is a topic that is well beyond my python knowledge -- I'm still trying to wrap my head around __init__.py vs __main__.py, and how for the love of all that is holy you enable relative imports for python pkg and python -m pkg "correctly". But the internet at large basically says "if you can use setuptools's setup, you need to because it is better (and hence why it exists in the first place).

Forcing py-setuptools to be available for all py-* (not forcing it to be used, just available), seems like it would solve this because pretty much everybody tries to first use py-setuptools and only when not found fallback on distutils.

Does that make sense? It's basically adding another meta layer to the python package people inherit from to write py-X.

@tgamblin tgamblin merged commit d2a52d6 into spack:develop Feb 21, 2017
@tgamblin
Copy link
Member

@svenevs: we could think about how we might add a setuptools dependency automatically to PythonPackage but given that there are PythonPackages that wouldn't have a setuptools dependency, and that we don't currently have a way to extend a package and remove a dependency (and adding that seems like more trouble than it might be worth), I'm inclined to just be explicit about the deps.

@adamjstewart's much improved spack create currently generates some boilerplate that hints at the need for setuptools. It may make sense to uncomment that by default when setup.py is detected.

@svenevs
Copy link
Member

svenevs commented Feb 21, 2017

It may make sense to uncomment that by default when setup.py is detected.

Oh yeah that's way better! Everything stays explicit, so no voodoo magic going on behind the scenes, and a best-faith-effort to use the "right" setup function from py-setuptools.

@adamjstewart adamjstewart deleted the fixes/setuptools branch March 24, 2017 22:10
diaena pushed a commit to diaena/spack that referenced this pull request May 26, 2017
…ored (spack#3198)

* Fix setuptools bootstrapping now that dependencies are no longer vendored
* Reorder patch and comments
* Use exact same patch as ActiveState/appdirs#84
* Use exact same patch as https://sourceforge.net/p/pyparsing/patches/10/
xavierandrade pushed a commit to xavierandrade/spack that referenced this pull request Jun 16, 2017
…ored (spack#3198)

* Fix setuptools bootstrapping now that dependencies are no longer vendored
* Reorder patch and comments
* Use exact same patch as ActiveState/appdirs#84
* Use exact same patch as https://sourceforge.net/p/pyparsing/patches/10/
amklinv pushed a commit that referenced this pull request Jul 17, 2017
…ored (#3198)

* Fix setuptools bootstrapping now that dependencies are no longer vendored
* Reorder patch and comments
* Use exact same patch as ActiveState/appdirs#84
* Use exact same patch as https://sourceforge.net/p/pyparsing/patches/10/
healther pushed a commit to electronicvisions/spack that referenced this pull request Jul 26, 2017
…ored (spack#3198)

* Fix setuptools bootstrapping now that dependencies are no longer vendored
* Reorder patch and comments
* Use exact same patch as ActiveState/appdirs#84
* Use exact same patch as https://sourceforge.net/p/pyparsing/patches/10/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants