-
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 extraneous host pinnings from libraft-headers-only. #2102
Remove extraneous host pinnings from libraft-headers-only. #2102
Conversation
I confirmed that the host environment no longer includes I'm still waiting for CI to complete to know about the run-exports being fixed for sure, but this should be a sufficient fix from what I can observe so far. |
/merge |
Double-confirmation: I downloaded the artifacts and I see what I expected: |
Thanks all! 🙏 |
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
We have
run
pinnings for librmm, spdlog, and fmt, so we don't need to list them inhost
as well. Currently, we are allowingfmt>=10.1.1,<11
inhost
, which is pulling a newer version 10.2.1 and adding a run-export that isfmt >=10.2.1,<11.0a0
, which disallowsfmt 10.1.1
contrary to our intentions.