On Thu, 1 Jul 2021, Luca Fancellu wrote: > > On 24 Jun 2021, at 00:33, Stefano Stabellini 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 > >> --- > >> .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" OK, thanks. I missed that addition. > > > >> +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 In that case, I think anything related to SPHINXBUILD and whether sphinx is installed or not, should be a separate patch at the beginning of the series. It could be committed independently before the rest of the series. When we get to this patch, SPHINXBUILD should be already there. > >> + $(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. OK :-/ > > > > > >> .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 Better to remove it or to move it to a separate patch > > > >> 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. Great > > > >> # -- 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. I would add them now even if they are not strictly required to parse the public headers. But this is the kind of thing where others might have a different opinion.