[PATCH v5 16/18] Documentation: Split public/private documentation

Kieran Bingham kieran.bingham at ideasonboard.com
Wed Aug 7 13:45:31 CEST 2024


Quoting Dan Scally (2024-08-07 09:30:22)
> Hi Laurent - thanks for the patch
> 
> 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.
> > +    return template.substitute(data)
> > +
> > +
> > +def main(argv):
> > +
> > +    # Parse command line arguments
> The comment can probably be skipped.
> > +    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.
> > +
> > +    # 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.

> >                     ],
> >                     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.

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