lib.evalModules: add graph attribute #403839

mightyiam · 2025-05-03T08:01:45Z

For various module evaluation design/refactor analysis purposes one may want to obtain a data structure that represents all the modules that took part of a configuration evaluation.

This PR adds an attribute to the return value of lib.evalModules with such a data structure.

Initially discussed here.

Things done

Co-authored-by: @A-jay98

Add a 👍 reaction to pull requests you find important.

nixos-discourse · 2025-05-31T08:16:40Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/obtaining-a-nixpkgs-module-system-configuration-modules-graph/63286/4

mightyiam · 2025-06-07T07:54:38Z

Should this new attribute be nested inside another attrset to reserve a place for more metrics in the future?

Does the design otherwise make sense?

To us it seems like this does not change a hot evaluation path.
We did not make any benchmark comparisons.

nixos-discourse · 2025-06-07T07:55:45Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/obtaining-a-nixpkgs-module-system-configuration-modules-graph/63286/5

nixos-discourse · 2025-06-07T07:56:40Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5547

hsjobeki · 2025-06-08T09:25:01Z

Since our CI only performs a nixpkgs evaluation, for this PR it would be nice to also include a full nixos evaluation comparison.
Which is what I did manually to support this PR:

Before: 98ce1f5ab866f073a94bfa7b0408f83c926a6202 (master @ Fri Jun 6 09:57:21 2025 +0200)
After: 4cae774beb0bf838e8380b33317d6573b9bd0ba0 (this PR )
Expr:

nixos = import ./nixos/lib/eval-config.nix {
  modules = [
    {
      fileSystems."/".device = "/dev/null";
      boot.loader.systemd-boot.enable = true;
    }
    {
      nixpkgs.system = "x86_64-linux";
    }
  ];
};

Command: NIX_SHOW_STATS=1 NIX_SHOW_STATS_PATH=stats_after.json nix-instantiate ./test.nix -A 'nixos.config.system.build.toplevel' --eval-only > /dev/null

Note: CPU, and time is noisy, but i.e. nrThunks should be reliably reproducible on your system.

Key	Before	After	Δ Abs	Δ %
cpuTime	2.150998	2.760706	0.609708	28.345345%
envs.bytes	101835320	102368096	532776	0.523174%
envs.elements	7267506	7304057	36551	0.502937%
envs.number	5461909	5491955	30046	0.550101%
gc.cycles	3	3	0	0.000000%
gc.heapSize	503767040	520544256	16777216	3.330352%
gc.totalBytes	756090320	757450912	1360592	0.179951%
list.bytes	13065432	13068024	2592	0.019839%
list.concats	163321	163334	13	0.007960%
list.elements	`1633179`	1633503	324	0.019839%
nrAvoided	5469102	5495155	26053	0.476367%
nrExprs	1389685	1390106	421	0.030295%
nrFunctionCalls	4813837	4837405	23568	0.489589%
nrLookups	2809486	2816136	6650	0.236698%
nrOpUpdateValuesCopied	10619533	10619952	419	0.003946%
nrOpUpdates	327979	327945	-34	-0.010367%
nrPrimOpCalls	2224074	2224614	540	0.024280%
nrThunks	6345976	6365930	19954	0.314435%
sets.bytes	267236128	267395280	159152	0.059555%
sets.elements	15401395	15409077	7682	0.049879%
sets.number	1300863	1303128	2265	0.174115%
sizes.Attr	16	16	0	0.000000%
sizes.Bindings	16	16	0	0.000000%
sizes.Env	8	8	0	0.000000%
sizes.Value	24	24	0	0.000000%
symbols.bytes	1019925	1020225	300	0.029414%
symbols.number	84364	84387	23	0.027263%
time.cpu	2.150998	2.760706	0.609708	28.345345%
time.gc	0.475000	0.615000	0.140000	29.473684%
time.gcFraction	0.220828	0.222769	0.001941	0.879143%
values.bytes	249627336	250148856	521520	0.208919%
values.number	10401139	10422869	21730	0.208919%

hsjobeki

Overall looking very nice. Some minor things should be fixed.

doc/module-system/module-system.chapter.md

lib/modules.nix

hsjobeki · 2025-06-08T09:45:19Z

lib/modules.nix

      #
-      # Filters a structure as emitted by collectStructuredModules by removing all disabled
+      # Filters a `[ StructuredModule ]` by removing all disabled


The term StructuredModule is never defined, would be nice to add a defining comment somewhere

I kind of don't want to. Perhaps unnecesarry?

lib/modules.nix

lib/tests/modules/graph/test.nix

roberth

I believe it's possible to significantly reduce the amount of memory retained during normal, non-graph evaluation.
Unfortunately, I can't reproduce the increase in gc.heapSize due to rounding (NixOS/nix#13336), so I'm unable to confirm as of now.

lib/modules.nix

lib/tests/modules.sh

lib/modules.nix

roberth · 2025-06-08T15:57:43Z

doc/module-system/module-system.chapter.md

+Represents all the modules that took part in the evaluation.
+It is a list of `ModuleGraph` where `ModuleGraph` has the following attributes:
+
+- `key`: `string`


file seems useful for troubleshooting and should also be in the module attrset.
This is not implemented yet.

Suggested change

- `key`: `string`

- `key`: `string` for the purpose of module deduplication and `disabledModules`

- `file`: `string` for the purpose of error messages and warnings

We applied the docstring for key and added a file. Please review to make sure we did that right thing.

hsjobeki · 2025-06-19T14:20:27Z

lib/tests/modules.sh

@@ -55,7 +59,7 @@ evalConfig() {
    local attr=$1
    shift
    local script="import ./default.nix { modules = [ $* ];}"
-    nix-instantiate --timeout 1 -E "$script" -A "$attr" --eval-only --show-trace --read-write-mode --json


I'd prefer keeping the nix-instantiate as is and provide the additional arguments that you need via checkExpression

That change has a subtle altering affect to checkConfigOutput like adding --strict and removing -A "$attr.
That change to the testing framework is probably possible, but i wouldn't change it, if there is no motivation behind that.

Fiy: Will need a bit more time to review the hard part of the changes 😄

You're right about the --strict. We've now moved --strict out of local-nix-instantiate and into `checkExpression.

Not sure what you mean with the -A "$attr".

It seems that now behavior of existing tests is preserved.

mightyiam · 2025-07-05T05:46:03Z

lib/modules.nix

+      # This function takes an empty attrset as an argument.
+      # It could theoretically be replaced with its body,
+      # but such a binding is avoided to allow for earlier grabage collection.
+      doCollect =
+        { }:
+        collectModules class (specialArgs.modulesPath or "") (regularModules ++ [ internalModule ]) (
+          {
+            inherit
+              lib
+              options
+              config
+              specialArgs
+              ;
+            _class = class;
+            _prefix = prefix;
+          }
+          // specialArgs
+        );


Here's the doCollect. Is the comment correct?

mightyiam · 2025-08-02T15:24:51Z

I used this recently and it was helpful to me 🪄

drupol · 2025-08-02T15:32:48Z

I used this recently and it was helpful to me 🪄

It would be great to see a practical use case for this, to illustrate a bit more what this does.

mightyiam · 2025-08-03T02:37:56Z

It gave me a list of all files in which I was defining flake.modules.nixos.pc. The alternative to that is error prone text search.

Co-authored-by: Shahar "Dawn" Or <[email protected]>

Co-authored-by: Ali Jamadi <[email protected]>

mightyiam · 2025-08-07T04:39:33Z

Spent some time rebasing this.

roberth

Awesome!

Resolved

nixos-discourse · 2025-08-07T16:34:24Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixpkgs-module-system-config-modules-graph/67722/1

mightyiam force-pushed the modules-graph branch from 37facfa to 84f3dee Compare May 10, 2025 05:07

mightyiam force-pushed the modules-graph branch 2 times, most recently from b8928a3 to 2a5c8ff Compare May 31, 2025 07:38

A-jay98 force-pushed the modules-graph branch from 2a5c8ff to d097006 Compare May 31, 2025 07:59

A-jay98 force-pushed the modules-graph branch from d097006 to 72cf6f2 Compare June 7, 2025 06:36

mightyiam force-pushed the modules-graph branch 2 times, most recently from e2f87c8 to 724fccd Compare June 7, 2025 07:48

github-actions bot added the 8.has: documentation This PR adds or changes documentation label Jun 7, 2025

mightyiam force-pushed the modules-graph branch from 724fccd to 4cae774 Compare June 7, 2025 07:50

mightyiam marked this pull request as ready for review June 7, 2025 07:54

nix-owners bot requested review from hsjobeki, GetPsyched, roberth, fricklerhandwerk and infinisil June 7, 2025 07:56

mightyiam changed the title ~~lib.evalModules: add modules tree report attribute~~ lib.evalModules: add graph attribute Jun 7, 2025

hsjobeki previously requested changes Jun 8, 2025

View reviewed changes

roberth reviewed Jun 8, 2025

View reviewed changes

lib/modules.nix Outdated Show resolved Hide resolved

lib/modules.nix Outdated Show resolved Hide resolved

lib/modules.nix Outdated Show resolved Hide resolved

hsjobeki reviewed Jun 8, 2025

View reviewed changes

lib/tests/modules.sh Outdated Show resolved Hide resolved

roberth reviewed Jun 8, 2025

View reviewed changes

mightyiam requested review from roberth and hsjobeki June 14, 2025 10:13

hsjobeki reviewed Jun 19, 2025

View reviewed changes

roberth mentioned this pull request Jun 21, 2025

nixos: Add meta.tests #413892

Open

13 tasks

nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 25, 2025

mightyiam force-pushed the modules-graph branch from 4426b3b to b667776 Compare June 28, 2025 05:22

ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jun 28, 2025

mightyiam force-pushed the modules-graph branch from b667776 to 8ab084b Compare June 28, 2025 05:39

A-jay98 force-pushed the modules-graph branch from 8ab084b to dfa25a7 Compare July 5, 2025 05:41

mightyiam commented Jul 5, 2025

View reviewed changes

mightyiam force-pushed the modules-graph branch from dfa25a7 to a9ea851 Compare August 1, 2025 13:47

mightyiam added a commit to mightyiam/infra that referenced this pull request Aug 1, 2025

https://github.com/NixOS/nixpkgs/pull/403839

4616998

mightyiam added a commit to mightyiam/infra that referenced this pull request Aug 1, 2025

https://github.com/NixOS/nixpkgs/pull/403839

b80fd03

mightyiam added a commit to mightyiam/infra that referenced this pull request Aug 2, 2025

https://github.com/NixOS/nixpkgs/pull/403839

3b2da3f

nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 5, 2025

A-jay98 and others added 2 commits August 7, 2025 11:27

lib/modules: extract isDisabled from filterModules

2c1ec14

Co-authored-by: Shahar "Dawn" Or <[email protected]>

lib.evalModules: add graph attribute

5186921

Co-authored-by: Ali Jamadi <[email protected]>

mightyiam force-pushed the modules-graph branch from a9ea851 to 5186921 Compare August 7, 2025 04:38

nixpkgs-ci bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 7, 2025

mightyiam added a commit to mightyiam/infra that referenced this pull request Aug 7, 2025

https://github.com/NixOS/nixpkgs/pull/403839

0a3bd26

roberth approved these changes Aug 7, 2025

View reviewed changes

roberth enabled auto-merge August 7, 2025 08:27

roberth merged commit 46343ce into NixOS:master Aug 7, 2025
24 of 27 checks passed

mightyiam deleted the modules-graph branch August 7, 2025 08:47

	- `key`: `string`
	- `key`: `string` for the purpose of module deduplication and `disabledModules`
	- `file`: `string` for the purpose of error messages and warnings

Uh oh!

lib.evalModules: add graph attribute #403839

lib.evalModules: add graph attribute #403839

Uh oh!

Conversation

mightyiam commented May 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Things done

Uh oh!

nixos-discourse commented May 31, 2025

Uh oh!

mightyiam commented Jun 7, 2025

Uh oh!

nixos-discourse commented Jun 7, 2025

Uh oh!

nixos-discourse commented Jun 7, 2025

Uh oh!

hsjobeki commented Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hsjobeki left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hsjobeki Jun 8, 2025

Choose a reason for hiding this comment

Uh oh!

mightyiam Jun 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

roberth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

roberth Jun 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mightyiam Jun 14, 2025

Choose a reason for hiding this comment

Uh oh!

hsjobeki Jun 19, 2025

Choose a reason for hiding this comment

Uh oh!

hsjobeki Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mightyiam Jun 28, 2025

Choose a reason for hiding this comment

Uh oh!

mightyiam Jul 5, 2025

Choose a reason for hiding this comment

Uh oh!

mightyiam commented Aug 2, 2025

Uh oh!

drupol commented Aug 2, 2025

Uh oh!

mightyiam commented Aug 3, 2025

Uh oh!

mightyiam commented Aug 7, 2025

Uh oh!

roberth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nixos-discourse commented Aug 7, 2025

Uh oh!

Uh oh!

mightyiam commented May 3, 2025 •

edited

Loading

hsjobeki commented Jun 8, 2025 •

edited

Loading

roberth Jun 8, 2025 •

edited

Loading

hsjobeki Jun 19, 2025 •

edited

Loading