Skip to content

lib.evalModules: Add extendModules and type to result#143207

Merged
roberth merged 7 commits intoNixOS:masterfrom
hercules-ci:evalModules-add-extendModules-and-type
Nov 1, 2021
Merged

lib.evalModules: Add extendModules and type to result#143207
roberth merged 7 commits intoNixOS:masterfrom
hercules-ci:evalModules-add-extendModules-and-type

Conversation

@roberth
Copy link
Member

@roberth roberth commented Oct 27, 2021

Motivation for this change

Primary goal: allow configuration systems to be embedded in other configuration systems more easily.

  • Allows the simultaneous construction of top-level invocations and
    submodule types. Any configuration system that previously only exposed evalConfig can now also expose a type easily.

  • Allow NixOS specialisations to be expressed cleanly

Side catch:

  • Improve specialisation option docs

Example integration of modules into a configuration system (à la NixOS / home manager / arion / etc)

let
  baseConfig = lib.types.evalModules {
    modules = myBuiltInModules;
    inherit specialArgs;
  };
in {
  inherit (baseConfig) type;
  evalConfig = configuration: baseConfig.extendModules { modules = [ configuration ]; };
}

This is the successor of #143133.

Closes #141733

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@roberth roberth requested a review from infinisil October 27, 2021 18:49
@roberth roberth requested review from edolstra and nbp as code owners October 27, 2021 18:49
@roberth roberth force-pushed the evalModules-add-extendModules-and-type branch 3 times, most recently from 498e5bc to 45ca3b6 Compare October 27, 2021 19:04
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 27, 2021
@roberth
Copy link
Member Author

roberth commented Oct 28, 2021

Tests pass and hashes are unchanged for nixosTests.nginx, nixosTests.specialisation and the manual ((import ./. {}).nixos {}).config.system.build.manual.manualHTML. EDIT: specialisation only until the "rephrase" commit, which changes option order. EDIT 2: Reverted that to expedite this pr.

@roberth roberth mentioned this pull request Oct 28, 2021
12 tasks
roberth added a commit to hercules-ci/arion that referenced this pull request Oct 28, 2021
TODO:
 - [ ] NixOS/nixpkgs#143207
 - [ ] switch to master or nixos-unstable with niv
roberth added a commit to hercules-ci/arion that referenced this pull request Oct 28, 2021
TODO:
 - [ ] NixOS/nixpkgs#143207
 - [ ] switch to master or nixos-unstable with niv
@roberth roberth mentioned this pull request Oct 28, 2021
2 tasks
@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 29, 2021
@dasJ dasJ mentioned this pull request Oct 29, 2021
16 tasks
@ofborg ofborg bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Oct 29, 2021
@roberth
Copy link
Member Author

roberth commented Oct 29, 2021

acme test also works.

I've dug through the issues and PRs and found no surprises.

@roberth roberth force-pushed the evalModules-add-extendModules-and-type branch from 90b7511 to dc5764a Compare October 29, 2021 13:05
@roberth
Copy link
Member Author

roberth commented Oct 29, 2021

I'm out of ideas to further validate and test this solution. Please review :)

@roberth roberth added 0.kind: enhancement Add something new or improve an existing system. 6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: module system About "NixOS" module system internals 8.has: documentation This PR adds or changes documentation labels Oct 29, 2021
Copy link
Member Author

@roberth roberth Oct 29, 2021

Choose a reason for hiding this comment

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

I'm pretty sure this broke cross-compiled specialisations before.

@roberth roberth added 0.kind: bug Something is broken 8.has: clean-up This PR removes packages or removes other cruft labels Oct 29, 2021
Allows the simultaneous construction of top-level invocations and
submodule types.

This helps structure configuration systems integration code.
Also works for (pkgs.nixos {}).type.
…serModules"

This reverts commit e2bea44.

While this commit was probably fine, I want to be conservative
with changes right before the release branch-off.
So far the extendModules commits have been adding and refactoring
expressions that did not affect derivation hashes, whereas this
commit changes the module ordering. I will open a separate PR for
it.
@roberth roberth force-pushed the evalModules-add-extendModules-and-type branch from dc5764a to 0b0d263 Compare November 1, 2021 08:45
@github-actions github-actions bot removed 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 1, 2021
@roberth roberth removed 0.kind: bug Something is broken 6.topic: cross-compilation Building packages on a different platform than they will be used on labels Nov 1, 2021
@ofborg ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 1, 2021
@roberth
Copy link
Member Author

roberth commented Nov 1, 2021

Considering that the design is already the result of discussion with @infinisil and I've validated the design in three use cases and I've reduced the potential impact to near zero by reverting the specialisations commit, I will go ahead and merge this in order not to stall progress. Limited existence/availability of maintainers for this very specific code was also a factor of consideration. I'll be happy to receive feedback after the merge and I am available (and capable) to resolve any issues if they do happen to arise. I don't merge my own PRs lightly.

@infinisil
Copy link
Member

Sounds good!

infinisil added a commit to infinisil/nixus that referenced this pull request Dec 22, 2021
Using NixOS/nixpkgs#143207 allows some nice
cleanup and fixes #41

This commit here notably requires a version after the above nixpkgs PR,
and versions before the above nixpkgs PR don't work with it. While it
would be possible to keep it backwards compatible, it doesn't seem worth
the trouble.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.kind: enhancement Add something new or improve an existing system. 6.topic: module system About "NixOS" module system internals 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: clean-up This PR removes packages or removes other cruft 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants