-
Notifications
You must be signed in to change notification settings - Fork 197
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
Remove duplicate librmm
runtime dependency
#1264
Conversation
`libraft-headers` has `librmm` listed as a `host` and `run` dependency in its recipe. Since `librmm` has a `run_exports` value, this causes there to be two `librmm` runtime dependency entries in the resulting `libraft-headers` conda package, each with slightly version specifiers. To prevent this from occurring, I've added `librmm` to the `ignore_run_exports_from` key in the `libraft-headers` recipe. Note that another way to solve this would be to remove `librmm` from the `run` dependencies, but this is inconsistent with the rest of the RAPIDS recipes.
This picture of the |
Similarly to rapidsai/raft#1264, this PR removes a duplicate `run` dependency on `librmm`. The current packages on Anaconda.org have a dependency on the `librmm` packages with the old date string format which have been removed from the `main` label. Additionally it increments the build number so that new packages are published as a result of this change.
Isn't this approach the better one, though? It lets librmm be the single source of truth dictating that linking to it also requires its presence as a runtime dependency. Maybe we merge this PR as is for consistency as you say, then go back through and refactor? |
This part is complicated by the fact that we use conda recipes with multiple outputs. If you take the Because The individual As a result of this arrangement, we manually add |
/merge |
Contributes to rapidsai/build-planning#54. The `libraft-headers` and `libraft-headers-only` conda packages are bundling `rmm`'s headers. I believe that's because the `librmm` conda package isn't available in the `libraft*` conda build environment, and as a result it's getting `rmm` via CPM (thanks to `rapids-cmake`). As a result, this project and any that depend on it are seeing warnings like the following in conda builds where `conda`'s `path_conflict` setting is set to `warn` or `prevent` (like #2245): ```text This transaction has incompatible packages due to a shared path. packages: rapidsai-nightly/linux-64::librmm-24.04.00a38-cuda11_240326_ga98931b9_38, rapidsai-nightly/linux-64::libraft-headers-only-24.04.00a93-cuda11_240326_g9637b3c2_93 path: 'include/rmm/mr/device/arena_memory_resource.hpp' ``` To fix that, this proposes the following changes: * make `librmm` a `host` dependency of the following conda packages: `libraft-headers-only`, `libraft-headers` ### Benefits of this change * slightly smaller `libraft-headers` and `libraft-headers-only` conda packages * reduced risk of runtime and installation issues caused by file clobbering ## Notes for reviewers ### History of changes to the `librmm` dependency for `libraft-headers`: * both `run` and `host`: #508 * both `run` and `host`, but ignoring its `run_exports`: #1264 * just `run`, but ignoring its `run_exports`: #2102 In particular, #2102 referred to the `host` dependency on `librmm` as "extraneous" but from a packaging perspective, I don't think it is. `librmm` being in `host` means it'll be present in the build environment, which means its headers will be *found* instead of *downloaded*, and therefore not packaging into the `libraft*` conda packages. ### How I tested this Built all the `raft` conda packages locally from `branch-24.06` and confirmed that they contain `rmm` headers. Then again from this branch and confirmed they were gone. ```shell docker run \ --rm \ --env-file "${PWD}/aws-creds.env" \ -v $(pwd):/opt/work \ -w /opt/work \ -it rapidsai/ci-conda:cuda12.2.2-ubuntu22.04-py3.10-amd64 \ bash CI="true" \ ci/build_cpp.sh # On 'branch-24.06', this showed the rmm headers being packaged. # On this branch, they're omitted. tar --bzip2 -tf \ /tmp/conda-bld-output/linux-64/libraft-headers-only-24.06.00a50-cuda12_240430_g1e0e2283_50.tar.bz2 \ | grep 'include/rmm' \ | wc -l ``` Also checked the CI logs from `conda-cpp-build` jobs here. On other recent PRs, I see CPM downloading `rmm` ... ```text -- CPM: Adding package [email protected] (branch-24.06) ``` ... and all the `rmm` headers getting installed as part of the `libraft-headers` package ```text -- Installing: /opt/conda/conda-bld/_h_env_placehold_placehold_..._placeho/include/rmm/cuda_stream.hpp ``` ([build link](https://github.com/rapidsai/raft/actions/runs/8904352932)) Here, I see `librmm` coming through via the conda package requirements ... ```text The following NEW packages will be INSTALLED: ... librmm: 24.06.00a17-cuda12_240430_g26fa9ecb_17 rapidsai-nightly ``` ... and being used instead of downloads via CPM ... ```text -- CPM: Using local package [email protected] ``` ... and no installation of the `rmm` headers as part of building any `libraft` packages. ([build link](https://github.com/rapidsai/raft/actions/runs/8910675575/job/24470450187?pr=2284)) Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) - Ray Douglass (https://github.com/raydouglass) URL: #2284
libraft-headers
haslibrmm
listed as ahost
andrun
dependency in its recipe.Since
librmm
has arun_exports
value, this causes there to be twolibrmm
runtime dependency entries in the resultinglibraft-headers
conda package, each with slightly version specifiers.To prevent this from occurring, I've added
librmm
to theignore_run_exports_from
key in thelibraft-headers
recipe.Note that another way to solve this would be to remove
librmm
from therun
dependencies, but this is inconsistent with the rest of the RAPIDS recipes.