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

Please consider undeprecating setup_requires #1742

Closed
ghost opened this issue Apr 13, 2019 · 23 comments
Closed

Please consider undeprecating setup_requires #1742

ghost opened this issue Apr 13, 2019 · 23 comments
Labels

Comments

@ghost
Copy link

ghost commented Apr 13, 2019

I have run into an issue with the deprecation of setup_requires: the recommended alternative appears to be PEP 517/build-system.requires, but that comes with its own issues:

  • apparently it breaks wheels unless you use special options like --no-binary. This breaks very common tools like tox which no longer can install a project if this package is a dependency - also, why would you want to not have wheels? this is wrong, my apologies

  • setup_requires isn't installed when invoking hooks.get_requires_for_build_wheel() as called by pip/pep517. However the new build-system.requires is. In practice this is an issue when putting a Cython project as a build dependency for .pxd imports: a .pxd dep is only actually required when building the wheel (where setup_requires will provide it) but not for basic package analysis (where build-system.requires per design provides it too, setup_requires won't). In practice since "providing it" means building the whole package, as a result obtaining any package metadata is greatly slowed down

  • pyproject.tomls build-system.requires is declarative but not exhaustive. the problem with that is that the dependencies are then spread over multiple places, e.g. no longer just requirements.txt parsed by setup.py - which can't be parsed similarly in the declarative pyproject.toml file with its static build_system.requires

Edit: a lot of this was initially quite misinformed so I corrected the reasons

@ghost
Copy link
Author

ghost commented Apr 13, 2019

EDIT FROM THE FUTURE: this part is somewhat irrelevant because I just accidentally broke something (so fully my mistake for the distraction, apologies 🙈 ) jump down here for remaining issue: #1742 (comment)


By the way since setup_requires is deprecated and therefore apparently it still uses easy_install because nobody fixed that (I suppose makes sense if it's deprecated) it doesn't work with Cython(!) because it breaks it with its sandbox: cython/cython#2730

But as written above, using pep518 breaks tox! And just putting it into install_requires works for all pip invocations outside I've seen, but also breaks in tox because through some random chance pip there decides to build the project itself before its deps which of course doesn't work if .pxd imports are needed (that requires the deps to be present in site-packages)

So as a result it appears it is currently impossible to rely on a dependency for .pxd cross-package import and have that project as a dependency for another that builds in tox. Isn't that a major issue? How is any of this supposed to work? All I'm seeing is two mechanisms, one deprecated and unable to deal with Cython, the other having really weird restrictions that break tox and wheels and whatnot, and now I can choose between the devil and the deep blue sea!

Edit: meant pep518 ("build-system".requires) and not pep517 (library)

Edit 2: most of this was wrong, hence the correction 😄

@benoit-pierre
Copy link
Member

The alternative to setup_requires is PEP 518, not necessarily PEP 517. What's the issue with pip's PEP 518/518 support and wheels?

@benoit-pierre
Copy link
Member

It would help if you provided a reproducible example of the issue you are running into.

@benoit-pierre
Copy link
Member

Assuming the issue is with your nettools package, using https:/JonasT/nettools/archive/a8b9236f985d468e49953f818dad3dcb1bbe2b77.zip with an added minimal pyproject.toml works fine:

[build-system]
requires = ["setuptools", "wheel", "cython"]

That is pip wheel . from a clean virtualenv (with only pip available) successfully build a wheel.

@benoit-pierre
Copy link
Member

Note that setuptools won't include pyproject.toml automatically, so you'll need a manifest entry if you want the source distribution to be complete, in MANIFEST.in:

include pyproject.toml

@ghost
Copy link
Author

ghost commented Apr 13, 2019 via email

@benoit-pierre
Copy link
Member

Again, can you point to a reproducible example?

@ghost
Copy link
Author

ghost commented Apr 13, 2019

I'll try to. This involves 3 projects with interdependencies and tox, so I'm not sure how quickly I can find out what constellation breaks it in detail. But it seems to be related to how tox builds the deps, and possibly --only-binary and other options it uses to build the wheel

Edit: also, the other two disadvantages I listed still remain. Declarative can be both good but also a huge disadvantage, and having dependencies somewhat arbitrarily split (there is a criteria of course but it's not very transparent) between two files and wildly different formats just is a little ugly

Edit 2: (I just had a broken package. Realized why later, see below!)

@ghost
Copy link
Author

ghost commented Apr 13, 2019

The exact error I believe was this @ tox/wheel: Could not build wheels for wobblui which use PEP 517 and cannot be installed directly.

@ghost
Copy link
Author

ghost commented Apr 14, 2019

@benoit-pierre Okay, so my MANIFEST.in was wrong and this lead to problems! Now I fixed that, and it almost works, but I get a really strange error on a dependent package and basically it doesn't work for some reason:

pypa/packaging-problems#252
(this issue now breaks it both inside and outside of tox)

But I suppose my ticket here wasn't that constructive given the wheel thing was just wrong, and the other points are admittedly a bit personal taste (I just don't think the multiple files or forcibly declarative approach just for half the things mixes well with how setuptools works right now) so I'll close the ticket for now even though it doesn't actually work, but see the above issue link for that

Edit: unintentional comment spam from my side (sorry again 😬 ) ends with this comment of my past self, next comment explains true issue more in-depth

@ghost ghost closed this as completed Apr 14, 2019
@ghost
Copy link
Author

ghost commented Jun 13, 2019

For what it's worth, I discovered using PEP518 (build-system.requires) instead of setup_requires has the following problem:

  • setup_requires is added to returned value of hooks.get_requires_for_build_wheel() as called by pip/pep517, but in practice not installed if one just needs to run setup.py and obtain hooks.get_requires_for_build_wheel() without actually building the wheel - which is necessary e.g. to obtain package metadata like dependencies.
  • and build-project.requires always is, per design, installed first, even when just hooks.get_requires_for_build_wheel() is run and the wheel is never built, affecting obtaining package metadata

This is a problem because it slows down obtaining basic package metadata greatly if you put in a build dependency that builds slowly. And many .pxd cimport targets tend to be larger Cython libs with native code compilation that is very slow. Some of my projects went from a few seconds metadata extraction to more than five minutes! And in some environments like https:/kivy/python-for-android this metadata analysis is necessary to automatically map to cross-compile patches and other tasks which turns abhorrently slow due to this.

I'm therefore reopening, and suggesting setup_requires is undeprecated (is it even still deprecated?) or something else is found to provide that behavior, also see discussion here: https://discuss.python.org/t/support-for-build-and-run-time-dependencies/1513/61?u=jtt

@pradyunsg
Copy link
Member

pradyunsg commented Jun 13, 2019

It would help if you provided a reproducible example of the issue you are running into.

@Jonast Could you provide a minimal example case that can be used to reproduce the issue you're reporting here?

@pradyunsg
Copy link
Member

pradyunsg commented Jun 13, 2019

@Jonast it does not help anyone if you comment at multiple locations for reporting the same issue - especially multiple times. You are reaching (mostly) the same folks on all those channels. To keep track of everything that's been discussed, I need to look at 2 Discourse threads and at least 3 issues spread across 3 repositories.

Could you please keep the discussion in one place, so that your issue and all possible options are all discussed in one place? That would make life easier for the project maintainers.

I'm gonna take the liberty to say that this issue is fine to have further discussion -- I'm happy to defer to setuptools maintainers to tell us a better place.

@ghost
Copy link
Author

ghost commented Jun 13, 2019

@pradyunsg I know this is off-topic, but since I did get multiple remarks like this over the last few days I would like to say something about it:

It can be super complicated to find the right place when not understanding yet what even the problem is. I try to make the best guess but from the outside the packaging ecosystem is really difficult to get a grasp on, so I will be almost inevitably wrong. When someone points out the correct place or I realize a discussion is actually indicating a bug that needs a ticket, I try my best to figure out where this actually should be and correct things. I also try to always refer to the next place, but I may simply mess things up because I usually report for much simpler projects involving no such "moves".

I am therefore hoping you won't hold any grudges and that you'll be forgiving also to people coming after me, because it is more difficult than it may look like 🤷‍♀️ my apologies and please have some mercy, we're not all as intimately familiar with things as you are! That would certainly help sometimes 😄


Sorry, I did actually forget about the example. This is a common way my pyproject.toml's look like right now:

[build-system]
requires = ["setuptools", "wheel", "Cython", "wobblui @ https:/wobblui/wobblui/archive/master.zip#egg=wobblui"]
build-backend = "setuptools.build_meta"

wobblui is a UI library I am working on from which I often want to do .pxd cimports for performance reasons. Sadly, it is also quite large so it may take a while to build.

Put the above pyproject.toml into a folder and add a trivial setup.py (e.g. just put from setuptools import setup; setup(name="bla")) and try to examine the metadata like this:

$ python3
Python 3.7.3 (default, May 11 2019, 00:45:16) 
[GCC 8.3.1 20190223 (Red Hat 8.3.1-2)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from pep517.envbuild import BuildEnvironment
>>> from pep517.wrappers import Pep517HookCaller
>>> import pytoml
>>> with open("pyproject.toml") as f:
...     build_sys = pytoml.load(f)['build-system']
... 
>>> env = BuildEnvironment()
>>> with env:
...     hooks = Pep517HookCaller(".", build_sys["build-backend"])
...     env.pip_install(build_sys["requires"])
...     reqs = hooks.get_requires_for_build_wheel({})

This is also the approach https:/kivy/python-for-android uses, and due to the build-system.requires entry it now takes a really long time to run. If you take that exact wobblui ref out of the pyproject.toml and move it into the setup.py as setup_requires=["wobblui ... "] then on the other hand doing that same analysis will only take a few seconds

@pradyunsg
Copy link
Member

pradyunsg commented Jun 13, 2019

It can be super complicated to find the right place when not understanding yet what even the problem is. I try to make the best guess but from the outside the packaging ecosystem is really difficult to get a grasp on, so I will be almost inevitably wrong.

Yep, I empathize with you here. This stuff can be pretty complicated so it is perfectly okay to be wrong. :)

I am therefore hoping you won't hold any grudges and that you'll be forgiving also to people coming after me, because it is more difficult than it may look like 🤷‍♀ my apologies and please have some mercy, we're not all as intimately familiar with things as you are! That would certainly help sometimes 😄

I didn't mean to come across as harsh (apologies if I came across as that) and I'm definitely not holding grudges. 🙈

As someone who's been trying to understand what exactly your issue is, I was mostly just suggesting this since it was getting a little difficult to keep track up with how this conversation was evolving.

I also try to always refer to the next place, but I may simply mess things up because I usually report for much simpler projects involving no such "moves".

Yep yep, I noticed. It'll be easier to point folks from other channels to chime in at a single "broad" place (like I've tried to do with this issue now). That way, it's easier for everyone to keep track of the discussion and the conversation can branch/flow as needed on a single platform and it's easier for everyone who has interest in the discussion can see how things evolved.


We definitely need to improve the situation with our communication channels to make them easier for new contributors. (btw, I do want to take inputs from you on how we could improve things for this, but let's do that elsewhere)


Thanks for the example! That should help us here. :)

@astrojuanlu
Copy link
Contributor

it is declarative. before you say "well that's intentional", the problem with that is that I like to have my dependencies in one place, currently requirements.txt

Comment from the peanut gallery: requirements.txt was never meant to list library requirements https://caremad.io/posts/2013/07/setup-vs-requirement/ and also, I guess you're kind of parsing it inside of your setup.py, which can lead to strange issues because some syntax supported in requirements.txt breaks setup().

@ghost
Copy link
Author

ghost commented Jun 30, 2019

@Juanlu001 first of all yeah that's really a nitpick, the main problem is the exploding package analysis time as illustrated in the example above. (which normally doesn't matter much, but is a huge problem with python-for-android because it causes 5-15 minutes additional build time which is massive) All the others are just nitpicks, but I felt it made sense to list them all for a full picture

As for the unsupported syntax, for me the problem is really the reverse: I'd like to use PEP 508 URLs and requirements.txt doesn't support them so I have quite some parsing to get them back in install_requires from a different format. So install_requires has better support for what I want to do than requirements.txt does - but I suppose this really differs for every use case. I wouldn't need a requirements.txt I just have it as additional convenience for some people who like to use it to set up their dev environment

@ghost
Copy link
Author

ghost commented Jul 13, 2019

I just decided to check for some more real world numbers, and in my current build for checking the deps of 7 packages, I just measured and I lose 13 minutes doing this. I'm pretty sure if there was a way to avoid compiling the cython deps just to examine a package I'd be going through that part in under a minute. So the time loss really is quite massive... 😢 This is on a first-gen AMD zen hexacore, so it's not the slowest machine ever

@ghost ghost changed the title Please undeprecate setup_requires or provide a reasonable alternative Please consider undeprecating setup_requires Jul 29, 2019
@McSinyx
Copy link
Contributor

McSinyx commented Jan 25, 2020

Where was it stated that setup_requires is deprecated if I may ask? I cannot seem to find that in PEP 518.

@ghost
Copy link
Author

ghost commented Jan 25, 2020

@McSinyx #1320 (comment) (also, some remarks about it here: #1784 )

@McSinyx
Copy link
Contributor

McSinyx commented Jan 25, 2020

Thanks, that should be officially documented elsewhere, but that's out of the scope of this issue I guess.

@jaraco
Copy link
Member

jaraco commented Oct 22, 2021

Regarding the performance issue, I think there are two strategies:

  • Replace the large, slow build dependency with a smaller version that does enough for the build to succeed.
  • Create a build artifact that pip could reasonably cache (a git repo can't be reasonably be cached).
  • Work with the packaging team to come up with a way to re-use build dependencies.

Setuptools won't be undeprecating setup_requires and I'm actively working to make more visible the deprecation and remove the functionality.

@jaraco jaraco closed this as completed Oct 22, 2021
@jaraco jaraco added the wontfix label Oct 22, 2021
@Hierosme
Copy link

Hierosme commented Oct 29, 2021

For me it type of breakout is not a good news. Like Python 2 to 3, it have take almost 10 years for back to reality.
That it not the first time where a PEP is totaly worng.

The PEP in question transform something it have been simple during years by something totaly strange, with nobody it have experience, and no clear documentation.

I not understand that need to break the work of world developper's.

My point is:
Please consider to limit the negative impact of it type a change.
I'm not sure maintain a Alias will be the end of the world.

Regards

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

No branches or pull requests

6 participants