Breno Leitao <[email protected]> writes:
> 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 located under Documentation/netlink/specs, parses one by one
> and generates a correspondent RST file for each YAML 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.
>
> Suggested-by: Jakub Kicinski <[email protected]>
> Signed-off-by: Breno Leitao <[email protected]>
In principle I like this approach better. There is one problem, though:
- In current kernels, on my machine, "make htmldocs" when nothing has
changed takes about 6s to complete.
- With this patch applied, it takes a little over 5 *minutes*.
Without having delved into it too far, I am guessing that the
unconditional recreation of the netlink RST files is causing the rebuild
of much of the documentation. Even so, I don't quite get it.
That, clearly, would need to be fixed before this can go in.
Thanks,
jon
On Fri, 17 Nov 2023 15:17:02 -0700 Jonathan Corbet wrote:
> In principle I like this approach better. There is one problem, though:
>
> - In current kernels, on my machine, "make htmldocs" when nothing has
> changed takes about 6s to complete.
>
> - With this patch applied, it takes a little over 5 *minutes*.
>
> Without having delved into it too far, I am guessing that the
> unconditional recreation of the netlink RST files is causing the rebuild
> of much of the documentation. Even so, I don't quite get it.
>
> That, clearly, would need to be fixed before this can go in.
FWIW on the C code-gen side we avoid touching the files if nothing
changed both at the Makefile level:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/tools/net/ynl/generated/Makefile#n28
And the tool itself actually generates to a tempfile and compares
if the output changed:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=2b7ac0c87d985c92e519995853c52b9649ea4b07
On Fri, Nov 17, 2023 at 04:39:39PM -0800, Jakub Kicinski wrote:
> On Fri, 17 Nov 2023 15:17:02 -0700 Jonathan Corbet wrote:
> > In principle I like this approach better. There is one problem, though:
> >
> > - In current kernels, on my machine, "make htmldocs" when nothing has
> > changed takes about 6s to complete.
> >
> > - With this patch applied, it takes a little over 5 *minutes*.
> >
> > Without having delved into it too far, I am guessing that the
> > unconditional recreation of the netlink RST files is causing the rebuild
> > of much of the documentation. Even so, I don't quite get it.
> >
> > That, clearly, would need to be fixed before this can go in.
>
> FWIW on the C code-gen side we avoid touching the files if nothing
> changed both at the Makefile level:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/tools/net/ynl/generated/Makefile#n28
I was able to do something similar and it works quite well. Only
regenerate what is not up-to-date. See below what I am doing. (I needed
to change the python to adapt)
> And the tool itself actually generates to a tempfile and compares
> if the output changed:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=2b7ac0c87d985c92e519995853c52b9649ea4b07
I am not planning to do it, since I would like to trust Make. Let me
know if you think this is important and I can do it also.
--
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 2f35793acd2a..dad6e2ecf082 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -97,7 +97,22 @@ quiet_cmd_sphinx = SPHINX $@ --> file://$(abspath $(BUILDDIR)/$3/$4)
cp $(if $(patsubst /%,,$(DOCS_CSS)),$(abspath $(srctree)/$(DOCS_CSS)),$(DOCS_CSS)) $(BUILDDIR)/$3/_stati
c/; \
fi
-htmldocs:
+YNL_TOOL:=$(srctree)/tools/net/ynl/ynl-gen-rst.py
+YNL_YAML_DIR:=$(srctree)/Documentation/netlink/specs
+YNL_RST_DIR:=$(srctree)/Documentation/networking/netlink_spec
+YNL_INDEX:=$(srctree)/Documentation/networking/netlink_spec/index.rst
+
+YNL_YAML_FILES := $(wildcard $(YNL_YAML_DIR)/*.yaml)
+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) -x # Generate the index
+
+%.rst: $(YNL_YAMLS_FILES)
+ $(YNL_TOOL) -i $(patsubst %.rst,%.yaml, $(@F)) # generate individual rst files
+
+htmldocs: $(YNL_INDEX)
@$(srctree)/scripts/sphinx-pre-install --version-check
@+$(foreach var,$(SPHINXDIRS),$(call loop_cmd,sphinx,html,$(var),,$(var)))
On Mon, 20 Nov 2023 11:55:26 -0800 Breno Leitao wrote:
> I am not planning to do it, since I would like to trust Make. Let me
> know if you think this is important and I can do it also.
Makefile is good enough for me.
> +$(YNL_INDEX): $(YNL_RST_FILES)
> + $(YNL_TOOL) -x # Generate the index
> +
> +%.rst: $(YNL_YAMLS_FILES)
> + $(YNL_TOOL) -i $(patsubst %.rst,%.yaml, $(@F)) # generate individual rst files
IDK what @F means, can the tool take one file at a time and then
we can make the rule a more usual:
%.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)))
On Mon, Nov 20, 2023 at 12:07:06PM -0800, Jakub Kicinski wrote:
> On Mon, 20 Nov 2023 11:55:26 -0800 Breno Leitao wrote:
> > I am not planning to do it, since I would like to trust Make. Let me
> > know if you think this is important and I can do it also.
>
> Makefile is good enough for me.
>
> > +$(YNL_INDEX): $(YNL_RST_FILES)
> > + $(YNL_TOOL) -x # Generate the index
> > +
> > +%.rst: $(YNL_YAMLS_FILES)
> > + $(YNL_TOOL) -i $(patsubst %.rst,%.yaml, $(@F)) # generate individual rst files
>
> IDK what @F means, can the tool take one file at a time and then
> we can make the rule a more usual:
'$(@F)' is the file-within-directory part of the file name of the
target. If the value of ‘$@’ is dir/foo.o then ‘$(@F)’ is foo.o. ‘$(@F)’
is equivalent to ‘$(notdir $@)’.
> %.rst: $(YNL_YAML_DIR)/%.yaml
> $(YNL_TOOL) -i $< -o $@
That is basically what it does now in the current implementation, but,
you don't need to pass the full path and no output file, since it knows
where to get the file and where to save it to.
If you are curious about the current python script, I've pushed it here:
https://github.com/leitao/linux/blob/netdev_discuss/tools/net/ynl/ynl-gen-rst.py
I can easily remove the paths inside the python file and only keep it in
the Makefile, so, we can use -i $< and -o $@.
On Mon, 20 Nov 2023 12:43:11 -0800 Breno Leitao wrote:
> > %.rst: $(YNL_YAML_DIR)/%.yaml
> > $(YNL_TOOL) -i $< -o $@
>
> That is basically what it does now in the current implementation, but,
> you don't need to pass the full path and no output file, since it knows
> where to get the file and where to save it to.
>
> If you are curious about the current python script, I've pushed it here:
> https://github.com/leitao/linux/blob/netdev_discuss/tools/net/ynl/ynl-gen-rst.py
>
> I can easily remove the paths inside the python file and only keep it in
> the Makefile, so, we can use -i $< and -o $@.
I think switching to -i / -o with full paths and removing the paths
from the generator is worthwhile.
We'll need to call the generator for another place sooner or later.
On Mon, Nov 20, 2023 at 01:14:24PM -0800, Jakub Kicinski wrote:
> On Mon, 20 Nov 2023 12:43:11 -0800 Breno Leitao wrote:
> > > %.rst: $(YNL_YAML_DIR)/%.yaml
> > > $(YNL_TOOL) -i $< -o $@
> >
> > That is basically what it does now in the current implementation, but,
> > you don't need to pass the full path and no output file, since it knows
> > where to get the file and where to save it to.
> >
> > If you are curious about the current python script, I've pushed it here:
> > https://github.com/leitao/linux/blob/netdev_discuss/tools/net/ynl/ynl-gen-rst.py
> >
> > I can easily remove the paths inside the python file and only keep it in
> > the Makefile, so, we can use -i $< and -o $@.
>
> I think switching to -i / -o with full paths and removing the paths
> from the generator is worthwhile.
>
> We'll need to call the generator for another place sooner or later.
I do agree with you. Let me update and send a V3 with these changes.