Support arbitrary keys in system.activationScripts

[quentinmit]

This changes nix-darwin's behavior to match NixOS's handling of this option. Each key can specify a list of dependencies to control ordering, and every element of system.activationScripts will be used.

This also introduces system.userActivationScripts, again mirroring NixOS, and moves the user-level keys from system.activationScripts.

Care is taken to continue handling legacy keys such as system.activationScripts.{preActivation,preUserActivation,postActivation,...} with the same behavior they have already had.

Previously, the org.nixos.activate-system launchd daemon only ran the etc and keyboard activation scripts. This change adds a new onlyOnRebuild setting to existing keys to preserve that behavior. Newly defined keys will run at both rebuild and boot time, matching NixOS's behavior.

Fixes #663.

[quentinmit]

I had a failing install-flake check so I dug deep into the logs.

First, it's complaining that /etc/nix/nix.conf already exists, but that seems to be normal based on looking at other recent PR's check logs. This is probably worth fixing as it means the CI doesn't actually test replacing nix.conf.

The nix-daemon activation script then runs, which checks if /etc/nix/nix.conf and /run/current-system/etc/nix/nix.conf are different, and triggers a SIGHUP. This was very confusing to me, until I realized that at the point the activation scripts run, /run/current-system is actually the old system.

It then waits for nix-daemon to start, but I'm guessing that what's happening is that nix-daemon hasn't actually shut down yet, so this check is a noop. Then in the next step, the nix-daemon isn't running (error: cannot connect to socket at '/nix/var/nix/daemon-socket/socket': Connection refused).

I'm guessing the nix-daemon activation script is just plain broken, and the fact that there are fewer activation scripts that run after it now is tickling a race condition that was previously dormant.

I reworked the code to check launchctl kickstart -p before the SIGHUP, which prints the pid of the running process. Then we can wait until the pid has changed. There's also launchctl kickstart -k that supposedly will kill the process and then restart it, but I can't tell what signal it will send, and presumably it doesn't use SIGHUP.

[rvem]

I was looking for the same feature and was surprised that someone already supported it 😅

Your solution works for my use-case

[LnL7]

In general I tried to keep compatibility with existing modules, however I intentionally excluded activation scripts from the public api for external modules. It would be nice if the module system provided better functionality for this but everything is global and thus accessible in all modules.

Allowing any imported code to do arbitrary things as root already doesn't seem like the best idea in the first place and while initially implementing the darwin modules I clearly noticed that almost none of the existing activation logic from NixOS could be reused as it's written with the assumption that it has full control over the entire system. Unlike with NixOS there's no way to rollback if activation breaks something for the host system.

[emilazy]

Currently I've just worked around this by lib.mkBefore/lib.mkAftering existing activation scripts in my modules, which seems strictly worse. Unfortunately I don't think there's any way to avoid trusting external modules with the current architecture, and if they break something I doubt the interface used to poke at the activation scripts would be the deciding factor in whether users blame nix-darwin for it.

[quentinmit]

In general I tried to keep compatibility with existing modules, however I intentionally excluded activation scripts from the public api for external modules. It would be nice if the module system provided better functionality for this but everything is global and thus accessible in all modules.

Allowing any imported code to do arbitrary things as root already doesn't seem like the best idea in the first place and while initially implementing the darwin modules I clearly noticed that almost none of the existing activation logic from NixOS could be reused as it's written with the assumption that it has full control over the entire system. Unlike with NixOS there's no way to rollback if activation breaks something for the host system.

I believe this PR preserves full compatibility with existing modules; users can still set exactly the same activation script keys and they will have exactly the same functionality, and they can also do the NixOS thing and define new activation script keys.

I agree it's a potentially dangerous interface, but you're already exposing the interface with preActivation/postActivation - this PR just makes it less painful to use from modules.

(BTW, the motivation for this is that I was starting to work on #618 and I found that there wasn't an easy way to add the required activationScript for it.)

[LnL7]

Currently I've just worked around this by lib.mkBefore/lib.mkAftering existing activation scripts in my modules, which seems strictly worse. Unfortunately I don't think there's any way to avoid trusting external modules with the current architecture.

I mostly agree with that, since everything is global there is indeed no way to actually prevent this. However currently at least insures that the included module was actually written with darwin in mind. The current changes would allow the following.

{
  imports = [ <nixpkgs/nixos/missing-functionality.nix> ];
  config = {
    something.enable = true;
  };
}

I would at least like this case like this to fail, one option could be to simply rename the option for activation snippets. This will still catch unintended use while enabling dynamic snippets and ordering both internally and for modules written for darwin specifically.

[emilazy]

Seems reasonable; how about something like system.darwin.activationScripts or darwin.activationScripts (or even darwin.system.activationScripts if we expect to need to mirror other parts of the options hierarchy for Darwin-specific things)?

P.S. I wish everything wasn't global. I wish we had proper static checks. :(

[emilazy]

(Home Manager uses home.activation FWIW; the other existing system setting with a NixOS overlap that seems like it could be dangerous is stateVersion so maybe that should be darwin.stateVersion too?)

[quentinmit]

I mostly agree with that, since everything is global there is indeed no way to actually prevent this. However currently at least insures that the included module was actually written with darwin in mind. The current changes would allow the following.

{
  imports = [ <nixpkgs/nixos/missing-functionality.nix> ];
  config = {
    something.enable = true;
  };
}

I would at least like this case like this to fail, one option could be to simply rename the option for activation snippets. This will still catch unintended use while enabling dynamic snippets and ordering both internally and for modules written for darwin specifically.

If you're okay with renaming it, we could name it system.darwinActivationScripts. I was trying to preserve compatibility with existing modules, which already use the name system.activationScripts{.preActivation,.postActivation,...}. I think user activation scripts can be left at system.userActivationScripts since those are less likely to break badly even if they're not darwin-specific.

[LnL7]

To clarify I'm ok with the changes if they don't remain under the same option as NixOS. For preActivation/postActivation we can use mkRenamedOptionModule to provide compatibility. I don't have a strong opinion on the new name, but maybe darwin.* is indeed a nice option instead of trying to keep it under the system.*.

[quentinmit]

To clarify I'm ok with the changes if they don't remain under the same option as NixOS. For preActivation/postActivation we can use mkRenamedOptionModule to provide compatibility. I don't have a strong opinion on the new name, but maybe darwin.* is indeed a nice option instead of trying to keep it under the system.*.

I don't believe mkRenamedOptionModule can be used since these are keys within an attrset.

[LnL7]

Yeah, could be that the implementation is a bit too simple to cover this case. But the same idea can be implemented using conditions and the warnings module.