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

Don't install setup_requires when run as a PEP 517 backend #2303

Closed
takluyver opened this issue Aug 4, 2020 · 7 comments · Fixed by #2306
Closed

Don't install setup_requires when run as a PEP 517 backend #2303

takluyver opened this issue Aug 4, 2020 · 7 comments · Fixed by #2306

Comments

@takluyver
Copy link
Member

PEP 517 separates the responsibilities around build requirements: the backend is responsible for saying what is required, and the frontend is responsible for ensuring they're available to the build.

In setuptools, build requirements are defined by setup_requires, and these get passed through to PEP 517's get_requires_for_build_* hooks. There is a monkeypatch to return them from these hooks and prevent setuptools trying to install them itself:

class Distribution(setuptools.dist.Distribution):
def fetch_build_eggs(self, specifiers):
specifier_list = list(map(str, parse_requirements(specifiers)))
raise SetupRequirementsError(specifier_list)
@classmethod
@contextlib.contextmanager
def patch(cls):
"""
Replace
distutils.dist.Distribution with this class
for the duration of this context.
"""
orig = distutils.core.Distribution
distutils.core.Distribution = cls
try:
yield
finally:
distutils.core.Distribution = orig

But something similar to that - preventing installation - should really be in place for all the PEP 517 hooks, because a PEP 517 backend isn't responsible for installing dependencies.

Why does this matter? Pip has the --no-build-isolation option, with which the caller can declare that they have taken care of build dependencies and pip should try to build the package in the current environment. This is useful for downstream packagers, and for experimenting with different versions of your build dependencies. But setuptools doesn't know about this, so it charges ahead and attempts to install things when that's not what you want.

The workaround I'm looking at is to only specify setup_requires if 'egg_info' in sys.argv, as is the case when the get_requires_for_build_* hooks are called. But this is clearly not ideal.

takluyver added a commit to takluyver/setuptools that referenced this issue Aug 6, 2020
Under PEP-517, installing build dependencies is up to the frontend.

Closes pypagh-2303
@jaraco
Copy link
Member

jaraco commented Aug 10, 2020

I'm more inclined to simply deprecate setup_requires rather than try to add sophistication to it.

@takluyver
Copy link
Member Author

Would there be any replacement way for a project to specify build requirements? We're specifically dealing with an optional build requirement - you can configure building with or without MPI integration - so we can't just put it in pyproject.toml. In this discourse thread, @pganssle suggested that setup_requires is the way to do that with setuptools.

@pganssle
Copy link
Member

In this discourse thread, @pganssle suggested that setup_requires is the way to do that with setuptools.

To clarify, I think I was saying that setup_requires makes this possible, not that it's particularly advisable.

Conditional dependencies of any sort are the bane of dependency resolvers and that sort of thing, so I think the proper way to solve this is to add "feature flag" / "optional features" to the standards metadata, rather than to actually recommend that people have conditional dependencies in setup_requires.

A possible compromise until we have a standards-based solution to the very common feature request of "optional" / "opportunistic" dependencies would be to deprecate the idea that setuptools would ever try to satisfy setup_requires at all, maybe?

It would be cleaner to add a new additional_build_requirements key or something and deprecate setup_requires entirely rather than subtly change the meaning of setup_requires (basically throwing warnings if setup_requires ever installs anything, then eventually stopping the installations entirely), but like I said I said I don't think we should be encouraging this behavior anyway, and I don't see any other valid use cases for additional_build_requirements.

@pganssle
Copy link
Member

Actually, come to think of it, I can think of one valid use case (though maybe this should be a modification to the pyproject.toml standard) — when you have a dependency for doing the build, but not for other PEP 517 operations like building the metadata. I think this distinction is often overlooked because it doesn't come up often, but things like dependency resolvers need to get reliable metadata, and if you have a build-time dependency on, for example, numpy, that only kicks in when building the extension, it could really slow down the "get reliable metadata" step (particularly if no wheel is available).

It might be worth adding in more explicit ways to list which dependencies apply to which step of the process.

@takluyver
Copy link
Member Author

Aha, sorry, I had over-optimistically read the thread as "setuptools supports this through the setup_requires option".

In our case, we have a fairly clear "normal" build (without MPI), and I expect the people building it with MPI support to be prepared to cope with packaging oddities that might come up. We've had this optional feature for years already, and we're trying to adapt it to a world with PEPs 518 & 517. So my interest in setup_requires is as something that more-or-less does what we need today, and could do it better in the future (hence this PR). I'm happy to consider new parameters and new standards, but right now I'm trying to find a solution that we can use in a release in the next few days.

@jaraco
Copy link
Member

jaraco commented Aug 11, 2020

Could you instead split out the behavior into two packages, one with the core support and another with the supplementary MPI support? Then the user could conditionally install just core or core + mpi (perhaps using extras as core[mpi]). The code would have to be aware of the existence of both and fall back to non-mpi behaviors if mpi isn't present, but otherwise, this would create two static builds that could both rely on pep517/518 and avoid setup_requires.

@takluyver
Copy link
Member Author

It may be possible - I don't know enough about MPI to say so definitively. I'd expect it to be a fair bit of work, and probably introduce new bugs. It's certainly not something I want to tackle for the next release. Building two binary flavours from the same source code is a fairly familiar solution, and one that we're generally pretty happy with in h5py.

Maybe we'll do our workaround for this for the next release, and then look at switching build system.

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

Successfully merging a pull request may close this issue.

3 participants