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

[FR] User configurable get_requires_for_build_wheel and get_requires_for_build_sdist #2854

Closed
1 task done
aragilar opened this issue Nov 4, 2021 · 10 comments · Fixed by #2897
Closed
1 task done
Labels
enhancement Needs Triage Issues that need to be evaluated for severity and status.

Comments

@aragilar
Copy link

aragilar commented Nov 4, 2021

What's the problem this feature will solve?

#2823 proposes to remove the legacy setup_requires option. Currently, setup_requires can be set statically (which has been replaced by the requires key in pyproject.toml), or dynamically during the running of the setup.py file. This latter method of dynamically setting the build requirements can be replaced more cleanly by the two optional PEP 517 hooks get_requires_for_build_wheel and get_requires_for_build_sdist. Currently setuptools does not allow users to override and/or set the output of these hooks.

Describe the solution you'd like

I'm not proposing a specific solution (I suspect the best solution will be the one that best matches what users need, and aligns with the internal design of setuptools), but here are three possible designs:

  1. Add two new arguments to setuptools.setup: sdist_requires and wheel_requires. These would be lists of requirements, similar to install_requirements. This would likely be the least change to existing setup.py files, but what that implies for the internal flow of setuptools is unclear. Additionally, this does not pass config_settings to the user code, which could be desired by users. It would be assumed that the presence of sdist_requires and wheel_requires would override setup_requires whilst that option was allowed.

  2. Like 1, but instead of the options being lists of requirements, instead a function matching the interface of the PEP 517 hooks would be given. This does require more changes to user code, but means that the user code can handle config_settings. I'm not sure there's much difference between 1 and 2 from a setuptools internal-design perspective though (as both are being handled completely within the setup.py module).

  3. Add sdist_requires and wheel_requires to setup.cfg. The values would follow the same format as build-backend from PEP 517, with the callables being pointed to matching the interface of the relevant hook. Depending on the internals of setuptools, this could allow setuptools to only run the setup.py file on the mandatory hooks.

Alternative Solutions

No response

Additional context

This came from #2823, from what I can see there's not been a discussion around this.

I'm also not aware of build backends using this in anger, last time I surveyed the ecosystem none of the backends provided a way to set this, so I wouldn't be surprised if this causes some breakage in some frontends (even if it is part of PEP 517).

Code of Conduct

  • I agree to follow the PSF Code of Conduct
@aragilar aragilar added enhancement Needs Triage Issues that need to be evaluated for severity and status. labels Nov 4, 2021
@jaraco
Copy link
Member

jaraco commented Nov 5, 2021

Thanks for filing this. Agreed it's something worth exploring. I'm especially interested in providing an interface to provide build-only requirements (not needed for sdist or metadata generation), as I'm aware of one case where users would like to include numpy during builds but don't want to pay the cost of installing numpy during metadata generation.

Can you provide an example of a problem you're trying to solve (or an example project where static build requirements is insufficient)? I acknowledge that PEP 518 build requirements are less flexible than setup_requires, but it's the dynamism of setuptools that the packaging ecosystem is attempting to avoid. I'd like to see a compelling use-case that would show the value of dynamic requirements before considering supporting them in this new packaging world.

The downside of dynamic requirements is that a downstream consumer can't inspect in advance what requirements might be needed, so it adds complexity to the build and limits the viable environments in which it can be installed.

As an extreme example, imagine a project that uses random.choice([build-dep1, build-dep2]) for its build dependencies. It would be near impossible for a downstream consumer to support that package in an offline installer; it couldn't determine which dependencies need to be present to build/install the package.

It's already hard enough to support the environment markers (a build on macOS might by satisfied by a different set of requirements than on Windows), but at least that system is well-defined and backed by supporting libraries. If backends start allowing arbitrary logic for specifying requirements, it becomes dramatically more difficult for vendors to work with the tools.

@aragilar
Copy link
Author

aragilar commented Nov 6, 2021

I 100% agree with not making things worse for downstream users, I'm aiming to make things better by providing the build metadata via standard mechanisms, rather than having to build things in odd ways.

My example use-case is being able to choose between an MPI and non-MPI build of h5py (with the non-MPI wheel being the default, and provided as a wheel on PyPI). Currently users can set HDF5_MPI="ON" which modifies setup_requires to include mpi4py, and tells the Cython code to enable the MPI features. This makes it much easier for users to build from source (which has to be done for MPI—there not a single MPI library, rather multiple different implementations and versions, and usually users don't get a choice of which one to use, it comes as part of the cluster), so keeping equivalent functionality would be nice.

I don't know of other libraries which have a similar use-case (I think you would need a library using C-extensions which in turn depends on a library with C-extensions and the first library cares about how the latter was configured), but I may be self-selecting out which libraries I use.

Some things that would be really nice, which are beyond the scope of this issue (as they probably require a PEP or updating existing PEPs) would be to store the versions used to build the sdist/wheel in the sdists/wheels metadata, and to pass the versions of the build dependencies installed to sdist/wheel builder (this could use config_settings). These would hopefully make it easier to manage the dynamism of get_requires_for_build_*.

One alternative solution to the example use-case above which I've just though of (which could go under alternative solutions, happy to copy it there if you want me to), is telling users to use non-python package managers such as spack, but I suspect that's not going to go over well with some users (given the pip vs. conda arguments...).

@abravalheri
Copy link
Contributor

abravalheri commented Nov 19, 2021

Hi @aragilar, have you considered adding a thin layer on top of setuptools.build_meta as defined in https://www.python.org/dev/peps/pep-0517/#in-tree-build-backends?

For example, you could have a _build_backend.py file:

from setuptools import build_meta as _orig

prepare_metadata_for_build_wheel = _orig.prepare_metadata_for_build_wheel
build_wheel = _orig.build_wheel
build_sdist = _orig.build_sdist


def get_requires_for_build_wheel(self, config_settings=None):
    return _orig.get_requires_for_build_wheel(config_settings) + [...]


def get_requires_for_build_sdist(self, config_settings=None):
    return _orig.get_requires_for_build_sdist(config_settings) + [...]

and then tell the front-end to use it via pyproject.toml

[build-system]
requires = ["setuptools", "wheel"]
build-backend = "_build_backend"
backend-path = ["."]

@layday
Copy link
Member

layday commented Nov 19, 2021

(Please don't put . on the path, create a dedicated folder.)

@aragilar
Copy link
Author

@abravalheri I did have the idea of going one step further: creating a meta backend which could defer each hook to a different backend as needed. I didn't write it as I suspect that would only cause more headaches for existing backend writers (who would get bits of their backend called, which could then lock down the changes they could make), as opposed to creating a backend toolkit which could be reused by projects.

@abravalheri
Copy link
Contributor

abravalheri commented Nov 21, 2021

It sounds a bit like a mad scientist, but why not? (Please call it frankenstein 🤣)

I did some very quick tests here and the backend wrapper approach seems to work fine.

@aragilar please let me know if you implement the wrapper in one of your projects. I would love to have a look in a real world use case.

@aragilar
Copy link
Author

aragilar commented Feb 3, 2022

Should this have been closed? I'm aware #2897 adds a possible workaround, but it might be nice to leave this open to track exposing the get_requires_for_build_wheel and get_requires_for_build_sdist in a more "official" way.

@webknjaz
Copy link
Member

webknjaz commented Dec 5, 2022

creating a meta backend which could defer each hook to a different backend as needed.

Interestingly, I've had a similar idea a few years ago. Except I was thinking about a backend that would allow stacking all the hooks from several backends in a user-specified order.

@webknjaz
Copy link
Member

webknjaz commented Dec 5, 2022

I did some very quick tests here and the backend wrapper approach seems to work fine.

Moreover, I've been using this approach successfully for quite a while. See : https://github.com/ansible/pylibssh/tree/8ac111d/bin/pep517_backend.

@webknjaz
Copy link
Member

webknjaz commented Dec 5, 2022

leave this open to track exposing the get_requires_for_build_wheel and get_requires_for_build_sdist in a more "official" way

These are PEP 517 hooks and this interface is already standardized. I don't think anything more than that is really needed. The proposed solution is universal and back-end agnostic, it's usually best not to have backend-specific hooks because that creates tight coupling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
5 participants