Repository navigation
Forward-merge release/26.10 into main - #919
Open
rapids-bot[bot] wants to merge 2 commits into
Open
rapids-bot[bot] wants to merge 2 commits into
rapids-bot[bot] wants to merge 2 commits into
Conversation
Closes #907 ## What the script does `ci/release/update-version.sh` bumps the RAPIDS version across the repo when a release branch is cut. It is a flat list of `sed_runner` calls, one per file, each rewriting a version string in place. ## The problems Running it on `main` today produces two bad substitutions and one that has never matched anything. A fourth bug stops it running at all through the no-context invocation both of its documented interfaces allow. ### 1. `CONTRIBUTING.md` gains a stray digit This is the reported bug. The minor-version group was a single `[[:digit:]]` instead of `[[:digit:]]\+`, so bumping `26.06` to `26.08` rewrote only `RAPIDS_VER=26.0` and left the trailing `6` behind: ```diff -export RAPIDS_VER=26.06 +export RAPIDS_VER=26.086 ``` That value is live on `release/26.08`. Any minor with two or more digits breaks the same way: the pattern eats one digit of the old minor and leaves the rest appended, so the next bump from today's `main` would give `RAPIDS_VER=26.106`. The other version-matching `sed_runner` calls already use the one-or-more form, apart from the `SECURITY.md` line in (3), which has the identical bug. ### 2. Trailing comments get deleted from `shared-workflows` refs `s|@.*|@ref|` is greedy to end of line, so rewriting a ref also deletes whatever follows it. Cutting `release/26.06` in 935ed5a stripped `# zizmor: ignore[unpinned-uses]` from all three `shared-workflows` `uses:` lines and broke pre-commit, which is what #877 was cleaning up. #877 fixed the fallout by adding `.github/zizmor.yml` and left the sed alone, so cutting `release/26.08` in d711b42 stripped the two remaining comments again. Those two are redundant now that the zizmor config exempts `rapidsai/shared-workflows/*` from `unpinned-uses`, so this is no longer a CI break. But the sed will still drop any trailing comment on any line it rewrites. ### 3. The `SECURITY.md` call does nothing `SECURITY.md` has no version string in it, so `s|[[:digit:]]\+\.[[:digit:]]-cuda|...|` has matched nothing since #877 added it. It carries the same missing `\+` as (1), so it could not have matched `06-cuda` even if a version were there. ### 4. The no-context invocation aborts ```console $ bash ci/release/update-version.sh 26.10.00 ci/release/update-version.sh: line 45: RAPIDS_RUN_CONTEXT: unbound variable ``` The header documents a primary interface with an optional `--run-context` flag and a fallback interface with an optional `RAPIDS_RUN_CONTEXT` variable, and says it defaults to `main` when neither is given. That default path reads the variable before checking whether it exists, so `set -u` kills the script before any substitution runs. ## The fix Four one-line changes, one commit each: 1. Add the missing `\+` so the `CONTRIBUTING.md` pattern matches a full minor version. 2. Anchor the workflow ref match to the token with `@[^[:space:]]\+` and drop the `g` flag, since there is one ref per line. Refs with no trailing comment, such as the one in `trigger-breaking-change-alert.yaml`, are still rewritten. 3. Delete the `SECURITY.md` call. Happy to restore it as `[[:digit:]]\+\.[[:digit:]]\+-cuda` instead if you would rather keep it as future-proofing. 4. Use `${RAPIDS_RUN_CONTEXT:-}` so the documented default actually happens. This one is fixed first, because it is what makes the other three reproducible locally. ## Testing Same approach as #877: ```shell ci/release/update-version.sh --run-context=release '26.10.00' git diff git grep -nE '26\.0[2468]' -- . ':!ci/release' # nothing stale survives git checkout -- . ci/release/update-version.sh '26.10.00' # no flag, no env var ``` Both context values were exercised through both interfaces, plus the bare default. CLI precedence over the env var still holds, and invalid values exit 1 without touching the tree. Running twice gives the same diff as running once. A release run followed by a main run returns the three workflow refs and the `Dockerfile` `RAPIDS_BRANCH` to `@main`, comments intact; the doc bumps correctly persist. I also ran the two new patterns against fixtures outside the repo. For (1): `26.06`, `26.6`, `26.10`, `26.100`, `9.1`, `26.06.00`, and a bare `RAPIDS_VER=` with no digits. The new pattern is right on all seven; the old one mangles four of them (`26.06`, `26.10`, `26.100`, `26.06.00`) and no-ops identically on the rest. For (2): a ref with a trailing comment, one without, a comment containing its own `@` (left alone, which is why the `g` goes), a 40-character SHA pin (still rewritten), and a `shared-actions` line (untouched, since the address is `/shared-workflows/`). `pre-commit run --all-files` passes. With a `GH_TOKEN` in the environment, `zizmor` additionally resolves tags and reports 12 pre-existing medium `ref-version-mismatch` findings on `actions/*` hash pins. Those are all present on `main` and none are in a file this PR touches. Only GNU sed was exercised. `\+` is already the idiom throughout the file and the bracket expressions are POSIX, so `sed -i.bak` portability should be unchanged. ## One thing I noticed Unrelated to the fix: docs on `main` are still at `26.06` (`CONTRIBUTING.md`, `dockerhub-readme.md`, `cuvs-bench/README.md`, `tests/container-canary/README.md`) while the Dockerfiles are at `26.10`, so the 26.08 and 26.10 burndowns look like they skipped them. I kept this PR to the script. Happy to add the doc bump here or in a follow-up. --------- Co-authored-by: Mike Sarahan <msarahan@gmail.com> Co-authored-by: Michael Sarahan <msarahan@nvidia.com>
Contributor
Author
|
FAILURE - Unable to forward-merge due to an error, manual merge is necessary. Do not use the IMPORTANT: When merging this PR, do not use the auto-merger (i.e. the |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Forward-merge triggered by push to release/26.10 that creates a PR to keep main up-to-date. If this PR is unable to be immediately merged due to conflicts, it will remain open for the team to manually merge. See forward-merger docs for more info.