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

Standardize run exports (and make analogous choices for pip packaging) for header-only libraries #92

Open
vyasr opened this issue Aug 12, 2024 · 1 comment

Comments

@vyasr
Copy link
Contributor

vyasr commented Aug 12, 2024

We are currently inconsistent with how header-only packages handle exports in conda. Consider:

With the ongoing work to add C++ RAPIDS wheels, we're going to see even more related issues like this because wheels have no concept like run exports, so if a header-only C++ package is a dependency of another one, then any consumer aiming to build against the second package needs the first available even though it is not actually a runtime requirement (concretely: a libcuspatial wheel would require a librmm wheel via libcudf to compile, but at runtime libcuspatial would only require libcudf and not librmm).

As discussed offline, we should remove the run exports in conda and be consistent in both pip and conda packaging by saying that consumers building against a package are responsible for bringing in transitive build-time dependencies that are not runtime dependencies. In particular, this means that transitive header-only dependencies must be explicitly specified by consumers at build time. This approach keeps runtime environments as slim as possible at the expensive of making package specs more verbose.

rapids-bot bot pushed a commit to rapidsai/cuspatial that referenced this issue Aug 29, 2024
Contributes to rapidsai/build-planning#33

`cuproj` does not need the `rmm` Python package... it only needs the RMM headers at build time. This proposes the following changes for `cuproj`:

* dropping the runtime requirement on `rmm` in wheels and conda packages
* switching the build requirement from `rmm` to `librmm` for wheels and conda packages
* removing unnecessary imports in the `test:` environment for conda packages

For more context on these changes, see rapidsai/build-planning#92.

## Notes for Reviewers

### Benefits of these changes

Faster conda builds (via dropping unnecessary dependencies).

Cheaper (in terms of bandwidth and disk space) installation of wheels and conda packages (via removing an unnecessary runtime dependency).

Reduces a source of network calls (and therefore CI instability) by removing some CPM downloads of RMM.

Before:

```text
-- CPM: Adding package rmm@24.10 (branch-24.10)
```

([build link](https://github.com/rapidsai/cuspatial/actions/runs/10618529204/job/29434041322#step:9:16754))

After (this PR):

```text
  -- CPM: Using local package rmm@24.10.0
```

([build link](https://github.com/rapidsai/cuspatial/actions/runs/10619138604/job/29436119470?pr=1448#step:9:11256))

### Is this required for `libcuspatial` wheel packaging?

No, it's just a side thing I noticed while working on that. The two are totally independent.

#

Authors:
  - James Lamb (https://github.com/jameslamb)

Approvers:
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)

URL: #1448
@vyasr
Copy link
Contributor Author

vyasr commented Sep 20, 2024

Some concerns raised by @wence- in rapidsai/rmm#1673 (comment) boil down to the fact that while header-only libraries are not required at runtime, there does need to be some way of indicating an ABI constraint such that all libraries in an environment that were built against some header-only library used ABI-compatible versions of that library. Concretely, without this you could have the user pull a librmm device_buffer out of a libcudf column and pass it to libcuml only to discover that under the hood libcudf and libcuml were compiled with incompatible versions of librmm (notice that I'm using the C++ libraries because in the Python case there is an implicit constraint by virtue of all the Python packages depending on rmm at runtime).

The ideal solution to fix this problem would be to define an ABI version metapackage for every header-only package so that dependents could use the metapackage as a runtime dependency to mutually constrain the ABI without needing to pull the headers into the runtime environment. However, that solution isn't ideal for us right now for a few reasons:

  1. RAPIDS libraries don't actually promise any ABI stability, so the metapackage's existence would be somewhat misleading.
  2. If RAPIDS libraries ever did want to start promising ABI stability, using a CalVer-based metapackage now would force starting future ABI versioning from some confusingly high number.
  3. The metapackage would add extra packaging overhead with minimal benefits now since our headers are quite small.

The upshot of this is that we may want to keep the run exports for now as an implicit ABI constraint. There is no equivalent solution for pip, but that should be less problematic since pip installation is really on relevant for the Python packages and those implicitly inherit the same constraint from the rmm Python package as mentioned above.

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

No branches or pull requests

1 participant