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

Port to rattler-build #1796

Open
wants to merge 59 commits into
base: branch-25.04
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
59 commits
Select commit Hold shift + click to select a range
8ba7466
WIP: convert to rattler-build
gforsyth Jan 24, 2025
a11cbe2
wip: broken, but it at least tries to build
gforsyth Jan 27, 2025
93633b4
restore meta.yaml with changes required for `conda-recipe-manager`
gforsyth Jan 27, 2025
8fd3ef3
Update conda/recipes/librmm/meta.yaml
gforsyth Jan 27, 2025
b501ca1
Update conda/recipes/librmm/meta.yaml
gforsyth Jan 28, 2025
17f9f13
builds, but compiles for each output which is bad
gforsyth Jan 28, 2025
38c6feb
build once and cache, then define separate outputs
gforsyth Jan 28, 2025
9b82fea
port rmm conda recipe to rattler build
gforsyth Jan 28, 2025
be5dbef
add comment about testing environmetn
gforsyth Jan 28, 2025
4994e5c
set GIT_* from env, fix missing torch file
gforsyth Jan 28, 2025
dff85ca
Fix path to `_torch_allocator.cpp` in `.gitignore`
gforsyth Jan 29, 2025
2d251cf
Disable pip check in import test
gforsyth Jan 29, 2025
be8c03f
Swap in `rattler-build` for `mambabuild`
gforsyth Jan 29, 2025
9303d66
install rattler-build
gforsyth Jan 29, 2025
84b9c14
explicitly set CMAKE_MAKE_PROGRAM
gforsyth Jan 29, 2025
c9e4fca
Revert "explicitly set CMAKE_MAKE_PROGRAM"
gforsyth Jan 29, 2025
5ae6a16
add `make` to build
gforsyth Jan 29, 2025
00230cc
add cmake to individual outputs
gforsyth Jan 29, 2025
1fc7c42
explicitly set output directory
gforsyth Jan 29, 2025
e554922
pass `-GNinja` to `build.sh`
gforsyth Jan 29, 2025
5df221c
remove output/ from gitignore
gforsyth Jan 29, 2025
272c466
remove old meta.yaml
gforsyth Jan 29, 2025
5799ee2
use env-var for rattler output dir
gforsyth Jan 29, 2025
1dd5a9a
update copyright
gforsyth Jan 29, 2025
2b769b9
Switch to v1 selectors
gforsyth Jan 30, 2025
9d053ae
Merge branch 'branch-25.04' into rattler-test
gforsyth Jan 30, 2025
dd98e59
Fix indentation
gforsyth Jan 30, 2025
33e7351
explicit channel specification in python build
gforsyth Jan 30, 2025
a9067b2
move env vars to `env` and set defaults
gforsyth Jan 31, 2025
c70f762
see if ninja gets used automatically without flag
gforsyth Jan 31, 2025
f1bf18f
Merge remote-tracking branch 'upstream/branch-25.04' into rattler-test
gforsyth Jan 31, 2025
fe8c6f4
set `SCCACHE_S3_KEY_PREFIX` from env
gforsyth Jan 31, 2025
2b9755b
overwrite unused envvar
gforsyth Feb 3, 2025
cdb334d
Remove manual env-var setting and rattler install
gforsyth Feb 5, 2025
d44eade
Merge branch 'branch-25.04' into rattler-test
gforsyth Feb 5, 2025
6f3b103
Update conda/recipes/librmm/recipe.yaml
gforsyth Feb 5, 2025
7ee96f4
Add missing right-brace
gforsyth Feb 5, 2025
de85796
remove unnecessary branching for cuda compiler
gforsyth Feb 6, 2025
8b2e801
Add note re; build ids and remove build-cache before upload
gforsyth Feb 7, 2025
a3bdd12
Remove `GIT_DESCRIBE_*` envvars from build and pipeline
gforsyth Feb 10, 2025
5489596
Explicitly list channels for build
gforsyth Feb 10, 2025
39856c1
Use double-quotes everywhere
gforsyth Feb 10, 2025
f0b65fe
Use `load_from_file` for `about` metadata, standardize ordering
gforsyth Feb 10, 2025
a05975f
Set missing `py_version`
gforsyth Feb 10, 2025
d51e58a
Remove `python` bounds
gforsyth Feb 10, 2025
de87ce8
Remove extraneous `else` from `conda-recipe-manager`
gforsyth Feb 10, 2025
f5e4669
Use `git` plugin to grab short sha
gforsyth Feb 10, 2025
7d17397
Restore python upper-bound
gforsyth Feb 10, 2025
9c4bca8
Remove `fmt` and `spdlog` from `host`
gforsyth Feb 10, 2025
5d0a244
Add lsp schema for rattler yaml
gforsyth Feb 10, 2025
13420c3
simplify sccache s3 key prefix
gforsyth Feb 10, 2025
384d2d4
remove upper bound on python and remove numba
gforsyth Feb 10, 2025
ea232d5
Merge branch 'branch-25.04' into rattler-test
gforsyth Feb 10, 2025
47c2e40
swap in `rapids-logger`
gforsyth Feb 10, 2025
0a236ed
formatting nits
gforsyth Feb 11, 2025
769c0a5
Select `rapidsai` channel based on build-type
gforsyth Feb 11, 2025
2aa166e
Merge branch 'branch-25.04' into rattler-test
gforsyth Feb 11, 2025
c5e8ce4
Remove all env-var defaults
gforsyth Feb 11, 2025
584afaa
Change license to SPDX-allowed format, remove `replace` in recipe
gforsyth Feb 11, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ rmm.egg-info/
python/build
python/*/build
python/rmm/docs/_build
python/rmm/**/librmm/**/*.cpp
!python/rmm/librmm/_torch_allocator.cpp
python/rmm/rmm/librmm/*.cpp
!python/rmm/rmm/librmm/_torch_allocator.cpp
gforsyth marked this conversation as resolved.
Show resolved Hide resolved
python/rmm/**/librmm/**/*.h
python/rmm/**/librmm/.nfs*
python/rmm/**/pylibrmm/**/*.cpp
Expand Down
25 changes: 23 additions & 2 deletions ci/build_cpp.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,30 @@ rapids-logger "Begin cpp build"

sccache --zero-stats

# This calls mambabuild when boa is installed (as is the case in the CI images)
RAPIDS_PACKAGE_VERSION=$(rapids-generate-version) rapids-conda-retry mambabuild conda/recipes/librmm
RAPIDS_PACKAGE_VERSION=$(rapids-generate-version)
export RAPIDS_PACKAGE_VERSION

if rapids-is-release-build; then
RAPIDS_CHANNEL=rapidsai
else
RAPIDS_CHANNEL=rapidsai-nightly
fi
Comment on lines +22 to +26
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor nit: I foresee this propagating everywhere. Should we turn this into a GHA tool? We could even make it return an array of usable channels so that we also have a centralized place for removing the nvidia channel once we no longer need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should. I'll get that set up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


# --no-build-id allows for caching with `sccache`
# more info is available at
# https://rattler.build/latest/tips_and_tricks/#using-sccache-or-ccache-with-rattler-build
rattler-build build --recipe conda/recipes/librmm \
--experimental \
gforsyth marked this conversation as resolved.
Show resolved Hide resolved
--no-build-id \
gforsyth marked this conversation as resolved.
Show resolved Hide resolved
--channel-priority disabled \
--output-dir "$RAPIDS_CONDA_BLD_OUTPUT_DIR" \
-c ${RAPIDS_CHANNEL} \
-c conda-forge \
-c nvidia

sccache --show-adv-stats

# remove build_cache directory
rm -rf "$RAPIDS_CONDA_BLD_OUTPUT_DIR"/build_cache

Comment on lines +42 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rapids-upload-conda-to-s3 cpp
25 changes: 23 additions & 2 deletions ci/build_python.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,30 @@ CPP_CHANNEL=$(rapids-download-conda-from-s3 cpp)

sccache --zero-stats

# This calls mambabuild when boa is installed (as is the case in the CI images)
RAPIDS_PACKAGE_VERSION=$(head -1 ./VERSION) rapids-conda-retry mambabuild -c "${CPP_CHANNEL}" conda/recipes/rmm
RAPIDS_PACKAGE_VERSION=$(head -1 ./VERSION)
export RAPIDS_PACKAGE_VERSION

if rapids-is-release-build; then
RAPIDS_CHANNEL=rapidsai
else
RAPIDS_CHANNEL=rapidsai-nightly
fi

# --no-build-id allows for caching with `sccache`
# more info is available at
# https://rattler.build/latest/tips_and_tricks/#using-sccache-or-ccache-with-rattler-build
rattler-build build --recipe conda/recipes/rmm \
--experimental \
--no-build-id \
--channel-priority disabled \
--output-dir "$RAPIDS_CONDA_BLD_OUTPUT_DIR" \
-c "${CPP_CHANNEL}" \
-c ${RAPIDS_CHANNEL} \
-c conda-forge \
-c nvidia

sccache --show-adv-stats

rm -rf "$RAPIDS_CONDA_BLD_OUTPUT_DIR"/build_cache

rapids-upload-conda-to-s3 python
110 changes: 0 additions & 110 deletions conda/recipes/librmm/meta.yaml

This file was deleted.

105 changes: 105 additions & 0 deletions conda/recipes/librmm/recipe.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# yaml-language-server: $schema=https://raw.githubusercontent.com/prefix-dev/recipe-format/main/schema.json
# Copyright (c) 2018-2025, NVIDIA CORPORATION.
schema_version: 1

context:
version: ${{ env.get("RAPIDS_PACKAGE_VERSION") }}
bdice marked this conversation as resolved.
Show resolved Hide resolved
cuda_version: ${{ (env.get("RAPIDS_CUDA_VERSION") | split("."))[:2] | join(".") }}
cuda_major: ${{ (env.get("RAPIDS_CUDA_VERSION") | split("."))[0] }}
date_string: ${{ env.get("RAPIDS_DATE_STRING") }}
head_rev: ${{ git.head_rev(".")[:8] }}

recipe:
name: librmm-split

cache:
source:
path: ../../..

requirements:
build:
- cmake ${{ cmake_version }}
- ninja
- ${{ compiler("c") }}
- ${{ compiler("cxx") }}
- ${{ compiler("cuda") }}
- cuda-version =${{ cuda_version }}
- ${{ stdlib("c") }}

build:
script:
file: build.sh
gforsyth marked this conversation as resolved.
Show resolved Hide resolved
secrets:
- AWS_ACCESS_KEY_ID
- AWS_SECRET_ACCESS_KEY
- AWS_SESSION_TOKEN

env:
CMAKE_C_COMPILER_LAUNCHER: ${{ env.get("CMAKE_C_COMPILER_LAUNCHER") }}
CMAKE_CUDA_COMPILER_LAUNCHER: ${{ env.get("CMAKE_CUDA_COMPILER_LAUNCHER") }}
CMAKE_CXX_COMPILER_LAUNCHER: ${{ env.get("CMAKE_CXX_COMPILER_LAUNCHER") }}
CMAKE_GENERATOR: ${{ env.get("CMAKE_GENERATOR") }}
PARALLEL_LEVEL: ${{ env.get("PARALLEL_LEVEL") }}
SCCACHE_BUCKET: ${{ env.get("SCCACHE_BUCKET") }}
SCCACHE_IDLE_TIMEOUT: ${{ env.get("SCCACHE_IDLE_TIMEOUT") }}
SCCACHE_REGION: ${{ env.get("SCCACHE_REGION") }}
SCCACHE_S3_USE_SSL: ${{ env.get("SCCACHE_S3_USE_SSL") }}
SCCACHE_S3_NO_CREDENTIALS: ${{ env.get("SCCACHE_S3_NO_CREDENTIALS") }}
SCCACHE_S3_KEY_PREFIX: librmm-${{ env.get("RAPIDS_CONDA_ARCH") }}

outputs:
- package:
name: librmm
version: ${{ version }}
build:
string: cuda${{ cuda_major }}_${{ date_string }}_${{ head_rev }}
script: install_librmm.sh
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A final thought, should we just inline these scripts? They're pretty much all one-liners. I find that the current split makes most recipes harder to parse, not easier. Curious what other reviewers think, but I consider inlining <3 scripts preferable to having a separate file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's a good idea and would remove one more layer of indirection

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider inlining <3 scripts preferable

I ❤️ inline scripts too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be different for cudf or other repos — some of the scripts are multiple lines or have a TON of flags.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say we inline where it's a simple one-liner and if it's more complicated than that (or has a bunch of flags) we stick with the standalone install script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider inlining <3 scripts preferable

I ❤️ inline scripts too.

Whoops lol I hope at least one of you guessed that I meant "<3 line scripts" 😂

Yeah I'm fine doing this on a case-by-case basis based on the rough 3-line heuristic.

requirements:
host:
- cmake ${{ cmake_version }}
- cuda-version =${{ cuda_version }}
run:
- ${{ pin_compatible("cuda-version", upper_bound="x", lower_bound="x") }}
- if: cuda_major == "11"
then: cudatoolkit
- rapids-logger =0.1
run_exports:
- ${{ pin_subpackage("librmm", upper_bound="x.x") }}
ignore_run_exports:
from_package:
- ${{ compiler("cuda") }}
tests:
- script:
- "test -d \"${PREFIX}/include/rmm\""
about:
gforsyth marked this conversation as resolved.
Show resolved Hide resolved
homepage: ${{ load_from_file("python/librmm/pyproject.toml").project.urls.Homepage }}
license: ${{ load_from_file("python/librmm/pyproject.toml").project.license.text }}
summary: ${{ load_from_file("python/librmm/pyproject.toml").project.description }}
- package:
name: librmm-tests
version: ${{ version }}
build:
string: cuda${{ cuda_major }}_${{ date_string }}_${{ head_rev }}
script: install_librmm_tests.sh
requirements:
host:
- cmake ${{ cmake_version }}
- cuda-version =${{ cuda_version }}
- if: cuda_major == "11"
then: cudatoolkit
else: cuda-cudart-dev
run:
- if: cuda_major == "11"
then: cudatoolkit
else: cuda-cudart
- ${{ pin_compatible("cuda-version", upper_bound="x", lower_bound="x") }}
- ${{ pin_subpackage("librmm", exact=True) }}
ignore_run_exports:
from_package:
- ${{ compiler("cuda") }}
- if: cuda_major == "11"
then: cuda-cudart-dev
about:
homepage: ${{ load_from_file("python/librmm/pyproject.toml").project.urls.Homepage }}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we load_from_file once into something in the context and then access that data? I don't know if rattler is smart enough to cache the file on its own.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem to work. In all of their documentation examples they repeatedly call load_from_file on the same file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird. I opened prefix-dev/rattler-build#1423. This seems like a bug in the Jinja parsing.

license: ${{ load_from_file("python/librmm/pyproject.toml").project.license.text | replace(" ", "-") }}
summary: librmm test & benchmark executables
90 changes: 0 additions & 90 deletions conda/recipes/rmm/meta.yaml

This file was deleted.

Loading
Loading