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.


Reply via email to