-
Notifications
You must be signed in to change notification settings - Fork 60
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 #1044
Merged
rapids-bot
merged 30 commits into
rapidsai:branch-0.39
from
jameslamb:rapids-build-backend
Jun 12, 2024
Merged
use rapids-build-backend #1044
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
d31ad60
use rapids-build-backend
jameslamb bfdc823
fix dependencies
jameslamb 745b40f
version tests
jameslamb 6cf9e4e
changes
jameslamb 74dbf00
fix
jameslamb 663320d
just run wheel builds
jameslamb 2f06f89
try moving libucx dependency
jameslamb 369d57e
put libucx back, it is needed in setup.py
jameslamb 0ed0e1d
Merge branch 'branch-0.39' of github.com:rapidsai/ucx-py into rapids-…
jameslamb 5fd0b04
test with https://github.com/rapidsai/rapids-build-backend/pull/41
jameslamb 893ff43
move other dependencies down to rapids-build-backend's table
jameslamb 2e2889f
wheel, not install
jameslamb 6c9fb3f
fix pip wheel invocation
jameslamb 15fde53
get better logs
jameslamb 7fbb1c1
fix pip wheel invocation
jameslamb 3e1e04f
re-enable wheel tests
jameslamb 473f306
move cudf dependency
jameslamb 0dddea7
empty commit to re-trigger CI
jameslamb 772ac92
use rapids-build-backend packages
jameslamb b964fee
re-enable conda builds
jameslamb d6241e5
fix data-gathering in conda recipe
jameslamb 39960dd
fix variable name
jameslamb 5cd1908
Merge branch 'branch-0.39' into rapids-build-backend
jameslamb 8a443be
Apply suggestions from code review
jameslamb 3238af2
try library install in post_create_environment
jameslamb 6d3fde7
Merge branch 'rapids-build-backend' of github.com:jameslamb/ucx-py in…
jameslamb dbd5722
fix keys
jameslamb 51c0153
consider rapids nightly index
jameslamb ec63083
fix formatting
jameslamb 5de66ec
use same __version__ test as other libraries, fix rtd config
jameslamb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ files: | |
- cuda | ||
- cuda_version | ||
- py_version | ||
- rapids_build_setuptools | ||
- run | ||
- test_python | ||
test_python: | ||
|
@@ -25,6 +26,14 @@ files: | |
pyproject_dir: . | ||
extras: | ||
table: build-system | ||
includes: | ||
- rapids_build_setuptools | ||
py_rapids_build: | ||
output: pyproject | ||
pyproject_dir: . | ||
extras: | ||
table: tool.rapids-build-backend | ||
key: requires | ||
includes: | ||
- build_python | ||
- depends_on_ucx_build | ||
|
@@ -109,8 +118,13 @@ dependencies: | |
common: | ||
- output_types: [conda, requirements, pyproject] | ||
packages: | ||
- setuptools>=64.0.0 | ||
- cython>=3.0.0 | ||
rapids_build_setuptools: | ||
common: | ||
- output_types: [conda, requirements, pyproject] | ||
packages: | ||
- rapids-build-backend>=0.3.1,<0.4.0dev0 | ||
- setuptools>=64.0.0 | ||
run: | ||
common: | ||
- output_types: [conda, requirements, pyproject] | ||
|
@@ -136,8 +150,6 @@ dependencies: | |
- matrix: {cuda: "11.*"} | ||
packages: | ||
- libucx-cu11==1.15.0 | ||
# NOTE: this fallback needs to be a real, suffixed version | ||
# so 'pip install .' (e.g. as used in docs builds) will work | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I accidentally left behind this comment in #1041 ... it's not accurate on |
||
- matrix: null | ||
packages: | ||
- libucx==1.15.0 | ||
|
@@ -160,8 +172,6 @@ dependencies: | |
- matrix: {cuda: "11.*"} | ||
packages: | ||
- libucx-cu11>=1.15.0,<1.16 | ||
# NOTE: this fallback needs to be a real, suffixed version | ||
# so "pip install ." (e.g. as used in docs builds) will work | ||
- matrix: null | ||
packages: | ||
- libucx>=1.15.0,<1.16 | ||
|
@@ -170,7 +180,6 @@ dependencies: | |
- output_types: [conda, requirements, pyproject] | ||
packages: | ||
- cloudpickle | ||
- cudf==24.8.* | ||
- dask | ||
- distributed | ||
- numba>=0.57 | ||
|
@@ -179,7 +188,20 @@ dependencies: | |
- pytest-rerunfailures | ||
- output_types: [conda] | ||
packages: | ||
- &cudf_conda cudf==24.8.*,>=0.0.0a0 | ||
- cupy>=12.0.0 | ||
specific: | ||
- output_types: [requirements, pyproject] | ||
packages: | ||
- cupy-cuda11x>=12.0.0 | ||
matrices: | ||
- matrix: {cuda: "12.*"} | ||
packages: | ||
- cudf-cu12==24.8.*,>=0.0.0a0 | ||
- cupy-cuda12x>=12.0.0 | ||
- matrix: {cuda: "11.*"} | ||
packages: | ||
- cudf-cu11==24.8.*,>=0.0.0a0 | ||
- &cupy_cu11 cupy-cuda11x>=12.0.0 | ||
- matrix: | ||
packages: | ||
- *cudf_conda | ||
- *cupy_cu11 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 thought the
.get(...)
was required because conda-build doesn't actually inject the Jinja variables on every pass and that led to problems in metadata. I'm not sure. We use the.get(...)
logic in dask-cuda, too. https://github.com/rapidsai/dask-cuda/blob/branch-24.08/conda/recipes/dask-cuda/meta.yamlThere 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.
It looks to me like this is working as expected.
For example, I see the right
license:
string there.And looking in one of the packages, all this metadata looks correct too:
Unless you can remember the specific reason to prefer
.get()
, I think we should move away from it in all conda recipes, to turn "silently ignored missing metadata" cases into "got a loud error at build time" cases. I actually just wrote that up in rapidsai/build-planning#72.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.
Ok cool! Go for it.
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.
@jakirkham may remember the specific reason why we used
.get(...)
in these recipes, I recall discussing it with him at some point.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 found a related thread: rapidsai/pynvjitlink#33 (comment)
I don't think it changes the fact that this is working and should be used for the specific places I've made this change in this PR, and doesn't need to block this PR. But might mean "never use
.get()
" is too strong.I'll post over in the
build-planning
issue.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.
Maybe
conda-build
has changed. 🤷♂️ In any case, I am happy to drop.get()
if this is working.