[{"id":27071,"web_url":"https://patchwork.libcamera.org/comment/27071/","msgid":"<168359215841.2007737.1464908094811998162@Monstersaurus>","date":"2023-05-09T00:29:18","subject":"Re: [libcamera-devel] [PATCH v1 1/2] Documentation: Add predefined\n\tmacros from config.h to Doxyfile","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2023-05-06 12:10:24)\n> libcamera creates a config.h file with predefine macros, and instructs\n\n/predefine/predefined/\n\n> the compiler to include it implicitly with the -include argument.\n> Doxygen has no support for implicit inclusion of headers, but has a\n> PREDEFINED configuration option for its preprocessor that lists\n> predefined macros. Populate it with the values from the config_h\n> configuration data object that is used for generate the config.h file,\n> to ensure that documentation matches the configuration options libcamera\n> has been built with.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> Documentation/Doxyfile.in | 3 ++-\n> Documentation/meson.build | 7 +++++++\n> 2 files changed, 9 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index 1447abdb7d8c..697a14d1dfe2 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -80,6 +80,7 @@ INCLUDE_FILE_PATTERNS = *.h\n> \n> PREDEFINED = __DOXYGEN__ \\\n> __cplusplus \\\n> - __attribute__(x)=\n> + __attribute__(x)= \\\n> + @PREDEFINED@\n> \n> HAVE_DOT = YES\n> diff --git a/Documentation/meson.build b/Documentation/meson.build\n> index 186461561f8d..ed581163345e 100644\n> --- a/Documentation/meson.build\n> +++ b/Documentation/meson.build\n> @@ -16,6 +16,13 @@ if doxygen.found() and dot.found()\n> cdata.set('TOP_BUILDDIR', meson.project_build_root())\n> cdata.set('OUTPUT_DIR', meson.current_build_dir())\n> \n> + doxygen_predefined = []\n> + foreach key : config_h.keys()\n> + doxygen_predefined += '@0@=@1@'.format(key, config_h.get(key))\n> + endforeach\n> +\n> + cdata.set('PREDEFINED', ' '.join(doxygen_predefined))\n> +\n\nGoing for a quick test, this generates:\n\nPREDEFINED = __DOXYGEN__ \\\n __cplusplus \\\n __attribute__(x)= \\\n HAVE_BACKTRACE=1 HAVE_DW=1 HAVE_GNUTLS=1 HAVE_IPA_PUBKEY=1 HAVE_LIBUDEV=1 HAVE_LOCALE_T=1 HAVE_SECURE_GETENV=1 HAVE_TRACING=1 HAVE_UNWIND=1 IPA_CONFIG_DIR=\"/etc/libcamera/ipa:/usr/share/libcamera/ipa\" IPA_MODULE_DIR=\"/usr/lib/x86_64-linux-gnu/libcamera\" IPA_PROXY_DIR=\"/usr/libexec/libcamera\" LIBCAMERA_DATA_DIR=\"/usr/share/libcamera\" LIBCAMERA_SYSCONF_DIR=\"/etc/libcamera\"\n\nWhich looks reasonable to me.\n\nYou could go one step further and match the style of the rest of the\nfile though which makes it easier to parse. Though it's unlikely for\nsomeone to be reading this specifically! (Unless they're checking the\nDoxyfile for parsing issues which maybe more readability would help!?)\n\n\n- cdata.set('PREDEFINED', ' '.join(doxygen_predefined))\n+ cdata.set('PREDEFINED', ' \\\\\\n\\t\\t\\t '.join(doxygen_predefined))\n\n\nMakes a cleaner:\n\nPREDEFINED = __DOXYGEN__ \\\n __cplusplus \\\n __attribute__(x)= \\\n HAVE_BACKTRACE=1 \\\n HAVE_DW=1 \\\n HAVE_GNUTLS=1 \\\n HAVE_IPA_PUBKEY=1 \\\n HAVE_LIBUDEV=1 \\\n HAVE_LOCALE_T=1 \\\n HAVE_SECURE_GETENV=1 \\\n HAVE_TRACING=1 \\\n HAVE_UNWIND=1 \\\n IPA_CONFIG_DIR=\"/etc/libcamera/ipa:/usr/share/libcamera/ipa\" \\\n IPA_MODULE_DIR=\"/usr/lib/x86_64-linux-gnu/libcamera\" \\\n IPA_PROXY_DIR=\"/usr/libexec/libcamera\" \\\n LIBCAMERA_DATA_DIR=\"/usr/share/libcamera\" \\\n LIBCAMERA_SYSCONF_DIR=\"/etc/libcamera\"\n\nI'm a bit bemused about how any of those would be relevant for the\ndocumentation yet though. How we build the documentation shouldn't be\nbased upon any of those keys ?\n\nBut maybe it helps to have them defined to match the build for\nconsistency somewhere.\n\nEither way,\n\nReviewed-by: Kieran Bingham \n\n\n\n> doxyfile = configure_file(input : 'Doxyfile.in',\n> output : 'Doxyfile',\n> configuration : cdata)\n> -- \n> Regards,\n> \n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 161D5BD16B\n\tfor ;\n\tTue, 9 May 2023 00:29:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 94D3E633B4;\n\tTue, 9 May 2023 02:29:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5176E60544\n\tfor ;\n\tTue, 9 May 2023 02:29:21 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id ECA814DB;\n\tTue, 9 May 2023 02:29:14 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683592163;\n\tbh=3AS20j2GLUpoV3Oe1hRoY1JxsdG07Gx27eFH5cLxLtg=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=dFl05/cnSayLUsO4MOH6a9K0ZNcaanLIWlv+IeXvu8cs4e4LvDyY7CnEpZ7jbEiiL\n\tdWx9xhQmdYSnqqBk57NlQgLPpMPzQSgqR+Et5SpWhzGYsMYE1bhPl0bGx974fjTf8w\n\t0M2dqeRWB52T2U3MtYo+Za0ggET4K+GLhpfoGUhZH49BRQhgGZwsdnnU/l592jP9OY\n\tR5JWAmgxhApuWPLoXAQLWC4iEeSzPKTd06YKpB9u+tY4roHluR/WVIbgNtoQbrbp9g\n\tdH6y5yRommKSlNHtTN20w/0j5IYtFmz7eV+XX2lYsCoLhLPYezA0rzO1Rjt5fTMILq\n\thBqSS+tQD7Hjw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683592155;\n\tbh=3AS20j2GLUpoV3Oe1hRoY1JxsdG07Gx27eFH5cLxLtg=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=sLHyAVwjKP52vFx5JC3/qAkfVlUUw9fEVcSNAxH0YtGVit0hDyu/oRxT+oaun8/FG\n\tV4luRF6uwS9s0RCEGpTmBJIcfXmFZnj+LBsjMfrolbEyVToKdKvhTMT9oxG1ib+RUD\n\tS2VdPU2Ai1bHEBcJ4G7cgXwVUWUxi+pAJ44xhn6c="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"sLHyAVwj\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230506111025.18669-2-laurent.pinchart@ideasonboard.com>","References":"<20230506111025.18669-1-laurent.pinchart@ideasonboard.com>\n\t<20230506111025.18669-2-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 09 May 2023 01:29:18 +0100","Message-ID":"<168359215841.2007737.1464908094811998162@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] Documentation: Add predefined\n\tmacros from config.h to Doxyfile","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27074,"web_url":"https://patchwork.libcamera.org/comment/27074/","msgid":"<20230509035550.GC30543@pendragon.ideasonboard.com>","date":"2023-05-09T03:55:50","subject":"Re: [libcamera-devel] [PATCH v1 1/2] Documentation: Add predefined\n\tmacros from config.h to Doxyfile","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, May 09, 2023 at 01:29:18AM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2023-05-06 12:10:24)\n> > libcamera creates a config.h file with predefine macros, and instructs\n> \n> /predefine/predefined/\n> \n> > the compiler to include it implicitly with the -include argument.\n> > Doxygen has no support for implicit inclusion of headers, but has a\n> > PREDEFINED configuration option for its preprocessor that lists\n> > predefined macros. Populate it with the values from the config_h\n> > configuration data object that is used for generate the config.h file,\n> > to ensure that documentation matches the configuration options libcamera\n> > has been built with.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Documentation/Doxyfile.in | 3 ++-\n> > Documentation/meson.build | 7 +++++++\n> > 2 files changed, 9 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> > index 1447abdb7d8c..697a14d1dfe2 100644\n> > --- a/Documentation/Doxyfile.in\n> > +++ b/Documentation/Doxyfile.in\n> > @@ -80,6 +80,7 @@ INCLUDE_FILE_PATTERNS = *.h\n> > \n> > PREDEFINED = __DOXYGEN__ \\\n> > __cplusplus \\\n> > - __attribute__(x)=\n> > + __attribute__(x)= \\\n> > + @PREDEFINED@\n> > \n> > HAVE_DOT = YES\n> > diff --git a/Documentation/meson.build b/Documentation/meson.build\n> > index 186461561f8d..ed581163345e 100644\n> > --- a/Documentation/meson.build\n> > +++ b/Documentation/meson.build\n> > @@ -16,6 +16,13 @@ if doxygen.found() and dot.found()\n> > cdata.set('TOP_BUILDDIR', meson.project_build_root())\n> > cdata.set('OUTPUT_DIR', meson.current_build_dir())\n> > \n> > + doxygen_predefined = []\n> > + foreach key : config_h.keys()\n> > + doxygen_predefined += '@0@=@1@'.format(key, config_h.get(key))\n> > + endforeach\n> > +\n> > + cdata.set('PREDEFINED', ' '.join(doxygen_predefined))\n> > +\n> \n> Going for a quick test, this generates:\n> \n> PREDEFINED = __DOXYGEN__ \\\n> __cplusplus \\\n> __attribute__(x)= \\\n> HAVE_BACKTRACE=1 HAVE_DW=1 HAVE_GNUTLS=1 HAVE_IPA_PUBKEY=1 HAVE_LIBUDEV=1 HAVE_LOCALE_T=1 HAVE_SECURE_GETENV=1 HAVE_TRACING=1 HAVE_UNWIND=1 IPA_CONFIG_DIR=\"/etc/libcamera/ipa:/usr/share/libcamera/ipa\" IPA_MODULE_DIR=\"/usr/lib/x86_64-linux-gnu/libcamera\" IPA_PROXY_DIR=\"/usr/libexec/libcamera\" LIBCAMERA_DATA_DIR=\"/usr/share/libcamera\" LIBCAMERA_SYSCONF_DIR=\"/etc/libcamera\"\n> \n> Which looks reasonable to me.\n> \n> You could go one step further and match the style of the rest of the\n> file though which makes it easier to parse. Though it's unlikely for\n> someone to be reading this specifically! (Unless they're checking the\n> Doxyfile for parsing issues which maybe more readability would help!?)\n> \n> \n> - cdata.set('PREDEFINED', ' '.join(doxygen_predefined))\n> + cdata.set('PREDEFINED', ' \\\\\\n\\t\\t\\t '.join(doxygen_predefined))\n> \n> \n> Makes a cleaner:\n> \n> PREDEFINED = __DOXYGEN__ \\\n> __cplusplus \\\n> __attribute__(x)= \\\n> HAVE_BACKTRACE=1 \\\n> HAVE_DW=1 \\\n> HAVE_GNUTLS=1 \\\n> HAVE_IPA_PUBKEY=1 \\\n> HAVE_LIBUDEV=1 \\\n> HAVE_LOCALE_T=1 \\\n> HAVE_SECURE_GETENV=1 \\\n> HAVE_TRACING=1 \\\n> HAVE_UNWIND=1 \\\n> IPA_CONFIG_DIR=\"/etc/libcamera/ipa:/usr/share/libcamera/ipa\" \\\n> IPA_MODULE_DIR=\"/usr/lib/x86_64-linux-gnu/libcamera\" \\\n> IPA_PROXY_DIR=\"/usr/libexec/libcamera\" \\\n> LIBCAMERA_DATA_DIR=\"/usr/share/libcamera\" \\\n> LIBCAMERA_SYSCONF_DIR=\"/etc/libcamera\"\n\nI like this :-)\n\n> I'm a bit bemused about how any of those would be relevant for the\n> documentation yet though. How we build the documentation shouldn't be\n> based upon any of those keys ?\n\nI noticed this when developing patch 2/2, doxygen didn't warn me about\nmissing documentation for the new IPAManager::pubKey() function.\n\nFor the public API I fully agree with you, it should make no difference.\nFor the internal API, it's not so clear cut. The documentation we\npublish on the website should cover all the internal API, but I\nunderstand it could be confusing for the developers if some features are\nnot available in the version of libcamera they run.\n\n> But maybe it helps to have them defined to match the build for\n> consistency somewhere.\n> \n> Either way,\n> \n> Reviewed-by: Kieran Bingham \n> \n> > doxyfile = configure_file(input : 'Doxyfile.in',\n> > output : 'Doxyfile',\n> > configuration : cdata)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 644ADBE08B\n\tfor ;\n\tTue, 9 May 2023 03:55:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA8B5633B4;\n\tTue, 9 May 2023 05:55:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0054260493\n\tfor ;\n\tTue, 9 May 2023 05:55:37 +0200 (CEST)","from pendragon.ideasonboard.com (softbank126090219015.bbtec.net\n\t[126.90.219.15])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9AD867CE;\n\tTue, 9 May 2023 05:55:30 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1683604539;\n\tbh=nOK9pC/EQuNUbOz8IaO2ziv9sRqQ+hCk4wE9IXdEHhU=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=dRawRxwtYvqcRvXWDFySbQ9R44c2Ejf1HBksxa6jPgI/KZDoNrwq/oKaOS0HDLIrR\n\tLU62jPMUZvLYG1gVVBiZsM4i94itiMzrjegvXjkSdVecpBzKoDZbvLh6s0isIZIQR5\n\tBbJP22xIp46pHBRDN5APqedRs8JCjGEaZTx1xbIZWenpb6VTOvAjRaZZeYzUfU50kO\n\th/W7wM02AIa5LLlfgKBFDOgoex+W/B6uPGPiQxYt6NbXyxjwncFjCZDg4lGu/rBJ3s\n\tRhdgtSk4nJav/zA5l9JfauQLUn6ooNy6BXmRB56rPZMixw9pv9uNsWXoulFe66iYMw\n\tGOFoqXmRbKaaw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1683604531;\n\tbh=nOK9pC/EQuNUbOz8IaO2ziv9sRqQ+hCk4wE9IXdEHhU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=MllRskMB5z4Z8i05hke8+ZraD19T+Xbift8r1JdV9d7UJWf4XH6JAe19lZEE61VlU\n\tcLlKhvJcPiiCB588DNTjP4jpTYyDDte/+ybXwCwp3xHRDUwFQuomwCW5Xi5gMCAwfC\n\tb5+dCYJngLs/yfReFWDF6KyTpfjC/+4olij6EfwA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"MllRskMB\"; dkim-atps=neutral","Date":"Tue, 9 May 2023 06:55:50 +0300","To":"Kieran Bingham ","Message-ID":"<20230509035550.GC30543@pendragon.ideasonboard.com>","References":"<20230506111025.18669-1-laurent.pinchart@ideasonboard.com>\n\t<20230506111025.18669-2-laurent.pinchart@ideasonboard.com>\n\t<168359215841.2007737.1464908094811998162@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<168359215841.2007737.1464908094811998162@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v1 1/2] Documentation: Add predefined\n\tmacros from config.h to Doxyfile","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]