move shared code into scripts, re-arrange layers, other small maintenance#356
move shared code into scripts, re-arrange layers, other small maintenance#356rapids-bot[bot] merged 16 commits intomainfrom
Conversation
| ```text | ||
| rapidsai/ci-conda:25.10-cuda13.0.2-ubuntu24.04-py3.13 | ||
| rapidsai/ci-conda:cuda13.0.2-ubuntu24.04-py3.13 | ||
| rapidsai/ci-conda:25.10-cuda13.1.0-ubuntu24.04-py3.13 |
There was a problem hiding this comment.
|
|
||
| if [[ "${#PACKAGES_TO_INSTALL[@]}" != "0" ]]; then | ||
| echo "could not find some necessary packages, temporarily installing them" | ||
| if type -f apt-get >/dev/null 2>&1; then |
There was a problem hiding this comment.
Using this form instead of LINUX_VER here because all ci-conda images, regardless of LINUX_VER, start from condaforge/miniforge3. That happens to be Ubuntu 24.04 right now.
Checking for apt-get is more future-proof than parsing out /etc/lsb-release or something, I think.
gforsyth
left a comment
There was a problem hiding this comment.
Looks great, James! I'm a big fan of consolidating all of the configuration and install paths into shared files.
I flagged a few small things -- nothing that will have any impact on execution, so feel free to fix or ignore as you see fit.
| @@ -1,4 +1,4 @@ | |||
| # SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | |||
There was a problem hiding this comment.
If pre-commit touches its own config, does it need to bump the copyright date?
I feel like we're on the edge of a horrible paradox.
There was a problem hiding this comment.
This diff looks weird now.
#58 had the same change (updating a hook version) that I had here. And that passed pre-commit.ci without the copyright bump. I am confused 🤔
There was a problem hiding this comment.
I don't understand how this passed without the copyright updated: https://results.pre-commit.ci/run/github/504062804/1769455570.c0AHX-kCQ5CvUHagdoIBvg
There was a problem hiding this comment.
But whatever either way, I think we want this. #58 changed this in 2026, the copyright date should expand to include 2026 🤷🏻
Co-authored-by: Gil Forsyth <gforsyth@users.noreply.github.com>
|
Thanks Gil! |
|
/merge |
Similar to rapidsai/docker#840, proposes moving more shared code into scripts, to reduce duplication.
Notes for reviewers
Moves shared code into bind-mounted scripts
Using scripts reduces duplication and therefore the risk of unintended inconsistencies.
Using bind mounts instead of
ADD/COPYkeeps the docker context small and ensures those files only needed at build time don't end up in the built images.Combines
RUNsteps into fewer layersThis can result in smaller images via better compression and avoids some per-layer overhead in building, pushing, and pulling.
I tried to do these only in cases where it was very unlikely to change the caching behavior (like combining 2 steps that create static files / directories) or where I felt it notably improved code clarity.
This images are pulled many many many more times than they're built and they're almost all built uncached on CI runnes, so I did not worry too much about the impact on build caching.
Updates examples in docs to CUDA 13.1.0
ref: rapidsai/build-planning#236