Skip to content

nixos/installer: include a substitutable copy of the manual#210966

Closed
pennae wants to merge 1 commit into
NixOS:masterfrom
pennae:installer-manual-cache
Closed

nixos/installer: include a substitutable copy of the manual#210966
pennae wants to merge 1 commit into
NixOS:masterfrom
pennae:installer-manual-cache

Conversation

@pennae

@pennae pennae commented Jan 15, 2023

Copy link
Copy Markdown
Contributor
Description of changes

the manual included in the installer can't be used for substitution in the installed system because it includes installer options that are most likely not available in the final system. add another copy of the manual built from a minimal config to the installer for exactly such substitution.

given this we can also reenable documentation in the installer tests, they'll copy the newly included version of the manual instead of trying to build it from scratch.

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:
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

the manual included in the installer can't be used for substitution in
the installed system because it includes installer options that are most
likely not available in the final system. add another copy of the manual
built from a minimal config to the installer for exactly such
substitution.

given this we can also reenable documentation in the installer tests,
they'll copy the newly included version of the manual instead of trying
to build it from scratch.
@pennae pennae requested review from ncfavier and vcunat January 15, 2023 19:08
@github-actions github-actions Bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jan 15, 2023
@ofborg ofborg Bot added 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 Jan 15, 2023
@ncfavier

Copy link
Copy Markdown
Member

installer options that are most likely not available in the final system

Can you give an example? Shouldn't these options just be marked invisible?

@pennae

pennae commented Jan 15, 2023

Copy link
Copy Markdown
Contributor Author

Can you give an example? Shouldn't these options just be marked invisible?

the isoImage hierarchy is probably the most common. we could mark them all invisible, but that feels like even more of a hack than including two version of the manual. (and would make their docs a whole lot harder to discover, which is already not that easy)

@ncfavier

Copy link
Copy Markdown
Member

Ah. Perhaps we should go the other way around then, and add the installer profiles here:?

{ documentation.nixos.extraModules = [ ./virtualisation/qemu-vm.nix ]; }

@pennae

pennae commented Jan 15, 2023

Copy link
Copy Markdown
Contributor Author

not entirely sure, to be honest? installer options will be mostly useful for people hacking on the installer, not for more general use. unlike the qemu options that are very useful for nixos-rebuild build-vm.

dunno. don't want to decide this; we'd just add the appropriate manual to the installer as it seems to be the least fragile and most contained option.

@pennae pennae closed this Jan 15, 2023
@ncfavier

Copy link
Copy Markdown
Member

Why close? I'm still investigating this

@pennae

pennae commented Jan 15, 2023

Copy link
Copy Markdown
Contributor Author

turns out we went in the completely wrong direction this is only a problem with the tests, not with the actual installer. 🤦

tests have a different modules tree, which is picked up by the manual build when options are referenced as ${options.module.something}, adding a nodes.<name>. infix.

@ncfavier

Copy link
Copy Markdown
Member

OK, here's what I've discovered:

  • isoImage and friends do not actually make the manuals differ, because they're not included in the base modules (module-list.nix) and includeAllModules is off by default.
  • for the manual to be a cache miss on the ISO,
    • it must not be cached in cache.nixos.org, which can happen if the ISO is built from an old revision
    • AND it must not be included in the ISO, which happens when documentation.doc.enable is false, which is the case since profiles/minimal: reduce size #173661

So it seems like the obvious fix for the ISO is to reënable documentation.doc.enable in https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/installer/cd-dvd/installation-cd-minimal.nix.

Not sure about the tests.

@pennae

pennae commented Jan 15, 2023

Copy link
Copy Markdown
Contributor Author

non-minimal installers have documentation enabled: https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/profiles/installation-device.nix#L26. not including it in minimal isos sounds somewhat reasonable, those aren't going to be very useful without an internet connection anyway. (turns out that will always override the minimal profile anyway, so the manua is always included. verified it against a minimal iso boot too, manual was definitely available)

@ncfavier

Copy link
Copy Markdown
Member

documentation.enable and documentation.nixos.enable isn't enough to get the HTML manual, you also need documentation.doc.enable. While it's not useful to have it in the minimal installer, it might be justified if it avoids expensive cache misses.

@pennae

pennae commented Jan 16, 2023

Copy link
Copy Markdown
Contributor Author

oh, yeah. hadn't thought to check for HTML, only for the manpages. including HTML as well would definitely make sense, yes.

@pennae pennae deleted the installer-manual-cache branch January 22, 2023 20:26
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: 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.

2 participants