Skip to content

nixos/documentation: Allow specifying extraSources#81557

Merged
infinisil merged 1 commit intoNixOS:masterfrom
bb010g:nixos-expose-extrasources
Mar 21, 2020
Merged

nixos/documentation: Allow specifying extraSources#81557
infinisil merged 1 commit intoNixOS:masterfrom
bb010g:nixos-expose-extrasources

Conversation

@bb010g
Copy link
Copy Markdown
Contributor

@bb010g bb010g commented Mar 2, 2020

Because there was absolutely no way of setting this without rewriting parts of the module otherwise.

Motivation for this change

When using https://github.com/DBCDK/morph with NixOS machines specifying documentation.nixos.includeAllModules = true;, the manuals of NixOS instantations were impure & repeatedly rebuilding due to both morph's use of temporary directories & Nixpkgs modules and the normal location of the morph network's repository being in a user's home directory along with our definition of Nixpkgs modules. Some examples from these manual builds:

<attrs>
  <attr name="declarations">
    <list>
      <string value="/tmp/morph-974414533/options.nix" />
    </list>
  <attr name="default">
<attrs>
  <attr name="declarations">
    <list>
      <string value="/home/bb010g/Documents/sysadmin/contoso/modules/nix-substituters.nix" />
    </list>
  <attr name="default">

NixOS's documentation machinery has a configuration option to deal with this, extraSources, but it's not exposed, and the expressions aren't structured in a way to allow sneaky, underhanded configuration either. You can't get around this without patching Nixpkgs. (This PR has been tested so far on our repository via applying pkgs.applyPatches { src = nixpkgs.outPath; patches = [ ./nixpkgs-nixos-expose-extrasources.patch ]; } before importing Nixpkgs. (Whee.))

With this change in place, the repository-directory impurity demonstrated can be eliminated with nixos.extraSources = [ ./. ]; from the repository root, and the morph impurity with an (approximate) patch to /data/eval-machines.nix of:

--- a/data/eval-machines.nix
+++ b/data/eval-machines.nix
@@ -26,6 +26,8 @@ rec {
             [ ({ config, lib, options, ... }: {
                 key = "deploy-stuff";
                 imports = [ ./options.nix ];
+                # Make doc builds determinisitic, even with our tempdir module imports
+                documentation.nixos.extraSources = [ ../. ];
                 # Provide a default hostname and deployment target equal
                 # to the attribute name of the machine in the model.
                 networking.hostName = lib.mkDefault machineName;

Sorry for the delay on PRing this; I know this should have been in way earlier for 20.03's sake. If possible, I'd like to get this backported to 20.03 for sanity in situtations like morph when

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs 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/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

This change is Reviewable

@ofborg ofborg Bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Mar 2, 2020
@nixos-discourse
Copy link
Copy Markdown

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

https://discourse.nixos.org/t/nixos-20-03-feature-freeze/5655/39

Copy link
Copy Markdown
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Other than my comment, this looks good to me!

Comment thread nixos/modules/misc/documentation.nix Outdated
Because there was absolutely no way of setting this without rewriting
parts of the module otherwise.
@bb010g bb010g force-pushed the nixos-expose-extrasources branch from 7535c42 to 34dd64b Compare March 21, 2020 02:05
@bb010g
Copy link
Copy Markdown
Contributor Author

bb010g commented Mar 21, 2020

Resolved.

@infinisil infinisil merged commit 9d0b3bf into NixOS:master Mar 21, 2020
@bb010g bb010g deleted the nixos-expose-extrasources branch March 21, 2020 06:28
@bb010g
Copy link
Copy Markdown
Contributor Author

bb010g commented Apr 21, 2020

Would it still be possible to land this on release-20.03?

@infinisil
Copy link
Copy Markdown
Member

Backported to 20.03 in 6df8f27 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants