> On 24 Jun 2021, at 00:33, Stefano Stabellini <sstabell...@kernel.org> wrote:
>
> On Mon, 10 May 2021, Luca Fancellu wrote:
>> Modify docs/Makefile to call doxygen and generate sphinx
>> html documentation given the doxygen XML output.
>>
>> Modify docs/conf.py sphinx configuration file to setup
>> the breathe extension that works as bridge between
>> sphinx and doxygen.
>>
>> Add some files to the .gitignore to ignore some
>> generated files for doxygen.
>>
>> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
>> ---
>> .gitignore | 6 ++++++
>> docs/Makefile | 42 +++++++++++++++++++++++++++++++++++++++---
>> docs/conf.py | 48 +++++++++++++++++++++++++++++++++++++++++++++---
>> 3 files changed, 90 insertions(+), 6 deletions(-)
>>
>> diff --git a/.gitignore b/.gitignore
>> index 1c2fa1530b..d271e0ce6a 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -58,6 +58,12 @@ docs/man7/
>> docs/man8/
>> docs/pdf/
>> docs/txt/
>> +docs/doxygen-output
>> +docs/sphinx
>> +docs/xen.doxyfile
>> +docs/xen.doxyfile.tmp
>> +docs/xen-doxygen/doxygen_include.h
>> +docs/xen-doxygen/doxygen_include.h.tmp
>> extras/mini-os*
>> install/*
>> stubdom/*-minios-config.mk
>> diff --git a/docs/Makefile b/docs/Makefile
>> index 8de1efb6f5..2f784c36ce 100644
>> --- a/docs/Makefile
>> +++ b/docs/Makefile
>> @@ -17,6 +17,18 @@ TXTSRC-y := $(sort $(shell find misc -name '*.txt'
>> -print))
>>
>> PANDOCSRC-y := $(sort $(shell find designs/ features/ misc/ process/ specs/
>> \( -name '*.pandoc' -o -name '*.md' \) -print))
>>
>> +# Directory in which the doxygen documentation is created
>> +# This must be kept in sync with breathe_projects value in conf.py
>> +DOXYGEN_OUTPUT = doxygen-output
>> +
>> +# Doxygen input headers from xen-doxygen/doxy_input.list file
>> +DOXY_LIST_SOURCES != cat "xen-doxygen/doxy_input.list"
>> +DOXY_LIST_SOURCES := $(realpath $(addprefix
>> $(XEN_ROOT)/,$(DOXY_LIST_SOURCES)))
Hi Stefano,
>
> I cannot find exactly who is populating doxy_input.list. I can see it is
> empty in patch #6. Does it get populated during the build?
doxy_input.list is the only file that should be modified by the developer when
he/she wants to add documentation
for a new file to be parsed by Doxygen, in my patch about documenting
grant_tables.h you can see I add
there the path “xen/include/public/grant_table.h"
>
>
>> +DOXY_DEPS := xen.doxyfile \
>> + xen-doxygen/mainpage.md \
>> + xen-doxygen/doxygen_include.h
>> +
>> # Documentation targets
>> $(foreach i,$(MAN_SECTIONS), \
>> $(eval DOC_MAN$(i) := $(patsubst man/%.$(i),man$(i)/%.$(i), \
>> @@ -46,8 +58,28 @@ all: build
>> build: html txt pdf man-pages figs
>>
>> .PHONY: sphinx-html
>> -sphinx-html:
>> - sphinx-build -b html . sphinx/html
>> +sphinx-html: $(DOXY_DEPS) $(DOXY_LIST_SOURCES)
>> +ifneq ($(SPHINXBUILD),no)
>
> This check on SPHINXBUILD is new, it wasn't there before. Why do we need
> it now? We are not really changing anything in regards to Sphinx, just
> adding Doxygen support. Or was it a mistake that it was missing even
> before this patch?
Yes this is new, I saw that we didn’t look if sphinx was installed in the
system, so now we did
>
>
>> + $(DOXYGEN) xen.doxyfile
>> + XEN_ROOT=$(realpath $(XEN_ROOT)) $(SPHINXBUILD) -b html . sphinx/html
>> +else
>> + @echo "Sphinx is not installed; skipping sphinx-html documentation."
>> +endif
>> +
>> +xen.doxyfile: xen.doxyfile.in xen-doxygen/doxy_input.list
>> + @echo "Generating $@"
>> + @sed -e "s,@XEN_BASE@,$(realpath $(XEN_ROOT)),g" $< \
>> + | sed -e "s,@DOXY_OUT@,$(DOXYGEN_OUTPUT),g" > $@.tmp
>> + @$(foreach inc,\
>> + $(DOXY_LIST_SOURCES),\
>> + echo "INPUT += \"$(inc)\"" >> $@.tmp; \
>> + )
>> + mv $@.tmp $@
>> +
>> +xen-doxygen/doxygen_include.h: xen-doxygen/doxygen_include.h.in
>> + @echo "Generating $@"
>> + @sed -e "s,@XEN_BASE@,$(realpath $(XEN_ROOT)),g" $< > $@.tmp
>> + @mv $@.tmp $@
>
> Is the absolute path required? If not, we can probably get rid of this
> generation step and simply have the relative path in
> xen-doxygen/doxygen_include.h. I think this could apply to
> xen.doxyfile.in above.
Unfortunately yes, the doxygen_include.h is a file that is included in every
documented header before
starting the doxygen parser, since we don’t have all the headers in one path,
it is impossible to have here
a relative path that is good for every header in Xen.
>
>
>> .PHONY: html
>> html: $(DOC_HTML) html/index.html
>> @@ -71,7 +103,11 @@ clean: clean-man-pages
>> $(MAKE) -C figs clean
>> rm -rf .word_count *.aux *.dvi *.bbl *.blg *.glo *.idx *~
>> rm -rf *.ilg *.log *.ind *.toc *.bak *.tmp core
>> - rm -rf html txt pdf sphinx/html
>> + rm -rf html txt pdf sphinx $(DOXYGEN_OUTPUT)
>> + rm -f xen.doxyfile
>> + rm -f xen.doxyfile.tmp
>> + rm -f xen-doxygen/doxygen_include.h
>> + rm -f xen-doxygen/doxygen_include.h.tmp
>>
>> .PHONY: distclean
>> distclean: clean
>> diff --git a/docs/conf.py b/docs/conf.py
>> index 50e41501db..a48de42331 100644
>> --- a/docs/conf.py
>> +++ b/docs/conf.py
>> @@ -13,13 +13,17 @@
>> # add these directories to sys.path here. If the directory is relative to the
>> # documentation root, use os.path.abspath to make it absolute, like shown
>> here.
>> #
>> -# import os
>> -# import sys
>> +import os
>> +import sys
>> # sys.path.insert(0, os.path.abspath('.'))
>>
>>
>> # -- Project information
>> -----------------------------------------------------
>>
>> +if "XEN_ROOT" not in os.environ:
>> + sys.exit("$XEN_ROOT environment variable undefined.")
>> +XEN_ROOT = os.path.abspath(os.environ["XEN_ROOT"])
>> +
>> project = u'Xen'
>> copyright = u'2019, The Xen development community'
>> author = u'The Xen development community'
>> @@ -35,6 +39,7 @@ try:
>> xen_subver = line.split(u"=")[1].strip()
>> elif line.startswith(u"export XEN_EXTRAVERSION"):
>> xen_extra = line.split(u"=")[1].split(u"$", 1)[0].strip()
>> +
>
> spurious change?
I think I’ve intentionally added a new line to separate the code from the
except: below,
but if it is a problem I can remove it
>
>
>> except:
>> pass
>> finally:
>> @@ -44,6 +49,15 @@ finally:
>> else:
>> version = release = u"unknown version"
>>
>> +try:
>> + xen_doxygen_output = None
>> +
>> + for line in open(u"Makefile"):
>> + if line.startswith(u"DOXYGEN_OUTPUT"):
>> + xen_doxygen_output = line.split(u"=")[1].strip()
>> +except:
>> + sys.exit("DOXYGEN_OUTPUT variable undefined.")
>
> This is a bit strange: isn't there a better way to get the
> DOXYGEN_OUTPUT variable than reading the Makefile?
>
> At that point I think it would be better to define DOXYGEN_OUTPUT a
> second time in conf.py. But maybe it could be passed as an evironmental
> variable?
Yes we could pass it as an environment variable as we do with XEN_ROOT,
I will fix it in a next release.
>
>
>> # -- General configuration
>> ---------------------------------------------------
>>
>> # If your documentation needs a minimal Sphinx version, state it here.
>> @@ -53,7 +67,8 @@ needs_sphinx = '1.4'
>> # Add any Sphinx extension module names here, as strings. They can be
>> # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
>> # ones.
>> -extensions = []
>> +# breathe -> extension that integrates doxygen xml output with sphinx
>> +extensions = ['breathe']
>>
>> # Add any paths that contain templates here, relative to this directory.
>> templates_path = ['_templates']
>> @@ -175,6 +190,33 @@ texinfo_documents = [
>> 'Miscellaneous'),
>> ]
>>
>> +# -- Options for Breathe extension
>> -------------------------------------------
>> +
>> +breathe_projects = {
>> + "Xen": "{}/docs/{}/xml".format(XEN_ROOT, xen_doxygen_output)
>> +}
>> +breathe_default_project = "Xen"
>> +
>> +breathe_domain_by_extension = {
>> + "h": "c",
>> + "c": "c",
>> +}
>> +breathe_separate_member_pages = True
>> +breathe_show_enumvalue_initializer = True
>> +breathe_show_define_initializer = True
>> +
>> +# Qualifiers to a function are causing Sphihx/Breathe to warn about
>> +# Error when parsing function declaration and more. This is a list
>> +# of strings that the parser additionally should accept as
>> +# attributes.
>> +cpp_id_attributes = [
>> + '__syscall', '__deprecated', '__may_alias',
>> + '__used', '__unused', '__weak',
>> + '__DEPRECATED_MACRO', 'FUNC_NORETURN',
>> + '__subsystem',
>
> Should we also have any of following:
>
> __packed
> __init
> __attribute__
> __aligned__
>
> in the list? In any case, we don't have to add them right now, we could
> add them later as we expand Doxygen coverage if they become needed.
Sure it is possible, I can add them now since I have to push a fix for this
patch
If you want.
Cheers,
Luca
>
>
>> +]
>> +c_id_attributes = cpp_id_attributes
>> +
>>
>> # -- Options for Epub output
>> -------------------------------------------------
