nixos/*: deprecate docbook option docs

[pennae]

Description of changes

deprecate docbook option docs and functions for creating docbook option docs. this very explicitly does not introduce warnings that could break a build to not break any systems when the 22.11 rolls around, and for 23.05 we'll want to remove docbook support for options entirely (without another warning).

a couple new module have been merged since the last conversion PR was started and a few options weren't converted even then, so we have to handle a few deprecation messages immediately as well.

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.11 Release Notes (or backporting 22.05 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.

[edolstra]

What is the advantage of literalExpression over literalExample? They seems to do the same thing, and literalExample has a more accurate name.

[pennae]

literalExample suggests it's only valid for examples, but it's also valid in defaultText (where it is just as useful)

[dasJ]

nix looks good, changelog and docs look good.
Have only skimmed the lib.mdDoc insertions but they looked good from what I can tell.
Have not reviewed the Python code

[infinisil]

This is also too fast. There needs to be a reasonable period for third-party tools to easily support multiple nixpkgs releases without causing warnings. This was previously a problem with iirc home-manager, where a deprecation warning was unavoidable due to older nixpkgs still needing support.

[pennae]

as far as we can tell there will be no way for us to do this transition that will not cause pains in third-party tools, waiting longer will only make them worse (cf python 2). anything that wants to support both can either go the trivial route with a let mdDoc = lib.mdDoc or lib.id; in prefix and using documentation that works with both renderers, or it can have two texts and switch between them based on pkgs.version. considering how much things change between releases it doesn't seem unreasonable to expect third-party things to add support quickly, while actually removing support for old docbook is a lot less pressing as long as the gates work. maybe we should instead add something to the docs system that reliably tells whether the underlying docs format is docbook or markdown?

(we also can't make any meaningful improvements to rendering of the options chapter/manpage before docbook is fully disallowed. can't find a way to insert raw roff sequences into a docbook manpage build or raw html into a docbook html build, which would leave us with rendering each docbook option individually and parsing the generated output to insert into what we'd render from markdown directly. 🙁)

[infinisil]

anything that wants to support both can either go the trivial route with a let mdDoc = lib.mdDoc or lib.id; in prefix and using documentation that works with both renderers, or it can have two texts and switch between them based on pkgs.version

Both of those are far from ideal and require a lot of work. Third-party tools should also have a chance for a smooth transition with an appropriate deprecation period. Merging this PR as-is would require home-manager, nix-darwin (and anything else rendering docs using the module system) to effectively immediately update all their docbook to markdown, support for which was only introduced a couple months ago.

This is how I think the deprecation strategy should look like:

• 22.11: Introduce support for markdown docs (treewide: markdown option docs #176146). Mention in the release notes that support for docbook will be deprecated in the next release and removed in the one after that. No warning for docbook yet. This allows third-party tools to read release notes and prepare for the next release, such that they won't cause any warnings for users.
• 23.05: Deprecate docbook support by adding a warning when mdDoc isn't used. The warning mentions that docbook support will be removed in the next released. This pushes everybody to use mdDoc, explicitly confirming that they have transitioned their code to markdown.
• 23.11: Remove docbook support either by using markdown implicitly for unannotated strings. This is only safe assuming everybody did the transition to 23.05, if somebody updates from older releases, they'd never get the warning to move to mdDoc, but considering the alternative of throwing an error for unannotated strings, that's a worthwhile tradeoff imo.
• (future): Slowly deprecate mdDoc with a warning, then an error, then a complete removal

So essentially there's two deprecation phases: An initial silent one for third-party tools to prepare a smooth NixOS/nixpkgs upgrade for their users, and a noisy one (warning) for end users. And if a third-party tool misses the first phase of deprecation, they will get complaints by their users during the second one.

Yes this means that we'll have to wait with the docbook removal for another year, but that's just how deprecation works with a stable API surface (which is expected of nixpkgs).

[infinisil]

Found the issue I was thinking of that shows how this problem happened in the past: #163451

[pennae]

while yes, markdown support for options is new in the scheme of things, it's been known for a good while now that docbook is on the way out. for that reason we'd much rather skip what you propose for 22.11. perhaps we could make the process easier on users by adding an option to disable the deprecation warnings?

dunno. we honestly don't have the energy to fight for doing this quickly, we just want the docs build to not take ten minutes each time. may someone else decide.

[roberth]

This will annoy third party module maintainers.
They'll receive issues and PRs about the warnings, while they need to keep using docbook until 22.05 is EOL.
You could prepare the deprecation though, with

Suggested change

	else
	lib.warn
	"literalDocBook is deprecated, use literalMD instead"
	{ _type = "literalDocBook"; inherit text; };
	else
	lib.warnIf (lib.isInOldestRelease 2211)
	"literalDocBook is deprecated, use literalMD instead"
	{ _type = "literalDocBook"; inherit text; };

[pennae]

wouldn't that only delay the annoyance to third-party modules by one release? if they want to not see warnings during the last release that supports both docbook and markdown they'll still have to write docs twice for that period or work with a common denominator. especially for modules that are rarely or never used with unstable, they'd see the same brick wall of deprecation notices, only later?

we honestly don't quite follow how moving the warnings into the future will make anything nicer for folks unless every module maintainer reads the complete release notes and acts upon them quickly (which seems rather unlikely)

[roberth]

They can't switch because they need to keep using docbook until 22.05 is EOL.

Delaying the warnings until the moment they can switch solves the problem for them.

For reference, this problem was described in #157667.
Machinery to automate deprecations was added in #163451.

[pennae]

@infinisil @roberth here's an idea: we deprecate everything very slowly indeed, but also add a temporary option to be removed together with docbook, eg documentation.nixos.options.renderFromMD that defaults to false and allows opting in to the fully docbook-less option docs world as soon as the option comes around. the unstabe manual should be built with both values for this option for time being, deprecation notices in the build should count as errors, and differences between the two build should count as errors as well.

[roberth]

add a temporary option to be removed together with docbook, eg documentation.nixos.options.renderFromMD that defaults to false and allows opting in to the fully docbook-less option docs world as soon as the option comes around.

sgtm

the unstabe manual should be built with both values for this option for time being, deprecation notices in the build should count as errors, and differences between the two build should count as errors as well.

Yes, for Nixpkgs' own CI we can and should be strict. I can think of two ways we should probably both implement

• add renderFromMD = true to the nixosTests; perhaps even as a channel blocker(?)
• configure the github action that builds the manual to set the option, or duplicate the action

These changes won't affect users, but keep nixpkgs prepared for the final change.

[pennae]

softened the deprecation policy a lot and added a check for manual equivalence by build type to the CI workflows. haven't added a test yet (not sure what the best way to do this would be). for now this probably shouldn't be a channel blocker, it doesn't seem quite important enough for that—once we're on the last stretch to removing docbook that would be a good thing to do though.

[roberth]

Could you add explanatory messages to these commands when they fail?
That way, the average contributor will know what to do in either of the three circumstances.
Would a failure in the last command only be caused by a programming error in the python script?

[pennae]

Could you add explanatory messages to these commands when they fail?

the first cannot fail unless the CI machine breaks, the second produces diagnostics as part of its output. added a little informational message to the last one.

Would a failure in the last command only be caused by a programming error in the python script?

currently it cannot fail at all (if anyting fails, it'll be the second nix-build). in a speculative future it could fail if the MD renderer produces a different output for any reason at all, even for stylistic choices. we might not be able to match the docbook-produced manual exactly, in which case the comparison would either have to be modified or maybe even removed again.

[pennae]

rebased to master because new docbook options appeared. no other changes.

[pennae]

is there anything that still needs to be done here? really want to get this over with before it needs another rebase for newly introduced options.

[pennae]

absent any further comments or change requests we'd like to merge this in a week. rebasing hasn't been necessary very often so far at least, which seems to be a good sign for the overall effort.

[infinisil]

Almost looking good to me!

[infinisil]

Why is this flag called renderFromMD when it doesn't do that? If it just warns for docbook shouldn't it be called something like allowDocbook = true/"warn"/"error"?

[pennae]

Why is this flag called renderFromMD when it doesn't do that

it does not yet, but once this is merged we plan to start working on a direct-from-MD renderer that will be used when this flag is set.

[infinisil]

It is very misleading though. How about having such a proposed allowDocbook and if we later allow markdown rendering, we can introduce another option renderBackend = "docbook"/"markdown". Then if "markdown" is chosen, allowDocbook is forcibly set to "error" or null (indicating docbook is not even detected anymore, so the option doesn't make sense)

[pennae]

• replaced renderFromMD : bool with renderer : enum [ "docbook+md" "markdown" ]
• using docbook with renderer = "markdown" is now treated as a hard error (which will become necessary in the future anyway)
• documentation updated to match

[infinisil]

I discussed this with @pennae on Matrix here, we concluded that allowDocbook was okay, for now both true and false values use the docbook render pipeline, but when markdown pipeline support is added, setting allowDocbook = false can use that one instead to speed up the doc build.

[infinisil]

Wait what does this mean? "It is always permissible to set this to true when it was set to false previously."?

[pennae]

that documentation is expected to not break if this has been set to false at some point and only markdown documentation added after that. setting it to false may break stuff, but unsetting it should definitely fix those breakages

[infinisil]

I see, how about rewording this a bit then, something like "Whether markdown option documentation is allowed is independent of this option."

[pennae]

with all problems now resolved we'll go ahead and merge ahead of more docbook being added. hopefully no large problems will pop up, if so we can always revert and redo or apply fixes as needed.

[vcunat]

@pennae: this PR broke nixpkgs manual: https://hydra.nixos.org/build/190038194

[zowoq]

this PR broke nixpkgs manual: https://hydra.nixos.org/build/190038194

#190751

[jtojnar]

Looks like lib.mdDoc is no longer documented in the NixOS manual: https://nixos.org/manual/nixos/unstable/#sec-option-declarations

This leads to confusion: #200724

[pennae]

#201187

[pennae]

@amjoseph-nixpkgs the error in your workflow is likely due to an interaction of #208407 and an update to iay being merged using the wrong function before your workflow was run. looks like it was fixed again in #209199.

apologies for all the trouble :(