Skip to content

docs: Switch from 'breathe' to 'docleaf' for API documentation generation #59570

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jul 3, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/doc-build.yml
Original file line number Diff line number Diff line change
@@ -85,7 +85,7 @@ jobs:
DOC_TARGET="html"
fi

DOC_TAG=${DOC_TAG} SPHINXOPTS="-q -W -t publish" make -C doc ${DOC_TARGET}
DOC_TAG=${DOC_TAG} SPHINXOPTS="-q -W --keep-going -t publish" make -C doc ${DOC_TARGET}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

drop? I wonder if we should actually default to this, I was not aware of this option!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed the commit that removed this and amended the commit message to indicate your approval.


- name: compress-docs
run: |
2 changes: 1 addition & 1 deletion doc/_extensions/zephyr/doxyrunner.py
Original file line number Diff line number Diff line change
@@ -9,7 +9,7 @@
============

This Sphinx plugin can be used to run Doxygen build as part of the Sphinx build
process. It is meant to be used with other plugins such as ``breathe`` in order
process. It is meant to be used with other plugins such as ``docleaf`` in order
to improve the user experience. The principal features offered by this plugin
are:

44 changes: 40 additions & 4 deletions doc/_extensions/zephyr/warnings_filter.py
Original file line number Diff line number Diff line change
@@ -24,7 +24,7 @@

import logging
import re
from typing import Dict, Any, List
from typing import Dict, Any, List, Optional

from sphinx.application import Sphinx
from sphinx.util.logging import NAMESPACE
@@ -41,6 +41,7 @@ class WarningsFilter(logging.Filter):
silent: If true, warning is hidden, otherwise it is shown as INFO.
name: Filter name.
"""

def __init__(self, expressions: List[str], silent: bool, name: str = "") -> None:
super().__init__(name)

@@ -52,7 +53,8 @@ def filter(self, record: logging.LogRecord) -> bool:
return True

for expression in self._expressions:
if re.match(expression, record.msg):
# The message isn't always a string so we convert it before regexing as we can only regex strings
if expression.match(str(record.msg)):
if self._silent:
return False
else:
@@ -63,6 +65,21 @@ def filter(self, record: logging.LogRecord) -> bool:
return True


class Expression:
"""
Encapsulate a log filter pattern and track if it ever matches a log line.
"""

def __init__(self, pattern):
self.pattern = pattern
self.matched = False

def match(self, str):
matches = bool(re.match(self.pattern, str))
self.matched = matches or self.matched
return matches


def configure(app: Sphinx) -> None:
"""Entry point.

@@ -74,8 +91,10 @@ def configure(app: Sphinx) -> None:
with open(app.config.warnings_filter_config) as f:
expressions = list()
for line in f.readlines():
if not line.startswith("#"):
expressions.append(line.rstrip())
if line.strip() and not line.startswith("#"):
expressions.append(Expression(line.rstrip()))

app.env.warnings_filter_expressions = expressions

# install warnings filter to all the Sphinx logger handlers
filter = WarningsFilter(expressions, app.config.warnings_filter_silent)
@@ -84,11 +103,28 @@ def configure(app: Sphinx) -> None:
handler.filters.insert(0, filter)


def finished(app: Sphinx, exception: Optional[Exception]):
"""
Prints out any patterns that have not matched a log line to allow us to clean up any that are not used.
"""
if exception:
# Early exit if there has been an exception as matching data is only
# valid for complete builds
return

expressions = app.env.warnings_filter_expressions

for expression in expressions:
if not expression.matched:
logging.warning(f"Unused expression: {expression.pattern}")


def setup(app: Sphinx) -> Dict[str, Any]:
app.add_config_value("warnings_filter_config", "", "")
app.add_config_value("warnings_filter_silent", True, "")

app.connect("builder-inited", configure)
app.connect("build-finished", finished)

return {
"version": __version__,
4 changes: 2 additions & 2 deletions doc/_static/css/custom.css
Original file line number Diff line number Diff line change
@@ -843,13 +843,13 @@ kbd, .kbd,
background-color: var(--navbar-scrollbar-active-color);
}

/* Breathe tweaks */
/* Docleaf tweaks */

.rst-content .section > dl > dd {
margin-left: 0;
}

.rst-content p.breathe-sectiondef-title {
.rst-content p.docleaf-sectiondef-title {
font-size: 115%;
color: var(--link-color);
}
15 changes: 8 additions & 7 deletions doc/conf.py
Original file line number Diff line number Diff line change
@@ -67,7 +67,7 @@
# -- General configuration ------------------------------------------------

extensions = [
"breathe",
"docleaf.doxygen",
"sphinx.ext.todo",
"sphinx.ext.extlinks",
"sphinx.ext.autodoc",
@@ -211,16 +211,17 @@
doxyrunner_fmt_vars = {"ZEPHYR_BASE": str(ZEPHYR_BASE), "ZEPHYR_VERSION": version}
doxyrunner_outdir_var = "DOXY_OUT"

# -- Options for Breathe plugin -------------------------------------------
# -- Options for Docleaf plugin -------------------------------------------

breathe_projects = {"Zephyr": str(doxyrunner_outdir / "xml")}
breathe_default_project = "Zephyr"
breathe_domain_by_extension = {
docleaf_projects = {"Zephyr": {"xml": str(doxyrunner_outdir / "xml"), "root": "../"}}
docleaf_default_project = "Zephyr"
docleaf_domain_by_extension = {
"h": "c",
"c": "c",
}
breathe_show_enumvalue_initializer = True
breathe_default_members = ("members", )
# Filters out any 'function' or 'variable' members that have 'all caps' names as
# they are likely unprocessed macro calls
docleaf_doxygen_skip = ["members:all_caps"]

cpp_id_attributes = [
"__syscall",
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/blob.rst
Original file line number Diff line number Diff line change
@@ -127,4 +127,3 @@ This section contains types and defines common to the BLOB Transfer models.

.. doxygengroup:: bt_mesh_blob
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/blob_cli.rst
Original file line number Diff line number Diff line change
@@ -93,4 +93,3 @@ API reference

.. doxygengroup:: bt_mesh_blob_cli
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/blob_srv.rst
Original file line number Diff line number Diff line change
@@ -67,4 +67,3 @@ API reference

.. doxygengroup:: bt_mesh_blob_srv
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/dfd_srv.rst
Original file line number Diff line number Diff line change
@@ -23,4 +23,3 @@ API reference

.. doxygengroup:: bt_mesh_dfd_srv
:project: Zephyr
:members:
2 changes: 0 additions & 2 deletions doc/connectivity/bluetooth/api/mesh/dfu.rst
Original file line number Diff line number Diff line change
@@ -221,8 +221,6 @@ This section lists the types common to the Device Firmware Update mesh models.

.. doxygengroup:: bt_mesh_dfu
:project: Zephyr
:members:

.. doxygengroup:: bt_mesh_dfu_metadata
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/dfu_cli.rst
Original file line number Diff line number Diff line change
@@ -11,4 +11,3 @@ API reference

.. doxygengroup:: bt_mesh_dfu_cli
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/dfu_srv.rst
Original file line number Diff line number Diff line change
@@ -68,4 +68,3 @@ API reference

.. doxygengroup:: bt_mesh_dfu_srv
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/lcd_cli.rst
Original file line number Diff line number Diff line change
@@ -17,4 +17,3 @@ API reference

.. doxygengroup:: bt_mesh_large_comp_data_cli
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/lcd_srv.rst
Original file line number Diff line number Diff line change
@@ -27,4 +27,3 @@ API reference

.. doxygengroup:: bt_mesh_large_comp_data_srv
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/od_cli.rst
Original file line number Diff line number Diff line change
@@ -26,4 +26,3 @@ API reference

.. doxygengroup:: bt_mesh_od_priv_proxy_cli
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/od_srv.rst
Original file line number Diff line number Diff line change
@@ -25,4 +25,3 @@ API reference

.. doxygengroup:: bt_mesh_od_priv_proxy_srv
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/op_agg_cli.rst
Original file line number Diff line number Diff line change
@@ -27,4 +27,3 @@ API reference

.. doxygengroup:: bt_mesh_op_agg_cli
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/op_agg_srv.rst
Original file line number Diff line number Diff line change
@@ -29,4 +29,3 @@ API reference

.. doxygengroup:: bt_mesh_op_agg_srv
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/priv_beacon_cli.rst
Original file line number Diff line number Diff line change
@@ -33,4 +33,3 @@ API reference

.. doxygengroup:: bt_mesh_priv_beacon_cli
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/priv_beacon_srv.rst
Original file line number Diff line number Diff line change
@@ -36,4 +36,3 @@ API reference

.. doxygengroup:: bt_mesh_priv_beacon_srv
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/rpr_cli.rst
Original file line number Diff line number Diff line change
@@ -129,4 +129,3 @@ API reference

.. doxygengroup:: bt_mesh_rpr_cli
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/rpr_srv.rst
Original file line number Diff line number Diff line change
@@ -27,4 +27,3 @@ API reference

.. doxygengroup:: bt_mesh_rpr_srv
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/srpl_cli.rst
Original file line number Diff line number Diff line change
@@ -28,4 +28,3 @@ API reference

.. doxygengroup:: bt_mesh_sol_pdu_rpl_cli
:project: Zephyr
:members:
1 change: 0 additions & 1 deletion doc/connectivity/bluetooth/api/mesh/srpl_srv.rst
Original file line number Diff line number Diff line change
@@ -27,4 +27,3 @@ API reference

.. doxygengroup:: bt_mesh_sol_pdu_rpl_srv
:project: Zephyr
:members:
10 changes: 5 additions & 5 deletions doc/contribute/documentation/generation.rst
Original file line number Diff line number Diff line change
@@ -52,7 +52,7 @@ The project's documentation contains the following items:
header [shape="rectangle" label="c header\ncomments"]
xml [shape="rectangle" label="XML"]
html [shape="rectangle" label="HTML\nweb site"]
sphinx[shape="ellipse" label="sphinx +\nbreathe,\ndocutils"]
sphinx[shape="ellipse" label="sphinx +\ndocleaf,\ndocutils"]
images -> sphinx
rst -> sphinx
conf -> sphinx
@@ -65,7 +65,7 @@ The project's documentation contains the following items:


The reStructuredText files are processed by the Sphinx documentation system,
and make use of the breathe extension for including the doxygen-generated API
and make use of the docleaf extension for including the doxygen-generated API
material. Additional tools are required to generate the
documentation locally, as described in the following sections.

@@ -226,16 +226,16 @@ build the documentation directly from there:
Filtering expected warnings
***************************

There are some known issues with Sphinx/Breathe that generate Sphinx warnings
There are some known issues with Sphinx/Docleaf that generate Sphinx warnings
even though the input is valid C code. While these issues are being considered
for fixing we have created a Sphinx extension that allows to filter them out
based on a set of regular expressions. The extension is named
``zephyr.warnings_filter`` and it is located at
``doc/_extensions/zephyr/warnings_filter.py``. The warnings to be filtered out
can be added to the ``doc/known-warnings.txt`` file.

The most common warning reported by Sphinx/Breathe is related to duplicate C
declarations. This warning may be caused by different Sphinx/Breathe issues:
The most common warning reported by Sphinx/Docleaf is related to duplicate C
declarations. This warning may be caused by different Sphinx/Docleaf issues:

- Multiple declarations of the same object are not supported
- Different objects (e.g. a struct and a function) can not share the same name
22 changes: 17 additions & 5 deletions doc/known-warnings.txt
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
# Each line should contain the regular expression of a known Sphinx warning
# that should be filtered out
.*Duplicate C declaration.*\n.*'\.\. c:.*:: dma_config'.*

# Function and (enum or struct) name
.*Duplicate C declaration.*\n.*'\.\. c:.*:: flash_img_check'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: zsock_fd_set'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: net_if_mcast_monitor'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: fs_statvfs'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*dmic_trigger.*'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: uint16_t id'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: dma_config'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: net_if_mcast_monitor'.*
.*Duplicate C declaration.*\n.*'\.\. c:struct:: bt_ots_init'.*

# Struct and typedef name
.*Duplicate C declaration.*\n.*'\.\. c:.*:: zsock_fd_set'.*

# Function and extern function
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*net_if_ipv4_addr_mask_cmp.*'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*net_if_ipv4_is_addr_bcast.*'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*net_if_ipv4_addr_lookup.*'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*net_if_ipv6_addr_lookup.*'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*net_if_ipv6_maddr_lookup.*'.*

# Common field names
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*struct in_addr.*'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*struct in6_addr.*'.*
.*Duplicate C declaration.*\n.*'\.\. c:.*:: .*struct net_if.*'.*
.*Duplicate C declaration.*\n.*'\.\. c:struct:: bt_ots_init'.*

# Clash with field of nested anonymous struct
.*Duplicate C declaration.*\n.*'\.\. c:member:: enum *bt_mesh_dfd_upload_phase bt_mesh_dfd_srv.phase'.*
.*Duplicate C declaration.*\n.*'\.\. c:member:: struct *bt_mesh_blob_xfer bt_mesh_dfu_cli.blob'.*
.*Duplicate C declaration.*\n.*'\.\. c:member:: struct *net_if *\* net_if_mcast_monitor.iface'.
Comment on lines +28 to +30
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: why didn't these fail in breathe?

Copy link
Collaborator Author

@michaeljones michaeljones Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. It would also be sensible to ask how many of the other patterns in this file we still need.

Docleaf isn't a line-for-line translation of Breathe into Rust. It is a new attempt to solve the same problem in a similar fashion. If it was easier to follow the logic the Breathe code base then a from-scratch re-write might not have been so appealing. All this is to say, I don't know why these didn't fail for Breathe and I'm not exactly sure why they are required for Docleaf.

Would you like clear answers to this before progressing? I can see why that would be appealing and feel like the correct approach. It'll take a bit of digging around though.

Edit: I'll dig into them. It'll be good to understand them more. Maybe a system we were can feed the names in as some config and docleaf can flag where it sees them in the rendering process or something.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial impression is that the root of the issue at times seems to be that the Sphinx C-domain does not have good handling for when structs, functions and other entities have the same name. This is allowed in C and present a few times in the Zephyr code base. This leads to duplicate targets being generated and triggering warnings from the Sphinx C Domain code.

It is mentioned here: sphinx-doc/sphinx#8241 (comment) in an issue which goes on to reference this pull request: sphinx-doc/sphinx#8313 as the solution but that seems to have been open for a long time.

As the references are parsed and stored by the C Domain which then generates the warnings for clashes, I don't see there being a clear way to work around them other than the current regexes. The question then becomes the difference between Breathe and Docleaf and that comes down to how much of the Doxygen XML the two systems attempt to render. In various edge cases, the XML becomes harder to make clear sense of and so the two systems potentially draw the lines at different points in different places. Neither are perfect.

I will work through the various cases (as highlighted by deleting all the lines in known-warnings.txt file) and document why each appears to be happening and attempt to improve Docleaf where possible. Any due to name clashes are potentially going to have to stay but I can see at least one case where field name is clashing with a field in a nested structure and that might be possible to avoid.

Copy link
Collaborator Author

@michaeljones michaeljones Jul 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a new commit which adds categories to the known-warnings.txt file attempting to indicate the source of the various issues. Some are due to entities sharing the same name and the Sphinx issues indicated before. Some are due to fields with the same name appearing in multiple anonymous structs. Some are due to Doxygen not indicating extern declarations in the XML output and so Docleaf and Breathe are unable to filter them out in any fashion. I have raised a ticket for this: doxygen/doxygen#10163 (Edit - and created a PR: doxygen/doxygen#10164)

It is possible to use the Doxygen @cond syntax to have it ignore parts of the file and so we could use that to explicit stop it processing the extern declarations but that would introduce more changes than the current known-warnings.txt patterns.

Finally the new lines are due to Docleaf attempting to render fields from anonymous structs which then clash with fields in the parent struct. I believe this didn't happen in Breathe as it doesn't attempt to render the members in some of these situations. For example, the breathe output for bt_mesh_dfd_srv is:

image

Whilst the docleaf output is:

image

With the actual code being:

struct bt_mesh_dfd_srv {
const struct bt_mesh_dfd_srv_cb *cb;
struct bt_mesh_model *mod;
struct bt_mesh_dfu_cli dfu;
struct bt_mesh_dfu_target targets[CONFIG_BT_MESH_DFD_SRV_TARGETS_MAX];
struct bt_mesh_blob_target_pull pull_ctxs[CONFIG_BT_MESH_DFD_SRV_TARGETS_MAX];
const struct bt_mesh_blob_io *io;
uint16_t target_cnt;
uint16_t slot_idx;
bool apply;
enum bt_mesh_dfd_phase phase;
struct bt_mesh_blob_cli_inputs inputs;
struct {
enum bt_mesh_dfd_upload_phase phase;
const struct bt_mesh_dfu_slot *slot;
const struct flash_area *area;
struct bt_mesh_blob_srv blob;
} upload;
};

Which shows that the anonymous struct members incorrectly appear in the body of the main struct. Unfortunately this is because the Doxygen xml output (structbt__mesh__dfd__srv.zip) doesn't distinguish between the two levels. It flattens everything. I'm not sure what is better or worse. Leaving out the details or having them poorly represented. I haven't yet looked to see if there is a doxygen issue for this but I fear it might involve changes to their XML structures to better support this and that seems like it is possibly unlikely.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would welcome input as to what we might want to address or account for at this stage or if the current state is acceptable in some fashion for moving forwards.

2 changes: 1 addition & 1 deletion scripts/requirements-doc.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# DOC: used to generate docs

breathe>=4.34
docleaf==0.8.0
sphinx~=5.0,!=5.2.0.post0
sphinx_rtd_theme~=1.0
sphinx-tabs