This is a simple script that parses the Netlink YAML spec files
(Documentation/netlink/specs/), and generates RST files to be rendered
in the Network -> Netlink Specification documentation page.
Create a python script that is invoked during 'make htmldocs', reads the
YAML specs input file and generate the correspondent RST file.
Create a new Documentation/networking/netlink_spec index page, and
reference each Netlink RST file that was processed above in this main
index.rst file.
In case of any exception during the parsing, dump the error and skip
the file.
Do not regenerate the RST files if the input files (YAML) were not
changed in-between invocations.
Suggested-by: Jakub Kicinski <[email protected]>
Signed-off-by: Breno Leitao <[email protected]>
----
Changelog:
V3:
* Do not regenerate the RST files if the input files were not
changed. In order to do it, a few things changed:
- Rely on Makefile more to find what changed, and trigger
individual file processing
- The script parses file by file now (instead of batches)
- Create a new option to generate the index file
V2:
* Moved the logic from a sphinx extension to a external script
* Adjust some formatting as suggested by Donald Hunter and Jakub
* Auto generating all the rsts instead of having stubs
* Handling error gracefully
---
Documentation/Makefile | 16 +-
Documentation/networking/index.rst | 1 +
.../networking/netlink_spec/.gitignore | 1 +
.../networking/netlink_spec/readme.txt | 4 +
tools/net/ynl/ynl-gen-rst.py | 388 ++++++++++++++++++
5 files changed, 409 insertions(+), 1 deletion(-)
create mode 100644 Documentation/networking/netlink_spec/.gitignore
create mode 100644 Documentation/networking/netlink_spec/readme.txt
create mode 100755 tools/net/ynl/ynl-gen-rst.py
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2f35793acd2a..5c156fbb6cdf 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -97,7 +97,21 @@ quiet_cmd_sphinx = SPHINX $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
cp $(if $(patsubst /%,,$(DOCS_CSS)),$(abspath $(srctree)/$(DOCS_CSS)),$(DOCS_CSS)) $(BUILDDIR)/$3/_static/; \
fi
-htmldocs:
+YNL_INDEX:=$(srctree)/Documentation/networking/netlink_spec/index.rst
+YNL_RST_DIR:=$(srctree)/Documentation/networking/netlink_spec
+YNL_YAML_DIR:=$(srctree)/Documentation/netlink/specs
+YNL_TOOL:=$(srctree)/tools/net/ynl/ynl-gen-rst.py
+
+YNL_RST_FILES_TMP := $(patsubst %.yaml,%.rst,$(wildcard $(YNL_YAML_DIR)/*.yaml))
+YNL_RST_FILES := $(patsubst $(YNL_YAML_DIR)%,$(YNL_RST_DIR)%, $(YNL_RST_FILES_TMP))
+
+$(YNL_INDEX): $(YNL_RST_FILES)
+ @$(YNL_TOOL) -o $@ -x
+
+$(YNL_RST_DIR)/%.rst: $(YNL_YAML_DIR)/%.yaml
+ @$(YNL_TOOL) -i $< -o $@
+
+htmldocs: $(YNL_INDEX)
@$(srctree)/scripts/sphinx-pre-install --version-check
@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 683eb42309cc..cb435c141794 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -55,6 +55,7 @@ Contents:
filter
generic-hdlc
generic_netlink
+ netlink_spec/index
gen_stats
gtp
ila
diff --git a/Documentation/networking/netlink_spec/.gitignore b/Documentation/networking/netlink_spec/.gitignore
new file mode 100644
index 000000000000..30d85567b592
--- /dev/null
+++ b/Documentation/networking/netlink_spec/.gitignore
@@ -0,0 +1 @@
+*.rst
diff --git a/Documentation/networking/netlink_spec/readme.txt b/Documentation/networking/netlink_spec/readme.txt
new file mode 100644
index 000000000000..6763f99d216c
--- /dev/null
+++ b/Documentation/networking/netlink_spec/readme.txt
@@ -0,0 +1,4 @@
+SPDX-License-Identifier: GPL-2.0
+
+This file is populated during the build of the documentation (htmldocs) by the
+tools/net/ynl/ynl-gen-rst.py script.
diff --git a/tools/net/ynl/ynl-gen-rst.py b/tools/net/ynl/ynl-gen-rst.py
new file mode 100755
index 000000000000..b6292109e236
--- /dev/null
+++ b/tools/net/ynl/ynl-gen-rst.py
@@ -0,0 +1,388 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0
+# -*- coding: utf-8; mode: python -*-
+
+"""
+ Script to auto generate the documentation for Netlink specifications.
+
+ :copyright: Copyright (C) 2023 Breno Leitao <[email protected]>
+ :license: GPL Version 2, June 1991 see linux/COPYING for details.
+
+ This script performs extensive parsing to the Linux kernel's netlink YAML
+ spec files, in an effort to avoid needing to heavily mark up the original
+ YAML file.
+
+ This code is split in three big parts:
+ 1) RST formatters: Use to convert a string to a RST output
+ 2) Parser helpers: Functions to parse the YAML data structure
+ 3) Main function and small helpers
+"""
+
+from typing import Any, Dict, List
+import os.path
+import sys
+import argparse
+import logging
+import yaml
+
+
+SPACE_PER_LEVEL = 4
+
+
+# RST Formatters
+# ==============
+def headroom(level: int) -> str:
+ """Return space to format"""
+ return " " * (level * SPACE_PER_LEVEL)
+
+
+def bold(text: str) -> str:
+ """Format bold text"""
+ return f"**{text}**"
+
+
+def inline(text: str) -> str:
+ """Format inline text"""
+ return f"``{text}``"
+
+
+def sanitize(text: str) -> str:
+ """Remove newlines and multiple spaces"""
+ # This is useful for some fields that are spread across multiple lines
+ return str(text).replace("\n", "").strip()
+
+
+def rst_fields(key: str, value: str, level: int = 0) -> str:
+ """Return a RST formatted field"""
+ return headroom(level) + f":{key}: {value}"
+
+
+def rst_definition(key: str, value: Any, level: int = 0) -> str:
+ """Format a single rst definition"""
+ return headroom(level) + key + "\n" + headroom(level + 1) + str(value)
+
+
+def rst_paragraph(paragraph: str, level: int = 0) -> str:
+ """Return a formatted paragraph"""
+ return headroom(level) + paragraph
+
+
+def rst_bullet(item: str, level: int = 0) -> str:
+ """Return a formatted a bullet"""
+ return headroom(level) + f" - {item}"
+
+
+def rst_subsection(title: str) -> str:
+ """Add a sub-section to the document"""
+ return f"{title}\n" + "-" * len(title)
+
+
+def rst_subsubsection(title: str) -> str:
+ """Add a sub-sub-section to the document"""
+ return f"{title}\n" + "~" * len(title)
+
+
+def rst_section(title: str) -> str:
+ """Add a section to the document"""
+ return f"\n{title}\n" + "=" * len(title)
+
+
+def rst_subtitle(title: str) -> str:
+ """Add a subtitle to the document"""
+ return "\n" + "-" * len(title) + f"\n{title}\n" + "-" * len(title) + "\n\n"
+
+
+def rst_title(title: str) -> str:
+ """Add a title to the document"""
+ return "=" * len(title) + f"\n{title}\n" + "=" * len(title) + "\n\n"
+
+
+def rst_list_inline(list_: List[str], level: int = 0) -> str:
+ """Format a list using inlines"""
+ return headroom(level) + "[" + ", ".join(inline(i) for i in list_) + "]"
+
+
+def rst_header() -> str:
+ """The headers for all the auto generated RST files"""
+ lines = []
+
+ lines.append(rst_paragraph(".. SPDX-License-Identifier: GPL-2.0"))
+ lines.append(rst_paragraph(".. NOTE: This document was auto-generated.\n\n"))
+
+ return "\n".join(lines)
+
+
+def rst_toctree(maxdepth: int = 2) -> str:
+ """Generate a toctree RST primitive"""
+ lines = []
+
+ lines.append(".. toctree::")
+ lines.append(f" :maxdepth: {maxdepth}\n\n")
+
+ return "\n".join(lines)
+
+
+# Parsers
+# =======
+
+
+def parse_mcast_group(mcast_group: List[Dict[str, Any]]) -> str:
+ """Parse 'multicast' group list and return a formatted string"""
+ lines = []
+ for group in mcast_group:
+ lines.append(rst_bullet(group["name"]))
+
+ return "\n".join(lines)
+
+
+def parse_do(do_dict: Dict[str, Any], level: int = 0) -> str:
+ """Parse 'do' section and return a formatted string"""
+ lines = []
+ for key in do_dict.keys():
+ lines.append(rst_paragraph(bold(key), level + 1))
+ lines.append(parse_do_attributes(do_dict[key], level + 1) + "\n")
+
+ return "\n".join(lines)
+
+
+def parse_do_attributes(attrs: Dict[str, Any], level: int = 0) -> str:
+ """Parse 'attributes' section"""
+ if "attributes" not in attrs:
+ return ""
+ lines = [rst_fields("attributes", rst_list_inline(attrs["attributes"]), level + 1)]
+
+ return "\n".join(lines)
+
+
+def parse_operations(operations: List[Dict[str, Any]]) -> str:
+ """Parse operations block"""
+ preprocessed = ["name", "doc", "title", "do", "dump"]
+ lines = []
+
+ for operation in operations:
+ lines.append(rst_section(operation["name"]))
+ lines.append(rst_paragraph(sanitize(operation["doc"])) + "\n")
+
+ for key in operation.keys():
+ if key in preprocessed:
+ # Skip the special fields
+ continue
+ lines.append(rst_fields(key, operation[key], 0))
+
+ if "do" in operation:
+ lines.append(rst_paragraph(":do:", 0))
+ lines.append(parse_do(operation["do"], 0))
+ if "dump" in operation:
+ lines.append(rst_paragraph(":dump:", 0))
+ lines.append(parse_do(operation["dump"], 0))
+
+ # New line after fields
+ lines.append("\n")
+
+ return "\n".join(lines)
+
+
+def parse_entries(entries: List[Dict[str, Any]], level: int) -> str:
+ """Parse a list of entries"""
+ lines = []
+ for entry in entries:
+ if isinstance(entry, dict):
+ # entries could be a list or a dictionary
+ lines.append(
+ rst_fields(entry.get("name", ""), sanitize(entry.get("doc", "")), level)
+ )
+ elif isinstance(entry, list):
+ lines.append(rst_list_inline(entry, level))
+ else:
+ lines.append(rst_bullet(inline(sanitize(entry)), level))
+
+ lines.append("\n")
+ return "\n".join(lines)
+
+
+def parse_definitions(defs: Dict[str, Any]) -> str:
+ """Parse definitions section"""
+ preprocessed = ["name", "entries", "members"]
+ ignored = ["render-max"] # This is not printed
+ lines = []
+
+ for definition in defs:
+ lines.append(rst_section(definition["name"]))
+ for k in definition.keys():
+ if k in preprocessed + ignored:
+ continue
+ lines.append(rst_fields(k, sanitize(definition[k]), 0))
+
+ # Field list needs to finish with a new line
+ lines.append("\n")
+ if "entries" in definition:
+ lines.append(rst_paragraph(":entries:", 0))
+ lines.append(parse_entries(definition["entries"], 1))
+ if "members" in definition:
+ lines.append(rst_paragraph(":members:", 0))
+ lines.append(parse_entries(definition["members"], 1))
+
+ return "\n".join(lines)
+
+
+def parse_attr_sets(entries: List[Dict[str, Any]]) -> str:
+ """Parse attribute from attribute-set"""
+ preprocessed = ["name", "type"]
+ ignored = ["checks"]
+ lines = []
+
+ for entry in entries:
+ lines.append(rst_section(entry["name"]))
+ for attr in entry["attributes"]:
+ type_ = attr.get("type")
+ attr_line = bold(attr["name"])
+ if type_:
+ # Add the attribute type in the same line
+ attr_line += f" ({inline(type_)})"
+
+ lines.append(rst_subsubsection(attr_line))
+
+ for k in attr.keys():
+ if k in preprocessed + ignored:
+ continue
+ lines.append(rst_fields(k, sanitize(attr[k]), 2))
+ lines.append("\n")
+
+ return "\n".join(lines)
+
+
+def parse_yaml(obj: Dict[str, Any]) -> str:
+ """Format the whole YAML into a RST string"""
+ lines = []
+
+ # Main header
+
+ lines.append(rst_header())
+
+ title = f"Family ``{obj['name']}`` netlink specification"
+ lines.append(rst_title(title))
+ lines.append(rst_paragraph(".. contents::\n"))
+
+ if "doc" in obj:
+ lines.append(rst_subtitle("Summary"))
+ lines.append(rst_paragraph(obj["doc"], 0))
+
+ # Operations
+ if "operations" in obj:
+ lines.append(rst_subtitle("Operations"))
+ lines.append(parse_operations(obj["operations"]["list"]))
+
+ # Multicast groups
+ if "mcast-groups" in obj:
+ lines.append(rst_subtitle("Multicast groups"))
+ lines.append(parse_mcast_group(obj["mcast-groups"]["list"]))
+
+ # Definitions
+ if "definitions" in obj:
+ lines.append(rst_subtitle("Definitions"))
+ lines.append(parse_definitions(obj["definitions"]))
+
+ # Attributes set
+ if "attribute-sets" in obj:
+ lines.append(rst_subtitle("Attribute sets"))
+ lines.append(parse_attr_sets(obj["attribute-sets"]))
+
+ return "\n".join(lines)
+
+
+# Main functions
+# ==============
+
+
+def parse_arguments() -> argparse.Namespace:
+ """Parse arguments from user"""
+ parser = argparse.ArgumentParser(description="Netlink RST generator")
+
+ parser.add_argument("-v", "--verbose", action="store_true")
+ parser.add_argument("-o", "--output", help="Output file name")
+
+ # Index and input are mutually exclusive
+ group = parser.add_mutually_exclusive_group()
+ group.add_argument(
+ "-x", "--index", action="store_true", help="Generate the index page"
+ )
+ group.add_argument("-i", "--input", help="YAML file name")
+
+ args = parser.parse_args()
+
+ if args.verbose:
+ logging.basicConfig(level=logging.DEBUG)
+
+ if args.input and not os.path.isfile(args.input):
+ logging.warning("%s is not a valid file.", args.input)
+ sys.exit(-1)
+
+ if not args.output:
+ logging.error("No output file specified.")
+ sys.exit(-1)
+
+ if os.path.isfile(args.output):
+ logging.debug("%s already exists. Overwriting it.", args.output)
+
+ return args
+
+
+def parse_yaml_file(filename: str) -> str:
+ """Transform the YAML specified by filename into a rst-formmated string"""
+ with open(filename, "r", encoding="utf-8") as spec_file:
+ yaml_data = yaml.safe_load(spec_file)
+ content = parse_yaml(yaml_data)
+
+ return content
+
+
+def write_to_rstfile(content: str, filename: str) -> None:
+ """Write the generated content into an RST file"""
+ logging.debug("Saving RST file to %s", filename)
+
+ with open(filename, "w", encoding="utf-8") as rst_file:
+ rst_file.write(content)
+
+
+def generate_main_index_rst(output: str) -> None:
+ """Generate the `networking_spec/index` content and write to the file"""
+ lines = []
+
+ lines.append(rst_header())
+ lines.append(rst_title("Netlink Specification"))
+ lines.append(rst_toctree(1))
+
+ index_dir = os.path.dirname(output)
+ logging.debug("Looking for .rst files in %s", index_dir)
+ for filename in os.listdir(index_dir):
+ if not filename.endswith(".rst") or filename == "index.rst":
+ continue
+ lines.append(f" {filename.replace('.rst', '')}\n")
+
+ logging.debug("Writing an index file at %s", output)
+ write_to_rstfile("".join(lines), output)
+
+
+def main() -> None:
+ """Main function that reads the YAML files and generates the RST files"""
+
+ args = parse_arguments()
+
+ if args.input:
+ logging.debug("Parsing %s", args.input)
+ try:
+ content = parse_yaml_file(os.path.join(args.input))
+ except Exception as exception:
+ logging.warning("Failed to parse %s.", args.input)
+ logging.warning(exception)
+ sys.exit(-1)
+
+ write_to_rstfile(content, args.output)
+
+ if args.index:
+ # Generate the index RST file
+ generate_main_index_rst(args.output)
+
+
+if __name__ == "__main__":
+ main()
--
2.34.1
On Tue, 21 Nov 2023 03:48:31 -0800 Breno Leitao wrote:
> This is a simple script that parses the Netlink YAML spec files
> (Documentation/netlink/specs/), and generates RST files to be rendered
> in the Network -> Netlink Specification documentation page.
>
> Create a python script that is invoked during 'make htmldocs', reads the
> YAML specs input file and generate the correspondent RST file.
>
> Create a new Documentation/networking/netlink_spec index page, and
> reference each Netlink RST file that was processed above in this main
> index.rst file.
>
> In case of any exception during the parsing, dump the error and skip
> the file.
>
> Do not regenerate the RST files if the input files (YAML) were not
> changed in-between invocations.
I can confirm that it does what it says and incremental make
htmldocs does not take forever.
Reviewed-by: Jakub Kicinski <[email protected]>
Thanks!
Hello:
This patch was applied to netdev/net-next.git (main)
by David S. Miller <[email protected]>:
On Tue, 21 Nov 2023 03:48:31 -0800 you wrote:
> This is a simple script that parses the Netlink YAML spec files
> (Documentation/netlink/specs/), and generates RST files to be rendered
> in the Network -> Netlink Specification documentation page.
>
> Create a python script that is invoked during 'make htmldocs', reads the
> YAML specs input file and generate the correspondent RST file.
>
> [...]
Here is the summary with links:
- [v3] Documentation: Document each netlink family
https://git.kernel.org/netdev/net-next/c/f061c9f7d058
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
On Tue, 21 Nov 2023, Breno Leitao <[email protected]> wrote:
> This is a simple script that parses the Netlink YAML spec files
> (Documentation/netlink/specs/), and generates RST files to be rendered
> in the Network -> Netlink Specification documentation page.
First of all, my boilerplate complaint: All extra processing for Sphinx
should really be done using Sphinx extensions instead of adding Makefile
hacks. I don't think it's sustainable to keep adding this stuff. We
chose Sphinx because it is extensible, and to avoid the Rube Goldberg
machine that the previous documentation build system was.
At the very least I would've expected to see Jon's ack on changes like
this.
The specific problem with this patch, now merged as commit f061c9f7d058
("Documentation: Document each netlink family"), is that it explicitly
writes intermediate files in the $(srctree). Even for O= builds. That's
one of the pitfalls of hacking it in Makefiles.
See below.
> Create a python script that is invoked during 'make htmldocs', reads the
> YAML specs input file and generate the correspondent RST file.
>
> Create a new Documentation/networking/netlink_spec index page, and
> reference each Netlink RST file that was processed above in this main
> index.rst file.
>
> In case of any exception during the parsing, dump the error and skip
> the file.
>
> Do not regenerate the RST files if the input files (YAML) were not
> changed in-between invocations.
>
> Suggested-by: Jakub Kicinski <[email protected]>
> Signed-off-by: Breno Leitao <[email protected]>
[snip]
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 2f35793acd2a..5c156fbb6cdf 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -97,7 +97,21 @@ quiet_cmd_sphinx = SPHINX $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
> cp $(if $(patsubst /%,,$(DOCS_CSS)),$(abspath $(srctree)/$(DOCS_CSS)),$(DOCS_CSS)) $(BUILDDIR)/$3/_static/; \
> fi
>
> -htmldocs:
> +YNL_INDEX:=$(srctree)/Documentation/networking/netlink_spec/index.rst
> +YNL_RST_DIR:=$(srctree)/Documentation/networking/netlink_spec
> +YNL_YAML_DIR:=$(srctree)/Documentation/netlink/specs
> +YNL_TOOL:=$(srctree)/tools/net/ynl/ynl-gen-rst.py
> +
> +YNL_RST_FILES_TMP := $(patsubst %.yaml,%.rst,$(wildcard $(YNL_YAML_DIR)/*.yaml))
> +YNL_RST_FILES := $(patsubst $(YNL_YAML_DIR)%,$(YNL_RST_DIR)%, $(YNL_RST_FILES_TMP))
> +
> +$(YNL_INDEX): $(YNL_RST_FILES)
> + @$(YNL_TOOL) -o $@ -x
> +
> +$(YNL_RST_DIR)/%.rst: $(YNL_YAML_DIR)/%.yaml
> + @$(YNL_TOOL) -i $< -o $@
> +
Right here.
> +htmldocs: $(YNL_INDEX)
> @$(srctree)/scripts/sphinx-pre-install --version-check
> @+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
>
> diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
> index 683eb42309cc..cb435c141794 100644
> --- a/Documentation/networking/index.rst
> +++ b/Documentation/networking/index.rst
> @@ -55,6 +55,7 @@ Contents:
> filter
> generic-hdlc
> generic_netlink
> + netlink_spec/index
> gen_stats
> gtp
> ila
> diff --git a/Documentation/networking/netlink_spec/.gitignore b/Documentation/networking/netlink_spec/.gitignore
> new file mode 100644
> index 000000000000..30d85567b592
> --- /dev/null
> +++ b/Documentation/networking/netlink_spec/.gitignore
> @@ -0,0 +1 @@
> +*.rst
And then goes on to git ignore the mess it made.
BR,
Jani.
--
Jani Nikula, Intel
Jani Nikula <[email protected]> writes:
> On Tue, 21 Nov 2023, Breno Leitao <[email protected]> wrote:
>> This is a simple script that parses the Netlink YAML spec files
>> (Documentation/netlink/specs/), and generates RST files to be rendered
>> in the Network -> Netlink Specification documentation page.
>
> First of all, my boilerplate complaint: All extra processing for Sphinx
> should really be done using Sphinx extensions instead of adding Makefile
> hacks. I don't think it's sustainable to keep adding this stuff. We
> chose Sphinx because it is extensible, and to avoid the Rube Goldberg
> machine that the previous documentation build system was.
So I feel like we've (me included) have kind of sent Breno around in
circles on this one. This *was* implemented as an extension once:
https://lore.kernel.org/netdev/[email protected]/
At that time it seemed too complex, and I thought that an external
script would lead to a simpler implementation overall. Perhaps I was
wrong.
I worry that a proliferation of extensions adds its own sort of
complexity and hazards - look at the things Vegard has fixed recently,
for example. Relatively few people can work in that environment, and
extensions can make our version-support troubles worse. So I'm not
fully sold on the idea that everything should be an extension,
especially if it can be expressed as a simple dependency and build step
in the makefile.
Some of the uglier makefile stuff we have is a different story...
Anyway, I apologize for my role in making this particular addition
harder than it needed to be. Perhaps, for the future, we should put
together and agree on a document (of all things) on how we think this
sort of functionality should be added.
Thanks,
jon
On Tue, 30 Jan 2024, Jonathan Corbet <[email protected]> wrote:
> Jani Nikula <[email protected]> writes:
>
>> On Tue, 21 Nov 2023, Breno Leitao <[email protected]> wrote:
>>> This is a simple script that parses the Netlink YAML spec files
>>> (Documentation/netlink/specs/), and generates RST files to be rendered
>>> in the Network -> Netlink Specification documentation page.
>>
>> First of all, my boilerplate complaint: All extra processing for Sphinx
>> should really be done using Sphinx extensions instead of adding Makefile
>> hacks. I don't think it's sustainable to keep adding this stuff. We
>> chose Sphinx because it is extensible, and to avoid the Rube Goldberg
>> machine that the previous documentation build system was.
>
> So I feel like we've (me included) have kind of sent Breno around in
> circles on this one. This *was* implemented as an extension once:
>
> https://lore.kernel.org/netdev/[email protected]/
>
> At that time it seemed too complex, and I thought that an external
> script would lead to a simpler implementation overall. Perhaps I was
> wrong.
>
> I worry that a proliferation of extensions adds its own sort of
> complexity and hazards - look at the things Vegard has fixed recently,
> for example.
If we're talking about the same things, I think one of the main problems
there was shelling out to an external script while it could all have
been trivially implemented directly in the extension. ;)
> Relatively few people can work in that environment, and
> extensions can make our version-support troubles worse. So I'm not
> fully sold on the idea that everything should be an extension,
> especially if it can be expressed as a simple dependency and build step
> in the makefile.
I think we're just going to have to agree to disagree here. And,
ultimately, it's your call as the documentation maintainer.
I'm sure some individual things are simple to put in the makefiles, but
I believe overall the entire thing would be simpler if we avoided that.
> Some of the uglier makefile stuff we have is a different story...
>
> Anyway, I apologize for my role in making this particular addition
> harder than it needed to be. Perhaps, for the future, we should put
> together and agree on a document (of all things) on how we think this
> sort of functionality should be added.
Perhaps. The problem at hand, though, is that after 'make
O=/path/to/build htmldocs' I have this cruft in my source tree:
$ git ls-files -oi --exclude-per-directory=.gitignore
Documentation/networking/netlink_spec/devlink.rst
Documentation/networking/netlink_spec/dpll.rst
Documentation/networking/netlink_spec/ethtool.rst
Documentation/networking/netlink_spec/fou.rst
Documentation/networking/netlink_spec/handshake.rst
Documentation/networking/netlink_spec/index.rst
Documentation/networking/netlink_spec/mptcp_pm.rst
Documentation/networking/netlink_spec/netdev.rst
Documentation/networking/netlink_spec/nfsd.rst
Documentation/networking/netlink_spec/ovs_datapath.rst
Documentation/networking/netlink_spec/ovs_flow.rst
Documentation/networking/netlink_spec/ovs_vport.rst
Documentation/networking/netlink_spec/rt_addr.rst
Documentation/networking/netlink_spec/rt_link.rst
Documentation/networking/netlink_spec/rt_route.rst
Documentation/networking/netlink_spec/tc.rst
I'm not even sure what the best way to fix that would be. (Apart from
turning it into an extension, of course. ;)
BR,
Jani.
--
Jani Nikula, Intel
On Tue, Jan 30, 2024 at 07:22:08AM -0700, Jonathan Corbet wrote:
> Jani Nikula <[email protected]> writes:
>
> > On Tue, 21 Nov 2023, Breno Leitao <[email protected]> wrote:
> >> This is a simple script that parses the Netlink YAML spec files
> >> (Documentation/netlink/specs/), and generates RST files to be rendered
> >> in the Network -> Netlink Specification documentation page.
> >
> > First of all, my boilerplate complaint: All extra processing for Sphinx
> > should really be done using Sphinx extensions instead of adding Makefile
> > hacks. I don't think it's sustainable to keep adding this stuff. We
> > chose Sphinx because it is extensible, and to avoid the Rube Goldberg
> > machine that the previous documentation build system was.
>
> So I feel like we've (me included) have kind of sent Breno around in
> circles on this one. This *was* implemented as an extension once:
>
> https://lore.kernel.org/netdev/[email protected]/
>
> At that time it seemed too complex, and I thought that an external
> script would lead to a simpler implementation overall. Perhaps I was
> wrong.
I think you are correct. I personally _think_ that the external script
is better, mainly because it is self contained, thus, easier to
maintain.
On 30/01/2024 17:06, Breno Leitao wrote:
> On Tue, Jan 30, 2024 at 07:22:08AM -0700, Jonathan Corbet wrote:
>> Jani Nikula <[email protected]> writes:
>>
>>> On Tue, 21 Nov 2023, Breno Leitao <[email protected]> wrote:
>>>> This is a simple script that parses the Netlink YAML spec files
>>>> (Documentation/netlink/specs/), and generates RST files to be rendered
>>>> in the Network -> Netlink Specification documentation page.
>>>
>>> First of all, my boilerplate complaint: All extra processing for Sphinx
>>> should really be done using Sphinx extensions instead of adding Makefile
>>> hacks. I don't think it's sustainable to keep adding this stuff. We
>>> chose Sphinx because it is extensible, and to avoid the Rube Goldberg
>>> machine that the previous documentation build system was.
>>
>> So I feel like we've (me included) have kind of sent Breno around in
>> circles on this one. This *was* implemented as an extension once:
>>
>> https://lore.kernel.org/netdev/[email protected]/
>>
>> At that time it seemed too complex, and I thought that an external
>> script would lead to a simpler implementation overall. Perhaps I was
>> wrong.
>
> I think you are correct. I personally _think_ that the external script
> is better, mainly because it is self contained, thus, easier to
> maintain.
From a cursory look at the two versions, the actual Python code to read
the YAML and write the reST is the same in both cases. (Breno, please
correct me if I'm wrong.)
It should be fairly easy to support both methods by moving most of the
code into a library and then having two thin wrappers around it, one
being a stand-alone command-line script and one being the Sphinx extension.
The stand-alone script could even be put directly in the library code,
just wrapped in "if __name__ == '__main__'".
A stand-alone script might be useful for debugging the output without
invoking the full sphinx-build machinery (i.e. basically having an easy
way to inspect and diff the output when making changes to the code).
Bottom line: This particular thing looks like a false dichotomy to me.
We should still fix the writing of .rst to $(srctree), though --
our use of parse-headers.pl seems to sidestep this by writing the
intermediate .rst output to Documentation/output/, I'll have to look a
bit more closely.
Vegard
On Tue, Jan 30, 2024 at 05:23:36PM +0100, Vegard Nossum wrote:
>
> On 30/01/2024 17:06, Breno Leitao wrote:
> > On Tue, Jan 30, 2024 at 07:22:08AM -0700, Jonathan Corbet wrote:
> > > Jani Nikula <[email protected]> writes:
> > >
> > > > On Tue, 21 Nov 2023, Breno Leitao <[email protected]> wrote:
> > > > > This is a simple script that parses the Netlink YAML spec files
> > > > > (Documentation/netlink/specs/), and generates RST files to be rendered
> > > > > in the Network -> Netlink Specification documentation page.
> > > >
> > > > First of all, my boilerplate complaint: All extra processing for Sphinx
> > > > should really be done using Sphinx extensions instead of adding Makefile
> > > > hacks. I don't think it's sustainable to keep adding this stuff. We
> > > > chose Sphinx because it is extensible, and to avoid the Rube Goldberg
> > > > machine that the previous documentation build system was.
> > >
> > > So I feel like we've (me included) have kind of sent Breno around in
> > > circles on this one. This *was* implemented as an extension once:
> > >
> > > https://lore.kernel.org/netdev/[email protected]/
> > >
> > > At that time it seemed too complex, and I thought that an external
> > > script would lead to a simpler implementation overall. Perhaps I was
> > > wrong.
> >
> > I think you are correct. I personally _think_ that the external script
> > is better, mainly because it is self contained, thus, easier to
> > maintain.
>
> From a cursory look at the two versions, the actual Python code to read
> the YAML and write the reST is the same in both cases. (Breno, please
> correct me if I'm wrong.)
You are correct. They are similar because Sphinx was not bringing much
value to what I was trying to do (or I was not able to explore all
Sphinx benefit - It was my very first Sphinx plug-in).
That said, the plug-in was basically a wrapper around "the Python code",
that was re-used for the one-off script. So, moving from one to another
was easy.
On 30/01/2024 17:23, Vegard Nossum wrote:
> We should still fix the writing of .rst to $(srctree), though -- our
> use of parse-headers.pl seems to sidestep this by writing the
> intermediate .rst output to Documentation/output/, I'll have to look
> a bit more closely.
I have now spent quite a bit of time investigating this.
The main result is that Sphinx really does NOT like it when documents
are located outside of the source root.
What makes the parse-headers.pl approach work is the 'kernel-include'
custom directive (from kernel_include.py) -- this allows you to include
files from outside the source root into a file that _is_ in the source
root. So the .rst files generated by parse-headers.pl are included using
this directive, rather than being treated as real documents that you can
refer to by their name.
If we did the same, one option would be to create one .rst file for each
yaml file currently in Documentation/netlink/specs/ and just have them
'kernel-include' the corresponding generated file. I don't think this is
a great idea because we'd always have to remember to create new files
when a corresponding .yaml file is added -- and it feels like an ugly
workaround since each file would just be a single line including the
generated file.
Sphinx 7.2.0 introduces a mapping of document names (like
"networking/index") to paths, and it looks doable to write an extension
that would discover additional .rst files from outside the source path
to process. But before this there is essentially no way to have files
outside the source root appear in the toctree.
But I wouldn't like to bump the minimum required version from 2.4 to 7.2
either. Also, the extension wouldn't be using a public/standard API so
it would essentially be unsupported and could break again at any time.
It might not even be possible at all even today.
Another idea would be to have another layer of directories in the output
directory, so e.g.:
$(BUILDDIR)/Documentation -> $(srctree)/Documentation (symlink)
$(BUILDDIR)/generated/...
Of course this would break ALL the web links like
https://docs.kernel.org/process/stable-kernel-rules.html
since it would now be
https://docs.kernel.org/Documentation/process/stable-kernel-rules.html
so I consider this a no-go as well. (We could consider creating N
symlinks instead; one for each directory under Documentation/ -- but
this again goes well into hacky territory IMHO.)
As far as I can tell, the most viable remaining option is to use a true
Sphinx extension that defines a new directive (that points directly to a
YAML file) and never write out the .rst to a file.
I think this was Jani's preferred solution as well.
Jon, Breno -- what do you think?
Vegard
Hi,
On Fri, 9 Feb 2024 15:47:16 +0100, Vegard Nossum wrote:
> On 30/01/2024 17:23, Vegard Nossum wrote:
>> We should still fix the writing of .rst to $(srctree), though -- our
>> use of parse-headers.pl seems to sidestep this by writing the
>> intermediate .rst output to Documentation/output/, I'll have to look
>> a bit more closely.
>
> I have now spent quite a bit of time investigating this.
>
> The main result is that Sphinx really does NOT like it when documents
> are located outside of the source root.
There is a hack extension named "kernel_include".
See Documentation/sphinx/kernel_include.py (no, I'm not saying
I understand it.)
The "kernel-include::" directive is exploited only in
Documentation/userspace-api/media/dvb/headers.rst.
I have no idea if Jon is happy to see another subsystem
to start exploiting this extension.
And you should be interested in seeing
Documentation/userspace-api/media/Makefile and how it is integrated
into Documentation/Makefile.
I think media people are doing similar things you'd like to do.
HTH, Akira