-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Docs: Add instructions about build_meta
wrappers as alternative to dynamic setup_requires
#2897
Conversation
build_meta
wrappers as alternative to dynamic setup_requires
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure I'd like to endorse this approach officially. I'm also uneasy exposing the internal API of the backend. I'd prefer instead to provide supported hooks for this purpose.
Although setup_requires is deprecated for installing packages, it's not deprecated for the PEP 517 build backend. e.g.:
# setup.py
import setuptools
import random
setuptools.setup(setup_requires=[random.choice(['dep_a', 'dep_b'])])
Of course, this change doesn't give selectability between dependencies for sdist and binary builds, that I'd like to support. For that, I'd like to provide build_requires
or similar, with support for static builds and dynamic ones.
My worry about this documentation is that it elevates the technique and encourages adopting of the approach, but I'd like to reserve this approach for very selective use-cases.
I really appreciate the work here, but I'm still leaning away from it.
Since the alternative is to let it linger, perhaps for a long time, I'm going to go ahead and accept the change, but I'm going to suggest we make the support "provisional" by making it clear that this approach is not recommended for any use case, but is available for those that absolutely need it. WDYT?
Hi @jaraco thanks for the comments and review. I understand the hesitation here, ideally everything should be done declaratively, but there are some complicated use cases that might require wrapping the backend.
A positive side of simply relying on PEP517 is that it avoids growing (even more) complexity on setuptools and the associated maintenance cost. (e.g. the selectability you mention, in theory is already supported - as documented in this PR - with no implementation needed).
I am not sure if I understood this comment. I think all the functions I used in the example are specified by the PEPs (517/518), which makes them automatically public right? There would be no need of exposing internal API, everything can be done by just following the standards.
Please feel free to close the PR if it does not match setuptools long-term vision. Personally, I think that this approach is a simple consequence of the standards, so marking it as provisional feels a bit weird (in theory this approach should be very stable and sound, unless there is a change in the standards), but I appreciate that the project might want to provide a better/easier way in the future. |
Regarding the tradeoff between the cost of implementing/maintaining a setuptools-specific solution and the easiness of use: Isn't it fair to assume that if a project reaches a point that it needs to separate build dependencies for sdist and whell, or to delay their installation, writing a very thin backend wrapper is straight forward? Are these needs likely/recurrent enough to justify adding complexity to setuptools? |
All good points. Let's proceed. |
When a pure build is requested, no compiled extensions are produced. The setuptools_rust dependency ends up imported-but-not-used. Teach setuptools to avoid build-requiring it in this case. Unfortunately, setuptools deprecated the setup_requires keyword which is intended to do this and replaced it with confusing advice that users should implement their own build backend that subclasses setuptools. It's a bit more busywork to do as the setuptools documentation suggests but at least the result is as expected. Closes: jelmer#1405 Bug: pypa/setuptools#2854 Buggy: pypa/setuptools#2897
IMO not very fair at all. It requires:
I could edit a single file, or I could edit 4 of them. Wouldn't it make so much more sense to have a single straightforward way to do this? Some of these requirements aren't even obvious. The merged documentation has an "IMPORTANT" to remind you of this but I don't think it's safe to assume everyone will realize this anyway.
Why do build backends such as setuptools exist if not to automate away the pointless busywork? |
I agree with this sentiment. Would you be willing to write up a new issue summarizing the concern and possibly laying out a proposed solution (or solutions)? |
Summary of changes
Added instructions about wrapping
setuptools.build_meta
for customising the backend.This is useful for dynamically specifying build requirements, according to PEP 517.
I believe this should be enough to close #2854.
Moreover by looking at the issues, I also have the impression that #1698 is already covered and could be closed.
Pull Request Checklist
changelog.d/
.(See documentation for details)