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-render-docs: add manual html renderer, use it for the nixos manual #217342

Merged
merged 18 commits into from
Mar 4, 2023
Merged

nixos-render-docs: add manual html renderer, use it for the nixos manual #217342

merged 18 commits into from
Mar 4, 2023

Conversation

pennae
Copy link
Contributor

@pennae pennae commented Feb 20, 2023

Description of changes

at last! the html renderer for the nixos manual (if ! allowDocBook). we reproduce the docbook-generated manuals almost exactly (as compared by the manual compare workflow scripts, which run each candidate through html-tidy first to allow for whitespace changes). we also do that in about 10% the time needed by the docbook manual toolchain (realizing a similar speedup to what manpages saw).

there's a good bit of docbook compat code in here, most of which can be either dropped when docbook goes away or formalized as the new (old) nixos manual style.

an epub converter should be not too far off from here, there's already infrastructure in place to allow arbitrary chunking of the input document and as far as we can tell our present epub manual is little more than a more aggressively chunked html manual packed in a zip file.

draft because this depends on #214709 for a couple options rendering internals, but we could extract those and merge this first if someone wants to expedite this.

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
  • Fits CONTRIBUTING.md.

@github-actions github-actions 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/` labels Feb 20, 2023
@pennae pennae marked this pull request as ready for review February 21, 2023 17:19
@pennae pennae requested a review from dasJ as a code owner February 21, 2023 17:19
{manpage} already exapnds to a link but akkoma wants to link to
a specific setting. split the mention for clarity.

networkd just straight up duplicated what {manpage} generates anyway, so
that link can go away completely.
our renderers carry significantly more state than markdown-it wants to
easily cater for, and the html renderer will need even more state still.
relying on the markdown-it-provided rendering functions has already
proven to be a nuisance, and since parsing and rendering are split well
enough we can just replace the rendering part with our own stuff outright.

this also frees us from the tyranny of having to set instance variables
before calling super().__init__ just to make sure that the renderer
creation callback has access to everything it needs.
these weren't used for anything. options never was (and does not contain
any information for the renderer that we *want* to honor), and env is
not used because typed renderer state is much more useful for all our cases.
ultimately it's the renderer that needs it, for the options rendering
that will be simplified in a bit.
we should really be rendering options at *rendering* time, not at parse
time. currently this is just an academic exercise, but the html renderer
will have to inspect the options.json data after the entire document has
been parsed, but before anything gets rendered.
the html renderer will need all of these functions as well. some
extensions will be needed, but we'll add those as they become necessary.
for most of our data classes we can use dataclasses.dataclass with
frozen=True or even plain named tuples. the TOC structure we'll need to
generate proper navigation links is most easily represented and used as
a cyclic structure though, and for that we can use neither. if we want
to make the TOC structures immutable (which seems like a good idea)
we'll need a hack of *some* kind, and this hack seems like the least intrusive.
check that all required headings are present during parsing, not during
rendering. building a correct TOC will need this since every TOC entry
needs a heading to set its title, and every included substructure needs
a title.

also improve the error message on repeated title headings slightly,
giving the end line turns out to not be very useful.
while not technically necessary for correct rendering of *contents* we
do need to disallow heading levels being skipped to build a correct
TOC. treating headings that have skipped a number of levels to actually
be headings that many levels up only gets confusing, and inserting
artifical intermediate headings suffers from problems, such as which ids
to use and what to call them.
without this we cannot build a TOC to arbitrary depth without generating
ids for headings, but generated ids are fragile and liable to either
break or point to different things if the manual changes shape. we
already have the convention that all headings should have an id, this
formalizes it.
text content in the toplevel file of a book will not render properly.
the first proper element will be a preface, part, or chapter anyway, and
those require includes to produce.

parts do not currently allow headings in the part file itself, but
that's mainly a renderer limitation. we can add support for headings in
part intros when we need them

in all other cases includes must be followed by either another include,
a heading, or end of file. text content could not be properly linked to
from a TOC without a preceding heading.
while docbook relies on external chunk-toc info to do chunking of the
rendered manual we have nothing of the sort for html. there it seems
easiest to add annotations to blocks to create new chunks. such
annotations could be extended to docbook to create the chunk-toc instead
of passing it in externally, but with docbook on the way out that seems
like a waste of effort.
the docbook toolchain uses docbook-xsl to generate its TOC, our html
renderer will have to do this on its own. this generator uses a very
straight-forward algorithm of only inspecting headings, but anything
else could be inspected as well. (examples come to mind, but those do
not have titles and would thus make for bad toc entries)

we also use path information (that will be taken from include block args
in the html renderer) to produce navigation information. the algorithm
we use mirrors what docbook does, linking to the next/previous files in
depth-first toc order.

toc entries are linked to the tokens they refer to for easy use later.
the basic html renderer. it doesn't have all the docbook compatibility
codes embedded into it, but there is a good amount. this renderer is
unaware of manual structure and does not traverse structural include
tokens (if it finds any it'll just fail), that task falls to derived
classes. once we have more uses for structural includes than just the
manual we may revisit this decision.
it's not hooked up to anything yet, but that will come soon. there's a
bit of docbook compat here that must be interoperable with the actual
docbook exporter, but luckily it's not all that much.
this will be necessary for html since there we have to do chunking into
multiple files ourselves. writing one file from the caller of the
converter and all others from within the converter is unnecessarily
spread out, and returning a dict of file names and their contents is not
quite as meaningful for docbook (which has only one file to begin with).
this converter is currently supposed to be able to reproduce the
docbook-generated html DOMs exactly, though not necessarily the
html *files*. it mirrors many docbook behaviours that seem rather odd,
such as top-level sections in chapters using the same heading depth as
understood by html as their parent chapters do. over time we can
hopefully remove all special casing needed to reproduce docbook
rendering, but for now at least it doesn't hurt *too* much.
this reproduces the docbook-generated html manual exactly enough to
appease the compare workflows while we still support both toolchains.
it's also a lot faster than the docbook toolchain, rendering the entire
html manual in about two seconds on this machine (while docbook needs
about 20).
@pennae pennae requested a review from K900 February 23, 2023 15:09
@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Feb 23, 2023

Yay! Thanks so much for putting in this huge effort.

It's a lot to review, so I started with trying to build on x86_64-darwin.

Build failed
cd ./doc
nix-build .
this derivation will be built:
  /nix/store/jsjk7gqsk463sa4bxp7hjmr3p6sc9nbl-nixpkgs-manual.drv
building '/nix/store/jsjk7gqsk463sa4bxp7hjmr3p6sc9nbl-nixpkgs-manual.drv'...
unpacking sources
unpacking source archive /nix/store/2lqirqf8kcnizf6lrss9fc6n0dsslqdj-doc
source root is doc
patching sources
ln: failed to create symbolic link './doc-support/result/3n4brrn616lgmsbrw5mc3ifmv6iy6n2m-doc-support': Permission denied
/nix/store/32knm2y5g4x541xav6r6y7rhij8cg7lx-stdenv-darwin/setup: line 138: pop_var_context: head of shell_variables not a function context
error: builder for '/nix/store/jsjk7gqsk463sa4bxp7hjmr3p6sc9nbl-nixpkgs-manual.drv' failed with exit code 1;
       last 6 log lines:
       > unpacking sources
       > unpacking source archive /nix/store/2lqirqf8kcnizf6lrss9fc6n0dsslqdj-doc
       > source root is doc
       > patching sources
       > ln: failed to create symbolic link './doc-support/result/3n4brrn616lgmsbrw5mc3ifmv6iy6n2m-doc-support': Permission denied
       > /nix/store/32knm2y5g4x541xav6r6y7rhij8cg7lx-stdenv-darwin/setup: line 138: pop_var_context: head of shell_variables not a function context
       For full logs, run 'nix log /nix/store/jsjk7gqsk463sa4bxp7hjmr3p6sc9nbl-nixpkgs-manual.drv'.

A local build with make also failed:

Make output
cd ./doc
nix-shell
make
(cd doc-support; nix-build)
trace: lib.zip is deprecated, use lib.zipAttrsWith instead
trace: warning: lib.readPathsFromFile is deprecated, use a list instead
trace: warning: replaceChars is a deprecated alias of replaceStrings, replace usages of it with replaceStrings.
trace: `lib.nixpkgsVersion` is deprecated, use `lib.version` instead!
trace: lib.crossLists is deprecated, use lib.cartesianProductOfSets instead
trace: Warning: `showVal` is deprecated and will be removed in the next release, please use `traceSeqN`
trace: warning: literalExample is deprecated, use literalExpression instead, or use literalDocBook for a non-Nix description.
/nix/store/3n4brrn616lgmsbrw5mc3ifmv6iy6n2m-doc-support
make: Circular manual-full.xml <- manual-full.xml dependency dropped.
xmllint --nonet --xinclude --noxincludenode manual.xml --output manual-full.xml
warning: failed to load external entity "functions/library/generated/index.xml"
functions/library.xml:13: element include: XInclude error : could not load functions/library/generated/index.xml, and no fallback was found
jing doc-support/result/docbook.rng manual-full.xml
/Users/tweag/src/nixpkgs/doc/manual-full.xml:1429:81: error: element "xi:include" not allowed here; expected the element end-tag or element "address", "anchor", "annotation", "bibliography", "bibliolist", "blockquote", "bridgehead", "calloutlist", "caution", "classsynopsis", "cmdsynopsis", "constraintdef", "constructorsynopsis", "destructorsynopsis", "epigraph", "equation", "example", "fieldsynopsis", "figure", "formalpara", "funcsynopsis", "glossary", "glosslist", "important", "index", "indexterm", "informalequation", "informalexample", "informalfigure", "informaltable", "itemizedlist", "literallayout", "mediaobject", "methodsynopsis", "msgset", "note", "orderedlist", "para", "procedure", "productionset", "programlisting", "programlistingco", "qandaset", "refentry", "remark", "revhistory", "screen", "screenco", "screenshot", "section", "segmentedlist", "sidebar", "simpara", "simplelist", "simplesect", "synopsis", "table", "task", "tip", "toc", "variablelist" or "warning"
make: *** [Makefile:45: validate] Error 1

@pennae
Copy link
Contributor Author

pennae commented Feb 23, 2023

that seems unrelated at first glance? so far we've done nothing to the nixpkgs manual (that we can recall), it's all been localized to ./nixos 😕

edit cannot reproduce that error on x86_64-linux it seems. we did mange to get a failure from a non-cleaned doc-support dir, but that's different:

nixpkgs-manual> unpacking sources
nixpkgs-manual> unpacking source archive /nix/store/hxxf85q93zgvfd1fz6wmphacb1fr7n1k-doc
nixpkgs-manual> source root is doc
nixpkgs-manual> patching sources
nixpkgs-manual> ln: failed to create symbolic link './doc-support/result': File exists
nixpkgs-manual> /nix/store/b09v23lirgvci3wzszh22mbkdfj0h0yq-stdenv-linux/setup: line 130: pop_var_context: head of shell_variables not a function context

@ncfavier
Copy link
Member

ncfavier commented Feb 23, 2023

ln: failed to create symbolic link './doc-support/result/3n4brrn616lgmsbrw5mc3ifmv6iy6n2m-doc-support': Permission denied

That definitely looks like a "dirty tree" issue. There's already a result symlink there so this ln interprets doc-support/result as a target directory rather than a target filename, and fails to create a link in that directory because it's in the store.

I think this should have been fixed by #215121?

EDIT: there's no result in doc/.gitignore, so maybe not...

EDIT: #217865

@ncfavier
Copy link
Member

Sorry, accidentally linked my PR to this one so that it closed it...

@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Feb 23, 2023

@pennae yeah sorry for the noise, I've built the Nixpkgs manual because I followed the wrong manual's instructions. It happens they look almost exactly the same and the bookmarks sit next to each other in my browser...

The NixOS manual build works and looks correct on superficial inspection; checked off x86_64-darwin in the top post.

We may want to update the build instructions for the manual though, because it currently says to build -A manual.x86_64-linux or architectures supported by NixOS, which is confusing and not true. Manually adding x86_64-darwin to supportedSystems in release.nix works fine, but I can't untangle the minimal change required to only affect the manual.

Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

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

Went through the commit messages to follow design decisions, without inspecting the actual code too deeply, only ensuring the gist of each diff matches the intention laid out by the corresponding commit message.

I really like the approach and the result. Thank you so much, it's a great improvement on many fronts, and will enable lots of future work to improve technical coherence and readability of the manuals.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-02-23-documentation-team-meeting-notes-29/25731/1

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/any-consensus-on-documentation-generation-tool-for-nixpkgs-manual/15550/33

@pennae
Copy link
Contributor Author

pennae commented Mar 1, 2023

barring new reviews or complaints we'd like to merge this tomorrow or friday. it's currently opt-in and thus probably won't hurt anyone, and the real test will only come with post-23.05 disabling docbook by default anyway.

@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Mar 1, 2023

@pennae To the best of my knowledge you're the closest to being the technical maintainers of the NixOS manual now, and I trust your judgement.

@jtojnar opinions?

@ncfavier ncfavier merged commit 45e44c5 into NixOS:master Mar 4, 2023
@pennae pennae deleted the nrd-html-manual branch March 4, 2023 18:29
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/tweag-nix-dev-update-45/26397/1

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: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants