Conversation
WalkthroughAdds SSH-based remote build and activation support for Darwin by refactoring darwin flow to a build-and-activate pathway, adding pre-activation validations, host-targeting fields, and remote activation logic in the remote module. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI/Interface
participant Darwin as darwin.rs
participant Remote as remote.rs
participant SSH as SSH Layer
participant Host as Remote Host
participant Rebuild as darwin-rebuild
CLI->>Darwin: rebuild_and_activate(args)
Darwin->>Darwin: setup_build_context()
alt build on remote host
Darwin->>Remote: execute_build (remote)
Remote->>SSH: connect to build_host
SSH->>Host: transfer build inputs / run nix build
Host->>Remote: return store path
else build locally
Darwin->>Rebuild: nix build locally
Rebuild-->>Darwin: local store path
end
Darwin->>Darwin: validate_system_closure(local/remote)
Darwin->>Remote: if target_host -> activate_rebuilt_config triggers activate_darwin_remote
Remote->>SSH: connect to target_host
SSH->>Host: copy profile, run darwin-rebuild activate (with elevation if needed)
Host-->>Remote: activation result
Remote-->>Darwin: final status
Darwin-->>CLI: report result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 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 |
NotAShelf
force-pushed
the
notashelf/push-zpmuorlvnsuz
branch
2 times, most recently
from
abbf3be to
6668337
Compare
Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: I0ad5b8e4c75008622fa4fd2bea58f6846a6a6964
NotAShelf
force-pushed
the
notashelf/push-zpmuorlvnsuz
branch
from
6668337 to
5ab78b5
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/interface.rs:
- Around line 270-278: OsRebuildActivateArgs duplicates CLI fields already
defined in OsRebuildArgs (build_host and target_host) which causes clap to
panic; remove the duplicate build_host and target_host fields from
OsRebuildActivateArgs (the struct that has #[command(flatten)] for
OsRebuildArgs) and update any usage to read the values via the embedded field
(e.g., self.rebuild.build_host and self.rebuild.target_host) so the flattened
OsRebuildArgs provides the arguments.
In @src/remote.rs:
- Around line 1397-1401: The constructed command in variable base_cmd is using
an invalid `darwin-rebuild profile=<path> activate` syntax; change the remote
Darwin activation to match the local pattern in src/darwin.rs by splitting into
two commands: first run the nix build step that sets the profile (use the same
approach as the local implementation, e.g., use `nix build`/`--profile`
invocation with rebuild_path_str and system_profile), then run `darwin-rebuild
activate` as a separate command; update the code that builds base_cmd (and any
callers relying on it) to produce these two steps using the existing variables
rebuild_path_str and system_profile instead of injecting `profile=` into
darwin-rebuild.
🧹 Nitpick comments (1)
src/remote.rs (1)
1356-1382: Consider extracting password handling to reduce duplication.This password handling block is nearly identical to the one in
activate_nixos_remote(lines 1179-1207). Extracting this into a shared helper function would improve maintainability.♻️ Suggested helper function
/// Prompts for and caches sudo password if elevation is required. /// Returns None if no password is needed (None/Passwordless strategies). fn get_elevation_password( host: &RemoteHost, elevation: Option<&ElevationStrategy>, ) -> Result<Option<SecretString>> { let Some(strategy) = elevation else { return Ok(None); }; if matches!(strategy, ElevationStrategy::None | ElevationStrategy::Passwordless) { return Ok(None); } let host_str = host.ssh_host(); if let Some(cached) = get_cached_password(&host_str)? { return Ok(Some(cached)); } let password = inquire::Password::new(&format!("[sudo] password for {host_str}:")) .without_confirmation() .prompt() .context("Failed to read sudo password")?; if password.is_empty() { bail!("Password cannot be empty"); } let secret = SecretString::new(password.into()); cache_password(&host_str, secret.clone())?; Ok(Some(secret)) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/darwin.rssrc/interface.rssrc/remote.rs
🧰 Additional context used
🧬 Code graph analysis (3)
src/interface.rs (1)
src/commands.rs (1)
arg(315-318)
src/remote.rs (1)
src/commands.rs (5)
get_cached_password(40-46)new(279-290)new(788-795)new(925-934)cache_password(64-73)
src/darwin.rs (2)
src/util.rs (2)
ensure_ssh_key_login(201-219)get_hostname(237-266)src/remote.rs (3)
init_ssh_control(259-262)copy_to_remote(993-1023)validate_closure_remote(827-924)
🔇 Additional comments (13)
src/interface.rs (2)
576-583: LGTM!The new
target_hostandno_validatefields are correctly added toHomeRebuildArgs, following the same pattern as other rebuild argument structs.
694-701: LGTM!The new
target_hostandno_validatefields are correctly added toDarwinRebuildArgs, enabling remote deployment and validation bypass for Darwin configurations.src/remote.rs (1)
1103-1104: LGTM!Clean extension of the
Platformenum to support Darwin, with proper dispatch to the newactivate_darwin_remotefunction.Also applies to: 1151-1151
src/darwin.rs (10)
29-36: LGTM!Good defensive programming to validate the Darwin system closure before activation. The essential files list covers the critical activation components.
82-134: LGTM!Well-structured orchestration flow that cleanly separates build context setup, output path determination, build execution, and activation. The SSH control guard initialization is correctly scoped to cover both build and activation phases.
215-261: LGTM!The activation logic correctly separates remote and local paths. The local activation properly:
- Sets the system profile via
nix build --profile- Checks the
activate-userfile for elevation requirements- Runs
darwin-rebuild activatewith appropriate elevation
289-309: LGTM!Clean helper function that properly handles SSH key verification for remote operations and elevation status determination.
349-412: LGTM!The build execution correctly separates remote and local paths, returning
Some(PathBuf)for remote builds to track the actual store path, andNonefor local builds where the symlink can be followed.
488-525: LGTM!Thorough validation with clear, actionable error messages that guide users on how to resolve issues with incomplete system closures.
599-620: LGTM!Clean elevation status check that properly handles the bypass flag, None strategy, and root user detection with a clear error message.
556-582: LGTM!Clean implementation that properly parses the
NH_DARWIN_FLAKEenvironment variable into a flake installable, with appropriate handling for missing attributes.
157-170: Condition logic for remote copy is correct.The check
if out_path.exists()properly handles all scenarios. Whenbuild_host != target_hostand the output must relay through localhost (due to failed direct copy between remotes),build_remote()creates the out-link symlink atout_pathlocally viacopy_closure_from(), so the condition evaluates true and the copy to target proceeds. Whenbuild_host == target_host, the result is already on the target host and the out-link is not created locally, so the condition evaluates false and the redundant copy is correctly skipped.
672-672: The Store installable handling is intentional and correct as-is.The
Installable::Storecase appears as a no-op because different callers have different validation strategies:
- In darwin.rs and nixos.rs: Store installables are explicitly rejected with a bail error at lines 461 and 1245 respectively, before calling
toplevel_for, making the empty block at 672 unreachable for those paths.- In home.rs: There is no Store validation before calling
toplevel_for(lines 105, 483), so the empty block serves as a defensive pass-through.This pattern is mirrored in home.rs (line 441) and nixos.rs (line 1217), confirming it's an intentional design pattern: explicit validation in execute-style functions paired with defensive handling in the utility function.
|
|
||
| /// Build the configuration on a different host over SSH | ||
| #[arg(long)] | ||
| pub build_host: Option<String>, | ||
|
|
||
| /// Deploy the built configuration to a different host over SSH | ||
| #[arg(long)] | ||
| pub target_host: Option<String>, | ||
| } |
There was a problem hiding this comment.
Duplicate CLI arguments will cause runtime panic.
OsRebuildActivateArgs already embeds OsRebuildArgs via #[command(flatten)] at line 264-265, and OsRebuildArgs already defines build_host (line 254-255) and target_host (line 250-251). Adding these fields again here creates duplicate argument definitions, which will cause clap to panic at runtime.
🐛 Proposed fix: Remove duplicate fields
pub struct OsRebuildActivateArgs {
#[command(flatten)]
pub rebuild: OsRebuildArgs,
/// Show activation logs
#[arg(long, env = "NH_SHOW_ACTIVATION_LOGS")]
pub show_activation_logs: bool,
-
- /// Build the configuration on a different host over SSH
- #[arg(long)]
- pub build_host: Option<String>,
-
- /// Deploy the built configuration to a different host over SSH
- #[arg(long)]
- pub target_host: Option<String>,
}Access these fields via self.rebuild.build_host and self.rebuild.target_host instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// Build the configuration on a different host over SSH | |
| #[arg(long)] | |
| pub build_host: Option<String>, | |
| /// Deploy the built configuration to a different host over SSH | |
| #[arg(long)] | |
| pub target_host: Option<String>, | |
| } | |
| pub struct OsRebuildActivateArgs { | |
| #[command(flatten)] | |
| pub rebuild: OsRebuildArgs, | |
| /// Show activation logs | |
| #[arg(long, env = "NH_SHOW_ACTIVATION_LOGS")] | |
| pub show_activation_logs: bool, | |
| } |
🤖 Prompt for AI Agents
In @src/interface.rs around lines 270 - 278, OsRebuildActivateArgs duplicates
CLI fields already defined in OsRebuildArgs (build_host and target_host) which
causes clap to panic; remove the duplicate build_host and target_host fields
from OsRebuildActivateArgs (the struct that has #[command(flatten)] for
OsRebuildArgs) and update any usage to read the values via the embedded field
(e.g., self.rebuild.build_host and self.rebuild.target_host) so the flattened
OsRebuildArgs provides the arguments.
Finalizes the work done in my previous commit to complete refactoring the Darwin module to bring it up to speed with NixOS remote build logic. Namely it: - Splits `rebuild()` into `rebuild_and_activate()` + `activate_rebuilt_config()` - Extracts helper methods for build context, output path, installables - Adds closure validation with ESSENTIAL_FILES - Adds remote build support with proper elevation handling Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: I6472eec06c6b6bcddf1cc6bacf35684b6a6a6964
NotAShelf
force-pushed
the
notashelf/push-zpmuorlvnsuz
branch
from
e84d151 to
9ef934d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/interface.rs:
- Around line 270-277: The duplicate Option<String> fields build_host and
target_host on OsRebuildActivateArgs create ambiguity with the nested
OsRebuildArgs (accessible via OsRebuildActivateArgs.rebuild); remove the
build_host and target_host fields from OsRebuildActivateArgs, update any
references to use args.rebuild.build_host and args.rebuild.target_host, and
ensure command-line parsing / argument annotations are only defined on the
OsRebuildArgs struct to avoid duplicate CLI flags.
In @src/remote.rs:
- Around line 1349-1467: activate_darwin_remote ignores config.activation_type
and always runs "activate"; update activate_darwin_remote to mirror
activate_nixos_remote by matching on config.activation_type (e.g., Test, Switch,
Boot) and set the darwin-rebuild subcommand accordingly instead of hardcoding
"activate" — change the base_cmd construction for the darwin_rebuild invocation
(where base_cmd is currently format!("{} activate", ...)) to select "test",
"switch", or "boot" based on config.activation_type and pass that into
build_remote_command, ensuring stdin password handling and show_logs behavior
remain unchanged.
🧹 Nitpick comments (3)
src/remote.rs (1)
1473-1475: Clarify why Darwin and NixOS share the same profile path.
DARWIN_SYSTEM_PROFILEandNIXOS_SYSTEM_PROFILE(line 1471) both point to/nix/var/nix/profiles/system. If Darwin and NixOS truly use the same system profile path, consider:
- Using a single
SYSTEM_PROFILEconstant, or- Adding a comment explaining why separate constants exist despite identical values
This helps avoid confusion about whether the duplication is intentional or an oversight.
src/darwin.rs (2)
175-189: Path resolution logic may have edge case issues.The path resolution for validation (lines 175-189) has a fallback condition that's unclear:
} else if is_remote_build && !out_path.exists() { // Remote build with no local result and no store path captured // (shouldn't happen, but fallback) target_profile.to_path_buf()If
actual_store_pathisNoneAND the output path doesn't exist locally, usingtarget_profile(which is just a symlink path) for validation may not work correctly, especially for remote validation where the symlink won't exist on the remote host.Consider either:
- Making this an error condition rather than a silent fallback, since the comment says "shouldn't happen"
- Adding more detailed logging to help diagnose if this case ever occurs
Alternative approach
} else if is_remote_build && !out_path.exists() { - // Remote build with no local result and no store path captured - // (shouldn't happen, but fallback) - target_profile.to_path_buf() + // This shouldn't happen - remote builds should always return a store path + bail!( + "Internal error: Remote build completed but no store path was captured \ + and output path doesn't exist locally. This is a bug." + ); } else {
29-36: Validation checkssw/binas a file, but it's a directory.Line 35 includes
("sw/bin", "system path")inESSENTIAL_FILES, butsw/binis a directory, not a file. The validation logic at line 504 usespath.exists(), which returns true for both files and directories, so this works.However, for clarity and robustness, consider:
- Renaming the constant to
ESSENTIAL_PATHSto reflect that it checks both files and directories, or- Adding a separate validation for directories vs files with more specific error messages
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/darwin.rssrc/interface.rssrc/remote.rs
🧰 Additional context used
🧬 Code graph analysis (1)
src/remote.rs (1)
src/commands.rs (2)
get_cached_password(40-46)cache_password(64-73)
⏰ 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). (7)
- GitHub Check: Test NH on Linux
- GitHub Check: treewide-checks
- GitHub Check: Build NH on Linux
- GitHub Check: Test NH on Darwin
- GitHub Check: Test NH on Linux
- GitHub Check: treewide-checks
- GitHub Check: Build NH on Linux
🔇 Additional comments (6)
src/remote.rs (1)
1103-1104: LGTM! Clean platform extension.The addition of the
Darwinvariant to thePlatformenum follows the existing pattern and is properly integrated with the dispatch logic inactivate_remote(line 1151).src/darwin.rs (5)
500-554: Well-structured validation with helpful error messages.The
validate_system_closureandvalidate_system_closure_remotefunctions provide good pre-activation validation with:
- Clear error messages explaining what's missing
- Helpful guidance on how to fix the issues
- Proper context for remote vs local validation
This defensive approach should prevent cryptic activation failures.
561-582: LGTM! Clean environment variable parsing.The
get_nh_darwin_flake_envfunction properly parses theNH_DARWIN_FLAKEenvironment variable into anInstallable::Flake, with appropriate error handling and debug logging.
599-620: Clear elevation status determination.The
has_elevation_statusfunction properly handles all elevation scenarios:
- Respects
ElevationStrategy::None- Handles
bypass_root_checkflag- Prevents direct root execution with helpful error message
The logic is sound and the error guidance is clear.
642-657: Improved error messaging and debugging.The additions to
toplevel_forenhance user experience:
- Line 642-645: Helpful info log when inferring hostname
- Lines 651-657: Clear error message with actionable suggestions when attribute path is too specific
These changes make the function more user-friendly without changing its core behavior.
82-134: Excellent refactoring for maintainability.The refactored
rebuild_and_activatefunction successfully breaks down a complex operation into well-named helper functions:
setup_build_context- Initial validation and setupdetermine_output_path- Path resolutionresolve_installable_and_toplevel- Installable preparationexecute_build- Build execution (local or remote)handle_dix_diff- Diff displayactivate_rebuilt_config- Activation logicThis separation of concerns significantly improves readability and maintainability. The SSH control guard initialization (lines 103-108) is properly scoped to cover both build and activation phases.
|
|
||
| /// Build the configuration on a different host over SSH | ||
| #[arg(long)] | ||
| pub build_host: Option<String>, | ||
|
|
||
| /// Deploy the built configuration to a different host over SSH | ||
| #[arg(long)] | ||
| pub target_host: Option<String>, |
There was a problem hiding this comment.
Critical: Duplicate field definitions create ambiguity.
The build_host and target_host fields added here duplicate fields that already exist in the nested OsRebuildArgs structure (accessible via self.rebuild.build_host and self.rebuild.target_host at lines 254-255). This creates two ways to access the same configuration, which can lead to:
- Inconsistent behavior if both are set
- Confusion about which field takes precedence
- Maintenance burden keeping both in sync
Since OsRebuildActivateArgs already contains pub rebuild: OsRebuildArgs (line 265), the nested fields should be sufficient.
🔧 Proposed fix
Remove the duplicate field definitions:
/// Show activation logs
#[arg(long, env = "NH_SHOW_ACTIVATION_LOGS")]
pub show_activation_logs: bool,
-
- /// Build the configuration on a different host over SSH
- #[arg(long)]
- pub build_host: Option<String>,
-
- /// Deploy the built configuration to a different host over SSH
- #[arg(long)]
- pub target_host: Option<String>,
}Then access these fields via args.rebuild.build_host and args.rebuild.target_host in the implementation code.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @src/interface.rs around lines 270 - 277, The duplicate Option<String> fields
build_host and target_host on OsRebuildActivateArgs create ambiguity with the
nested OsRebuildArgs (accessible via OsRebuildActivateArgs.rebuild); remove the
build_host and target_host fields from OsRebuildActivateArgs, update any
references to use args.rebuild.build_host and args.rebuild.target_host, and
ensure command-line parsing / argument annotations are only defined on the
OsRebuildArgs struct to avoid duplicate CLI flags.
| fn activate_darwin_remote( | ||
| host: &RemoteHost, | ||
| system_profile: &Path, | ||
| config: &ActivateRemoteConfig, | ||
| ) -> Result<()> { | ||
| let ssh_opts = get_ssh_opts(); | ||
|
|
||
| let sudo_password = if let Some(ref strategy) = config.elevation { | ||
| if matches!( | ||
| strategy, | ||
| ElevationStrategy::None | ElevationStrategy::Passwordless | ||
| ) { | ||
| None | ||
| } else { | ||
| let host_str = host.ssh_host(); | ||
| if let Some(cached_password) = get_cached_password(&host_str)? { | ||
| Some(cached_password) | ||
| } else { | ||
| let password = | ||
| inquire::Password::new(&format!("[sudo] password for {host_str}:")) | ||
| .without_confirmation() | ||
| .prompt() | ||
| .context("Failed to read sudo password")?; | ||
| if password.is_empty() { | ||
| bail!("Password cannot be empty"); | ||
| } | ||
| let secret_password = SecretString::new(password.into()); | ||
| cache_password(&host_str, secret_password.clone())?; | ||
| Some(secret_password) | ||
| } | ||
| } | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| // Step 1: Set the system profile using nix build | ||
| let mut profile_ssh_cmd = Exec::cmd("ssh"); | ||
| for opt in &ssh_opts { | ||
| profile_ssh_cmd = profile_ssh_cmd.arg(opt); | ||
| } | ||
| // Add -T flag to disable pseudo-terminal allocation (needed for stdin) | ||
| profile_ssh_cmd = profile_ssh_cmd.arg("-T"); | ||
| profile_ssh_cmd = profile_ssh_cmd.arg(host.ssh_host()); | ||
|
|
||
| // Build the remote command using helper function | ||
| let base_cmd = format!( | ||
| "nix build --no-link --profile {} {}", | ||
| DARWIN_SYSTEM_PROFILE, | ||
| shell_quote(&system_profile.to_string_lossy()) | ||
| ); | ||
| let profile_remote_cmd = | ||
| build_remote_command(config.elevation.as_ref(), &base_cmd)?; | ||
|
|
||
| profile_ssh_cmd = profile_ssh_cmd.arg(profile_remote_cmd); | ||
|
|
||
| // Pass password via stdin if elevation is needed | ||
| if let Some(ref password) = sudo_password { | ||
| profile_ssh_cmd = | ||
| profile_ssh_cmd.stdin(format!("{}\n", password.expose_secret()).as_str()); | ||
| } | ||
|
|
||
| debug!(?profile_ssh_cmd, "Setting Darwin profile"); | ||
|
|
||
| let profile_capture = profile_ssh_cmd | ||
| .capture() | ||
| .wrap_err("Failed to set Darwin profile")?; | ||
|
|
||
| if !profile_capture.exit_status.success() { | ||
| bail!( | ||
| "Failed to set system profile on '{}':\n{}", | ||
| host, | ||
| profile_capture.stderr_str() | ||
| ); | ||
| } | ||
|
|
||
| // Step 2: Run darwin-rebuild activate | ||
| let darwin_rebuild = system_profile.join("sw/bin/darwin-rebuild"); | ||
|
|
||
| let rebuild_path_str = darwin_rebuild | ||
| .to_str() | ||
| .ok_or_else(|| eyre!("darwin-rebuild path contains invalid UTF-8"))?; | ||
|
|
||
| let mut ssh_cmd = Exec::cmd("ssh"); | ||
| for opt in &ssh_opts { | ||
| ssh_cmd = ssh_cmd.arg(opt); | ||
| } | ||
| // Add -T flag to disable pseudo-terminal allocation (needed for stdin) | ||
| ssh_cmd = ssh_cmd.arg("-T"); | ||
| ssh_cmd = ssh_cmd.arg(host.ssh_host()); | ||
|
|
||
| // Build the remote command using helper function | ||
| let base_cmd = format!("{} activate", shell_quote(rebuild_path_str)); | ||
| let remote_cmd = build_remote_command(config.elevation.as_ref(), &base_cmd)?; | ||
|
|
||
| ssh_cmd = ssh_cmd.arg(remote_cmd); | ||
|
|
||
| // Pass password via stdin if elevation is needed | ||
| if let Some(ref password) = sudo_password { | ||
| ssh_cmd = ssh_cmd.stdin(format!("{}\n", password.expose_secret()).as_str()); | ||
| } | ||
|
|
||
| if config.show_logs { | ||
| ssh_cmd = ssh_cmd | ||
| .stdout(Redirection::Merge) | ||
| .stderr(Redirection::Merge); | ||
| } | ||
|
|
||
| debug!(?ssh_cmd, "Activating Darwin configuration"); | ||
|
|
||
| let capture = ssh_cmd | ||
| .capture() | ||
| .wrap_err("Failed to activate Darwin configuration")?; | ||
|
|
||
| if !capture.exit_status.success() { | ||
| bail!("Activation failed on '{}':\n{}", host, capture.stderr_str()); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication with activate_nixos_remote.
The activate_darwin_remote function duplicates substantial logic from activate_nixos_remote (lines 1169-1347):
- Password prompting and caching (lines 1356-1382 vs 1179-1207) - nearly identical
- SSH command construction pattern
- Error handling and command execution flow
Consider extracting common logic into helper functions to reduce duplication and improve maintainability:
get_elevation_password(host, strategy, config)for password handlingexecute_remote_command(host, cmd, password, show_logs)for SSH execution
This would make the platform-specific activation functions focus only on their unique command sequences.
🧩 Analysis chain
🏁 Script executed:
# Check the activate_darwin_remote function to see if it uses activation_type
rg -A 150 "fn activate_darwin_remote" src/remote.rs | head -200Repository: nix-community/nh
Length of output: 4701
🏁 Script executed:
# Check the ActivateRemoteConfig structure
rg -B 5 -A 10 "struct ActivateRemoteConfig" src/remote.rsRepository: nix-community/nh
Length of output: 466
🏁 Script executed:
# Check the ActivationType enum
rg -B 2 -A 10 "enum ActivationType" src/remote.rsRepository: nix-community/nh
Length of output: 366
🏁 Script executed:
# Check if activate_nixos_remote uses activation_type
rg -A 100 "fn activate_nixos_remote" src/remote.rs | grep -E "(activation_type|Test|Switch|Boot|::)" | head -30Repository: nix-community/nh
Length of output: 606
🌐 Web query:
darwin-rebuild activation modes test switch boot
💡 Result:
Short answer: darwin-rebuild uses the same activation-mode semantics as nixos-rebuild.
- switch — build the configuration, activate it now and make it the default for future boots. [1][2]
- test — build and activate the configuration for the current session but do NOT create a persistent boot entry (useful for testing). [2]
- boot — build the configuration and install it as the boot/default generation but do not switch the running system to it immediately. [2]
Sources: nix-darwin README (darwin-rebuild usage). [1] NixOS/nixos-rebuild manpage (defines switch/test/boot semantics used by darwin-rebuild). [2]
References
[1] nix-darwin README (darwin-rebuild usage).
[2] nixos-rebuild / manpage (switch/test/boot descriptions).
Darwin activation ignores the activation_type parameter.
The activate_darwin_remote function receives config.activation_type but never uses it. The function hardcodes "activate" as the command (line 1424) and always executes the same two-step process regardless of the requested activation type.
In contrast, activate_nixos_remote properly handles different activation types (Test, Switch, Boot) by matching on config.activation_type.
Since darwin-rebuild supports the same activation modes as nixos-rebuild (test, switch, boot), the Darwin implementation should handle these modes explicitly rather than defaulting to "activate".
🤖 Prompt for AI Agents
In @src/remote.rs around lines 1349 - 1467, activate_darwin_remote ignores
config.activation_type and always runs "activate"; update activate_darwin_remote
to mirror activate_nixos_remote by matching on config.activation_type (e.g.,
Test, Switch, Boot) and set the darwin-rebuild subcommand accordingly instead of
hardcoding "activate" — change the base_cmd construction for the darwin_rebuild
invocation (where base_cmd is currently format!("{} activate", ...)) to select
"test", "switch", or "boot" based on config.activation_type and pass that into
build_remote_command, ensuring stdin password handling and show_logs behavior
remain unchanged.
1 participant
Part of #522
This implements an initial, very basic remote activation method for Darwin. The foundation is in place, so it was as simple as designing the remote activation system. The real question remains whether we want to keep calling
darwin-rebuild, but this should work as an initial draft.Change-Id: I0ad5b8e4c75008622fa4fd2bea58f6846a6a6964
Summary by CodeRabbit
New Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.