On 12. 9. 2024 22:09, Thomas Monjalon wrote:
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.

Ack, I'll rename the variable.


[...]
--- /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


Ack.

+
+"""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".


Ack.

+
+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?

I'll add this comment above the variable:
# The names of packages used in import statements may be different from distribution package names.
# We get distribution package names from pyproject.toml.
# _EXTRA_DEPS adds those import names which don't match their distribution package name.


[...]
+++ 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?

Yes.

I feel the need for dependencies should be explained in the script.

From my point of view, the script gets the dependencies and it's up to the caller how they use the list of dependencies.

The caller is conf.py and there's a bit of an explanation:
# Get missing DTS dependencies. Add path to buildtools to find the get_missing_imports function.

And then:
# Ignore missing imports from DTS dependencies.

So basically get the dependencies so we know what to ignore.

But I could add something to the script if this is not enough.


[...]
--- 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 <bl...@debian.org>
+# initialize common Doxygen configuration
+cdata = configuration_data()
+
+subdir('dts')

Why inserting DTS first before generating DPDK API doc?


I wanted to put it before subdir_done(). Maybe we could put subdir('dts') in the else branch and also at the end of the meson.build file. That could be better.

[...]
  # 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.

It's not stricly needed, but it produces better documentation, with links to Python docs for classes and other things found there.

For example:
:class:`~argparse.Action` in a docstring will link to https://docs.python.org/3/library/argparse.html#argparse.Action


+
+    # 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


Ack.


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

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



Reply via email to