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

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

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