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

[RFC] handling CUDA-suffixed dependencies needed at setuptools.setup() time #39

Closed
jameslamb opened this issue Jun 5, 2024 · 3 comments · Fixed by #41
Closed

[RFC] handling CUDA-suffixed dependencies needed at setuptools.setup() time #39

jameslamb opened this issue Jun 5, 2024 · 3 comments · Fixed by #41
Assignees
Labels
question Further information is requested

Comments

@jameslamb
Copy link
Member

Description

Consider a project with the following characteristics:

  • uses a setup.py running setuptools.setup()
  • needs to import {library} in that setup.py, prior to setup() being called
  • {library} is only distributed via CUDA-suffixed wheels (e.g. {library}-cu11)

How should such cases be handled? Is it possible to make them work with rapids-build-backend?

Details

Consider the specific case of ucx-py.

Its setup.py includes stuff like this that runs before setuptools.setup() is executed:

def _find_libucx_libs_and_headers():
    import libucx
    module_dir = os.path.dirname(libucx.__file__)
    libs = glob.glob(f"{module_dir}/**/lib*.so*", recursive=True)
    ...
    return list(lib_dirs), list(header_dirs)

libucx_lib_dirs, libucx_header_dirs = _find_libucx_libs_and_headers()

ext_modules = [
    Extension(
        ...
        include_dirs=[libucx_header_dirs],
        library_dirs=[libucx_lib_dirs],
    ),
    ...
]

setup(
    ext_modules=ext_modules,
    cmdclass={"build_ext": new_build_ext},
    ...
)

(ucx-wheels/setup.py)

The key line is that import libucx.

By the time that line runs, libucx-cu11 or libucx-cu12 needs to have been installed. It that seems pip wheel ends up running setup.py before rapids-build-backend has had a chance to replace the contents of pyproject.toml.

With libucx==1.15.0 (unsuffixed) in [build-system], like this:

[build-system]
build-backend = "rapids_build_backend.build"
requires = [
    "cython>=3.0.0",
    "libucx==1.15.0",
    "rapids-build-backend>=0.3.0,<0.4.0dev0",
    "setuptools>=64.0.0",
]

Wheel-building fails like this:

ERROR: Could not find a version that satisfies the requirement libucx==1.15.0 (from versions: none)
ERROR: No matching distribution found for libucx==1.15.0

(example build link)

With it omitted from that table and moved down into [tool.rapids-build-backend], wheel building fails like this:

ModuleNotFoundError: No module named 'libucx'

(example build link)

Options I can think of

(in no particular order)

  1. still use rapids-build-backend, but preserve the sed-replacing of just libucx in wheel-building scripts
    • in other words... solve this case-by-case and not generally
  2. run rapids-dependency-file-generator --matrix "cuda=${RAPIDS_CUDA_VERSION%.*}" in build scripts prior to pip wheel
    to update pyproject.toml in place
  3. install libucx at build time by some other mechanism, omit it from [build-system] table, run pip wheel --no-build-isolation
  4. in setup.py, customize the command run by python setup.py build_ext
    • such that it does all the import libucx stuff when it's called to build extensions (instead of when setuptools.setup() is called)
    • for example, I think we could subclass Cython.build_ext and just override the build_extension() method (Cython/distutils/build_ext.py#L82)
    • more details in "Extending or Customizing Setuptools" (setuptools docs)

Request for comment

Other options?

Any of these that definitely won't work?

How should we proceed?

References

Created after starting rapidsai/ucx-py#1044.

@vyasr
Copy link
Contributor

vyasr commented Jun 5, 2024

Something seems off here. Let me try and reproduce locally. The sequence here seems to suggest that somehow it's going down a legacy code path.

@vyasr
Copy link
Contributor

vyasr commented Jun 5, 2024

I'll post my (edited) Slack comment here for the record.

What we have here is the original chicken and egg problem of setup_requires. The canonical way for projects to specify build requirements prior to PEP 518 was via a list setup_requires provided as an argument to (setuptools|distutils).setup. I've never investigated the mechanism by which setup_requires works, but the classic example of Cython builds demonstrate how setup_requires was never really sufficient because you could need requirements prior to the invocation of the setup function, which means that in general anything in setup_requires wouldn't actually be available early enough for use in cases like the ucx one above anyway.

That being said, it seems that the setuptools PEP 517 build backend (setuptools.build_meta) is achieving backwards compatibility with legacy projects by implementing its get_requires_for_build_wheel as an invocation of setup.py in which some internals are patched so that the setup_requires list is what ends up being returned. I didn't go too deep down that rabbit hole, but the upshot is that RBB's assumption that the underlying build backend's get_requires_for_build_wheel can be safely invoked without the requirements specified in RBB's own requirements list is incorrect for setuptools. If running setup.py is how you get the build backend dependencies, then RBB can't call setuptools.build_meta.get_requires_for_build_wheel without first installing the dependencies listed in RBB's own build requirements table as necessities for that.

The only really viable solution I see is to special-case RBB's get_requires_for_build_wheel implementation for setuptools. If a project is using setuptools as the underlying build backend, rapids_build_backend.get_requires_for_build_wheel should check for the presence of a setup.py file. If that file exists (it doesn't have to, modern uses of setuptools can be pyproject.toml or setup.cfg only), then we should grep the text for setup_requires. If that's found, RBB should error indicating that this isn't supported and all those dependencies should instead be moved to RBB's list. Then, rapids_build_backend.get_requires_for_build_wheel can simply skip calling the setuptools implementation.

@jameslamb jameslamb self-assigned this Jun 5, 2024
@jameslamb
Copy link
Member Author

Adding some links for my own reference.

Here's where setuptools.build_meta.get_requires_for_build_wheel() is defined:

https://github.com/pypa/setuptools/blob/f91fa3d9fc7262e0467e4b2f84fe463f8f8d23cf/setuptools/build_meta.py#L324-L325

That calls setuptools.build_meta._get_build_requires(), which calls setuptools.build_meta._BuildMetaBackend._get_build_requires()

https://github.com/pypa/setuptools/blob/f91fa3d9fc7262e0467e4b2f84fe463f8f8d23cf/setuptools/build_meta.py#L295

which ends up reading in the setup.py file and exec-ing it

https://github.com/pypa/setuptools/blob/f91fa3d9fc7262e0467e4b2f84fe463f8f8d23cf/setuptools/build_meta.py#L307-L311

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

Successfully merging a pull request may close this issue.

2 participants