Message-ID: <20250613141355.1bba92fc@foz.lan>
Date: Fri, 13 Jun 2025 14:13:55 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Donald Hunter <donald.hunter@...il.com>
Cc: Linux Doc Mailing List <linux-doc@...r.kernel.org>, Jonathan Corbet
 <corbet@....net>, linux-kernel@...r.kernel.org, Akira Yokosawa
 <akiyks@...il.com>, "David S. Miller" <davem@...emloft.net>, Ignacio
 Encinas Rubio <ignacio@...cinas.com>, Marco Elver <elver@...gle.com>, Shuah
 Khan <skhan@...uxfoundation.org>, Eric Dumazet <edumazet@...gle.com>, Jan
 Stancek <jstancek@...hat.com>, Paolo Abeni <pabeni@...hat.com>, Ruben
 Wauters <rubenru09@....com>, joel@...lfernandes.org,
 linux-kernel-mentees@...ts.linux.dev, lkmm@...ts.linux.dev,
 netdev@...r.kernel.org, peterz@...radead.org, stern@...land.harvard.edu,
 Breno Leitao <leitao@...ian.org>
Subject: Re: [PATCH v2 00/12] Don't generate netlink .rst files inside
 $(srctree)

Em Fri, 13 Jun 2025 12:05:56 +0100
Donald Hunter <donald.hunter@...il.com> escreveu:

> Mauro Carvalho Chehab <mchehab+huawei@...nel.org> writes:
> 
> > As discussed at:
> >    https://lore.kernel.org/all/20250610101331.62ba466f@foz.lan/
> >
> > changeset f061c9f7d058 ("Documentation: Document each netlink family")
> > added a logic which generates *.rst files inside $(srctree). This is bad when
> > O=<BUILDDIR> is used.
> >
> > A recent change renamed the yaml files used by Netlink, revealing a bad
> > side effect: as "make cleandocs" don't clean the produced files, symbols 
> > appear duplicated for people that don't build the kernel from scratch.
> >
> > There are some possible solutions for that. The simplest one, which is what
> > this series address, places the build files inside Documentation/output. 
> > The changes to do that are simple enough, but has one drawback,
> > as it requires a (simple) template file for every netlink family file from
> > netlink/specs. The template is simple enough:
> >
> >         .. kernel-include:: $BUILDDIR/networking/netlink_spec/<family>.rst  
> 
> I think we could skip describing this since it was an approach that has
> now been dropped.

Ok. Will drop on next versions.

> 
> > Part of the issue is that sphinx-build only produces html files for sources
> > inside the source tree (Documentation/). 
> >
> > To address that, add an yaml parser extension to Sphinx.
> >
> > It should be noticed that this version has one drawback: it increases the
> > documentation build time. I suspect that the culprit is inside Sphinx
> > glob logic and the way it handles exclude_patterns. What happens is that
> > sphinx/project.py uses glob, which, on my own experiences, it is slow
> > (due to that, I ended implementing my own glob logic for kernel-doc).
> >
> > On the plus side, the extension is flexible enough to handle other types
> > of yaml files, as the actual yaml conversion logic is outside the extension.  
> 
> I don't think the extension would handle anything other than the Netlink
> yaml specs, and I don't think that should be a goal of this patchset.

Not necessarily. We do have already DT yaml files (although there's
a separate process to handle those outside the tree). Nothing prevents
we end having more. See, the way Sphinx parser works is that it will cover
all files with *.yaml extension no matter where it is located within the
tree. We may end needing to use it for something else as well (*).

(*) at the last Media Summit, we did have some discussions about using
    either yaml or rst for sensor documentation.

> > With this version, there's no need to add any template file per netlink/spec
> > file. Yet, the Documentation/netlink/spec.index.rst require updates as
> > spec files are added/renamed/removed. The already-existing script can
> > handle it automatically by running:
> >
> >             tools/net/ynl/pyynl/ynl_gen_rst.py -x  -v -o Documentation/netlink/specs/index.rst  
> 
> I think this can be avoided by using the toctree glob directive in the
> index, like this:
> 
> =============================
> Netlink Family Specifications
> =============================
> 
> .. toctree::
>    :maxdepth: 1
>    :glob:
> 
>    *
> 
> This would let you have a static index file.

Didn't know about such option. If it works with the parser, it sounds good 
enough.

> 
> > ---
> >
> > v2:
> > - Use a Sphinx extension to handle netlink files.
> >
> > v1:
> > - Statically add template files to as networking/netlink_spec/<family>.rst
> >
> > Mauro Carvalho Chehab (12):
> >   tools: ynl_gen_rst.py: create a top-level reference
> >   docs: netlink: netlink-raw.rst: use :ref: instead of :doc:  
> 
> I suggest combining the first 2 patches.
> 
> >   docs: netlink: don't ignore generated rst files  
> 
> Maybe leave this patch to the end and change the description to be a
> cleanup of the remants of the old approach.

Ok for me, but I usually prefer keeping one patch per logical change.
In this case, one patch adding support at the tool; the other one
improving docs to benefit from the new feature.

> Further comments on specific commits
> 
> >   tools: ynl_gen_rst.py: make the index parser more generic
> >   tools: ynl_gen_rst.py: Split library from command line tool
> >   scripts: lib: netlink_yml_parser.py: use classes
> >   tools: ynl_gen_rst.py: do some coding style cleanups
> >   scripts: netlink_yml_parser.py: improve index.rst generation
> >   docs: sphinx: add a parser template for yaml files
> >   docs: sphinx: parser_yaml.py: add Netlink specs parser  
> 
> Please combine these 2 patches. The template patch just introduces noise
> into the series and makes it harder to review.

Ok.

> >   docs: use parser_yaml extension to handle Netlink specs
> >   docs: conf.py: don't handle yaml files outside Netlink specs
> >
> >  .pylintrc                                     |   2 +-
> >  Documentation/Makefile                        |  17 -
> >  Documentation/conf.py                         |  17 +-
> >  Documentation/netlink/specs/index.rst         |  38 ++
> >  Documentation/networking/index.rst            |   2 +-
> >  .../networking/netlink_spec/.gitignore        |   1 -
> >  .../networking/netlink_spec/readme.txt        |   4 -
> >  Documentation/sphinx/parser_yaml.py           |  80 ++++
> >  .../userspace-api/netlink/netlink-raw.rst     |   6 +-
> >  scripts/lib/netlink_yml_parser.py             | 394 ++++++++++++++++++
> >  tools/net/ynl/pyynl/ynl_gen_rst.py            | 378 +----------------
> >  11 files changed, 544 insertions(+), 395 deletions(-)
> >  create mode 100644 Documentation/netlink/specs/index.rst
> >  delete mode 100644 Documentation/networking/netlink_spec/.gitignore
> >  delete mode 100644 Documentation/networking/netlink_spec/readme.txt
> >  create mode 100755 Documentation/sphinx/parser_yaml.py
> >  create mode 100755 scripts/lib/netlink_yml_parser.py  

Thanks,
Mauro

Powered by blists - more mailing lists