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

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

Thanks for the comments, Yoan.

> > diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
> > index 32c18ee472..98923b1467 100644
> > --- a/doc/guides/tools/dts.rst
> > +++ b/doc/guides/tools/dts.rst
> > @@ -335,3 +335,32 @@ There are three tools used in DTS to help with code checking, style and formatti
> >   These three tools are all used in ``devtools/dts-check-format.sh``,
> >   the DTS code check and format script.
> >   Refer to the script for usage: ``devtools/dts-check-format.sh -h``.
> > +
> > +
> > +Building DTS API docs
> > +---------------------
> > +
> > +To build DTS API docs, install the dependencies with Poetry, then enter its shell:
> > +
> > +   .. code-block:: console
>
> I believe the code-block line should not be indented. As it is, it did
> not generate properly when I built the doc. Same for the other code
> block below.
>

Good catch, thanks.

> > diff --git a/dts/doc/meson.build b/dts/doc/meson.build
> > new file mode 100644
> > index 0000000000..8e70eabc51
> > --- /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')
> > +sphinx_apidoc = find_program('sphinx-apidoc')
> > +
> > +if not sphinx.found() or not sphinx_apidoc.found()
> > +    subdir_done()
> > +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, '--append-syspath', '--force',
> > +            '--module-first', '--separate', '-V', meson.project_version(),
> > +            '-o', dts_api_build_dir, '--no-toc', '--implicit-namespaces',
> > +            dts_api_framework_dir],
> > +        build_by_default: false)
> > +doc_targets += dts_api_src
> > +doc_target_names += 'DTS_API_sphinx_sources'
> > +
> > +cp = find_program('cp')
> > +cp_index = custom_target('cp_index',
> > +        input: 'doc-index.rst',
> > +        output: 'index.rst',
> > +        depends: dts_api_src,
> > +        command: [cp, '@INPUT@', join_paths(dts_api_build_dir, 'index.rst')],
> > +        build_by_default: false)
> > +doc_targets += cp_index
> > +doc_target_names += 'DTS_API_sphinx_index'
> > +
> > +extra_sphinx_args = ['-a', '-c', doc_guides_source_dir]
> > +if get_option('werror')
> > +    extra_sphinx_args += '-W'
> > +endif
> > +
> > +htmldir = join_paths(get_option('datadir'), 'doc', 'dpdk')
> > +dts_api_html = custom_target('dts_api_html',
> > +        output: 'html',
> > +        depends: cp_index,
> > +        command: ['DTS_ROOT=@0@'.format(dts_dir),
> > +            sphinx_wrapper, sphinx, meson.project_version(),
> > +            dts_api_build_dir, dts_api_build_dir,
> > +            '--dts-root', dts_dir, extra_sphinx_args],
> > +        build_by_default: false,
> > +        install: false,
> > +        install_dir: htmldir)
>
> I don't entirely understand what this command do. I suspect it's the one
> that create and populate the html directory in build/doc/api/dts/

Yes, this one generates the sphinx html docs from the .rst files
generated in the first step.

> The main thing that confuse me is this htmldir. Mine seemed to point to
> share/doc/dpdk. Such a directory doesn't seems to be built (or I could
> not find where). That might be intended given install is set to false.
> But in that case, why bother setting it?
> The thing that worries me more is that dpdk/doc/guide/meson.build do
> build some html and have the same htmldir. If we were to enable the
> install in both, wouldn't the html overwrite each other (in particular
> index.html)?
> If my understanding is correct, I believe htmldir should either be
> removed, or set to a different value (<datadir>/doc/dpdk/dts ?)

These install related configs are basically a placeholder (install:
false) and a copy-paste from the doxygen API generation because meson
required these to be filles (IIRC).
I was hoping Bruce would give me some guidance in this.

Bruce, how should we install the DTS API docs? I'm not really sure how
exactly the meson install procedure works, what's going to be copied
and so on. I don't really know what to do with this.

>
> > +doc_targets += dts_api_html
> > +doc_target_names += 'DTS_API_HTML'
> > diff --git a/dts/meson.build b/dts/meson.build
> > new file mode 100644
> > index 0000000000..17bda07636
> > --- /dev/null
> > +++ b/dts/meson.build
> > @@ -0,0 +1,16 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2023 PANTHEON.tech s.r.o.
> > +
> > +doc_targets = []
> > +doc_target_names = []
> > +dts_dir = meson.current_source_dir()
> > +
> > +subdir('doc')
> > +
> > +if doc_targets.length() == 0
> > +    message = 'No docs targets found'
> > +else
> > +    message = 'Built docs:'
> > +endif
> > +run_target('dts/doc', command: [echo, message, doc_target_names],
> > +    depends: doc_targets)
> > diff --git a/meson.build b/meson.build
> > index 39cb73846d..4d34dc531c 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -85,6 +85,7 @@ subdir('app')
> >
> >   # build docs
> >   subdir('doc')
> > +subdir('dts')
> >
> >   # build any examples explicitly requested - useful for developers - and
> >   # install any example code into the appropriate install path
>
> When building from scratch, I had several warning/errors.
>
> $ meson build
> [usual meson output block...]
> WARNING: Target "dts/doc" has a path separator in its name.
> This is not supported, it can cause unexpected failures and will become
> a hard error in the future.

This is likely due to a newer meson version that I'm using, which is
0.53.2 because that's used in CI. We can rename it to satisfy newer
versions.

> Configuring rte_build_config.h using configuration
> Message:
> =================
> Applications Enabled
> =================
> [...]
>
> If you change the target name, remember to change it in the doc too.
>
>
>
> $ ninja -C build dts/doc
> [create all the rst...]
> [3/4] Generating dts_api_html with a custom command.
> dpdk/dts/framework/remote_session/interactive_shell.py:docstring of
> framework.remote_session.interactive_shell.InteractiveShell:25: WARNING:
> Definition list ends without a blank line; unexpected unindent.
>
> dpdk/dts/framework/remote_session/interactive_shell.py:docstring of
> framework.remote_session.interactive_shell.InteractiveShell:27: ERROR:
> Unexpected indentation.
>
> dpdk/dts/framework/testbed_model/common.py:docstring of
> framework.testbed_model.common.MesonArgs:3: ERROR: Unexpected indentation.
>
> dpdk/dts/framework/testbed_model/common.py:docstring of
> framework.testbed_model.common.MesonArgs:4: WARNING: Block quote ends
> without a blank line; unexpected unindent.
>
> dpdk/dts/framework/testbed_model/linux_session.py:docstring of
> framework.testbed_model.linux_session.LshwOutput:10: ERROR: Unexpected
> indentation.
>
> dpdk/dts/framework/testbed_model/linux_session.py:docstring of
> framework.testbed_model.linux_session.LshwOutput:13: WARNING: Block
> quote ends without a blank line; unexpected unindent.
>
> dpdk/dts/framework/testbed_model/sut_node.py:docstring of
> framework.testbed_model.sut_node.SutNode.create_eal_parameters:3: ERROR:
> Unexpected indentation.
>
> dpdk/dts/framework/testbed_model/sut_node.py:docstring of
> framework.testbed_model.sut_node.SutNode.create_eal_parameters:6:
> WARNING: Block quote ends without a blank line; unexpected unindent.
>
> dpdk/dts/framework/testbed_model/sut_node.py:docstring of
> framework.testbed_model.sut_node.SutNode.create_eal_parameters:18:
> ERROR: Unexpected indentation.
>
> dpdk/dts/framework/testbed_model/sut_node.py:docstring of
> framework.testbed_model.sut_node.SutNode.create_eal_parameters:20:
> WARNING: Block quote ends without a blank line; unexpected unindent.
>
> dpdk/dts/framework/test_suite.py:docstring of
> framework.test_suite.TestSuite:1: ERROR: Unknown target name: "test".
>
> dpdk/dts/framework/test_suite.py:docstring of
> framework.test_suite.TestSuite:1: ERROR: Unknown target name: "test_perf".
>

These errors are here because the docstrings are either incomplete or
not yet reformatted. These will be addressed in a new version that's
coming soon.

> If I then try to rerun ninja, those errors don't appear, so it seems to
> happen mostly on fresh build.
>

Yes, that is expected.

This made me look into how we're running sphinx-build. We're using:
  -a                write all files (default: only write new and changed files)

in the hope of forcing a full rebuild, but that doesn't seem to be
working properly, if at all. What I actually want sphinx to do is to
update the .html files after a rebuild and -a doesn't help with that.

I've tried the -E option instead and that seems to be working - it
updates the modified .html files so I'll replace -a with -E in the new
version.