Improve Home Manager support: profiles/suites, modules, extern, flake outputs #156

Pacman99 · 2021-03-09T21:03:43Z

A really simple method of implementing #119.

This relies on a feature that I added in home-manager master for module options of extraSpecialArgs and sharedModules, nix-community/home-manager#1793. I could try and get that backported to 20.09. But I thought I'd get some feedback on these changes first.

nrdxp · 2021-03-13T07:06:41Z

bors try

bors · 2021-03-13T07:06:44Z

try

Configuration problem:
bors.toml: not found

Pacman99 · 2021-03-14T08:45:11Z

Updated to match the lib changes.

Also I dropped hmActivationPackages and this time exported homeConfigurations with a hyphen seperating hosts and users. So all host/user pairs get exported as host-user.

I think this is significantly simpler. I only exported the activation packages in the first place due to nix's issues with nested outputs. So separating with a hyphen allows for easy debugging. And with the flk script, it really only affects someone who manually builds the activation package and even that isn't that bad.

nrdxp · 2021-03-14T20:15:43Z

This already is looking amazing, once you get it ready for review and it's approved, you can merge it yourself when your confident it's done. Good work.

bors delegate+

bors · 2021-03-14T20:15:43Z

✌️ Pacman99 can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

Pacman99 · 2021-03-14T20:52:50Z

This already is looking amazing, once you get it ready for review and it's approved, you can merge it yourself when your confident it's done. Good work.

Wow thats cool! Thanks! I think I might try to get the sharedModules and extraSpecialArgs PR backported to 20.09 in home-manager so I can get this merged sooner.

nrdxp · 2021-03-14T22:55:33Z

Alternatively, I was thinking of upgrading the nixos flake to nixos-unstable for the time being, at least until the next stable release.

Pacman99 · 2021-03-14T22:57:51Z

Ohh that makes things simpler. Can you do that as a separate commit? There might be other things to fix after upgrading, that doesn't make sense to include in this PR.

nrdxp · 2021-03-14T22:59:53Z

Yeah, I'll try to manage it soon. I'm working on cleaning up some other things as well. Hopefully by the end of the day.

Pacman99 · 2021-03-14T23:47:06Z

I dropped the commit updating the flake, so this should theoretically merge cleanly once the flake is updated. But I can fix any other conflicts once you upgrade.

Pacman99 · 2021-03-15T00:27:31Z

I added some more defaults to the hmConfig. Although, optimally, that would not be needed with #127.

nrdxp · 2021-03-15T17:43:54Z

I've update my local config to nixos-unstable and it seems to be working fine, after some fixes. However, I can't push it up yet, as it somehow broke the ci runner and all tests are failing with an exception. Once I figure it out, I'll push it.

Pacman99 · 2021-03-15T17:46:19Z

I think the check for devshell on aarch64 will fail. Because nix flake check tries to evaluate it(not build) and nixUnstable is still broken for aarch64 on nixos-unstable - hydra hasn't updated the channel with the fix yet.

nrdxp · 2021-03-15T17:52:29Z

Yeah I saw that, but actually every single derivation is failing due to an exception in hercules-ci's cpp code. I am gonna try reverting the update to the hercules flake and see if that helps. If not, it may be related to the update to Nix. I believe I can work around it either way, hopefully won't take too long.

nrdxp · 2021-03-16T02:36:41Z

It seems fixing the ci-agent is going to be slightly more involved than I initially suspected. hercules-ci/hercules-ci-agent#291

We may have to rely on upstream to patch it before it works. I was thinking I could use a container as a workaround, but the containers seem to use the hosts nix-daemon, so it didn't pan out.

So the alternatives seem to be:

to simply update the revision and run an older version on the ci-runner for now until a fix is found (will probably have to go with this one)
let it stay broken and manually merge this and other PR's hoping a bunch of stuff doesn't break (probably not this one)
use a virtual machine (might work but performance may suffer, I'll experiment)

Pacman99 · 2021-03-16T03:19:44Z

Is the ci runner on an old revision evaluate the checks ok? I think the first option is the best so at least we can keep changing things.

nrdxp · 2021-03-16T03:50:48Z

yes it seems to be working again after downgrading. Assuming all these checks pass, I'll push the changes with the nixos-unstable update.

nrdxp · 2021-03-16T03:54:38Z

Before we merge this, be sure to rebase after I push the update, because I'm also going to make a change to the bors.toml. After merging my last PR as a squash merge, I decided I don't like it, and am going to use a regular merge commit instead. BORS pulls the toml from the PR and not the main branch so it's important you have the new one.

Pacman99 · 2021-03-16T03:58:45Z

Ok thats makes sense, will do. Yeah squash merges aren't great for larger PR's.

nrdxp · 2021-03-20T04:23:41Z

besides the merge conflicts, is this still blocked?

Pacman99 · 2021-03-20T22:31:43Z

I rebased and fixed conflicts. But I can't actually test this new version, so after the update I will have to rebase again and run through any errors that may have come up after the rebase.

Also I switched to use user@host pairs based on the pr in home-manager that adds --flake support to home manager: nix-community/home-manager#1856.

nrdxp

My review was pretty brief on this as I don't really have a ton of time and don't want to hold it up any longer. I've already delegated merge permission to you, so if you've tested it and believe it is ready. Please go ahead and merge it.

nrdxp · 2021-03-23T02:18:11Z

bors try

bors · 2021-03-23T02:18:20Z

try

Build failed:

ci/hercules/evaluation

Pacman99 · 2021-03-23T14:48:37Z

This is ready for final review. I updated the home-manager input in this PR(first commit), and rebased with core. Checks succeed for me. I added some docs to users/README.md, so I think thats the last thing to check - @blaggacao I would appreciate your input for that, you're usually good with docs..
bors try

bors · 2021-03-23T14:51:17Z

try

Build succeeded:

flake.nix

hosts/default.nix

nrdxp

I don't think any of my comments are major blockers. Overall this looks great. Good job on another great PR.

nrdxp · 2021-03-23T16:41:57Z

CHANGELOG.md

@@ -43,6 +43,7 @@
 - iso: avoid systemd service startup [\#202](https://github.com/divnix/devos/pull/202)
 - hosts/devosSystem: pass modules as attrset [\#198](https://github.com/divnix/devos/pull/198)
 - doc: enact bootstrapping section [\#196](https://github.com/divnix/devos/pull/196)
+- flake: nixos -\> nixos-unstable [\#192](https://github.com/divnix/devos/pull/192)


why is this here? Isn't it already in core?

rebase error, fixed

…-manager only defaults

Pacman99 · 2021-03-23T17:18:29Z

I'm going to go ahead and merge, this should be complete.
bors r+

bors · 2021-03-23T17:20:18Z

Build succeeded:

nrdxp

We are gonna need a follow up, cleanup/enhancement PR. I wouldn't mind doing it, if nobody else wishes to do so. I've mentions a copule places that need fixing.

nrdxp · 2021-03-24T22:40:21Z

lib/devos/devosSystem.nix

+              home-manager.useUserPackages = lib.mkForce false;
+              home-manager.sharedModules = [
+                {
+                  home.packages = config.environment.systemPackages;


hindsight review:

As it turns out, this may be a bit too aggressive. We may have to have a follow up PR to remove it, as it includes a lot of unrelated system packages and, worse, causes package conflicts in the community branch.

Yeah thats fine we can remove that line. It worked for me and I didn't think it would cause any issues.

nrdxp · 2021-03-24T22:42:08Z

hosts/default.nix

+        useGlobalPkgs = true;
+        useUserPackages = true;
+
+        extraSpecialArgs = extern.userSpecialArgs // { suites = suites.user; };


We may need to add inherit (config.nixpkgs) pkgs as home-manager doesn't seem to have a pkgs argument. This may be a bug upstream however, as I believe home-manager is supposed to pass in packages. In practice though, at least with this implementation, I had to add a pkgs arg to get doom-emacs to work, for example.

Are you sure thats the problem? Because home-manager already does that when useGlobalPkgs is true.

" using the system configuration's pkgs
argument in Home Manager. This disables the Home Manager
options nixpkgs.*"
^ from hm nixos module

I ran into these issues while trying to merge this into community. I just pushed the merge so you should now be able to reproduce by simply checking out the latest community branch and running flk home NixOS nixos

Perhaps shared modules don't get passed a pkgs arg, since that is how doom-emacs is entering the configuration. Not sure if this intentional by upstream or a bug. I'll search hm issue tracker to see if I can find some additional context.

Could be true, I tried adding pkgs to the hm section in users/nixos and used it to add vim to home.packages and that works fine. So your theory does make sense.

In home manager, the ways pkgs is set is using _module.args.pkgs in home-manager/modules/modules.nix. And I wonder if doesn't get passed to modules that were imported before that. But that doesn't make sense to me, since the module system should be recursive.

I believe I've isolated the problem. Gonna fix and test locally then will link to an new home-manager PR if I'm correct.

okay, I've offered a minimally invasive solution in nix-community/home-manager#1879

Pacman99 · 2021-03-24T22:49:00Z

We are gonna need a follow up, cleanup/enhancement PR. I wouldn't mind doing it, if nobody else wishes to do so. I've mentions a copule places that need fixing.

I likely won't be able to test the fixes since I can't reproduce the issues.

Pacman99 marked this pull request as draft March 9, 2021 21:03

nrdxp self-requested a review March 10, 2021 06:35

Pacman99 force-pushed the first-user branch 3 times, most recently from 313d5a2 to 79f9bfc Compare March 14, 2021 08:41

Pacman99 force-pushed the first-user branch from 79f9bfc to e19a013 Compare March 14, 2021 23:40

Pacman99 marked this pull request as ready for review March 14, 2021 23:48

Pacman99 force-pushed the first-user branch from d62b4a0 to 50f6272 Compare March 15, 2021 00:25

Pacman99 force-pushed the first-user branch from 50f6272 to d6dcdf8 Compare March 15, 2021 16:54

Pacman99 force-pushed the first-user branch from d6dcdf8 to 1d441bd Compare March 16, 2021 21:50

Pacman99 force-pushed the first-user branch from ad1862b to b1c3552 Compare March 20, 2021 22:28

Pacman99 force-pushed the first-user branch from b1c3552 to 26f6357 Compare March 21, 2021 16:26

nrdxp approved these changes Mar 22, 2021

View reviewed changes

bors bot added a commit that referenced this pull request Mar 23, 2021

Try #156:

14812ed

Pacman99 force-pushed the first-user branch from 26f6357 to 681d9e5 Compare March 23, 2021 14:45

bors bot added a commit that referenced this pull request Mar 23, 2021

Try #156:

829fd70

Pacman99 force-pushed the first-user branch from 681d9e5 to 71ed9bc Compare March 23, 2021 14:56

nrdxp reviewed Mar 23, 2021

View reviewed changes

flake.nix Outdated Show resolved Hide resolved

nrdxp reviewed Mar 23, 2021

View reviewed changes

hosts/default.nix Show resolved Hide resolved

Pacman99 force-pushed the first-user branch from 71ed9bc to ed9d34f Compare March 23, 2021 16:40

nrdxp approved these changes Mar 23, 2021

View reviewed changes

Pacman99 added 5 commits March 23, 2021 09:43

flake/home: release -> master

0e5aa65

users: add and export user modules and import them in hosts

416a52f

users: wire up all user profiles to userSuites and pass to home-manager

cc52cd9

export homeConfigurations for all user@host pairs and add better home…

dc9d987

…-manager only defaults

users: add Home Manager section to doc

6d51ed0

Pacman99 force-pushed the first-user branch from ed9d34f to 6d51ed0 Compare March 23, 2021 16:43

nrdxp approved these changes Mar 23, 2021

View reviewed changes

bors bot merged commit 74c23ce into divnix:core Mar 23, 2021

Pacman99 deleted the first-user branch March 23, 2021 17:21

nrdxp reviewed Mar 24, 2021

View reviewed changes

Pacman99 mentioned this pull request Apr 11, 2021

Improve Home Manager support: profiles/suites, modules, extern, flake outputs #119

Closed

Improve Home Manager support: profiles/suites, modules, extern, flake outputs #156

Improve Home Manager support: profiles/suites, modules, extern, flake outputs #156

Conversation

Pacman99 commented Mar 9, 2021

nrdxp commented Mar 13, 2021

bors bot commented Mar 13, 2021

try

Pacman99 commented Mar 14, 2021

nrdxp commented Mar 14, 2021

bors bot commented Mar 14, 2021

Pacman99 commented Mar 14, 2021

nrdxp commented Mar 14, 2021

Pacman99 commented Mar 14, 2021

nrdxp commented Mar 14, 2021

Pacman99 commented Mar 14, 2021

Pacman99 commented Mar 15, 2021

nrdxp commented Mar 15, 2021

Pacman99 commented Mar 15, 2021

nrdxp commented Mar 15, 2021

nrdxp commented Mar 16, 2021

Pacman99 commented Mar 16, 2021

nrdxp commented Mar 16, 2021

nrdxp commented Mar 16, 2021

Pacman99 commented Mar 16, 2021

nrdxp commented Mar 20, 2021

Pacman99 commented Mar 20, 2021

nrdxp left a comment

Choose a reason for hiding this comment

nrdxp commented Mar 23, 2021

bors bot commented Mar 23, 2021

try

Pacman99 commented Mar 23, 2021

bors bot commented Mar 23, 2021

try

nrdxp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pacman99 commented Mar 23, 2021

bors bot commented Mar 23, 2021

nrdxp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hindsight review:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nrdxp Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

nrdxp Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pacman99 Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

nrdxp Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pacman99 commented Mar 24, 2021

nrdxp Mar 24, 2021 •

edited

Loading

nrdxp Mar 24, 2021 •

edited

Loading

Pacman99 Mar 24, 2021 •

edited

Loading

nrdxp Mar 24, 2021 •

edited

Loading