nixos/*: split docs build

[pennae]

Motivation for this change

almost all options documentation is constant, only very few options reference pkgs in attributes that end up in documentation. this fact can be used to build most of options doc in a derivation that can then be cached, and merging the rest of the docs (which can evaluated more cheaply now) separately.

doing so reduces nixos-rebuild time quite significantly (by 20% or more), reducing build time on our system from 9.4 seconds to 7.4 seconds. disabling docs entirely reduces build time to 7.1 seconds, making the docs build as close to free as we can currently get.

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, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.05 Release Notes (or backporting 21.11 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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[pennae]

most of the modules that have gained an exemption from sandboxed doc builds either interpolate the version of the package they use into doc attributes, or they set relatedPackages. the former isn't easily fixed, the latter could be fixed by separating relatedPackages info from the modules themselves or removing the feature altogether. separating probably doesn't gain us much before there are a lot of users of this feature, but since it's been around for quite a while and hasn't seen much uptake that doesn't seem likely. (and if many users did appear we'd probably lose all the build time improvements the split provices, so maybe it'd be better to remove relatedPackages info as it exists now?)

[roberth]

I don't think you should need this for docs.

Duplicating the evalModules logic is not great for maintainability. Would it be feasible to use extendModules when module filtering is supported by the module system itself?

[pennae]

you're right, that isn't needed here. wonder where that even came from now, probably from attempts to put nixpkgs.nix into the sandbox (which failed anyway) 🤔

re evalModules: we really have to duplicate this because both parts of the build must run with _module.check = false, otherwise the build failed when were testing. not duplicating would be nice, but the savings of this PR come mostly from the docs build not reusing the eval we already have in any way. always open for suggestions though!

[roberth]

You could do essentially extendModules { modules = [ { config._module.args.check = lib.mkForce false; } ]; } for check.
The only challenge is removing some modules.

[pennae]

"some" is a bit of an understatement, there's over 1400 of them at time of writing 😄 filtering loaded modules like that would be possible if that facility existied, but evaluating a few user modules from scratch is probably faster. 🤔

[cole-h]

Finally making good on my 2-week-old promise...

[roberth]

could be fixed by separating relatedPackages info from the modules themselves or removing the feature altogether.

I find it surprising that a bunch of mere strings are causing such an issue. Perhaps the feature could be less strict, though I wouldn't have noticed in practice if you removed it.

[pennae]

I find it surprising that a bunch of mere strings are causing such an issue. Perhaps the feature could be less strict, though I wouldn't have noticed in practice if you removed it.

it's not the strings that cause the issue, but how they are handled. the docs mechanism uses those strings as attrnames into pkgs and pulls the package description into the docs.

[roberth]

I find it surprising that a bunch of mere strings are causing such an issue. Perhaps the feature could be less strict, though I wouldn't have noticed in practice if you removed it.

it's not the strings that cause the issue, but how they are handled. the docs mechanism uses those strings as attrnames into pkgs and pulls the package description into the docs.

Perhaps these could be replaced by links to search.nixos.org or something. Using the show= query param, example: https://search.nixos.org/packages?show=hercules-ci-agent&sort=relevance&query=hercules-ci-agent

[pennae]

turning them into links sounds like a good idea. the manpage build currently renders links in bold red, if we set the link text to something like pkgs.<attribute name> and dropped the package metadata we'd get a nice html manual and the manpage would be sufficient to look up information. does that sound sensible?

[roberth]

Yeah, go for it :)

[roberth]

For includeAllModules, you'd only have to add the user-provided modules, right? (in addition to p.wrong)

[pennae]

not sure. extraModules was included in the docs build when we started, so we just kept it.

as far as we understand extraModules (via NIXOS_EXTRA_MODULE_PATH) are always loaded but not included by default for performance reasons. it would be nice to include them in the cached part of the build, but like user modules they can reference arbitrary paths so that didn't work out :(

[ajs124]

This (bisect says fc614c3) seems to have broken the installer tests, therefore blocking the nixos channels.

The test fails with:

machine # [    8.959436] dhcpcd[827]: eth0: adding default route via fe80::2
machine # error: path '/nix/store/j02qf8rza5g930jazw4k71g0nigc063f-nixos-22.05pre-git' is not valid
machine # (use '--show-trace' to show detailed location information)
machine: output: 
Test "Perform the installation" failed with error: "command `nixos-install < /dev/null >&2` failed (exit code 1)"

From what I remember the installer tests use closureInfo to populate the store in the vm. So some part of that probably fails now.

[vcunat]

So (partial?) revert for now to unblock channels?

[pennae]

#153620 has a proposed fix. if that can't be merged quickly let's revert fc614c3 or set documentation.nixos.options.splitBuild = false by default

[vcunat]

Unfortunately that still doesn't completely fix the regressions caused by this PR on nixosTests.installer.simple, so channel blocking continues. Some dependencies are now unreachable from within the installer tests, giving lines like:

machine # error: unable to download 'https://salsa.debian.org/debian/keyutils/raw/4cecffcb8e2a2aa4ef41777ed40e4e4bcfb2e5bf/debian/patches/Make-build-reproducible.patch

[vcunat]

Usually these were addressed by extending system.extraDependencies in nixos/tests/installer.nix with the right packages (not their build_ dependencies which happen to show up). But I haven't looked into what exactly is newly required here and if it might be an undesirable addition instead.

[pennae]

d'oh. didn't let the installer test run through, so we didn't see those. will open a PR to disable the docs split for now until the installer issues are fully sorted.

[vcunat]

What I see is certainly just a test-only problem, so perhaps disable in the installer tests only? (unless a good extraDependencies addition in there will occur to us very soon)

[vcunat]

What I see failing certainly are dependencies to generate docs (docbook, manual, manpages).

[pennae]

we had tried disabling docs in the installer yesterday and had no success. could've been because tiredness, but at the moment i'd rather disable the split build to unblock the channel and have more time to analyse things.

[zowoq]

7e28421 broke kubernetes as well.

PR to revert: #153947

[zowoq]

And I can't even revert it because it breaks the manual.

[pennae]

@zowoq does it unbreak when reverting the kubernetes commits and setting meta.buildDocsInSandbox = false for all kubernetes modules?

[pennae]

seems so, the test does complete and the manual builds. #153951

[bb010g]

This PR broke my NixOS configuration (with modules that customize existing modules) when switching from 21.11 to 22.05, and I had to bisect (git bisect start release-22.05 release-21.11 && git bisect run nix-instantiate nixos -A system) to find this PR.

Here's a sample evaluation failure:

error: The option `security.pam.services.<name>.text' does not exist. Definition values:
       - In `/etc/nixos/modules/modules/pam.nix':
           {
             _type = "override";
             content = ''
               session optional ${pkgs.pam}/lib/security/pam_umask.so 0022
             '';
           ...

       … while evaluating the attribute 'options'

I'm currently trying to figure out how to make this build again without turning nixos.splitOptionDocBuild off. It looks like specialArgs = { options = allOpts; …; } isn't working as intended.

[pennae]

can you share a larger example that causes the errors you're seeing? could be that the doc split optimization simply isn't applicable in your case and turning it off could be the only option.

[nixos-discourse]

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

https://discourse.nixos.org/t/rebuilding-system-using-niv-fails-in-nixos-22-05/19667/7

[nixos-discourse]

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

https://discourse.nixos.org/t/a-look-at-nixos-nixpkgs-evaluation-times-over-the-years/65114/20