[v8,5/5] dts: add API doc generation

Message ID 20240712085724.21065-6-juraj.linkes@pantheon.tech (mailing list archive)
State Superseded, archived
Delegated to: Juraj Linkeš
Headers
Series dts: API docs generation |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation fail Compilation issues
ci/intel-Testing fail Testing issues
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS

Commit Message

Juraj Linkeš July 12, 2024, 8:57 a.m. UTC
The tool used to generate DTS API docs is Sphinx, which is already in
use in DPDK. The same configuration is used to preserve style with one
DTS-specific configuration (so that the DPDK docs are unchanged) that
modifies how the sidebar displays the content.

Sphinx generates the documentation from Python docstrings. The docstring
format is the Google format [0] which requires the sphinx.ext.napoleon
extension. The other extension, sphinx.ext.intersphinx, enables linking
to objects in external documentations, such as the Python documentation.

There are two requirements for building DTS docs:
* The same Python version as DTS or higher, because Sphinx imports 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>
Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Tested-by: Luca Vizzarro <luca.vizzarro@arm.com>
Tested-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 buildtools/call-sphinx-build.py |  3 +++
 doc/api/doxy-api-index.md       |  3 +++
 doc/api/doxy-api.conf.in        |  2 ++
 doc/api/meson.build             |  4 ++++
 doc/guides/conf.py              | 33 +++++++++++++++++++++++++++++++-
 doc/guides/meson.build          |  1 +
 doc/guides/tools/dts.rst        | 34 ++++++++++++++++++++++++++++++++-
 dts/doc/meson.build             | 27 ++++++++++++++++++++++++++
 dts/meson.build                 | 16 ++++++++++++++++
 meson.build                     |  1 +
 10 files changed, 122 insertions(+), 2 deletions(-)
 create mode 100644 dts/doc/meson.build
 create mode 100644 dts/meson.build
  

Comments

Thomas Monjalon July 30, 2024, 1:51 p.m. UTC | #1
12/07/2024 10:57, Juraj Linkeš:
> The tool used to generate DTS API docs is Sphinx, which is already in
> use in DPDK. The same configuration is used to preserve style with one
> DTS-specific configuration (so that the DPDK docs are unchanged) that
> modifies how the sidebar displays the content.

What is changed in the sidebar?


> --- a/doc/api/doxy-api-index.md
> +++ b/doc/api/doxy-api-index.md
> @@ -244,3 +244,6 @@ The public API headers are grouped by topics:
>    [experimental APIs](@ref rte_compat.h),
>    [ABI versioning](@ref rte_function_versioning.h),
>    [version](@ref rte_version.h)
> +
> +- **tests**:
> +  [**DTS**](@dts_api_main_page)

OK looks good


> --- a/doc/api/doxy-api.conf.in
> +++ b/doc/api/doxy-api.conf.in
> @@ -124,6 +124,8 @@ SEARCHENGINE            = YES
>  SORT_MEMBER_DOCS        = NO
>  SOURCE_BROWSER          = YES
>  
> +ALIASES                 = "dts_api_main_page=@DTS_API_MAIN_PAGE@"

Why is it needed?
That's the only way to reference it in doxy-api-index.md?
Would be nice to explain in the commit log.

> --- a/doc/api/meson.build
> +++ b/doc/api/meson.build
> +# A local reference must be relative to the main index.html page
> +# The path below can't be taken from the DTS meson file as that would
> +# require recursive subdir traversal (doc, dts, then doc again)

This comment is really obscure.

> +cdata.set('DTS_API_MAIN_PAGE', join_paths('..', 'dts', 'html', 'index.html'))

Oh I think I get it:
	- DTS_API_MAIN_PAGE is the Meson variable
	- dts_api_main_page is the Doxygen variable


> +# Napoleon enables the Google format of Python doscstrings, used in DTS
> +# Intersphinx allows linking to external projects, such as Python docs, also used in DTS

Close sentences with a dot, it is easier to read.

> +extensions = ['sphinx.ext.napoleon', 'sphinx.ext.intersphinx']
> +
> +# DTS Python docstring options
> +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'
> +napoleon_numpy_docstring = False
> +napoleon_attr_annotations = True
> +napoleon_preprocess_types = True
> +add_module_names = False
> +toc_object_entries = True
> +toc_object_entries_show_parents = 'hide'
> +intersphinx_mapping = {'python': ('https://docs.python.org/3', None)}
> +
> +dts_root = environ.get('DTS_ROOT')

Why does it need to be passed as an environment variable?
Isn't it a fixed absolute path?

> +if dts_root:
> +    path.append(dts_root)
> +    # DTS Sidebar config
> +    html_theme_options = {
> +        'collapse_navigation': False,
> +        'navigation_depth': -1,
> +    }

[...]

> +To build DTS API docs, install the dependencies with Poetry, then enter its shell:

I don't plan to use Poetry on my machine.
Can we simply describe the dependencies even if the versions are not specified?

> +
> +.. code-block:: console
> +
> +   poetry install --no-root --with docs
> +   poetry shell
> +
> +The documentation is built using the standard DPDK build system.
> +After executing the meson command and entering Poetry's shell, build the documentation with:
> +
> +.. code-block:: console
> +
> +   ninja -C build dts-doc

Don't we rely on the Meson option "enable_docs"?
> +
> +The output is generated in ``build/doc/api/dts/html``.
> +
> +.. Note::

In general the RST expressions are lowercase.

> +
> +   Make sure to fix any Sphinx warnings when adding or updating docstrings,
> +   and also run the ``devtools/dts-check-format.sh`` script and address any issues it finds.

It looks like something to write in the contributing guide.


> +++ b/dts/doc/meson.build
> @@ -0,0 +1,27 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2023 PANTHEON.tech s.r.o.
> +
> +sphinx = find_program('sphinx-build', required: false)
> +sphinx_apidoc = find_program('sphinx-apidoc', required: false)
> +
> +if not sphinx.found() or not sphinx_apidoc.found()

You should include the option "enable_docs" here.

> +    subdir_done()
> +endif
  
Juraj Linkeš Aug. 1, 2024, 1:03 p.m. UTC | #2
On 30. 7. 2024 15:51, Thomas Monjalon wrote:
> 12/07/2024 10:57, Juraj Linkeš:
>> The tool used to generate DTS API docs is Sphinx, which is already in
>> use in DPDK. The same configuration is used to preserve style with one
>> DTS-specific configuration (so that the DPDK docs are unchanged) that
>> modifies how the sidebar displays the content.
> 
> What is changed in the sidebar?
> 

These are the two changes:
html_theme_options = {
     'collapse_navigation': False,
     'navigation_depth': -1,
}

The first allows you to explore the structure without needing to enter 
any specific section - it puts the + at each section so everything is 
expandable.
The second just means that each section can be fully expanded (there's 
no limit).

> 
>> --- a/doc/api/doxy-api-index.md
>> +++ b/doc/api/doxy-api-index.md
>> @@ -244,3 +244,6 @@ The public API headers are grouped by topics:
>>     [experimental APIs](@ref rte_compat.h),
>>     [ABI versioning](@ref rte_function_versioning.h),
>>     [version](@ref rte_version.h)
>> +
>> +- **tests**:
>> +  [**DTS**](@dts_api_main_page)
> 
> OK looks good
> 
> 
>> --- a/doc/api/doxy-api.conf.in
>> +++ b/doc/api/doxy-api.conf.in
>> @@ -124,6 +124,8 @@ SEARCHENGINE            = YES
>>   SORT_MEMBER_DOCS        = NO
>>   SOURCE_BROWSER          = YES
>>   
>> +ALIASES                 = "dts_api_main_page=@DTS_API_MAIN_PAGE@"
> 
> Why is it needed?
> That's the only way to reference it in doxy-api-index.md?
> Would be nice to explain in the commit log.
> 

I can add something to the commit log. The questions are answered below, 
in your other related comment.

>> --- a/doc/api/meson.build
>> +++ b/doc/api/meson.build
>> +# A local reference must be relative to the main index.html page
>> +# The path below can't be taken from the DTS meson file as that would
>> +# require recursive subdir traversal (doc, dts, then doc again)
> 
> This comment is really obscure.
> 

I guess it is. I just wanted to explain that there's not way to do this 
without spelling out the path this way. At least I didn't find a way.
Should I remove the comment or reword it?

>> +cdata.set('DTS_API_MAIN_PAGE', join_paths('..', 'dts', 'html', 'index.html'))
> 
> Oh I think I get it:
> 	- DTS_API_MAIN_PAGE is the Meson variable
> 	- dts_api_main_page is the Doxygen variable
> 

Yes, this is a way to make it work. Maybe there's something else (I'm 
not that familiar with Doxygen), but from what I can tell, there wasn't 
a command line option that would set a variable (passing the path form 
Meson to Doxygen) and nothing else I found worked.

Is this solution ok? If we want to explore something else, is there 
someone with more experience with Doxygen who could help?

> 
>> +# Napoleon enables the Google format of Python doscstrings, used in DTS
>> +# Intersphinx allows linking to external projects, such as Python docs, also used in DTS
> 
> Close sentences with a dot, it is easier to read.
> 

Ack.

>> +extensions = ['sphinx.ext.napoleon', 'sphinx.ext.intersphinx']
>> +
>> +# DTS Python docstring options
>> +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'
>> +napoleon_numpy_docstring = False
>> +napoleon_attr_annotations = True
>> +napoleon_preprocess_types = True
>> +add_module_names = False
>> +toc_object_entries = True
>> +toc_object_entries_show_parents = 'hide'
>> +intersphinx_mapping = {'python': ('https://docs.python.org/3', None)}
>> +
>> +dts_root = environ.get('DTS_ROOT')
> 
> Why does it need to be passed as an environment variable?
> Isn't it a fixed absolute path?
> 

The path to DTS needs to be passed in some way (and added to sys.path) 
so that Sphinx knows where the sources are in order to import them.

Do you want us to not pass the path, but just hardcode it here? I didn't 
really think about that, maybe that could work.

>> +if dts_root:
>> +    path.append(dts_root)
>> +    # DTS Sidebar config
>> +    html_theme_options = {
>> +        'collapse_navigation': False,
>> +        'navigation_depth': -1,
>> +    }
> 
> [...]
> 
>> +To build DTS API docs, install the dependencies with Poetry, then enter its shell:
> 
> I don't plan to use Poetry on my machine.
> Can we simply describe the dependencies even if the versions are not specified?
> 

The reason we don't list the dependencies anywhere is that doing it with 
Poetry is much easier (and a bit safer, as Poetry is going to install 
tested versions).

But I can add references to the two relevant sections of 
dts/pyproject.toml which contain the dependencies with a note that they 
can be installed with pip (and I guess that would be another 
dependency), but at that point it's that not much different than using 
Poetry.

>> +
>> +.. code-block:: console
>> +
>> +   poetry install --no-root --with docs
>> +   poetry shell
>> +
>> +The documentation is built using the standard DPDK build system.
>> +After executing the meson command and entering Poetry's shell, build the documentation with:
>> +
>> +.. code-block:: console
>> +
>> +   ninja -C build dts-doc
> 
> Don't we rely on the Meson option "enable_docs"?

I had a discussion about this with Bruce, but I can't find it anywhere, 
so here's what I remember:
1. We didn't want to tie the dts api doc build to dpdk doc build because 
of the dependencies.
2. There's a way to build docs without the enable_docs option (running 
ninja with the target), which is what we added for dts. This doesn't tie 
the dts api doc build to the dpdk doc build.
3. We had an "enable_dts_docs" Meson option in the past (to keep it 
separate from dpdk doc build), but decided to drop it. My memory is hazy 
on this, but I think it was, again, because of the additional steps 
needed to bring up the dependency (poetry shell) - at that point, 
supporting just the ninja build way is sufficient. Bruce may shed more 
light on this.

>> +
>> +The output is generated in ``build/doc/api/dts/html``.
>> +
>> +.. Note::
> 
> In general the RST expressions are lowercase.
> 

Ack.

>> +
>> +   Make sure to fix any Sphinx warnings when adding or updating docstrings,
>> +   and also run the ``devtools/dts-check-format.sh`` script and address any issues it finds.
> 
> It looks like something to write in the contributing guide.
> 

I could add it there, where is the right place? In patches.rst, section 
"Checking the Patches"?

> 
>> +++ b/dts/doc/meson.build
>> @@ -0,0 +1,27 @@
>> +# SPDX-License-Identifier: BSD-3-Clause
>> +# Copyright(c) 2023 PANTHEON.tech s.r.o.
>> +
>> +sphinx = find_program('sphinx-build', required: false)
>> +sphinx_apidoc = find_program('sphinx-apidoc', required: false)
>> +
>> +if not sphinx.found() or not sphinx_apidoc.found()
> 
> You should include the option "enable_docs" here.
> 
>> +    subdir_done()
>> +endif
> 
> 
>
  
Thomas Monjalon Aug. 1, 2024, 3:07 p.m. UTC | #3
01/08/2024 15:03, Juraj Linkeš:
> On 30. 7. 2024 15:51, Thomas Monjalon wrote:
> > 12/07/2024 10:57, Juraj Linkeš:
> >> The tool used to generate DTS API docs is Sphinx, which is already in
> >> use in DPDK. The same configuration is used to preserve style with one
> >> DTS-specific configuration (so that the DPDK docs are unchanged) that
> >> modifies how the sidebar displays the content.
> > 
> > What is changed in the sidebar?
> > 
> 
> These are the two changes:
> html_theme_options = {
>      'collapse_navigation': False,
>      'navigation_depth': -1,
> }
> 
> The first allows you to explore the structure without needing to enter 
> any specific section - it puts the + at each section so everything is 
> expandable.
> The second just means that each section can be fully expanded (there's 
> no limit).

OK interesting, you may add a comment # unlimited depth


> >> +# A local reference must be relative to the main index.html page
> >> +# The path below can't be taken from the DTS meson file as that would
> >> +# require recursive subdir traversal (doc, dts, then doc again)
> > 
> > This comment is really obscure.
> 
> I guess it is. I just wanted to explain that there's not way to do this 
> without spelling out the path this way. At least I didn't find a way.
> Should I remove the comment or reword it?

May be removed I think.

> >> +cdata.set('DTS_API_MAIN_PAGE', join_paths('..', 'dts', 'html', 'index.html'))
> > 
> > Oh I think I get it:
> > 	- DTS_API_MAIN_PAGE is the Meson variable
> > 	- dts_api_main_page is the Doxygen variable
> > 
> 
> Yes, this is a way to make it work. Maybe there's something else (I'm 
> not that familiar with Doxygen), but from what I can tell, there wasn't 
> a command line option that would set a variable (passing the path form 
> Meson to Doxygen) and nothing else I found worked.
> 
> Is this solution ok? If we want to explore something else, is there 
> someone with more experience with Doxygen who could help?

Yes it's OK like that.


> >> +dts_root = environ.get('DTS_ROOT')
> > 
> > Why does it need to be passed as an environment variable?
> > Isn't it a fixed absolute path?
> 
> The path to DTS needs to be passed in some way (and added to sys.path) 
> so that Sphinx knows where the sources are in order to import them.
> 
> Do you want us to not pass the path, but just hardcode it here? I didn't 
> really think about that, maybe that could work.

I think hardcode is better here.


> >> +To build DTS API docs, install the dependencies with Poetry, then enter its shell:
> > 
> > I don't plan to use Poetry on my machine.
> > Can we simply describe the dependencies even if the versions are not specified?
> 
> The reason we don't list the dependencies anywhere is that doing it with 
> Poetry is much easier (and a bit safer, as Poetry is going to install 
> tested versions).
> 
> But I can add references to the two relevant sections of 
> dts/pyproject.toml which contain the dependencies with a note that they 
> can be installed with pip (and I guess that would be another 
> dependency), but at that point it's that not much different than using 
> Poetry.

I want to use my system package manager.
I am from this old school thinking we should have a single package manager in a system.

> >> +.. code-block:: console
> >> +
> >> +   poetry install --no-root --with docs
> >> +   poetry shell
> >> +
> >> +The documentation is built using the standard DPDK build system.
> >> +After executing the meson command and entering Poetry's shell, build the documentation with:
> >> +
> >> +.. code-block:: console
> >> +
> >> +   ninja -C build dts-doc
> > 
> > Don't we rely on the Meson option "enable_docs"?
> 
> I had a discussion about this with Bruce, but I can't find it anywhere, 
> so here's what I remember:
> 1. We didn't want to tie the dts api doc build to dpdk doc build because 
> of the dependencies.

Sure
But we could just skip if dependencies are not met?

> 2. There's a way to build docs without the enable_docs option (running 
> ninja with the target), which is what we added for dts. This doesn't tie 
> the dts api doc build to the dpdk doc build.

Yes

> 3. We had an "enable_dts_docs" Meson option in the past (to keep it 
> separate from dpdk doc build), but decided to drop it. My memory is hazy 
> on this, but I think it was, again, because of the additional steps 
> needed to bring up the dependency (poetry shell) - at that point, 
> supporting just the ninja build way is sufficient. Bruce may shed more 
> light on this.


> >> +   Make sure to fix any Sphinx warnings when adding or updating docstrings,
> >> +   and also run the ``devtools/dts-check-format.sh`` script and address any issues it finds.
> > 
> > It looks like something to write in the contributing guide.
> > 
> 
> I could add it there, where is the right place? In patches.rst, section 
> "Checking the Patches"?

Yes
  
Juraj Linkeš Aug. 2, 2024, 10:48 a.m. UTC | #4
On 1. 8. 2024 17:07, Thomas Monjalon wrote:
> 01/08/2024 15:03, Juraj Linkeš:
>> On 30. 7. 2024 15:51, Thomas Monjalon wrote:
>>> 12/07/2024 10:57, Juraj Linkeš:
>>>> The tool used to generate DTS API docs is Sphinx, which is already in
>>>> use in DPDK. The same configuration is used to preserve style with one
>>>> DTS-specific configuration (so that the DPDK docs are unchanged) that
>>>> modifies how the sidebar displays the content.
>>>
>>> What is changed in the sidebar?
>>>
>>
>> These are the two changes:
>> html_theme_options = {
>>       'collapse_navigation': False,
>>       'navigation_depth': -1,
>> }
>>
>> The first allows you to explore the structure without needing to enter
>> any specific section - it puts the + at each section so everything is
>> expandable.
>> The second just means that each section can be fully expanded (there's
>> no limit).
> 
> OK interesting, you may add a comment # unlimited depth
> 
> 

Ack.

>>>> +# A local reference must be relative to the main index.html page
>>>> +# The path below can't be taken from the DTS meson file as that would
>>>> +# require recursive subdir traversal (doc, dts, then doc again)
>>>
>>> This comment is really obscure.
>>
>> I guess it is. I just wanted to explain that there's not way to do this
>> without spelling out the path this way. At least I didn't find a way.
>> Should I remove the comment or reword it?
> 
> May be removed I think.
> 

Ok, I'll remove it.

>>>> +cdata.set('DTS_API_MAIN_PAGE', join_paths('..', 'dts', 'html', 'index.html'))
>>>
>>> Oh I think I get it:
>>> 	- DTS_API_MAIN_PAGE is the Meson variable
>>> 	- dts_api_main_page is the Doxygen variable
>>>
>>
>> Yes, this is a way to make it work. Maybe there's something else (I'm
>> not that familiar with Doxygen), but from what I can tell, there wasn't
>> a command line option that would set a variable (passing the path form
>> Meson to Doxygen) and nothing else I found worked.
>>
>> Is this solution ok? If we want to explore something else, is there
>> someone with more experience with Doxygen who could help?
> 
> Yes it's OK like that.
> 
> 

Ack.

>>>> +dts_root = environ.get('DTS_ROOT')
>>>
>>> Why does it need to be passed as an environment variable?
>>> Isn't it a fixed absolute path?
>>
>> The path to DTS needs to be passed in some way (and added to sys.path)
>> so that Sphinx knows where the sources are in order to import them.
>>
>> Do you want us to not pass the path, but just hardcode it here? I didn't
>> really think about that, maybe that could work.
> 
> I think hardcode is better here.
> 

I tried implementing this, but I ran into an issue with this:
dts_root = environ.get('DTS_ROOT')
if dts_root:
     path.append(dts_root)
     # DTS Sidebar config
     html_theme_options = {
         'collapse_navigation': False,
         'navigation_depth': -1,
     }

The sidebar configuration is conditional, so we have to pass something 
to indicate dts build. I'll change it so that we look for 'dts' in src 
in call-sphinx-build.py (we're in the dts doc directory, indicating dts 
build) and set the DTS_BUILD env var which we can use in conf.py. I 
didn't find a better way to do this as conf.py doesn't have any 
information about the build itself (and no path that conf.py has access 
to points to anything dts). Here's how it'll look:

if environ.get('DTS_BUILD'):
     path.append(path_join(dirname(dirname(dirname(__file__))), 'dts'))
     # DTS Sidebar config.
     html_theme_options = {
         'collapse_navigation': False,
         'navigation_depth': -1,  # unlimited depth
     }

> 
>>>> +To build DTS API docs, install the dependencies with Poetry, then enter its shell:
>>>
>>> I don't plan to use Poetry on my machine.
>>> Can we simply describe the dependencies even if the versions are not specified?
>>
>> The reason we don't list the dependencies anywhere is that doing it with
>> Poetry is much easier (and a bit safer, as Poetry is going to install
>> tested versions).
>>
>> But I can add references to the two relevant sections of
>> dts/pyproject.toml which contain the dependencies with a note that they
>> can be installed with pip (and I guess that would be another
>> dependency), but at that point it's that not much different than using
>> Poetry.
> 
> I want to use my system package manager.
> I am from this old school thinking we should have a single package manager in a system.
> 

I understand and would also prefer that, but it just doesn't work for 
Python. Not all packages are available from the package managers, and 
Python projects should not use system packages as there are frequently 
version mismatches between the system packages and what the project 
needs (the APIs could be different as well as behavior; a problem we've 
seen with Scapy). Poetry is one of the tools that tries to solve this 
well-known Python limitation.

I've done a quick search of what's available in Ubuntu and two packages 
aren't available, types-PyYAML (which maybe we could do without, I'll 
have to test) and aenum (which is currently needed for the capabilities 
patch; if absolutely necessary, maybe I could find a solution without 
aenum). But even with this we can't be sure that the system package 
versions will work.

>>>> +.. code-block:: console
>>>> +
>>>> +   poetry install --no-root --with docs
>>>> +   poetry shell
>>>> +
>>>> +The documentation is built using the standard DPDK build system.
>>>> +After executing the meson command and entering Poetry's shell, build the documentation with:
>>>> +
>>>> +.. code-block:: console
>>>> +
>>>> +   ninja -C build dts-doc
>>>
>>> Don't we rely on the Meson option "enable_docs"?
>>
>> I had a discussion about this with Bruce, but I can't find it anywhere,
>> so here's what I remember:
>> 1. We didn't want to tie the dts api doc build to dpdk doc build because
>> of the dependencies.
> 
> Sure
> But we could just skip if dependencies are not met?
> 

Maybe we could add a script that would check the dependencies. I'll see 
what I can do.

>> 2. There's a way to build docs without the enable_docs option (running
>> ninja with the target), which is what we added for dts. This doesn't tie
>> the dts api doc build to the dpdk doc build.
> 
> Yes
> 
>> 3. We had an "enable_dts_docs" Meson option in the past (to keep it
>> separate from dpdk doc build), but decided to drop it. My memory is hazy
>> on this, but I think it was, again, because of the additional steps
>> needed to bring up the dependency (poetry shell) - at that point,
>> supporting just the ninja build way is sufficient. Bruce may shed more
>> light on this.
> 
> 
>>>> +   Make sure to fix any Sphinx warnings when adding or updating docstrings,
>>>> +   and also run the ``devtools/dts-check-format.sh`` script and address any issues it finds.
>>>
>>> It looks like something to write in the contributing guide.
>>>
>>
>> I could add it there, where is the right place? In patches.rst, section
>> "Checking the Patches"?
> 
> Yes
> 

Ack.

>
  
Thomas Monjalon Aug. 2, 2024, 1:53 p.m. UTC | #5
02é x/08/2024 12:48, Juraj Linkeš:
> On 1. 8. 2024 17:07, Thomas Monjalon wrote:
> > 01/08/2024 15:03, Juraj Linkeš:
> >> On 30. 7. 2024 15:51, Thomas Monjalon wrote:
> >>> 12/07/2024 10:57, Juraj Linkeš:
> >>>> +dts_root = environ.get('DTS_ROOT')
> >>>
> >>> Why does it need to be passed as an environment variable?
> >>> Isn't it a fixed absolute path?
> >>
> >> The path to DTS needs to be passed in some way (and added to sys.path)
> >> so that Sphinx knows where the sources are in order to import them.
> >>
> >> Do you want us to not pass the path, but just hardcode it here? I didn't
> >> really think about that, maybe that could work.
> > 
> > I think hardcode is better here. xFalse,
>          'navigation_depth': -1,
>      }
> 
> The sidebar configuration is conditional, so we have to pass something 
> to indicate dts build. I'll change it so that we look for 'dts' in src 
> in call-sphinx-build.py (we're in the dts doc directory, indicating dts 
> build) and set the DTS_BUILD env var which we can use in conf.py. I 
> didn't find a better way to do this as conf.py doesn't have any 
> information about the build itself (and no path that conf.py has access 
> to points to anything dts). Here's how it'll look:
> 
> if environ.get('DTS_BUILD'):
>      path.append(path_join(dirname(dirname(dirname(__file__))), 'dts'))
>      # DTS Sidebar config.
>      html_theme_options = {
>          'collapse_navigation': False,
>          'navigation_depth': -1,  # unlimited depth
>      }

OK


> >>>> +To build DTS API docs, install the dependencies with Poetry, then enter its shell:
> >>>
> >>> I don't plan to use Poetry on my machine.
> >>> Can we simply describe the dependencies even if the versions are not specified?
> >>
> >> The reason we don't list the dependencies anywhere is that doing it with
> >> Poetry is much easier (and a bit safer, as Poetry is going to install
> >> tested versions).
> >>
> >> But I can add references to the two relevant sections of
> >> dts/pyproject.toml which contain the dependencies with a note that they
> >> can be installed with pip (and I guess that would be another
> >> dependency), but at that point it's that not much different than using
> >> Poetry.
> > 
> > I want to use my system package manager.
> > I am from this old school thinking we should have a single package manager in a system.
> > 
> 
> I understand and would also prefer that, but it just doesn't work for 
> Python. Not all packages are available from the package managers, and 
> Python projects should not use system packages as there are frequently 
> version mismatches between the system packages and what the project 
> needs (the APIs could be different as well as behavior; a problem we've 
> seen with Scapy). Poetry is one of the tools that tries to solve this 
> well-known Python limitation.

I fully agree for DTS runtime.
I'm expecting the dependencies are more tolerant for DTS doc.

> I've done a quick search of what's available in Ubuntu and two packages 
> aren't available, types-PyYAML (which maybe we could do without, I'll 
> have to test) and aenum (which is currently needed for the capabilities 
> patch; if absolutely necessary, maybe I could find a solution without 
> aenum). But even with this we can't be sure that the system package 
> versions will work.

We need them all to generate the documentation?

> >>>> +.. code-block:: console
> >>>> +
> >>>> +   poetry install --no-root --with docs
> >>>> +   poetry shell
> >>>> +
> >>>> +The documentation is built using the standard DPDK build system.
> >>>> +After executing the meson command and entering Poetry's shell, build the documentation with:
> >>>> +
> >>>> +.. code-block:: console
> >>>> +
> >>>> +   ninja -C build dts-doc
> >>>
> >>> Don't we rely on the Meson option "enable_docs"?
> >>
> >> I had a discussion about this with Bruce, but I can't find it anywhere,
> >> so here's what I remember:
> >> 1. We didn't want to tie the dts api doc build to dpdk doc build because
> >> of the dependencies.
> > 
> > Sure
> > But we could just skip if dependencies are not met?
> 
> Maybe we could add a script that would check the dependencies. I'll see 
> what I can do.

OK thanks
  
Juraj Linkeš Aug. 5, 2024, 9:04 a.m. UTC | #6
On 2. 8. 2024 15:53, Thomas Monjalon wrote:
> 02é x/08/2024 12:48, Juraj Linkeš:
>> On 1. 8. 2024 17:07, Thomas Monjalon wrote:
>>> 01/08/2024 15:03, Juraj Linkeš:
>>>> On 30. 7. 2024 15:51, Thomas Monjalon wrote:
>>>>> 12/07/2024 10:57, Juraj Linkeš:
>>>>>> +dts_root = environ.get('DTS_ROOT')
>>>>>
>>>>> Why does it need to be passed as an environment variable?
>>>>> Isn't it a fixed absolute path?
>>>>
>>>> The path to DTS needs to be passed in some way (and added to sys.path)
>>>> so that Sphinx knows where the sources are in order to import them.
>>>>
>>>> Do you want us to not pass the path, but just hardcode it here? I didn't
>>>> really think about that, maybe that could work.
>>>
>>> I think hardcode is better here. xFalse,
>>           'navigation_depth': -1,
>>       }
>>
>> The sidebar configuration is conditional, so we have to pass something
>> to indicate dts build. I'll change it so that we look for 'dts' in src
>> in call-sphinx-build.py (we're in the dts doc directory, indicating dts
>> build) and set the DTS_BUILD env var which we can use in conf.py. I
>> didn't find a better way to do this as conf.py doesn't have any
>> information about the build itself (and no path that conf.py has access
>> to points to anything dts). Here's how it'll look:
>>
>> if environ.get('DTS_BUILD'):
>>       path.append(path_join(dirname(dirname(dirname(__file__))), 'dts'))
>>       # DTS Sidebar config.
>>       html_theme_options = {
>>           'collapse_navigation': False,
>>           'navigation_depth': -1,  # unlimited depth
>>       }
> 
> OK
> 
> 
>>>>>> +To build DTS API docs, install the dependencies with Poetry, then enter its shell:
>>>>>
>>>>> I don't plan to use Poetry on my machine.
>>>>> Can we simply describe the dependencies even if the versions are not specified?
>>>>
>>>> The reason we don't list the dependencies anywhere is that doing it with
>>>> Poetry is much easier (and a bit safer, as Poetry is going to install
>>>> tested versions).
>>>>
>>>> But I can add references to the two relevant sections of
>>>> dts/pyproject.toml which contain the dependencies with a note that they
>>>> can be installed with pip (and I guess that would be another
>>>> dependency), but at that point it's that not much different than using
>>>> Poetry.
>>>
>>> I want to use my system package manager.
>>> I am from this old school thinking we should have a single package manager in a system.
>>>
>>
>> I understand and would also prefer that, but it just doesn't work for
>> Python. Not all packages are available from the package managers, and
>> Python projects should not use system packages as there are frequently
>> version mismatches between the system packages and what the project
>> needs (the APIs could be different as well as behavior; a problem we've
>> seen with Scapy). Poetry is one of the tools that tries to solve this
>> well-known Python limitation.
> 
> I fully agree for DTS runtime.
> I'm expecting the dependencies are more tolerant for DTS doc.
> 
>> I've done a quick search of what's available in Ubuntu and two packages
>> aren't available, types-PyYAML (which maybe we could do without, I'll
>> have to test) and aenum (which is currently needed for the capabilities
>> patch; if absolutely necessary, maybe I could find a solution without
>> aenum). But even with this we can't be sure that the system package
>> versions will work.
> 
> We need them all to generate the documentation?
> 

We actually may not. The Python docstrings are part of the code (stored 
in the __docstring__ attribute of everything), Sphinx (more precisely 
the autodoc extension [0]) imports all the code to access the docstrings 
and to do that, it needs the dependencies.

However, I found a config option that mocks imports from the specified 
modules [1], so what we can do is list the missing modules there (and we 
can build without the dependencies). If we do this, we could emit a 
warning from Sphinx, although the resulting docs don't seem any 
different according to the basic tests I did.

So this means we can do a build with both passing the target and passing 
-Denable_docs, provided I won't encounter anything wild. I'll change the 
docs accordingly.

[0] 
https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#ensuring-the-code-can-be-imported
[1] 
https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#confval-autodoc_mock_imports

>>>>>> +.. code-block:: console
>>>>>> +
>>>>>> +   poetry install --no-root --with docs
>>>>>> +   poetry shell
>>>>>> +
>>>>>> +The documentation is built using the standard DPDK build system.
>>>>>> +After executing the meson command and entering Poetry's shell, build the documentation with:
>>>>>> +
>>>>>> +.. code-block:: console
>>>>>> +
>>>>>> +   ninja -C build dts-doc
>>>>>
>>>>> Don't we rely on the Meson option "enable_docs"?
>>>>
>>>> I had a discussion about this with Bruce, but I can't find it anywhere,
>>>> so here's what I remember:
>>>> 1. We didn't want to tie the dts api doc build to dpdk doc build because
>>>> of the dependencies.
>>>
>>> Sure
>>> But we could just skip if dependencies are not met?
>>
>> Maybe we could add a script that would check the dependencies. I'll see
>> what I can do.
> 
> OK thanks
> 
>
  

Patch

diff --git a/buildtools/call-sphinx-build.py b/buildtools/call-sphinx-build.py
index 693274da4e..dff8471560 100755
--- a/buildtools/call-sphinx-build.py
+++ b/buildtools/call-sphinx-build.py
@@ -14,10 +14,13 @@ 
 parser.add_argument('version')
 parser.add_argument('src')
 parser.add_argument('dst')
+parser.add_argument('--dts-root', default=None)
 args, extra_args = parser.parse_known_args()
 
 # set the version in environment for sphinx to pick up
 os.environ['DPDK_VERSION'] = args.version
+if args.dts_root:
+    os.environ['DTS_ROOT'] = args.dts_root
 
 sphinx_cmd = [args.sphinx] + extra_args
 
diff --git a/doc/api/doxy-api-index.md b/doc/api/doxy-api-index.md
index f9283154f8..cc214ede46 100644
--- a/doc/api/doxy-api-index.md
+++ b/doc/api/doxy-api-index.md
@@ -244,3 +244,6 @@  The public API headers are grouped by topics:
   [experimental APIs](@ref rte_compat.h),
   [ABI versioning](@ref rte_function_versioning.h),
   [version](@ref rte_version.h)
+
+- **tests**:
+  [**DTS**](@dts_api_main_page)
diff --git a/doc/api/doxy-api.conf.in b/doc/api/doxy-api.conf.in
index a8823c046f..c94f02d411 100644
--- a/doc/api/doxy-api.conf.in
+++ b/doc/api/doxy-api.conf.in
@@ -124,6 +124,8 @@  SEARCHENGINE            = YES
 SORT_MEMBER_DOCS        = NO
 SOURCE_BROWSER          = YES
 
+ALIASES                 = "dts_api_main_page=@DTS_API_MAIN_PAGE@"
+
 EXAMPLE_PATH            = @TOPDIR@/examples
 EXAMPLE_PATTERNS        = *.c
 EXAMPLE_RECURSIVE       = YES
diff --git a/doc/api/meson.build b/doc/api/meson.build
index b828b1ed66..ffc75d7b5a 100644
--- a/doc/api/meson.build
+++ b/doc/api/meson.build
@@ -41,6 +41,10 @@  cdata.set('WARN_AS_ERROR', 'NO')
 if get_option('werror')
     cdata.set('WARN_AS_ERROR', 'YES')
 endif
+# A local reference must be relative to the main index.html page
+# The path below can't be taken from the DTS meson file as that would
+# require recursive subdir traversal (doc, dts, then doc again)
+cdata.set('DTS_API_MAIN_PAGE', join_paths('..', 'dts', 'html', 'index.html'))
 
 # configure HTML Doxygen run
 html_cdata = configuration_data()
diff --git a/doc/guides/conf.py b/doc/guides/conf.py
index 8b440fb2a9..b442a1f76c 100644
--- a/doc/guides/conf.py
+++ b/doc/guides/conf.py
@@ -9,7 +9,7 @@ 
 from os import environ
 from os.path import basename, dirname
 from os.path import join as path_join
-from sys import argv, stderr
+from sys import argv, stderr, path
 
 import configparser
 
@@ -23,6 +23,37 @@ 
           file=stderr)
     pass
 
+# Napoleon enables the Google format of Python doscstrings, used in DTS
+# Intersphinx allows linking to external projects, such as Python docs, also used in DTS
+extensions = ['sphinx.ext.napoleon', 'sphinx.ext.intersphinx']
+
+# DTS Python docstring options
+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'
+napoleon_numpy_docstring = False
+napoleon_attr_annotations = True
+napoleon_preprocess_types = True
+add_module_names = False
+toc_object_entries = True
+toc_object_entries_show_parents = 'hide'
+intersphinx_mapping = {'python': ('https://docs.python.org/3', None)}
+
+dts_root = environ.get('DTS_ROOT')
+if dts_root:
+    path.append(dts_root)
+    # DTS Sidebar config
+    html_theme_options = {
+        'collapse_navigation': False,
+        'navigation_depth': -1,
+    }
+
 stop_on_error = ('-W' in argv)
 
 project = 'Data Plane Development Kit'
diff --git a/doc/guides/meson.build b/doc/guides/meson.build
index 51f81da2e3..8933d75f6b 100644
--- a/doc/guides/meson.build
+++ b/doc/guides/meson.build
@@ -1,6 +1,7 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2018 Intel Corporation
 
+doc_guides_source_dir = meson.current_source_dir()
 sphinx = find_program('sphinx-build', required: get_option('enable_docs'))
 
 if not sphinx.found()
diff --git a/doc/guides/tools/dts.rst b/doc/guides/tools/dts.rst
index 515b15e4d8..77df7a0378 100644
--- a/doc/guides/tools/dts.rst
+++ b/doc/guides/tools/dts.rst
@@ -292,7 +292,12 @@  and try not to divert much from it.
 The :ref:`DTS developer tools <dts_dev_tools>` will issue warnings
 when some of the basics are not met.
 
-The code must be properly documented with docstrings.
+The API documentation, which is a helpful reference when developing, may be accessed
+in the code directly or generated with the :ref:`API docs build steps <building_api_docs>`.
+When adding new files or modifying the directory structure,
+the corresponding changes must be made to DTS api doc sources in ``dts/doc``.
+
+Speaking of which, the code must be properly documented with docstrings.
 The style must conform to the `Google style
 <https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings>`_.
 See an example of the style `here
@@ -427,6 +432,33 @@  the DTS code check and format script.
 Refer to the script for usage: ``devtools/dts-check-format.sh -h``.
 
 
+.. _building_api_docs:
+
+Building DTS API docs
+---------------------
+
+To build DTS API docs, install the dependencies with Poetry, then enter its shell:
+
+.. code-block:: console
+
+   poetry install --no-root --with docs
+   poetry shell
+
+The documentation is built using the standard DPDK build system.
+After executing the meson command and entering Poetry's shell, build the documentation with:
+
+.. code-block:: console
+
+   ninja -C build dts-doc
+
+The output is generated in ``build/doc/api/dts/html``.
+
+.. Note::
+
+   Make sure to fix any Sphinx warnings when adding or updating docstrings,
+   and also run the ``devtools/dts-check-format.sh`` script and address any issues it finds.
+
+
 Configuration Schema
 --------------------
 
diff --git a/dts/doc/meson.build b/dts/doc/meson.build
new file mode 100644
index 0000000000..01b7b51034
--- /dev/null
+++ b/dts/doc/meson.build
@@ -0,0 +1,27 @@ 
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2023 PANTHEON.tech s.r.o.
+
+sphinx = find_program('sphinx-build', required: false)
+sphinx_apidoc = find_program('sphinx-apidoc', required: false)
+
+if not sphinx.found() or not sphinx_apidoc.found()
+    subdir_done()
+endif
+
+dts_doc_api_build_dir = join_paths(doc_api_build_dir, 'dts')
+
+extra_sphinx_args = ['-E', '-c', doc_guides_source_dir, '--dts-root', dts_dir]
+if get_option('werror')
+    extra_sphinx_args += '-W'
+endif
+
+htmldir = join_paths(get_option('datadir'), 'doc', 'dpdk', 'dts')
+dts_api_html = custom_target('dts_api_html',
+        output: 'html',
+        command: [sphinx_wrapper, sphinx, meson.project_version(),
+            meson.current_source_dir(), dts_doc_api_build_dir, extra_sphinx_args],
+        build_by_default: false,
+        install: get_option('enable_docs'),
+        install_dir: htmldir)
+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..e8ce0f06ac
--- /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 8b248d4505..835973a0ce 100644
--- a/meson.build
+++ b/meson.build
@@ -87,6 +87,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