Re: [RFC PATCH v2 3/4] dts: add doc generation

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

On Tue, May 9, 2023 at 11:40 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, May 09, 2023 at 11:23:50AM +0200, Juraj Linkeš wrote:
> > On Fri, May 5, 2023 at 3:29 PM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Fri, May 05, 2023 at 01:13:34PM +0200, Juraj Linkeš wrote:
> > > > On Fri, May 5, 2023 at 12:57 PM Bruce Richardson
> > > > <bruce.richardson@intel.com> wrote:
> > > > >
> > > > > On Thu, May 04, 2023 at 02:37:48PM +0200, Juraj Linkeš wrote:
> > > > > > The tool used to generate developer docs is sphinx, which is already
> > > > > > used in DPDK. The configuration is kept the same to preserve the style.
> > > > > >
> > > > > > Sphinx generates the documentation from Python docstrings. The docstring
> > > > > > format most suitable for DTS seems to be the Google format [0] which
> > > > > > requires the sphinx.ext.napoleon extension.
> > > > > >
> > > > > > There are two requirements for building DTS docs:
> > > > > > * The same Python version as DTS or higher, because Sphinx import the
> > > > > >   code.
> > > > > > * Also the same Python packages as DTS, for the same reason.
> > > > > >
> > > > > > [0] https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings
> > > > > >
> > > > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > > > ---
> > > > > >  doc/api/meson.build      |  1 +
> > > > > >  doc/guides/conf.py       | 22 ++++++++++++++----
> > > > > >  doc/guides/meson.build   |  1 +
> > > > > >  doc/guides/tools/dts.rst | 29 +++++++++++++++++++++++
> > > > > >  dts/doc/doc-index.rst    | 20 ++++++++++++++++
> > > > > >  dts/doc/meson.build      | 50 ++++++++++++++++++++++++++++++++++++++++
> > > > > >  dts/meson.build          | 16 +++++++++++++
> > > > > >  meson.build              |  1 +
> > > > > >  meson_options.txt        |  2 ++
> > > > > >  9 files changed, 137 insertions(+), 5 deletions(-)
> > > > > >  create mode 100644 dts/doc/doc-index.rst
> > > > > >  create mode 100644 dts/doc/meson.build
> > > > > >  create mode 100644 dts/meson.build
> > > > > >
> > > > > <snip>
> > > > >
> > > > > > diff --git a/dts/doc/meson.build b/dts/doc/meson.build
> > > > > > new file mode 100644
> > > > > > index 0000000000..db2bb0bed9
> > > > > > --- /dev/null
> > > > > > +++ b/dts/doc/meson.build
> > > > > > @@ -0,0 +1,50 @@
> > > > > > +# SPDX-License-Identifier: BSD-3-Clause
> > > > > > +# Copyright(c) 2023 PANTHEON.tech s.r.o.
> > > > > > +
> > > > > > +sphinx = find_program('sphinx-build', required: get_option('enable_dts_docs'))
> > > > > > +sphinx_apidoc = find_program('sphinx-apidoc', required: get_option('enable_dts_docs'))
> > > > > > +
> > > > > > +if sphinx.found() and sphinx_apidoc.found()
> > > > > > +endif
> > > > > > +
> > > > > > +dts_api_framework_dir = join_paths(dts_dir, 'framework')
> > > > > > +dts_api_build_dir = join_paths(doc_api_build_dir, 'dts')
> > > > > > +dts_api_src = custom_target('dts_api_src',
> > > > > > +        output: 'modules.rst',
> > > > > > +        command: ['SPHINX_APIDOC_OPTIONS=members,show-inheritance',
> > > > >
> > > > > This gives errors when I try to configure a build, even without docs
> > > > > enabled.
> > > > >
> > > > >         ~/dpdk.org$ meson setup build-test
> > > > >         The Meson build system
> > > > >         Version: 1.0.1
> > > > >         Source dir: /home/bruce/dpdk.org
> > > > >         ...
> > > > >         Program sphinx-build found: YES (/usr/bin/sphinx-build)
> > > > >         Program sphinx-build found: YES (/usr/bin/sphinx-build)
> > > > >         Program sphinx-apidoc found: YES (/usr/bin/sphinx-apidoc)
> > > > >
> > > > >         dts/doc/meson.build:12:0: ERROR: Program 'SPHINX_APIDOC_OPTIONS=members,show-inheritance' not found or not executable
> > > > >
> > > > >         A full log can be found at /home/bruce/dpdk.org/build-test/meson-logs/meson-log.txt
> > > > >
> > > > > Assuming these need to be set in the environment, I think you can use the
> > > > > "env" parameter to custom target instead.
> > > > >
> > > >
> > > > I used meson 0.53.2 as that seemed to be the version I should target
> > > > (it's used in .ci/linux-setup.sh) which doesn't support the argument
> > > > (I originally wanted to use it, but they added it in 0.57.0). I didn't
> > > > see the error with 0.53.2.
> > > >
> > > > Which version should I target? 1.0.1?
> > > >
> > > > > > +            sphinx_apidoc, '--append-syspath', '--force',
> > > > > > +            '--module-first', '--separate',
> > > > > > +            '--doc-project', 'DTS', '-V', meson.project_version(),
> > > > > > +            '-o', dts_api_build_dir,
> > > > > > +            dts_api_framework_dir],
> > > > > > +        build_by_default: get_option('enable_dts_docs'))
> > > > > > +doc_targets += dts_api_src
> > > > > > +doc_target_names += 'DTS_API_sphinx_sources'
> > > > > > +
> > >
> > > I didn't try with 0.53.2 - let me test that, see if the error goes away. We
> > > may need different calls based on the meson version.
> > >
> > > Is there no other way to pass this data rather than via the environment?
> > >
> >
> > Certainly. For background, I wanted to do the same thing we do for
> > DPDK_VERSION, but that would require adding an additional parameter to
> > call-sphinx-build.py, which wouldn't be used by the other call of
> > call-sphinx-build.py (the one that builds doc guides), so I skipped
> > the parameter and set the env var before the call.
> >
> > There are a few options that come to mind:
> > 1. Use the parameter. There are two sub-options here, either make the
> > parameter positional and mandatory and then we'd have an awkward call
> > that builds the guides or I could make it optional, but that would
> > make the script a bit more complex (some argparse logic would be
> > needed).
> > 2. Hard-code the path into conf.py.
> > 3. Have separate conf.py files. Maybe we could make this work with symlinks.
> >
> > There could be something else. I like adding the optional parameter. I
> > don't know the policy on buildtools script complexity so let me know
> > what you think.
> >
> If the parameter would be just unused for the main doc build, and cause no
> issues, I don't see why we can't just put it into the main conf.py file. We
> can add a comment explaining that it only applies for the DTS doc. However,
> option 1, with an extra optional parameter doesn't seem so bad to me
> either. Using argparse in the build script doesn't seem like a problem
> either, if it's necessary. Maybe others have other opinions, though.
>

I'll just submit the version with argparse and we'll see how it looks.

> /Bruce