Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/documentation: Allow specifying extraSources #81557

Merged
merged 1 commit into from Mar 21, 2020

Conversation

bb010g
Copy link
Member

@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

@nixos-discourse
Copy link

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
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!

nixos/modules/misc/documentation.nix Outdated Show resolved Hide resolved
Because there was absolutely no way of setting this without rewriting
parts of the module otherwise.
@bb010g
Copy link
Member 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
Member Author

bb010g commented Apr 21, 2020

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

@infinisil
Copy link
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants