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.

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

nixos system-activation: support activation scripts run in a user context #47898

Merged
merged 3 commits into from Oct 5, 2018

Conversation

peterhoeg
Copy link
Member

Motivation for this change

#47842 for 18.09

Cc: @samueldr @vcunat

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Fits CONTRIBUTING.md.

Copy link
Member

@samueldr samueldr left a comment

Choose a reason for hiding this comment

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

Same as the other PR; though verified and it works as expected with a custom activation script snippet.

@samueldr samueldr merged commit 9d5f0ba into NixOS:release-18.09 Oct 5, 2018
(mapAttrsToList (name: value: "${pkgs.su}/bin/su ${name} -c ${pkgs.libsForQt5.kservice}/bin/kbuildsycoca5")
(filterAttrs (n: v: v.isNormalUser) config.users.users)));
# Update the start menu for each user that is currently logged in
system.userActivationScripts.plasmaSetup = "${pkgs.libsForQt5.kservice}/bin/kbuildsycoca5";
Copy link
Member

Choose a reason for hiding this comment

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

Why use a user-activation script instead of just creating a user systemd unit?

Copy link
Member

Choose a reason for hiding this comment

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

You missed that system.userActivationScripts is implemented via a systemd unit?

@grahamc
Copy link
Member

grahamc commented Oct 6, 2018 via email

@vcunat
Copy link
Member

vcunat commented Oct 6, 2018

The system activation script is ran once and with system privileges. This is to be ran for each (active) user with their privileges.

@grahamc
Copy link
Member

grahamc commented Oct 6, 2018 via email

@peterhoeg peterhoeg deleted the f/activation_1809 branch October 7, 2018 04:43
@peterhoeg
Copy link
Member Author

Right. Based on the definition it seems it will run on every login.

That's not entirely true. The system level activation scripts do run on every boot, but the user context scripts are only triggered for users logged in (we get a list of active sessions from logind).

I’m just not sure why we need the concept of a user activation script when a one shot user unit works just the same.

That's how it's implemented - as a oneshot unit. We still need a way to trigger it though so that is what the system level activation script handles.

This is in contrast to system activation scripts which aren’t typically implementable as systemd units.

Well, it's actually moving that way - @jameysharp has done some nice work in #47563.

@grahamc
Copy link
Member

grahamc commented Oct 7, 2018 via email

@peterhoeg peterhoeg restored the f/activation_1809 branch October 7, 2018 13:43
@jameysharp
Copy link
Contributor

Ooh, I like it. Although I think I'd like it even better if nixos-activation.service were instead nixos-activation.target. Then instead of system.userActivationScripts, you'd have systemd.user.services with wantedBy = [ "nixos-activation.target" ];.

@peterhoeg peterhoeg deleted the f/activation_1809 branch October 8, 2018 01:17
@peterhoeg
Copy link
Member Author

A target definitely makes more sense than a service.

system.userActivationScripts is the interface where the systemd unit(s)/target is the implementation. I think it makes sense to keep system.userActivationScripts as we can then implement it in any way we want.

And thinking a little more about this, how about this:

  1. We change the .service to a .target as you suggest
  2. Instead of writing out a massive script to be triggered by one unit, we can instead generate a unit per system.userActivationScripts script and hook that into the target

That will allow us to do granular activation.

@jameysharp
Copy link
Contributor

I feel like putting a lot of disclaimers on this response: I don't speak for anyone else; my feelings won't be hurt if you ignore my suggestions; etc. That said, if I were responsible for this decision, I know what I would do here, and since I got invited into this thread I guess I might as well share?

My major preference here is around having fewer ways to do the same things, and choosing the more general option, when that doesn't significantly impact usability.

Requiring people to use systemd.user.services directly has some usability cost, because they need to specify some extra things: wantedBy and serviceConfig.Type = "oneshot" at least, maybe others. I don't think that's a huge burden, though.

On the other hand, it's more flexible because they can use before or after to control the ordering of different snippets, or add packages to $PATH for a single snippet, or all sorts of other things.

It also doesn't require introducing new NixOS-specific concepts. Instead of saying "we have this notion of a user activation script, here's what that means," you can just say "every logged in user will have this target started in their session when the system configuration changes, do whatever you want with that, here's a link to the relevant systemd documentation if you aren't familiar with it." This feels conceptually simpler to me.

I'd argue that the worst option would be to do both. Saying "here's system.userActivationScripts as the easy way to do this, but it's implemented in terms of systemd.user.services this way and you can do that yourself if you need more flexibility" would mean you don't get the encapsulation you need to be able to change the implementation easily, at which point there's very little benefit over just having people write user service descriptions directly.

One more thought: configuration.nix(5) documents options including restartTriggers and restartIfChanged for systemd.user.services, but as far as I can tell there's no implementation for those options for user services, only for system services. Perhaps the best option here is to actually implement those options and just use them instead of a "nixos-activation.target"?

So... do whatever you like with this, but that's my personal feeling on the best direction to go here.

@peterhoeg
Copy link
Member Author

@jameysharp - I'm not ignoring you. I know how disheartening it can be when putting in a lot of thought into something, writing it up and then to be met with silence, but time's been a little tight on this side. I want us to work on getting something solid out of this and will get back to you shortly.

@jameysharp
Copy link
Contributor

No worries! 😄 I've been busy myself. If you want further opinions from me, let me know; otherwise I'm happy to leave this alone at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants