doc: use stricter, more minimal module evals#1212
doc: use stricter, more minimal module evals#12120xda157 merged 6 commits intonix-community:masterfrom
Conversation
47357db to
65236b5
Compare
|
shouldn't using absolute paths with |
65236b5 to
edbc82d
Compare
I'm in favour of keeping PR scope small; this makes it easier to review the changes and also reduces the pressure on contributors wanting to make small gradual improvements. Refactoring the module path style can't be done with this PR alone, as we'd additionally need to remove the Should be doable with a small follow up PR, though 🙂 |
Sounds good |
2ab82a7 to
58da2a3
Compare
58da2a3 to
43c52e7
Compare
43c52e7 to
6d24e7e
Compare
dddfcae to
7f06836
Compare
d319af2 to
99526e1
Compare
99526e1 to
9d745f6
Compare
dc40c6c to
2ceab87
Compare
2ceab87 to
16a622c
Compare
trueNAHO
left a comment
There was a problem hiding this comment.
From b80a8d6ae9e0b2e8e9d3e799e5d4dd56c6b7c3ec Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Fri, 13 Jun 2025 09:56:12 +0100 Subject: [PATCH 1/5] stylix: use a static example in `mkEnableWallpaper` --- stylix/target.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stylix/target.nix b/stylix/target.nix index f8b6861b2..98a84f8e0 100644 --- a/stylix/target.nix +++ b/stylix/target.nix @@ -113,7 +113,7 @@ lib.literalExpression "config.stylix.image != null" else false; - example = config.stylix.image == null; + example = true; }; mkEnableIf =
Following up on the discussion from #1212 (comment), I believe the true intention is to simply negate default such that true and false are used in default and example:
- example = config.stylix.image == null;
+ example = !(config.stylix.image != null && autoEnable);In that case, a cleaner solution would be to get the default value into scope with an explicit let-in scope (or a silly rec):
- example = config.stylix.image == null;
+ example = !default;From eddff6b48d0b5435ced05a97e2201a4cec7dd472 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Fri, 13 Jun 2025 10:17:16 +0100 Subject: [PATCH 2/5] stylix: set defaultText in font size options --- stylix/fonts.nix | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/stylix/fonts.nix b/stylix/fonts.nix index 4d5dbb72f..1234df1a5 100644 --- a/stylix/fonts.nix +++ b/stylix/fonts.nix @@ -58,7 +58,12 @@ in mkFontSizeOption = { default, target }: lib.mkOption { - inherit default; + default = if builtins.isInt default then default else cfg.sizes.${default}; + defaultText = + if builtins.isInt default then + default + else + lib.literalExpression "config.stylix.fonts.sizes.${default}"; description = '' The font size used for ${target}. @@ -94,12 +99,12 @@ in terminal = mkFontSizeOption { target = "terminals and text editors"; - default = cfg.sizes.applications; + default = "applications"; }; popups = mkFontSizeOption { target = "notifications, popups, and other overlay elements of the desktop"; - default = cfg.sizes.desktop; + default = "desktop"; }; };
I think the previous behaviour was better because it is simpler and more obvious by avoiding an additional string indirection.
From 44d85260ea146ab96f60fc3e21fb05a45149e5bb Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Sun, 4 May 2025 03:03:00 +0100 Subject: [PATCH 3/5] doc: use minimal module evals We don't actually need fully blown NixOS or home-manager configurations just to read the declared `options`. Instead, we can directly call `lib.evalModules` to build a minimal configuration containing only stylix modules. --- doc/default.nix | 37 +++++++++++-------------------------- doc/eval_compat.nix | 25 +++++++++++++++++++++++++ doc/hm_compat.nix | 10 ++++++++++ flake/packages.nix | 2 -- 4 files changed, 46 insertions(+), 28 deletions(-) create mode 100644 doc/eval_compat.nix create mode 100644 doc/hm_compat.nix diff --git a/doc/default.nix b/doc/default.nix index 04f090231..343e69010 100644 --- a/doc/default.nix +++ b/doc/default.nix @@ -2,9 +2,6 @@ lib, pkgs, inputs, - nixosSystem, - homeManagerConfiguration, - system, callPackage, writeText, stdenvNoCC, @@ -17,38 +14,26 @@ let # Prefix to remove from option declaration file paths. rootPrefix = toString ../. + "/"; - nixosConfiguration = nixosSystem { - inherit system; - modules = [ - inputs.home-manager.nixosModules.home-manager - inputs.self.nixosModules.stylix - ]; - }; - - homeConfiguration = homeManagerConfiguration { - inherit pkgs; - modules = [ - inputs.self.homeModules.stylix - { - home = { - homeDirectory = "/home/book"; - stateVersion = "22.11"; - username = "book"; - }; - } - ]; - }; + evalDocs = + module: + lib.evalModules { + modules = [ ./eval_compat.nix ] ++ lib.toList module; + specialArgs = { inherit pkgs; }; + }; # TODO: Include Nix Darwin options platforms = { home_manager = { name = "Home Manager"; - configuration = homeConfiguration; + configuration = evalDocs [ + inputs.self.homeModules.stylix + ./hm_compat.nix + ]; }; nixos = { name = "NixOS"; - configuration = nixosConfiguration; + configuration = evalDocs inputs.self.nixosModules.stylix; }; }; diff --git a/doc/eval_compat.nix b/doc/eval_compat.nix new file mode 100644 index 000000000..63f26d0ab --- /dev/null +++ b/doc/eval_compat.nix @@ -0,0 +1,25 @@ +{ lib, ... }: +{ + options = { + # Many modules assume they can do `options.programs ? foo`. + # They should probably do `options ? programs.foo` instead, + # but we can work around the issue with a stub option: + programs.__stub = lib.mkSinkUndeclaredOptions { }; + + # The config.lib option, as found in NixOS and home-manager. + # Many option declarations depend on `config.lib.stylix`, + # so we need to provide a `lib` option. + lib = lib.mkOption { + type = lib.types.attrsOf lib.types.attrs; + description = '' + This option allows modules to define helper functions, constants, etc. + ''; + default = { }; + visible = false; + }; + }; + + # Third-party options are not included in the module eval, + # so disable checking options definitions have matching declarations + config._module.check = false; +} diff --git a/doc/hm_compat.nix b/doc/hm_compat.nix new file mode 100644 index 000000000..d3d3ebcf4 --- /dev/null +++ b/doc/hm_compat.nix @@ -0,0 +1,10 @@ +{ lib, ... }: +{ + # Many modules assume they can do `options.services ? foo`. + # They should probably do `options ? services.foo` instead, + # but we can work around the issue with a stub option: + options.services.__stub = lib.mkSinkUndeclaredOptions { }; + + # Some modules use home-manager's `osConfig` arg + config._module.args.osConfig = null; +} diff --git a/flake/packages.nix b/flake/packages.nix index 97e3fd205..995137da7 100644 --- a/flake/packages.nix +++ b/flake/packages.nix @@ -22,8 +22,6 @@ { doc = pkgs.callPackage ../doc { inherit inputs; - inherit (inputs.nixpkgs.lib) nixosSystem; - inherit (inputs.home-manager.lib) homeManagerConfiguration; }; serve-docs = pkgs.callPackage ../doc/server.nix { inherit (config.packages) doc;
This looks great!
Considering how small the doc/eval_compat.nix and doc/hm_compat.nix modules are, it might be better to just inline them, since they are also used only once.
The following comment, which is copy-pasted twice, should mention the __stub attribute being an arbitrary name:
# Many modules assume they can do `options.services ? foo`.
# They should probably do `options ? services.foo` instead,
# but we can work around the issue with a stub option:What about the following differently structured explanation that is also wrapped
at 80 characters (wrap accordingly in the code):
# Declare the arbitrarily named __stub attribute to allow modules to evaluate
# 'options.<ATTRIBUTE> ? <OPTION>'.
#
# TODO: Replace 'options.<ATTRIBUTE> ? <OPTION>' instances with 'options ?
# <ATTRIBUTE>.<OPTION>' to remove this __stub workaround.From 10bd1ef594712a632ce1c6d782b965fc97a7ee03 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Sun, 4 May 2025 03:40:30 +0100 Subject: [PATCH 4/5] doc: ensure `pkgs` is not used in option docs Providing a stub `pkgs` instance that has no contents ensures the docs does not _read_ `pkgs` anywhere. --- doc/default.nix | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/doc/default.nix b/doc/default.nix index 343e69010..491592b70 100644 --- a/doc/default.nix +++ b/doc/default.nix @@ -14,11 +14,24 @@ let # Prefix to remove from option declaration file paths. rootPrefix = toString ../. + "/"; + # A stub pkgs used while evaluating the stylix modules for the docs + noPkgs = { + # Needed for type-checking + inherit (pkgs) _type; + + # Permit access to (pkgs.formats.foo { }).type + formats = builtins.mapAttrs (_: fmt: args: { + inherit (fmt args) type; + }) pkgs.formats; + }; + evalDocs = module: lib.evalModules { modules = [ ./eval_compat.nix ] ++ lib.toList module; - specialArgs = { inherit pkgs; }; + specialArgs = { + pkgs = noPkgs; + }; }; # TODO: Include Nix Darwin options
LGTM.
From 16a622ca67c26c1e931e7627ec338e28ca0f4bff Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Tue, 10 Jun 2025 19:33:29 +0100 Subject: [PATCH 5/5] doc: ensure `config` is not used in option docs With a few exceptions for `lib.stylix` functions. Once these exceptions are removed, we will have solved #98. --- doc/default.nix | 36 ++++++++++++++++++++++++++++++++++++ doc/eval_compat.nix | 22 ++++------------------ 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/doc/default.nix b/doc/default.nix index 491592b70..16ad3f706 100644 --- a/doc/default.nix +++ b/doc/default.nix @@ -25,12 +25,48 @@ let }) pkgs.formats; }; + # A stub config used while evaluating the stylix modules for the docs + # + # To resolve #98, this should be simplified to `noConfig = {}`. + # However, currently we access option-declaring functions via + # `config.lib.stylix`.
It might be better to replace #98 with https://github.com/nix-community/stylix/issues/98 in the commit message and the code comment to make the reference easier accessible. Also, the second paragraph in the code comment might benefit from a TODO: or NOTE: marker, although TODO: seems more appropriate here.
+ noConfig = + let + configuration = evalDocs { + # The config.lib option, as found in NixOS and home-manager. + # Currently required by the `target.nix` module. + options.lib = lib.mkOption { + type = lib.types.attrsOf lib.types.attrs; + description = '' + This option allows modules to define helper functions, constants, etc. + ''; + default = { }; + visible = false; + }; + + # The target.nix module defines functions that are currently needed to + # declare options + imports = [ ../stylix/target.nix ]; + }; + in + { + lib.stylix = { + inherit (configuration.config.lib.stylix) + mkEnableIf + mkEnableTarget + mkEnableTargetWith + mkEnableWallpaper + ; + }; + }; + evalDocs = module: lib.evalModules { modules = [ ./eval_compat.nix ] ++ lib.toList module; specialArgs = { pkgs = noPkgs; + config = noConfig; }; }; diff --git a/doc/eval_compat.nix b/doc/eval_compat.nix index 63f26d0ab..19d85ff53 100644 --- a/doc/eval_compat.nix +++ b/doc/eval_compat.nix @@ -1,23 +1,9 @@ { lib, ... }: { - options = { - # Many modules assume they can do `options.programs ? foo`. - # They should probably do `options ? programs.foo` instead, - # but we can work around the issue with a stub option: - programs.__stub = lib.mkSinkUndeclaredOptions { }; - - # The config.lib option, as found in NixOS and home-manager. - # Many option declarations depend on `config.lib.stylix`, - # so we need to provide a `lib` option. - lib = lib.mkOption { - type = lib.types.attrsOf lib.types.attrs; - description = '' - This option allows modules to define helper functions, constants, etc. - ''; - default = { }; - visible = false; - }; - }; + # Many modules assume they can do `options.programs ? foo`. + # They should probably do `options ? programs.foo` instead, + # but we can work around the issue with a stub option: + options.programs.__stub = lib.mkSinkUndeclaredOptions { }; # Third-party options are not included in the module eval, # so disable checking options definitions have matching declarations
LGTM.
This defeats the point of the change. The current patch was discussed briefly here: #1212 (comment) EDIT: I thought about this some more. Since the function already assumes
Sorry, I'm unclear what behaviour you prefer? Could you post a concrete suggestion? The intention of the change was to allow fonts to specify a string (representing a stylix font option name) or an int (which should be a literal value). In the case that an option is referenced, a
If you prefer. I moved them to separate files as I find that idiomatic for modules. Plus it keeps implementation details "out of site" so they don't distract from the detail of the main code. They were bigger when I first implemented them though, now they are smaller there's a stronger argument for inlining.
SGTM 👍 EDIT: Done
SGTM 👍 EDIT: Done |
16a622c to
9f3f5be
Compare
Previously the example evaluated `config.stylix.image`, which should not be done by the docs. Since `mkEnableWallpaper` already assumes `autoEnable` is static when it evaluates `defaultText`, we can do the same for `example`.
We don't actually need fully blown NixOS or home-manager configurations just to read the declared `options`. Instead, we can directly call `lib.evalModules` to build a minimal configuration containing only stylix modules.
9f3f5be to
d38823b
Compare
The docs no longer needs access to arbitrary flake inputs. We should enforce this strictly by taking `self` instead.
Providing a stub `pkgs` instance that has no contents ensures the docs does not _read_ `pkgs` anywhere.
With a few exceptions for `lib.stylix` functions. Once these exceptions are removed, we will have solved the issue with NixOS's documentation.nixos.includeAllModules option. See nix-community#98
d38823b to
3015a06
Compare
trueNAHO
left a comment
There was a problem hiding this comment.
Testing
I've tested that the docs still build.
I've spot checked that a few pages look the same as the current docs.
For reference, applying the
diff --git a/doc/default.nix b/doc/default.nix
index 25b8d50..0a0462e 100644
--- a/doc/default.nix
+++ b/doc/default.nix
@@ -372,7 +372,7 @@ let
# Permalink to view a source file on GitHub. If the commit isn't known,
# then fall back to the latest commit.
- declarationCommit = self.rev or "master";
+ declarationCommit = "8c1421ae02475a874f2a09cc4a7ad6de63fbc9e8";
declarationPermalink = "https://github.com/nix-community/stylix/blob/${declarationCommit}";
# Renders a single option declaration. Example output:patch on top of this patchset and comparing it with its master branch results in the following harmless diff:
diff '--color=auto' -r result-master/options/modules/gnome.html result-1212/options/modules/gnome.html
284c284
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
diff '--color=auto' -r result-master/options/modules/hyprlock.html result-1212/options/modules/hyprlock.html
284c284
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
diff '--color=auto' -r result-master/options/modules/kde.html result-1212/options/modules/kde.html
306c306
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
diff '--color=auto' -r result-master/options/modules/lightdm.html result-1212/options/modules/lightdm.html
285c285
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
diff '--color=auto' -r result-master/options/modules/regreet.html result-1212/options/modules/regreet.html
285c285
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
diff '--color=auto' -r result-master/options/modules/sway.html result-1212/options/modules/sway.html
283c283
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
diff '--color=auto' -r result-master/options/modules/swaylock.html result-1212/options/modules/swaylock.html
284c284
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
diff '--color=auto' -r result-master/options/modules/wayfire.html result-1212/options/modules/wayfire.html
284c284
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
diff '--color=auto' -r result-master/options/platforms/home_manager.html result-1212/options/platforms/home_manager.html
662c662
< <pre><code class="language-nix">10
---
> <pre><code class="language-nix">config.stylix.fonts.sizes.desktop
691c691
< <pre><code class="language-nix">12
---
> <pre><code class="language-nix">config.stylix.fonts.sizes.applications
diff '--color=auto' -r result-master/options/platforms/nixos.html result-1212/options/platforms/nixos.html
648c648
< <pre><code class="language-nix">10
---
> <pre><code class="language-nix">config.stylix.fonts.sizes.desktop
677c677
< <pre><code class="language-nix">12
---
> <pre><code class="language-nix">config.stylix.fonts.sizes.applications
diff '--color=auto' -r result-master/print.html result-1212/print.html
5611c5611
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
6352c6352
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
6881c6881
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
7408c7408
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
9246c9246
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
9978c9978
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
10098c10098
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
10937c10937
< <pre><code class="language-nix">true
---
> <pre><code class="language-nix">config.stylix.image == null
12495c12495
< <pre><code class="language-nix">10
---
> <pre><code class="language-nix">config.stylix.fonts.sizes.desktop
12524c12524
< <pre><code class="language-nix">12
---
> <pre><code class="language-nix">config.stylix.fonts.sizes.applications
13263c13263
< <pre><code class="language-nix">10
---
> <pre><code class="language-nix">config.stylix.fonts.sizes.desktop
13292c13292
< <pre><code class="language-nix">12
---
> <pre><code class="language-nix">config.stylix.fonts.sizes.applications
diff '--color=auto' -r result-master/searcher-bdf088f8.js result-1212/searcher-bdf088f8.js
527c527
< loadScript(path_to_root + 'searchindex-0eb7c674.js', 'search-index');
---
> loadScript(path_to_root + 'searchindex-d12312c2.js', 'search-index');
Only in result-master: searchindex-0eb7c674.js
Only in result-1212: searchindex-d12312c2.jsFollowing up on the discussion from #1212 (comment), I believe the true intention is to simply negate
defaultsuch thattrueandfalseare used indefaultandexample:- example = config.stylix.image == null; + example = !(config.stylix.image != null && autoEnable);In that case, a cleaner solution would be to get the
defaultvalue into scope with an explicitlet-inscope (or a sillyrec):- example = config.stylix.image == null; + example = !default;This defeats the point of the change.
!defaultcan only be used whendefaultis known to be static. Evaluatingdefaultin the docs should be avoided at all costs, unless you control thedefaultvalue and know it is static.The current patch was discussed briefly here: #1212 (comment)
EDIT: I thought about this some more. Since the function already assumes
autoEnablecan be read safely in docs values, we can match thedefaultTextimpl in theexampleimpl. If the assumption ever becomes incorrect, the stricter docs impl should catch it for bothexampleanddefaultText.
From ace49e0c0a7452aaf8c38284422950a5762e5038 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Fri, 13 Jun 2025 09:56:12 +0100 Subject: [PATCH 1/6] stylix: only read `autoEnable` in `mkEnableWallpaper`'s example Previously the example evaluated `config.stylix.image`, which should not be done by the docs. Since `mkEnableWallpaper` already assumes `autoEnable` is static when it evaluates `defaultText`, we can do the same for `example`. --- stylix/target.nix | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/stylix/target.nix b/stylix/target.nix index f8b6861b2..0b12847c5 100644 --- a/stylix/target.nix +++ b/stylix/target.nix @@ -113,7 +113,11 @@ lib.literalExpression "config.stylix.image != null" else false; - example = config.stylix.image == null; + example = + if autoEnable then + lib.literalExpression "config.stylix.image == null" + else + true; }; mkEnableIf =
LGTM.
I think the previous behaviour was better because it is simpler and more obvious by avoiding an additional string indirection.
Sorry, I'm unclear what behaviour you prefer? Could you post a concrete suggestion?
Initially, I meant reverting this patch.
The intention of the change was to allow fonts to specify a string (representing a stylix font option name) or an int (which should be a literal value). In the case that an option is referenced, a
defaultTextis added. This is similar in-principal tomkPackageOption.
I somehow missed that the purpose is to provide defaultText when possible. In that case, this sounds like a nice addition.
From bc29d5400278b478462a517b02c951304ee6974d Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Fri, 13 Jun 2025 10:17:16 +0100 Subject: [PATCH 2/6] stylix: set defaultText in font size options --- stylix/fonts.nix | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/stylix/fonts.nix b/stylix/fonts.nix index 4d5dbb72f..1234df1a5 100644 --- a/stylix/fonts.nix +++ b/stylix/fonts.nix @@ -58,7 +58,12 @@ in mkFontSizeOption = { default, target }: lib.mkOption { - inherit default; + default = if builtins.isInt default then default else cfg.sizes.${default}; + defaultText = + if builtins.isInt default then + default + else + lib.literalExpression "config.stylix.fonts.sizes.${default}"; description = '' The font size used for ${target}. @@ -94,12 +99,12 @@ in terminal = mkFontSizeOption { target = "terminals and text editors"; - default = cfg.sizes.applications; + default = "applications"; }; popups = mkFontSizeOption { target = "notifications, popups, and other overlay elements of the desktop"; - default = cfg.sizes.desktop; + default = "desktop"; }; };
LGTM.
Considering how small the
doc/eval_compat.nixanddoc/hm_compat.nixmodules are, it might be better to just inline them, since they are also used only once.If you prefer. I moved them to separate files as I find that idiomatic for modules. Plus it keeps implementation details "out of site" so they don't distract from the detail of the main code.
They were bigger when I first implemented them though, now they are smaller there's a stronger argument for inlining.
Personally, I would almost certainly inspect those files every time when reading this code to know what this means. In that case, the file is just an indirection with little gain since inlining it does not really clutter the main code.
However, the current approach is also fine for me. Do as you want.
From d087dd4f9cb8ab7ea80862e485af3ae248e95a30 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Sun, 4 May 2025 03:03:00 +0100 Subject: [PATCH 3/6] doc: use minimal module evals We don't actually need fully blown NixOS or home-manager configurations just to read the declared `options`. Instead, we can directly call `lib.evalModules` to build a minimal configuration containing only stylix modules. --- doc/default.nix | 37 +++++++++++-------------------------- doc/eval_compat.nix | 26 ++++++++++++++++++++++++++ doc/hm_compat.nix | 12 ++++++++++++ flake/packages.nix | 2 -- 4 files changed, 49 insertions(+), 28 deletions(-) create mode 100644 doc/eval_compat.nix create mode 100644 doc/hm_compat.nix diff --git a/doc/default.nix b/doc/default.nix index 04f090231..343e69010 100644 --- a/doc/default.nix +++ b/doc/default.nix @@ -2,9 +2,6 @@ lib, pkgs, inputs, - nixosSystem, - homeManagerConfiguration, - system, callPackage, writeText, stdenvNoCC, @@ -17,38 +14,26 @@ let # Prefix to remove from option declaration file paths. rootPrefix = toString ../. + "/"; - nixosConfiguration = nixosSystem { - inherit system; - modules = [ - inputs.home-manager.nixosModules.home-manager - inputs.self.nixosModules.stylix - ]; - }; - - homeConfiguration = homeManagerConfiguration { - inherit pkgs; - modules = [ - inputs.self.homeModules.stylix - { - home = { - homeDirectory = "/home/book"; - stateVersion = "22.11"; - username = "book"; - }; - } - ]; - }; + evalDocs = + module: + lib.evalModules { + modules = [ ./eval_compat.nix ] ++ lib.toList module; + specialArgs = { inherit pkgs; }; + }; # TODO: Include Nix Darwin options platforms = { home_manager = { name = "Home Manager"; - configuration = homeConfiguration; + configuration = evalDocs [ + inputs.self.homeModules.stylix + ./hm_compat.nix + ]; }; nixos = { name = "NixOS"; - configuration = nixosConfiguration; + configuration = evalDocs inputs.self.nixosModules.stylix; }; }; diff --git a/doc/eval_compat.nix b/doc/eval_compat.nix new file mode 100644 index 000000000..08822105b --- /dev/null +++ b/doc/eval_compat.nix @@ -0,0 +1,26 @@ +{ lib, ... }: +{ + options = { + # Declare the arbitrarily named __stub attribute to allow modules to evaluate + # 'options.programs ? «OPTION»'. + # + # TODO: Replace 'options.programs ? «OPTION»' instances with + # 'options ? programs.«OPTION»' to remove this __stub workaround. + programs.__stub = lib.mkSinkUndeclaredOptions { }; + + # The config.lib option, as found in NixOS and home-manager. + # Many option declarations depend on `config.lib.stylix`. + lib = lib.mkOption { + type = lib.types.attrsOf lib.types.attrs; + description = '' + This option allows modules to define helper functions, constants, etc. + ''; + default = { }; + visible = false; + }; + }; + + # Third-party options are not included in the module eval, + # so disable checking options definitions have matching declarations + config._module.check = false; +} diff --git a/doc/hm_compat.nix b/doc/hm_compat.nix new file mode 100644 index 000000000..b0654e40e --- /dev/null +++ b/doc/hm_compat.nix @@ -0,0 +1,12 @@ +{ lib, ... }: +{ + # Declare the arbitrarily named __stub attribute to allow modules to evaluate + # 'options.services ? «OPTION»'. + # + # TODO: Replace 'options.services ? «OPTION»' instances with + # 'options ? services.«OPTION»' to remove this __stub workaround. + options.services.__stub = lib.mkSinkUndeclaredOptions { }; + + # Some modules use home-manager's `osConfig` arg + config._module.args.osConfig = null; +} diff --git a/flake/packages.nix b/flake/packages.nix index 70390d4db..7990ecb78 100644 --- a/flake/packages.nix +++ b/flake/packages.nix @@ -22,8 +22,6 @@ { doc = pkgs.callPackage ../doc { inherit inputs; - inherit (inputs.nixpkgs.lib) nixosSystem; - inherit (inputs.home-manager.lib) homeManagerConfiguration; }; serve-docs = pkgs.callPackage ../doc/server.nix { inherit (config.packages) doc;
LGTM.
From 85d9237d49715a0ed81c9ffa50263308f24581ba Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Thu, 19 Jun 2025 23:50:54 +0100 Subject: [PATCH 4/6] doc: replace `inputs` input with `self` The docs no longer needs access to arbitrary flake inputs. We should enforce this strictly by taking `self` instead. --- doc/default.nix | 8 ++++---- flake/packages.nix | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/default.nix b/doc/default.nix index 343e69010..1253c33cb 100644 --- a/doc/default.nix +++ b/doc/default.nix @@ -1,7 +1,7 @@ { lib, pkgs, - inputs, + self, callPackage, writeText, stdenvNoCC, @@ -27,13 +27,13 @@ let home_manager = { name = "Home Manager"; configuration = evalDocs [ - inputs.self.homeModules.stylix + self.homeModules.stylix ./hm_compat.nix ]; }; nixos = { name = "NixOS"; - configuration = evalDocs inputs.self.nixosModules.stylix; + configuration = evalDocs self.nixosModules.stylix; }; }; @@ -324,7 +324,7 @@ let # Permalink to view a source file on GitHub. If the commit isn't known, # then fall back to the latest commit. - declarationCommit = inputs.self.rev or "master"; + declarationCommit = self.rev or "master"; declarationPermalink = "https://github.com/nix-community/stylix/blob/${declarationCommit}"; # Renders a single option declaration. Example output: diff --git a/flake/packages.nix b/flake/packages.nix index 7990ecb78..d603d2af1 100644 --- a/flake/packages.nix +++ b/flake/packages.nix @@ -21,7 +21,7 @@ )) { doc = pkgs.callPackage ../doc { - inherit inputs; + inherit (inputs) self; }; serve-docs = pkgs.callPackage ../doc/server.nix { inherit (config.packages) doc;
LGTM. Nice improvement.
From 935708448705060f5f99bc2cff3141588568ded8 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Sun, 4 May 2025 03:40:30 +0100 Subject: [PATCH 5/6] doc: ensure `pkgs` is not used in option docs Providing a stub `pkgs` instance that has no contents ensures the docs does not _read_ `pkgs` anywhere. --- doc/default.nix | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/doc/default.nix b/doc/default.nix index 1253c33cb..1b887713d 100644 --- a/doc/default.nix +++ b/doc/default.nix @@ -14,11 +14,24 @@ let # Prefix to remove from option declaration file paths. rootPrefix = toString ../. + "/"; + # A stub pkgs used while evaluating the stylix modules for the docs + noPkgs = { + # Needed for type-checking + inherit (pkgs) _type; + + # Permit access to (pkgs.formats.foo { }).type + formats = builtins.mapAttrs (_: fmt: args: { + inherit (fmt args) type; + }) pkgs.formats; + }; + evalDocs = module: lib.evalModules { modules = [ ./eval_compat.nix ] ++ lib.toList module; - specialArgs = { inherit pkgs; }; + specialArgs = { + pkgs = noPkgs; + }; }; # TODO: Include Nix Darwin options
LGTM.
From 3015a06644fb888d933fe1593a9d96385df22173 Mon Sep 17 00:00:00 2001 From: Matt Sturgeon <matt@sturgeon.me.uk> Date: Tue, 10 Jun 2025 19:33:29 +0100 Subject: [PATCH 6/6] doc: ensure `config` is not used in option docs With a few exceptions for `lib.stylix` functions. Once these exceptions are removed, we will have solved the issue with NixOS's documentation.nixos.includeAllModules option. See https://github.com/nix-community/stylix/issues/98 --- doc/default.nix | 35 +++++++++++++++++++++++++++++++++++ doc/eval_compat.nix | 25 ++++++------------------- 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/doc/default.nix b/doc/default.nix index 1b887713d..25b8d5039 100644 --- a/doc/default.nix +++ b/doc/default.nix @@ -25,12 +25,47 @@ let }) pkgs.formats; }; + # A stub config used while evaluating the stylix modules for the docs + # + # TODO: remove all dependency on `config` and simplify to `noConfig = null`. + # Doing that should resolve https://github.com/nix-community/stylix/issues/98 + noConfig = + let + configuration = evalDocs { + # The config.lib option, as found in NixOS and home-manager. + # Required by the `target.nix` module. + options.lib = lib.mkOption { + type = lib.types.attrsOf lib.types.attrs; + description = '' + This option allows modules to define helper functions, constants, etc. + ''; + default = { }; + visible = false; + }; + + # The target.nix module defines functions that are currently needed to + # declare options + imports = [ ../stylix/target.nix ]; + }; + in + { + lib.stylix = { + inherit (configuration.config.lib.stylix) + mkEnableIf + mkEnableTarget + mkEnableTargetWith + mkEnableWallpaper + ; + }; + }; + evalDocs = module: lib.evalModules { modules = [ ./eval_compat.nix ] ++ lib.toList module; specialArgs = { pkgs = noPkgs; + config = noConfig; }; }; diff --git a/doc/eval_compat.nix b/doc/eval_compat.nix index 08822105b..daf4b87fe 100644 --- a/doc/eval_compat.nix +++ b/doc/eval_compat.nix @@ -1,24 +1,11 @@ { lib, ... }: { - options = { - # Declare the arbitrarily named __stub attribute to allow modules to evaluate - # 'options.programs ? «OPTION»'. - # - # TODO: Replace 'options.programs ? «OPTION»' instances with - # 'options ? programs.«OPTION»' to remove this __stub workaround. - programs.__stub = lib.mkSinkUndeclaredOptions { }; - - # The config.lib option, as found in NixOS and home-manager. - # Many option declarations depend on `config.lib.stylix`. - lib = lib.mkOption { - type = lib.types.attrsOf lib.types.attrs; - description = '' - This option allows modules to define helper functions, constants, etc. - ''; - default = { }; - visible = false; - }; - }; + # Declare the arbitrarily named __stub attribute to allow modules to evaluate + # 'options.programs ? «OPTION»'. + # + # TODO: Replace 'options.programs ? «OPTION»' instances with + # 'options ? programs.«OPTION»' to remove this __stub workaround. + options.programs.__stub = lib.mkSinkUndeclaredOptions { }; # Third-party options are not included in the module eval, # so disable checking options definitions have matching declarations
LGTM.
This patchset LGTM. Let's wait on an additional review before merging this.
danth
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks for working on this :))
|
Successfully created backport PR for |
Summary
doc: use minimal module evals
We don't actually need fully blown NixOS or home-manager configurations just to read the declared
options.Instead, we can directly call
lib.evalModulesto build a minimal configuration containing only stylix modules.This is faster, simpler, more compartmentalised, etc.
This should allow for removing the option declaration location based filtering. Draft PR: #1215.
This may also make adding nix-darwin docs easier, in the future.
doc: ensure
pkgsis not used in option docsUsing a stubbed instance of
pkgswhile evaling the modules for the docs ensures no options accidentally use real packages in their description/default/example text.This highlighted an issue that was resolved in #1225.
doc: ensure
configis not used in option docsWith a few exceptions for
lib.stylixfunctions. These are needed for declaring options and are currently accessed viaconfig:mkEnableIfmkEnableTargetmkEnableTargetWithmkEnableWallpaperOnce these exceptions are removed, we will have solved #98.
Additionally,
mkTargetis used for creating entire modules (including options), but that is already accessed without usingconfig.A potential solution is to pass a
stylibto modules viaimportApply(or similar), as discussed here: #98 (comment)However that shouldn't block this PR and will be easier to reason about as independent PRs.
Testing
I've tested that the docs still build.
I've spot checked that a few pages look the same as the current docs.
Issues
The additional strictness imposed by this PR has uncovered pre-existing issues with options, which are contributing to #98. Not only does this cause issues with nixos docs, it also means that "dynamic" values are not documented correctly.
Most of these cases were fixed in #1244, however a few other cases have been found and fixed as part of this PR.
To be clear: these are not new issues introduced by this PR; this PR does not need to fix them, as workarounds are implemented.
cc @awwpotato
Things done