-
-
Notifications
You must be signed in to change notification settings - Fork 493
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
The Plan, phase 1 #1341
base: master
Are you sure you want to change the base?
The Plan, phase 1 #1341
Conversation
f2bd544
to
454a716
Compare
|
||
{ | ||
options = { | ||
system.primaryUser = lib.mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have an option to disable primary user, so that dogfooding finds problems with not having a primary user sooner?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized the default is null
, so never mind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, leaving it at the default will ensure your system doesn’t depend on any primary user and give a hopefully‐helpful error message if you set anything that requires it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in practice, it may be useful for us to
nest this intousers.users.*.homebrew
instead, at the expense of
being an unsupported setup if used to its full potential. Since
that would be a breaking change to the inteface anyway, I think
addinghomebrew.user
for now is acceptable.
I understand the motive here, but are you sure it's not better to make more breaking changes in one go? I'm worried that going through another great migration would agitate some.
I previously thought that too – that we should figure out the correct interfaces and write good migration documentation and tools and get all the breaking changes over with in one go – but the end result was that this work stalled out for years because there’s a lot of API design and implementation work to get things to a good state: we need to decide which things should be global or per‐user; we need to decide what should stay in nix-darwin and what should be deduplicated with Home Manager; for things like our If we tried to do it all at once, this work would have spent even longer in development hell and be unreviewably big. For the longest time i wasn’t sure how to resolve that, but after discussing on Matrix I realized that introducing a In the case of Homebrew, |
This adds an optional explicit `homebrew.user` option that allows users to avoid setting `system.primaryUser`, partly as a proof of concept of what the interfaces should look like in the future. Homebrew only officially support one global installation, so a singleton matches upstream’s expectations; in practice, it may be useful for us to nest this into `users.users.*.homebrew` instead, at the expense of being an unsupported setup if used to its full potential. Since that would be a breaking change to the inteface anyway, I think adding `homebrew.user` for now is acceptable. (I think one native Apple Silicon and one Rosetta 2 Homebrew installation – under `/opt/homebrew` and `/usr/local` respectively – may be exceptions to this lack of upstream support, but that would be complicated to support even with `users.users.*.homebrew`.) I’m not entirely sure where in system activation this should go. Probably after the user defaults and launch agents stuff, to match the existing logic in user activation, and I lean towards doing it as late as possible; too early and we might not have the users and groups required to bootstrap a Homebrew installation set up, but as Homebrew installations could be fiddly and fail, doing it in the middle could leave a partially‐activated system. Probably it should be done in a launch agent or something instead, but this is my best guess as to the appropriate place for now. The downside is that activation scripts generally won’t be able to assume that the Homebrew prefix is populated according to the current configuration, but they probably shouldn’t be depending on that anyway?
I’m not *completely* certain that this handles user agents correctly. There is a deprecated command, `launchctl asuser`, that executes a command in the Mach bootstrap context of another user`. <https://scriptingosx.com/2020/08/running-a-command-as-another-user/> claims that this is required when loading and unloading user agents, but I haven’t tested this. Our current launchd agent logic is pretty weird and broken already anyway, so unless this actively regresses things I’d lean towards keeping it like this until we can move over entirely to `launchctl bootstrap`/`launchctl kickstart`, which aren’t deprecated and can address individual users directly. Someone should definitely test it more extensively than I have, though.
System activation scripts shouldn’t (and soon won’t be able to) rely on `$HOME` being the primary user’s.
These can’t be relied upon in a post‐user‐activation world. Technically a breaking change, if anyone has their home directory outside of `/Users` or is using `root` for this, but, well, I did my best and these are legacy defaults anyway.
(With some tweaks to handle `nix.enable` and order it at a more sensible position in the `$PATH`.) The installers actually install Nix into `root`’s profile for some reason, which means that the path’s prioritization backfires when the script runs as root and we’re managing the Nix installation. When running `darwin-rebuild` as a normal user, this wasn’t a problem. Maybe we should just have a check to make sure there’s no conflicting Nix in `root`’s profile – it seems pretty bad for `root` to get the wrong Nix – but it would trigger for almost everyone, which seems kind of annoying. I guess we could automatically remove it from `root`’s profile if it matches what’s in `/nix/var/nix/profiles/default`… This reverts commit 02232f7.
454a716
to
229a7c6
Compare
The `activate-system` daemon will now run all the checks, which seems like probably a good idea anyway?
The checks should no longer depend on `/run`, so this avoids modifying the system before they run.
229a7c6
to
a387b4b
Compare
It’s finally time.
This PR eliminates user activation, addressing #96 by following the first phase of the plan from #1016 (comment), fulfilling a goal I started drafting patches for in 2023 before I was even a nix-darwin maintainer.
I strongly recommend reviewing this one commit‐by‐commit. See the commit messages for much more detail and for things I’m still uncertain about. I’ve been running this on my own system for a while now, but it could definitely use a staged roll‐out before we actually hit the merge button. We should also probably also reach out to authors of third‐party modules that will be affected once it’s mostly baked, though at least anything that breaks should mostly break loudly. (Adding text to our existing
activationScripts.{checks,userDefaults,userLaunchd,homebrew}.text
definitions is an exception – those will run asroot
now. But I can’t think of a particularly graceful way to handle that case, and anyway it’s not exactly something we fully support in the first place.)Closes: #96