nixos: add --install-bootloader for explicit bootloader installation#427
nixos: add --install-bootloader for explicit bootloader installation#427
--install-bootloader for explicit bootloader installation#427Conversation
WalkthroughAdds Command::set_env builder to set a single environment variable; adds OsRebuildArgs::install_bootloader CLI flag; updates nixos rebuild flows to optionally inject NIXOS_INSTALL_BOOTLOADER=1, run a separate bootloader activation command, and log completion/debug info. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant ORA as OsRebuildArgs
participant MAIN as rebuild flow
participant CMD as Command builder
participant BOOT as switch_to_configuration cmd
participant SH as Shell/Exec
U->>ORA: invoke rebuild (test/switch) with args
ORA->>MAIN: start rebuild
MAIN->>CMD: build primary command (variant: test/switch)
MAIN->>CMD: preserve_envs(["NIXOS_INSTALL_BOOTLOADER"])
alt install_bootloader == true
MAIN->>BOOT: build switch_to_configuration command
BOOT->>BOOT: set_env("NIXOS_INSTALL_BOOTLOADER","1")
BOOT->>SH: execute (with required env)
SH-->>BOOT: result / error
alt error
BOOT-->>MAIN: propagate "Bootloader activation failed"
end
end
CMD->>SH: execute primary command
SH-->>MAIN: result
MAIN->>U: log Completed {variant} (out_path)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code graph analysis (1)src/nixos.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/commands.rs(1 hunks)src/interface.rs(1 hunks)src/nixos.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/nixos.rs (1)
src/commands.rs (4)
new(156-167)new(611-619)new(733-742)elevate(171-174)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: treewide-checks
- GitHub Check: Test NH on Linux
- GitHub Check: Build NH on Linux
- GitHub Check: Build NH on Linux
- GitHub Check: Test NH on Linux
🔇 Additional comments (2)
src/nixos.rs (1)
320-336: Implementation looks correct.The conditional environment variable injection works as expected. The
set_envcall (line 329) will override thepreserve_envsentry (line 326) wheninstall_bootloaderis true, since both insert into the sameHashMap. This ensures explicit control takes precedence over environment preservation.Note: Consider whether the
Bootvariant (lines 338-347) should also support the--install-bootloaderflag, as theBootcommand typically installs the bootloader by setting it as the default boot option. The current implementation only coversTestandSwitch.src/commands.rs (1)
238-250: LGTM! Clean builder method implementation.The
set_envmethod follows the established builder pattern and correctly inserts anEnvAction::Setentry. The implementation is consistent with existing methods likepreserve_envsand will work correctly with the environment application logic inapply_env_to_execandbuild_sudo_cmd.
NH previously supported passing `NIXOS_INSTALL_BOOTLOADER` variable to the `switch-to-configuration`, which in turn forced an installation of the bootloader. This behaviour was supported, but implcit. As per #424 we now support the `--install-bootloader` flag that makes this behaviour explicit and documented. Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: I6a6a69643f4b16816728dcaa6a66b675f9fe0ff2
8d1df42 to
b0a7b1c
Compare
NH previously supported passing
NIXOS_INSTALL_BOOTLOADERvariable to theswitch-to-configuration, which in turn forced an installation of the bootloader. This behaviour was supported, but implcit. As per #424 we now support the--install-bootloaderflag that makes this behaviour explicit and documented.Change-Id: I6a6a69643f4b16816728dcaa6a66b675f9fe0ff2
Summary by CodeRabbit
New Features
Documentation
Chores