[PATCH v5 16/18] Documentation: Split public/private documentation
Dan Scally
dan.scally at ideasonboard.com
Wed Aug 7 15:45:03 CEST 2024
Hi Laurent
On 07/08/2024 14:31, Laurent Pinchart wrote:
> On Wed, Aug 07, 2024 at 12:45:31PM +0100, Kieran Bingham wrote:
>> Quoting Dan Scally (2024-08-07 09:30:22)
>>> On 05/08/2024 15:36, Laurent Pinchart wrote:
>>>> From: Daniel Scally <dan.scally at ideasonboard.com>
>>>>
>>>> The API reference pages generated by Doxygen are comprehensive, but
>>>> therefore quite overwhelming for application developers who will
>>>> likely never need to use the majority of the library's objects. To
>>>> reduce the complexity of the documentation, split it into two runs of
>>>> doxygen.
>>>>
>>>> The first run of doxygen is for the public API. We pass a specific list
>>>> of source files to parse, which is built from the arrays of public
>>>> headers and sources in meson build files. This ensures that we only
>>>> generate the documentation for code from those files.
>>>>
>>>> A custom Python script is needed to add the list of input files to
>>>> Doxyfile, as several of the objects included in the header and source
>>>> array are custom_tgt objects, which can't be handled as strings to
>>>> populate a variable in the configuration data.
>>>>
>>>> The headers defining the Extensible and Object classes (class.h and
>>>> object.h respectively), as well as the corresponding source files, are
>>>> excluded from the public API documentation despite being referenced in
>>>> the meson public headers and sources arrays. This is due to the fact
>>>> that public API classes inherit from Extensible and Object, making the
>>>> Extensible and Object classes part of the public ABI. Those two base
>>>> classes are however implementation details and must not be accessed
>>>> directly by application code.
>>>>
>>>> The second run of doxygen is for the internal API. This contains
>>>> documentation for all of the library's objects as it currently does.
>>>> This set will now be output into build/Documentation/internal-api-html.
>> Sounds good.
>>
>>>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
>>>> Co-developed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>> ---
>>> The python script to generate the list of input files is definitely better than scattered foreach
>>> loops collecting a list in a hacky way - good improvement.
>>>
>>>> .../{Doxyfile.in => Doxyfile-common.in} | 24 -------
>>>> Documentation/Doxyfile-internal.in | 30 ++++++++
>>>> Documentation/Doxyfile-public.in | 20 ++++++
>>>> Documentation/gen-doxyfile.py | 46 ++++++++++++
>>>> Documentation/meson.build | 70 +++++++++++++++++--
>>>> 5 files changed, 160 insertions(+), 30 deletions(-)
>>>> rename Documentation/{Doxyfile.in => Doxyfile-common.in} (62%)
>>>> create mode 100644 Documentation/Doxyfile-internal.in
>>>> create mode 100644 Documentation/Doxyfile-public.in
>>>> create mode 100755 Documentation/gen-doxyfile.py
>>>>
>>>> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile-common.in
>>>> similarity index 62%
>>>> rename from Documentation/Doxyfile.in
>>>> rename to Documentation/Doxyfile-common.in
>>>> index 6e5a3830ec38..a70aee430e77 100644
>>>> --- a/Documentation/Doxyfile.in
>>>> +++ b/Documentation/Doxyfile-common.in
>>>> @@ -17,20 +17,11 @@ EXTENSION_MAPPING = h=C++
>>>>
>>>> TOC_INCLUDE_HEADINGS = 0
>>>>
>>>> -INTERNAL_DOCS = YES
>>>> CASE_SENSE_NAMES = YES
>>>>
>>>> QUIET = YES
>>>> WARN_AS_ERROR = @WARN_AS_ERROR@
>>>>
>>>> -INPUT = "@TOP_SRCDIR@/Documentation" \
>>>> - "@TOP_SRCDIR@/include/libcamera" \
>>>> - "@TOP_SRCDIR@/src/ipa/ipu3" \
>>>> - "@TOP_SRCDIR@/src/ipa/libipa" \
>>>> - "@TOP_SRCDIR@/src/libcamera" \
>>>> - "@TOP_BUILDDIR@/include/libcamera" \
>>>> - "@TOP_BUILDDIR@/src/libcamera"
>>>> -
>>>> FILE_PATTERNS = *.c \
>>>> *.cpp \
>>>> *.dox \
>>>> @@ -38,19 +29,6 @@ FILE_PATTERNS = *.c \
>>>>
>>>> RECURSIVE = YES
>>>>
>>>> -EXCLUDE = @TOP_SRCDIR@/include/libcamera/base/span.h \
>>>> - @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_sysfs.h \
>>>> - @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_udev.h \
>>>> - @TOP_SRCDIR@/include/libcamera/internal/ipc_pipe_unixsocket.h \
>>>> - @TOP_SRCDIR@/src/libcamera/device_enumerator_sysfs.cpp \
>>>> - @TOP_SRCDIR@/src/libcamera/device_enumerator_udev.cpp \
>>>> - @TOP_SRCDIR@/src/libcamera/ipc_pipe_unixsocket.cpp \
>>>> - @TOP_SRCDIR@/src/libcamera/pipeline/ \
>>>> - @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \
>>>> - @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \
>>>> - @TOP_BUILDDIR@/include/libcamera/ipa/soft_ipa_interface.h \
>>>> - @TOP_BUILDDIR@/src/libcamera/proxy/
>>>> -
>>>> EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
>>>> @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h \
>>>> @TOP_BUILDDIR@/include/libcamera/ipa/ipu3_*.h \
>>>> @@ -73,8 +51,6 @@ EXCLUDE_SYMBOLS = libcamera::BoundMethodArgs \
>>>>
>>>> EXCLUDE_SYMLINKS = YES
>>>>
>>>> -HTML_OUTPUT = api-html
>>>> -
>>>> GENERATE_LATEX = NO
>>>>
>>>> MACRO_EXPANSION = YES
>>>> diff --git a/Documentation/Doxyfile-internal.in b/Documentation/Doxyfile-internal.in
>>>> new file mode 100644
>>>> index 000000000000..5e0310091416
>>>> --- /dev/null
>>>> +++ b/Documentation/Doxyfile-internal.in
>>>> @@ -0,0 +1,30 @@
>>>> +# SPDX-License-Identifier: CC-BY-SA-4.0
>>>> +
>>>> + at INCLUDE_PATH = @TOP_BUILDDIR@/Documentation
>>>> + at INCLUDE = Doxyfile-common
>>>> +
>>>> +HIDE_UNDOC_CLASSES = NO
>>>> +HIDE_UNDOC_MEMBERS = NO
>>>> +HTML_OUTPUT = internal-api-html
>>>> +INTERNAL_DOCS = YES
>>>> +
>>>> +INPUT = "@TOP_SRCDIR@/Documentation" \
>>>> + "@TOP_SRCDIR@/include/libcamera" \
>>>> + "@TOP_SRCDIR@/src/ipa/ipu3" \
>>>> + "@TOP_SRCDIR@/src/ipa/libipa" \
>>>> + "@TOP_SRCDIR@/src/libcamera" \
>>>> + "@TOP_BUILDDIR@/include/libcamera" \
>>>> + "@TOP_BUILDDIR@/src/libcamera"
>>>> +
>>>> +EXCLUDE = @TOP_SRCDIR@/include/libcamera/base/span.h \
>>>> + @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_sysfs.h \
>>>> + @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_udev.h \
>>>> + @TOP_SRCDIR@/include/libcamera/internal/ipc_pipe_unixsocket.h \
>>>> + @TOP_SRCDIR@/src/libcamera/device_enumerator_sysfs.cpp \
>>>> + @TOP_SRCDIR@/src/libcamera/device_enumerator_udev.cpp \
>>>> + @TOP_SRCDIR@/src/libcamera/ipc_pipe_unixsocket.cpp \
>>>> + @TOP_SRCDIR@/src/libcamera/pipeline/ \
>>>> + @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \
>>>> + @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \
>>>> + @TOP_BUILDDIR@/include/libcamera/ipa/soft_ipa_interface.h \
>>>> + @TOP_BUILDDIR@/src/libcamera/proxy/
>>>> diff --git a/Documentation/Doxyfile-public.in b/Documentation/Doxyfile-public.in
>>>> new file mode 100644
>>>> index 000000000000..36bb57584a07
>>>> --- /dev/null
>>>> +++ b/Documentation/Doxyfile-public.in
>>>> @@ -0,0 +1,20 @@
>>>> +# SPDX-License-Identifier: CC-BY-SA-4.0
>>>> +
>>>> + at INCLUDE_PATH = @TOP_BUILDDIR@/Documentation
>>>> + at INCLUDE = Doxyfile-common
>>>> +
>>>> +HIDE_UNDOC_CLASSES = YES
>>>> +HIDE_UNDOC_MEMBERS = YES
>>>> +HTML_OUTPUT = api-html
>>>> +INTERNAL_DOCS = NO
>>>> +
>>>> +INPUT = "@TOP_SRCDIR@/Documentation" \
>>>> + ${inputs}
>>>> +
>>>> +EXCLUDE = @TOP_SRCDIR@/include/libcamera/base/class.h \
>>>> + @TOP_SRCDIR@/include/libcamera/base/object.h \
>>>> + @TOP_SRCDIR@/include/libcamera/base/span.h \
>>>> + @TOP_SRCDIR@/src/libcamera/base/class.cpp \
>>>> + @TOP_SRCDIR@/src/libcamera/base/object.cpp
>>>> +
>>>> +PREDEFINED += __DOXYGEN_PUBLIC__
>>>> diff --git a/Documentation/gen-doxyfile.py b/Documentation/gen-doxyfile.py
>>>> new file mode 100755
>>>> index 000000000000..c2a4184dd195
>>>> --- /dev/null
>>>> +++ b/Documentation/gen-doxyfile.py
>>>> @@ -0,0 +1,46 @@
>>>> +#!/usr/bin/env python3
>>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>>> +# Copyright (C) 2024, Google Inc.
>>> Does this mean it's drawn from elsewhere? Or is it a copy-paste error?
>>>
>>>> +#
>>>> +# Author: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>> +#
>>>> +# Generate Doxyfile from a template
>>>> +
>>>> +import argparse
>>>> +import os
>>>> +import string
>>>> +import sys
>>>> +
>>>> +
>>>> +def fill_template(template, data):
>>>> +
>>>> + template = open(template, 'rb').read()
>>>> + template = template.decode('utf-8')
>>>> + template = string.Template(template)
>>> I'd add a line break here.
> Done.
>
>>>> + return template.substitute(data)
>>>> +
>>>> +
>>>> +def main(argv):
>>>> +
>>>> + # Parse command line arguments
>>> The comment can probably be skipped.
> I'm fine with that.
>
>
>>>> + parser = argparse.ArgumentParser()
>>>> + parser.add_argument('-o', dest='output', metavar='file',
>>>> + type=argparse.FileType('w', encoding='utf-8'),
>>>> + default=sys.stdout,
>>>> + help='Output file name (default: standard output)')
>>>> + parser.add_argument('template', metavar='doxyfile.tmpl', type=str,
>>>> + help='Doxyfile template')
>>>> + parser.add_argument('inputs', type=str, nargs='*',
>>>> + help='Input files')
>>>> +
>>>> + args = parser.parse_args(argv[1:])
>>>> +
>>>> + inputs = [f'"{os.path.realpath(input)}"' for input in args.inputs]
>>>> + data = fill_template(args.template, {'inputs': (' \\\n' + ' ' * 25).join(inputs)})
>>>> + args.output.write(data)
>>>> +
>>>> + return 0
>>>> +
>>>> +
>>>> +if __name__ == '__main__':
>>>> + sys.exit(main(sys.argv))
>>>> diff --git a/Documentation/meson.build b/Documentation/meson.build
>>>> index 1d84ed815b50..bf28233a2ce8 100644
>>>> --- a/Documentation/meson.build
>>>> +++ b/Documentation/meson.build
>>>> @@ -24,9 +24,9 @@ if doxygen.found() and dot.found()
>>>>
>>>> cdata.set('PREDEFINED', ' \\\n\t\t\t '.join(doxygen_predefined))
>>>>
>>>> - doxyfile = configure_file(input : 'Doxyfile.in',
>>>> - output : 'Doxyfile',
>>>> - configuration : cdata)
>>>> + doxyfile_common = configure_file(input : 'Doxyfile-common.in',
>>>> + output : 'Doxyfile-common',
>>>> + configuration : cdata)
>>>>
>>>> doxygen_public_input = [
>>>> libcamera_base_public_headers,
>>>> @@ -50,17 +50,75 @@ if doxygen.found() and dot.found()
>>>> doxygen_internal_input += [ipu3_ipa_sources]
>>>> endif
>>>>
>>>> - custom_target('doxygen',
>>>> + # We run doxygen twice - the first run excludes internal API objects as it
>>>> + # is intended to document the public API only. A second run covers all of
>>>> + # the library's objects for libcamera developers. Common configuration is
>>>> + # set in an initially generated Doxyfile, which is then included by the two
>>>> + # final Doxyfiles. We collected a list of the public sources in
>>>> + # doxygen_public_sources to pass to the public run and include it in cdata
>>>> + # here - although both Doxyfile and Doxyfile-internal will also receive
>>>> + # the same parameter they simply ignore it.
>>> This comment needs correcting now as it references the older method.
> I'll fix it.
>
>>>> +
>>>> + # This is the "public" run of doxygen generating an abridged version of the
>>>> + # API's documentation.
>>>> +
>>>> + doxyfile_tmpl = configure_file(input : 'Doxyfile-public.in',
>>>> + output : 'Doxyfile-public.tmpl',
>>>> + configuration : cdata)
>>>> +
>>>> + doxyfile = custom_target('doxyfile-public',
>>>> + input : [
>>>> + doxygen_public_input,
>>>> + ],
>>>> + output : 'Doxyfile-public',
>>>> + command : [
>>>> + 'gen-doxyfile.py',
>>>> + '-o', '@OUTPUT@',
>>>> + doxyfile_tmpl,
>>>> + '@INPUT@',
>>>> + ])
>>>> +
>>>> + custom_target('doxygen-public',
>>>> input : [
>>>> doxyfile,
>>>> - doxygen_public_input,
>>>> - doxygen_internal_input,
>>>> + doxyfile_common,
>> At least in my mail client - the indentation looks odd here and might
>> want checking.
> It looks fine in the source code as far as I can tell. Can you fix your
> mail client ? :-)
>
>>>> ],
>>>> output : 'api-html',
>>>> command : [doxygen, doxyfile],
>>>> install : true,
>>>> install_dir : doc_install_dir,
>>>> install_tag : 'doc')
>>>> +
>>>> + # This is the internal documentation, which hard-codes a list of directories
>>>> + # to parse in its doxyfile.
>>>> +
>>>> + doxyfile_tmpl = configure_file(input : 'Doxyfile-internal.in',
>>>> + output : 'Doxyfile-internal.tmpl',
>>>> + configuration : cdata)
>>>> +
>>>> + doxyfile = custom_target('doxyfile-internal',
>>>> + input : [
>>>> + doxygen_public_input,
>>>> + doxygen_internal_input,
>>>> + ],
>>>> + output : 'Doxyfile-internal',
>>>> + command : [
>>>> + 'gen-doxyfile.py',
>>>> + '-o', '@OUTPUT@',
>>>> + doxyfile_tmpl,
>>>> + '@INPUT@',
>>>> + ])
>>> I think we can skip the script for the internal run -there's no
>>> templated values to be replaced and the generated Doxyfile-internal
>>> is identical to the Doxyfile-internal.tmpl.
>> I wonder if that's "future proofing" but I don't mind either way.
> The goal was to keep the code flow identical, and I considered
> future-proofing too. I don't mind dropping it though.
Fair enough; leave it in then (unless you've already dropped it...I also don't mind either way so
whatever state it's currently in, is fine).
Is an R-b from me appropriate here given I'm down as the author? If so:
Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>
>> I have nothing much to add except a potential whitespace comment on top
>> of Dan's comments, so with those handled:
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>>
>>>> +
>>>> + custom_target('doxygen-internal',
>>>> + input : [
>>>> + doxyfile,
>>>> + doxyfile_common,
>>>> + ],
>>>> + output : 'internal-api-html',
>>>> + command : [doxygen, doxyfile],
>>>> + install : true,
>>>> + install_dir : doc_install_dir,
>>>> + install_tag : 'doc-internal')
>>>> endif
>>>>
>>>> #
More information about the libcamera-devel
mailing list