Re: [PATCH v19 5/5] dts: add API doc generation

[Thomas Monjalon <thomas@monjalon.net>]

21/08/2024 17:02, Juraj Linkeš:
> +if 'dts' in src:
> +    os.environ['DTS_BUILD'] = "y"

That's more precisely "DTS doc build".
I think the variable name DTS_BUILD may be confusing.

[...]
> --- /dev/null
> +++ b/buildtools/get-dts-runtime-deps.py
> @@ -0,0 +1,95 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2024 PANTHEON.tech s.r.o.
> +#

extra blank line above can be removed

> +
> +"""Utilities for DTS dependencies.
> +
> +The module can be used as an executable script,
> +which verifies that the running Python version meets the version requirement of DTS.
> +The script exits with the standard exit codes in this mode (0 is success, 1 is failure).

Given it is just doing a check by default,
the script name could be "check-dts-requirements".

> +
> +The module also contains a function, get_missing_imports,
> +which looks for runtime dependencies in the DTS pyproject.toml file
> +and returns a list of module names used in an import statement (import packages) that are missing.
> +This function is not used when the module is run as a script and is available to be imported.
> +"""
[...]
> +    req_deps = _get_dependencies(_DTS_DEP_FILE_PATH)
> +    req_deps.pop('python')
> +
> +    for req_dep, dep_data in (req_deps | _EXTRA_DEPS).items():

Please could you explain somewhere why _EXTRA_DEPS is needed?

[...]
> +++ b/doc/api/dts/meson.build
> @@ -0,0 +1,31 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 PANTHEON.tech s.r.o.
> +
> +sphinx = find_program('sphinx-build', required: get_option('enable_docs'))
> +if not sphinx.found()
> +    subdir_done()
> +endif
> +
> +python_ver_satisfied = run_command(get_dts_runtime_deps, check: false).returncode()
> +if python_ver_satisfied != 0
> +    subdir_done()
> +endif

Looks simple.
So if I have the right Python but some dependencies are missing,
it will still work the same, right?
I feel the need for dependencies should be explained in the script.

[...]
> --- a/doc/api/meson.build
> +++ b/doc/api/meson.build
> @@ -1,6 +1,11 @@
>  # SPDX-License-Identifier: BSD-3-Clause
>  # Copyright(c) 2018 Luca Boccassi <bluca@debian.org>
>  
> +# initialize common Doxygen configuration
> +cdata = configuration_data()
> +
> +subdir('dts')

Why inserting DTS first before generating DPDK API doc?

[...]
>  # set up common Doxygen configuration
> -cdata = configuration_data()
>  cdata.set('VERSION', meson.project_version())
>  cdata.set('API_EXAMPLES', join_paths(dpdk_build_root, 'doc', 'api', 'examples.dox'))
>  cdata.set('OUTPUT', join_paths(dpdk_build_root, 'doc', 'api'))
> diff --git a/doc/guides/conf.py b/doc/guides/conf.py
> index 0f7ff5282d..d7f3030838 100644
> --- a/doc/guides/conf.py
> +++ b/doc/guides/conf.py
> @@ -10,7 +10,7 @@
>  from os.path import basename
>  from os.path import dirname
>  from os.path import join as path_join
> -from sys import argv, stderr
> +from sys import argv, stderr, path
>  
>  import configparser
>  
> @@ -58,6 +58,48 @@
>               ("tools/devbind", "dpdk-devbind",
>                "check device status and bind/unbind them from drivers", "", 8)]
>  
> +# DTS API docs additional configuration
> +if environ.get('DTS_BUILD'):
> +    extensions = ['sphinx.ext.napoleon', 'sphinx.ext.autodoc', 'sphinx.ext.intersphinx']
> +    # Napoleon enables the Google format of Python doscstrings.
> +    napoleon_numpy_docstring = False
> +    napoleon_attr_annotations = True
> +    napoleon_preprocess_types = True
> +
> +    # Autodoc pulls documentation from code.
> +    autodoc_default_options = {
> +        'members': True,
> +        'member-order': 'bysource',
> +        'show-inheritance': True,
> +    }
> +    autodoc_class_signature = 'separated'
> +    autodoc_typehints = 'both'
> +    autodoc_typehints_format = 'short'
> +    autodoc_typehints_description_target = 'documented'
> +
> +    # Intersphinx allows linking to external projects, such as Python docs.
> +    intersphinx_mapping = {'python': ('https://docs.python.org/3', None)}

I'm not sure about the need for this intersphinx.

> +
> +    # DTS docstring options.
> +    add_module_names = False
> +    toc_object_entries = True
> +    toc_object_entries_show_parents = 'hide'
> +    # DTS Sidebar config.
> +    html_theme_options = {
> +        'collapse_navigation': False,
> +        'navigation_depth': -1,  # unlimited depth
> +    }
> +
> +    # Add path to DTS sources so that Sphinx can find them.
> +    dpdk_root = dirname(dirname(dirname(__file__)))
> +    path.append(path_join(dpdk_root, 'dts'))
> +
> +    # Get missing DTS dependencies. Add path to buildtools to find the get_missing_imports function.
> +    path.append(path_join(dpdk_root, 'buildtools'))
> +    import importlib
> +    # Ignore missing imports from DTS dependencies.
> +    autodoc_mock_imports = importlib.import_module('get-dts-runtime-deps').get_missing_imports()
[...]
> +the corresponding changes must be made to DTS api doc sources in ``doc/api/dts``.

api -> API


Except minor corrections and explanations, it looks good.
You can add my ack to the final version.

Acked-by: Thomas Monjalon <thomas@monjalon.net>