-
Notifications
You must be signed in to change notification settings - Fork 1.3k
chore(nix): improve flake with dynamic version and build optimization #550
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
base: main
Are you sure you want to change the base?
chore(nix): improve flake with dynamic version and build optimization #550
Conversation
- Read version dynamically from package.json instead of hardcoding - Add lib.fileset source filtering to exclude node_modules and build artifacts - Update update-flake.sh to support dynamic version pattern - Add hash change detection to skip unnecessary rebuilds - Improve error handling with automatic rollback on failure - Update specs to reflect dynamic version behavior
- nix-installer-action: v13 → v21 - magic-nix-cache-action: v8 → v13 - Update validation message for unchanged flake.nix
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR updates Nix flake maintenance: flake.nix reads version from package.json and uses a fileset for src; update-flake.sh uses a placeholder hash to derive/skip updates and improves extraction/error handling; CI workflow bumps Nix-related action versions and updates messaging; docs and specs adjusted accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/CI
participant Script as update-flake.sh
participant Pkg as package.json
participant Flake as flake.nix
participant Nix as Nix Build
participant Git as Git
User->>Script: run update-flake.sh
Script->>Script: ensure pnpm-lock.yaml exists
Script->>Pkg: read version
Pkg-->>Script: version
Script->>Flake: verify flake reads version from package.json
alt dynamic version present
Script->>Flake: write placeholder hash
Script->>Nix: build --no-link (capture output)
Nix-->>Script: output including actual hash
Script->>Script: extract calculated hash
alt hash same as current
Script-->>User: ℹ️ No changes needed (exit 0)
else hash differs
Script->>Flake: update hash in flake.nix
Script->>Nix: re-verify build
Nix-->>Script: build succeeds
Script->>Git: stage changes (ready to commit)
Script-->>User: ✅ Updated hash (recommend test/verify/commit)
end
else dynamic version missing
Script-->>User: ❌ Error: flake.nix must read version from package.json
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryImproves Nix flake maintainability and build performance through dynamic version reading and source filtering. Key improvements:
All changes align with the updated specs in Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Script as update-flake.sh
participant PkgJSON as package.json
participant Flake as flake.nix
participant Nix as Nix Build
Dev->>Script: ./scripts/update-flake.sh
Script->>PkgJSON: Read version
PkgJSON-->>Script: 0.23.0
Script->>Flake: Check dynamic version pattern
alt Dynamic pattern found
Script->>Script: ✓ Dynamic version confirmed
else Hardcoded version
Script->>Dev: ⚠️ Warning: hardcoded version
end
Script->>Flake: Extract current hash
Flake-->>Script: sha256-current...
Script->>Flake: Set placeholder hash
Script->>Nix: nix build --no-link (expected to fail)
Nix-->>Script: Error: got: sha256-correct...
Script->>Script: Parse correct hash from error
alt Hash unchanged
Script->>Flake: Restore correct hash
Script->>Dev: ✓ Hash already up-to-date
else Hash changed
Script->>Flake: Update with correct hash
Script->>Nix: nix build --no-link (verify)
alt Build succeeds
Nix-->>Script: Success
Script->>Dev: ✅ Updated successfully
else Build fails
Nix-->>Script: Error
Script->>Flake: Restore original hash
Script->>Dev: ❌ Build verification failed
end
end
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
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.
Pull request overview
This PR enhances the Nix flake configuration to read version dynamically from package.json and improves build performance through source filtering. It also upgrades the update-flake.sh script with better error handling, early-exit detection for unchanged hashes, and improved UX with colored output.
Changes:
- Dynamic version reading from
package.jsoninflake.nix(eliminates manual version sync) - Source filtering with
lib.filesetto excludenode_modules/and unnecessary files from Nix builds - Enhanced
update-flake.shscript with hash change detection, automatic rollback on errors, and colored output - CI workflow updates: bumped Nix actions to latest versions (v13→v21, v8→v13)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| flake.nix | Added dynamic version reading from package.json and source filtering with lib.fileset |
| scripts/update-flake.sh | Enhanced with hash change detection, error rollback, colored output, and improved UX |
| scripts/README.md | Updated documentation to reflect dynamic version behavior |
| openspec/specs/ci-nix-validation/spec.md | Updated spec to document dynamic version validation |
| openspec/changes/archive/2026-01-09-add-flake-update-script/specs/flake-update-script/spec.md | Updated spec for new script behavior including rollback and no-op detection |
| .github/workflows/ci.yml | Bumped Nix installer and cache actions to latest versions |
| .changeset/nix-flake-improvements.md | Added changeset documenting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/update-flake.sh`:
- Around line 52-71: Replace the GNU-only grep -oP calls used to derive
CORRECT_HASH from BUILD_OUTPUT with POSIX-compatible sed parsing: take
BUILD_OUTPUT and run it through sed (using -n and -E/extended regex) to search
for the "sha256-..." token after the "got:" or "got:\s+" context and print the
first match, assigning that to CORRECT_HASH; update both places where
CORRECT_HASH is computed and ensure you still fall back to the alternate pattern
if the first sed extraction yields empty, leaving CURRENT_HASH, PLACEHOLDER,
BUILD_OUTPUT and CORRECT_HASH variable usage unchanged.
- Fix grep pattern on line 37 to include opening parenthesis - Replace GNU grep -oP with portable sed alternatives (lines 53, 68, 70) - Ensures script works on both Linux and macOS (BSD sed/grep)
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/update-flake.sh`:
- Around line 99-107: The current conditional uses `if nix build --no-link 2>&1
| grep -q "warning: Git tree.*is dirty"; then` which treats any non-matching
output (including build failures) as success; change the logic to capture the
exit status and output of `nix build --no-link` into a variable, check the
command exit code first (fail if non-zero), then separately grep the output for
the "Git tree.*is dirty" warning; update the branches around that captured
output so that when exit code != 0 you print a failure and exit non‑zero, when
exit code == 0 and dirty warning present you print the yellow dirty-tree
message, and when exit code == 0 and no warning you print the green success
message (reference the existing shell snippet containing the `nix build
--no-link 2>&1 | grep -q "warning: Git tree.*is dirty"` condition and variables
BLUE/YELLOW/GREEN).
Fix logic bug where build failures were incorrectly reported as success. The script now: - Captures build exit code and output separately - Fails fast if build returns non-zero exit code - Only checks for 'dirty tree' warning if build succeeded This addresses CodeRabbit review feedback on line 101-107.
Summary
Improves Nix flake maintainability and build performance with dynamic version reading and source filtering.
Changes
Nix flake (
flake.nix)package.jsoninstead of hardcodinglib.filesetsource filtering to excludenode_modules/and build artifactslibfor cleaner codeUpdate script (
scripts/update-flake.sh)CI (
ci.yml)nix-installer-action: v13 → v21magic-nix-cache-action: v8 → v13Documentation
scripts/README.mdto reflect dynamic version behaviorTesting
nix flake checkpassesscripts/update-flake.shdetects up-to-date hashscripts/update-flake.shcorrects incorrect hashMotivation
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.