auto bump with new build system#1366
auto bump with new build system#1366daniel-noland wants to merge 3 commits intopr/mvachhar/new-build-systemfrom
Conversation
Rewrites scripts/bump.sh to use npins for all pin management (previously it only handled dpdk-sys). Adds a new `bump-pins` just recipe that invokes the rewritten script. Removes the now-obsolete gen-pins.sh and dpdk-sys.env which are superseded by the new approach. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Updates rust-overlay pin to latest. This is the result of running the newly added bump-pins recipe. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Adds a 'bump nix pins' step that runs `just bump-pins` before the cargo upgrade logic, creating a distinct commit for nix pin updates. Keeping nix and cargo bumps in separate commits makes git bisect easier. Also updates the PR branch and title to reflect that the workflow now covers both nix and cargo dependency bumps, and includes nix pin update status in the PR body. Signed-off-by: Daniel Noland <daniel@githedgehog.com>
daniel-noland
changed the base branch from
main
to
pr/mvachhar/new-build-system
There was a problem hiding this comment.
Pull request overview
This PR updates the automated dependency bump workflow to also refresh Nix/npins pins as part of scheduled “bump” runs, aligning Nix pin regeneration with the new build system flow.
Changes:
- Removed the standalone pin-generation script and folded pin (re)initialization into
scripts/bump.sh. - Added a
just bump-pinsrecipe and integrated it into.github/workflows/bump.ymlbefore running Cargo upgrades. - Updated
npins/sources.json(example: rust-overlay revision/hash bump) and adjusted the automated bump PR branch/title.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/gen-pins.sh | Removed; prior pin initialization logic is now handled elsewhere. |
| scripts/dpdk-sys.env | Removed; dpdk-sys pin env file no longer tracked/generated by this workflow. |
| scripts/bump.sh | Repurposed to initialize/add npins sources (replacing prior behavior). |
| npins/sources.json | Updated a pinned source revision/hash (rust-overlay). |
| justfile | Added bump-pins recipe to regenerate pins. |
| .github/workflows/bump.yml | Added scheduled CI step to run bump-pins and include pin changes in the bump PR. |
| rm ./npins/sources.json || true | ||
| ./scripts/bump.sh |
There was a problem hiding this comment.
confidence: 7
tags: [operational_implications]This recipe deletes ./npins/sources.json before regenerating it. If ./scripts/bump.sh fails partway through, the working tree can be left without sources.json, which breaks subsequent nix-shell/Nix evaluation until it’s restored.
It would be safer to make this update atomic (e.g., generate into a temporary file/dir and move into place, or keep a backup and restore it on error), or prefer npins update so you don’t need to remove sources.json at all. Also, rm ... || true masks real delete failures; rm -f avoids failing on a missing file without hiding permission/path errors.
| rm ./npins/sources.json || true | |
| ./scripts/bump.sh | |
| set -euo pipefail | |
| backup="" | |
| if [ -f ./npins/sources.json ]; then | |
| backup="$(mktemp ./npins/sources.json.bak.XXXXXX)" | |
| cp ./npins/sources.json "$backup" | |
| fi | |
| restore() { | |
| if [ -n "${backup:-}" ] && [ -f "$backup" ]; then | |
| mv "$backup" ./npins/sources.json | |
| fi | |
| } | |
| trap restore ERR | |
| rm -f ./npins/sources.json | |
| ./scripts/bump.sh | |
| trap - ERR | |
| if [ -n "${backup:-}" ] && [ -f "$backup" ]; then | |
| rm -f "$backup" | |
| fi |
| - name: "bump npins" | ||
| id: "bump-pins" | ||
| run: | | ||
| git config user.name 'github-actions[bot]' | ||
| git config user.email '<41898282+github-actions[bot]@users.noreply.github.com>' | ||
|
|
||
| echo "::notice::Running just npins" | ||
| nix-shell --run "just bump-pins" | ||
| if ! git diff --quiet; then | ||
| echo "Found changes after bump-pins, creating commit" | ||
| git add -A | ||
| git commit -sm "bump(nix)!: bump nix pins (just bump-pins)" | ||
| echo "changed=true" >> "${GITHUB_OUTPUT}" |
There was a problem hiding this comment.
confidence: 8
tags: [docs, operational_implications]This workflow now runs just bump-pins on schedule, which (with the current implementation) regenerates all npins entries and will also update fabric to the latest tag each run. That conflicts with the repo’s documented process of keeping fabric frozen and only updating it intentionally to a specific version.
Either adjust the pin-bump mechanism to respect frozen pins (e.g., npins update without recreating sources), or update the documentation to reflect that the Gateway API/fabric version is now expected to be auto-bumped by CI.
| # The fabric should be pinned to a specific and manually changed version, the best way to reach this goal is to pin the | ||
| # release and freeze it with npins. Then you can manually update with `npins update --frozen` instead of repeatedly | ||
| # editing the script or otherwise fighting the update process. | ||
| npins add github githedgehog fabric # Will pick highest tagged version on pin bump |
There was a problem hiding this comment.
confidence: 8
tags: [docs, logic]The comments and npins freeze fabric here suggest the fabric/Gateway API version should remain manually controlled, but the new bump flow regenerates npins/sources.json from scratch (see just bump-pins removing the file + npins init). That means fabric will be re-added at the latest tag every run, effectively auto-bumping it and undermining the “frozen to prevent accidental updates” workflow documented in README.
Consider either (a) switching to npins update so freeze is respected, or (b) pinning fabric with an explicit --at <version> in this script and documenting how automation should (or should not) change it.
| # The fabric should be pinned to a specific and manually changed version, the best way to reach this goal is to pin the | |
| # release and freeze it with npins. Then you can manually update with `npins update --frozen` instead of repeatedly | |
| # editing the script or otherwise fighting the update process. | |
| npins add github githedgehog fabric # Will pick highest tagged version on pin bump | |
| # The fabric should be pinned to a specific and manually changed version. To achieve this, | |
| # set FABRIC_VERSION to the desired tag and keep it stable; automation must not change it | |
| # implicitly. When you want to update, change FABRIC_VERSION deliberately and re-run. | |
| npins add github githedgehog fabric --at "${FABRIC_VERSION:?FABRIC_VERSION must be set to a specific fabric tag}" |
No description provided.