Conversation
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true📝 WalkthroughWalkthroughChangesThe Installable defaults
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/nh-installable/src/defaults.rs (1)
182-210: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding direct tests for
resolve_flake_directorysymlink handling.The symlinked-
dirvs. symlinked-flake.nixbranches are non-trivial and untested directly (existing tests only coveros_installable_atwith plain directories). A couple oftempfile+std::os::unix::fs::symlink-based tests would guard this logic against regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/nh-installable/src/defaults.rs` around lines 182 - 210, The symlink-handling branches in resolve_flake_directory lack direct coverage. Add focused Unix tests using tempfile directories and symlink to verify both a symlinked directory containing flake.nix and a plain directory whose flake.nix is symlinked, asserting the returned resolved directory and relevant error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/nh-installable/src/defaults.rs`:
- Around line 121-142: Update os_installable_at to check for
system_directory/default.nix after checking system.nix and before falling back
to <nixpkgs/nixos>. Preserve the existing debug logging and file installable
behavior for both discovered Nix files.
---
Nitpick comments:
In `@crates/nh-installable/src/defaults.rs`:
- Around line 182-210: The symlink-handling branches in resolve_flake_directory
lack direct coverage. Add focused Unix tests using tempfile directories and
symlink to verify both a symlinked directory containing flake.nix and a plain
directory whose flake.nix is symlinked, asserting the returned resolved
directory and relevant error behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fb66cf7f-1e83-4324-9311-b41501972010
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
crates/nh-installable/Cargo.tomlcrates/nh-installable/src/defaults.rscrates/nh-installable/src/lib.rs
| fn os_installable_at( | ||
| system_directory: &Path, | ||
| find_system: impl FnOnce() -> Result<Option<PathBuf>>, | ||
| ) -> Result<Installable> { | ||
| if let Some(installable) = find_flake(system_directory, "os", "NH_OS_FLAKE")? | ||
| { | ||
| return Ok(installable); | ||
| } | ||
|
|
||
| if let Some(path) = find_system()? { | ||
| debug!(path = %path.display(), "Using <nixos-system>"); | ||
| return Ok(file(path)); | ||
| } | ||
|
|
||
| let system_nix = system_directory.join("system.nix"); | ||
| if system_nix.is_file() { | ||
| debug!(path = %system_nix.display(), "Using system.nix"); | ||
| return Ok(file(system_nix)); | ||
| } | ||
|
|
||
| Ok(file(PathBuf::from("<nixpkgs/nixos>"))) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
git ls-files crates/nh-installable/src/defaults.rs
printf '\n--- outline ---\n'
ast-grep outline crates/nh-installable/src/defaults.rs --view expanded
printf '\n--- relevant lines ---\n'
sed -n '1,220p' crates/nh-installable/src/defaults.rs | cat -nRepository: nix-community/nh
Length of output: 8642
🏁 Script executed:
set -euo pipefail
sed -n '260,360p' crates/nh-installable/src/defaults.rs | cat -n
printf '\n--- references to resolve_file ---\n'
rg -n "resolve_file\(" -S .
printf '\n--- references to system.nix/default.nix in this area ---\n'
rg -n "system\.nix|default\.nix|nixpkgs/nixos|nixos-system" crates/nh-installable/src/defaults.rsRepository: nix-community/nh
Length of output: 5037
os_installable_at should also fall back to default.nix. resolve_file already prefers system.nix and then default.nix for OS directories, but this default path only checks system.nix before falling back to <nixpkgs/nixos>, so a bare nh os switch can behave differently from an explicit /etc/nixos path when only default.nix exists.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/nh-installable/src/defaults.rs` around lines 121 - 142, Update
os_installable_at to check for system_directory/default.nix after checking
system.nix and before falling back to <nixpkgs/nixos>. Preserve the existing
debug logging and file installable behavior for both discovered Nix files.
…mode nh now tries `default.nix` and `system.nix` akin to `nixos-rebuild` for explicit compatibility behaviour AFTER flake options were exhausted. This only applies to the default, flakes-first behaviour. For users already configuring nh to respect their non-flake setups, there is no behavioural changes. Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: I65bd9b5a49fe69a76cda12ffadc941e06a6a6964
b1bcc26 to
56a4341
Compare
nh now tries
default.nixandsystem.nixakin tonixos-rebuildfor explicit compatibility behaviour AFTER flake options were exhausted. This only applies to the default, flakes-first behaviour. For users already configuring nh to respect their non-flake setups, there is no behavioural changes.Change-Id: I65bd9b5a49fe69a76cda12ffadc941e06a6a6964
Add a 👍 reaction to pull requests you find important.