-
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
Use rapids-build-backend. #1502
Use rapids-build-backend. #1502
Conversation
4eca92d
to
8c9c4df
Compare
Yep that worked (build link), thanks @vyasr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of small things, but approving since I don't need to look at this again unless something comes up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed this in the raft PR and it reminded me: do we also need to add a disable-cuda in the build.sh invocations?
Yes, but it needs to be |
Alright I've updated this with the following changes:
That's enough changes that I think this should get another review if you have time @vyasr . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple more small notes, but approving assuming that they're addressed.
SKBUILD_CMAKE_ARGS="${EXTRA_CMAKE_ARGS}" python -m pip install \ | ||
--no-build-isolation \ | ||
--no-deps \ | ||
--config-settings rapidsai.disable-cuda=true \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just pushed one more change... remembered that pip
accepts --config-settings
(with an s
), not --config-setting
.
https://pip.pypa.io/en/stable/cli/pip_install/#cmdoption-C
Having --config-setting
(no s
) didn't break anything in CI or even display any warnings, so maybe newer versions of pip
have code to capture both. But either way, I think --config-settings
(with the s
) is the right thing for pip install
, given that's what is in that tool's docs.
I'll apply that change to the cudf
PR. cc @KyleFromNVIDIA that should be applied to the other rapids-build-backend
PRs as well.
I just pushed one more tiny change (#1502 (comment)). I think all the review comments have been addressed, so if CI passes, I'll merge this. |
/merge |
Contributes to rapidsai/build-planning#31 Follow-up to #1502 * ensures that `update-version.sh` does not remove alpha specs like `,>=0.0.0a0` in `pyproject.toml` and conda environment files * consolidates `rapids-build-backend` versions in `dependencies.yaml` - *since I was pushing a new commit here anyway, figured I'd take the opportunity to include that simplification recommended in rapidsai/cudf#15245 (comment) Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) - Bradley Dice (https://github.com/bdice) URL: #1572
…build-backend configuration (#1582) Contributes to rapidsai/build-planning#31 Contributes to rapidsai/dependency-file-generator#89 #1502 was one of the first `rapids-build-backend` PRs merged across RAPIDS. Since it was merged, we've made some small adjustments to the approach for `rapids-build-backend`. This catches `rmm` up with those changes: * consolidates version-handling in `ci/build_cpp.sh` * removes `commit-file` configuration in `pyproject.toml` - *as of rapidsai/rapids-build-backend#30, this is no longer necessary if the project's top-level directory is `{project_name}.replace("-", "_")* - *and anyway, it was changed from `commit-file` to `commit-files` in that PR, so `commit-file` was being silently ignored here* * uses `--file-key` instead of `--file_key` in `rapids-dependency-file-generator` calls Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #1582
…build-backend configuration (#1582) Contributes to rapidsai/build-planning#31 Contributes to rapidsai/dependency-file-generator#89 #1502 was one of the first `rapids-build-backend` PRs merged across RAPIDS. Since it was merged, we've made some small adjustments to the approach for `rapids-build-backend`. This catches `rmm` up with those changes: * consolidates version-handling in `ci/build_cpp.sh` * removes `commit-file` configuration in `pyproject.toml` - *as of rapidsai/rapids-build-backend#30, this is no longer necessary if the project's top-level directory is `{project_name}.replace("-", "_")* - *and anyway, it was changed from `commit-file` to `commit-files` in that PR, so `commit-file` was being silently ignored here* * uses `--file-key` instead of `--file_key` in `rapids-dependency-file-generator` calls Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #1582
I think the README file needs to be updated. |
Could you please raise this in an issue or PR? |
@jakirkham Opened an issue here: #1609 . |
Fixes #1609 Follow-up to #1502 Adds `rapids-build-backend` to the documentation on how to build `rmm` from source. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) - Jiaming Yuan (https://github.com/trivialfis) URL: #1614
Description
This PR uses
rapids-build-backend
to simplify wheel builds and reduce the complexity of various CI/build scripts.See also:
Checklist