pypa / setuptools

Official project repository for the Setuptools build system
https://pypi.org/project/setuptools/
MIT License
2.54k stars 1.2k forks source link

Please consider undeprecating setup_requires #1742

Closed ghost closed 3 years ago

ghost commented 5 years ago

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:

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

ghost commented 5 years ago

EDIT FROM THE FUTURE: this part is somewhat irrelevant because I just accidentally broke something (so fully my mistake for the distraction, apologies :see_no_evil: ) jump down here for remaining issue: https://github.com/pypa/setuptools/issues/1742#issuecomment-501609619


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: https://github.com/cython/cython/issues/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 :smile:

benoit-pierre commented 5 years ago

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 commented 5 years ago

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

benoit-pierre commented 5 years ago

Assuming the issue is with your nettools package, using https://github.com/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 commented 5 years ago

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 commented 5 years ago

Yes that message appears if you use it in a dep that itself is a build requires dep of your main project in tox.

Edit from way later: the message appeared because my package was broken :smile: apologies from future me

benoit-pierre commented 5 years ago

Again, can you point to a reproducible example?

ghost commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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:

~~https://github.com/pypa/packaging-problems/issues/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 :grimacing: ) ends with this comment of my past self, next comment explains true issue more in-depth

ghost commented 5 years ago

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

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://github.com/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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 :woman_shrugging: my apologies and please have some mercy, we're not all as intimately familiar with things as you are! That would certainly help sometimes :smile:


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://github.com/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://github.com/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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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... :cry: This is on a first-gen AMD zen hexacore, so it's not the slowest machine ever

McSinyx commented 4 years ago

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

ghost commented 4 years ago

@McSinyx https://github.com/pypa/setuptools/issues/1320#issuecomment-380093653 (also, some remarks about it here: https://github.com/pypa/setuptools/issues/1784 )

McSinyx commented 4 years ago

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

jaraco commented 3 years ago

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

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

Hierosme commented 3 years ago

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