Skip to content
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.

Sign up for GitHub

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

darwin.rs: handle new root only activation #238

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

darwin.rs: handle new root only activation #238

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
caf77d5
darwin.rs: handle new root only activation
khaneliman Mar 18, 2025
aee93d4
darwin.rs: future proof removal of `activate-user` script
khaneliman Mar 18, 2025
dc33934
darwin.rs: use darwin-rebuild
khaneliman Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions src/darwin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,22 @@ impl DarwinRebuildArgs {
.dry(self.common.dry)
.run()?;

let switch_to_configuration = out_path.get_path().join("activate-user");

Command::new(switch_to_configuration)
.message("Activating configuration for user")
.dry(self.common.dry)
.run()?;

let switch_to_configuration = out_path.get_path().join("activate");

Command::new(switch_to_configuration)
.elevate(true)
let darwin_rebuild = out_path.get_path().join("sw/bin/darwin-rebuild");
Copy link

@isabelroses isabelroses Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this introduces a dependency on darwin-rebuild which is not always present.

If you can’t or don’t want to use darwin-rebuild activate, then you
should skip running activate-user for post‐user‐activation
configurations and continue running activate as root.

Copy link
Author

@khaneliman khaneliman Mar 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what situation is it not present? This is the darwin-rebuild from the built configuration.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was done from conversations with emilazy on matrix recommending i switch the implementation from using the activation scripts directly to the built darwin-rebuild binary.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what situation is it not present? This is the darwin-rebuild from the built configuration.

system.tools.darwin-rebuild.enable = false

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn’t set system.tools.darwin-rebuild.enable on a system you want to switch configurations on. We should probably add a stronger warning to that option and maybe hide it, since it’s extremely niche. In the long run we will hopefully have separate apply/switch-to-configuration scripts, but currently I don’t think this is a configuration activation tools should worry about supporting.

let activate_user = out_path.get_path().join("activate-user");

// Determine if we need to elevate privileges
let needs_elevation = !activate_user
.try_exists()
.context("Failed to check if activate-user file exists")?
|| std::fs::read_to_string(&activate_user)
.context("Failed to read activate-user file")?
.contains("# nix-darwin: deprecated");

// Create and run the activation command with or without elevation
Command::new(darwin_rebuild)
.arg("activate")
.message("Activating configuration")
.elevate(needs_elevation)
.dry(self.common.dry)
.run()?;
}
Expand Down