[{"id":28148,"web_url":"https://patchwork.libcamera.org/comment/28148/","msgid":"<20231123095730.GI15697@pendragon.ideasonboard.com>","date":"2023-11-23T09:57:30","subject":"Re: [libcamera-devel] [PATCH v2 2/6] controls: Update argument\n\thandling for controls generation scripts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Tue, Nov 21, 2023 at 02:53:46PM +0000, Naushir Patuck via libcamera-devel wrote:\n> The template file to the gen-controls.py and gen-py-controls.py is now\n> passed in through the '-t' command line argument instead of being a\n> positional argument. This will allow multiple input files to be\n> provided to the scripts in a future commit.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> include/libcamera/meson.build | 7 ++++---\n> src/libcamera/meson.build | 7 ++++---\n> src/py/libcamera/gen-py-controls.py | 2 +-\n> src/py/libcamera/meson.build | 18 ++++++++----------\n> utils/gen-controls.py | 2 +-\n> 5 files changed, 18 insertions(+), 18 deletions(-)\n> \n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 2c8c0258c95e..5fb772e6dd14 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -41,12 +41,13 @@ control_source_files = {\n> control_headers = []\n> \n> foreach header, mode : control_source_files\n> - input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')\n> + input_files = files('../../src/libcamera/' + header +'.yaml')\n> + template_file = files(header + '.h.in')\n> control_headers += custom_target(header + '_h',\n> input : input_files,\n> output : header + '.h',\n> - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> - '--mode', mode],\n> + command : [gen_controls, '-o', '@OUTPUT@',\n> + '--mode', mode, '-t', template_file, '@INPUT@'],\n> install : true,\n> install_dir : libcamera_headers_install_dir)\n> endforeach\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index e49bf850b355..05ee38daf22b 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -128,12 +128,13 @@ endif\n> control_sources = []\n> \n> foreach source, mode : control_source_files\n> - input_files = files(source +'.yaml', source + '.cpp.in')\n> + input_files = files(source +'.yaml')\n> + template_file = files(source + '.cpp.in')\n\nHmmm... I think we have a problem here. The custom_target() input\nargument is used to track dependencies, and cause the target to be\nrebuild if any of the dependencies has changed. Passing the template\nfile through '-t' is fine, but dropping it from the inputs means that\ndependencies won't be tracked properly. You can test this by modifying\nthe template file (without touching anything else) and seeing if all the\nsource files get rebuilt.\n\n> control_sources += custom_target(source + '_cpp',\n> input : input_files,\n> output : source + '.cpp',\n> - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> - '--mode', mode])\n> + command : [gen_controls, '-o', '@OUTPUT@',\n> + '--mode', mode, '-t', template_file, '@INPUT@'])\n> endforeach\n> \n> libcamera_sources += control_sources\n> diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> index 8f14cdafe313..ea28f0139f23 100755\n> --- a/src/py/libcamera/gen-py-controls.py\n> +++ b/src/py/libcamera/gen-py-controls.py\n> @@ -93,7 +93,7 @@ def main(argv):\n> help='Output file name. Defaults to standard output if not specified.')\n> parser.add_argument('input', type=str,\n> help='Input file name.')\n> - parser.add_argument('template', type=str,\n> + parser.add_argument('-t', dest='template', type=str, required=True,\n> help='Template file name.')\n\nWhile at it, could you make the arguments sorted alphabetically ? As we\nhave '--mode', I would make the template argument\n\n parser.add_argument('--template', '-', dest='template', type=str,\n required=True, help='Template file name')\n\nand change '--mode' to '--mode', '-m' for consistency.\n\nA \"while at it\" comment in the commit message is fine with me, unless\nyou insist splitting it in a separate patch :-)\n\n> parser.add_argument('--mode', type=str, required=True,\n> help='Mode is either \"controls\" or \"properties\"')\n> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> index f58c7198ee9e..1c3ea1843ac0 100644\n> --- a/src/py/libcamera/meson.build\n> +++ b/src/py/libcamera/meson.build\n> @@ -28,29 +28,27 @@ pycamera_sources = files([\n> \n> # Generate controls\n> \n> -gen_py_controls_input_files = files([\n> - '../../libcamera/control_ids.yaml',\n> - 'py_controls_generated.cpp.in',\n> -])\n> +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> +gen_py_controls_template = files('py_controls_generated.cpp.in')\n> \n> gen_py_controls = files('gen-py-controls.py')\n> \n> pycamera_sources += custom_target('py_gen_controls',\n> input : gen_py_controls_input_files,\n> output : ['py_controls_generated.cpp'],\n> - command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])\n> + command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@',\n> + '-t', gen_py_controls_template, '@INPUT@'])\n> \n> # Generate properties\n> \n> -gen_py_property_enums_input_files = files([\n> - '../../libcamera/property_ids.yaml',\n> - 'py_properties_generated.cpp.in',\n> -])\n> +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> +gen_py_properties_template = files('py_properties_generated.cpp.in')\n> \n> pycamera_sources += custom_target('py_gen_properties',\n> input : gen_py_property_enums_input_files,\n> output : ['py_properties_generated.cpp'],\n> - command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])\n> + command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@',\n> + '-t', gen_py_properties_template, '@INPUT@'])\n> \n> # Generate formats\n> \n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index 8f2f8fdb02c3..8af33d29cd07 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -361,7 +361,7 @@ def main(argv):\n> help='Output file name. Defaults to standard output if not specified.')\n> parser.add_argument('input', type=str,\n> help='Input file name.')\n> - parser.add_argument('template', type=str,\n> + parser.add_argument('-t', dest='template', type=str, required=True,\n> help='Template file name.')\n\nSame comment as above.\n\n> parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],\n> help='Mode of operation')","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 53634C3220\n\tfor ;\n\tThu, 23 Nov 2023 09:57:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C1EFF629BC;\n\tThu, 23 Nov 2023 10:57:25 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 49B4961DAA\n\tfor ;\n\tThu, 23 Nov 2023 10:57:24 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4CA5FDB7;\n\tThu, 23 Nov 2023 10:56:52 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700733445;\n\tbh=zu+YKG1eLQB61N9kp9wMdhOULCJ790GbuT20VzO3Pa0=;\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=Xogu+NrEyfd0jYC32pJkcDFDKA0Oe8lyMsWSfeS+jyXL7f4EMWwjWrSVtMarG6+dv\n\tKsFCsxMUTrovMN1qQrO3W5YWDce/yoSD5gkzSs2VtjvuvzPhCD1VXd6szDR+Jhua+n\n\tgFaFa0H7J9O9etzwI0kD1300STCstVZcXgKMCXfS+PbLWLuxjaFLXBEpiuVjncUKee\n\tpqe3nyXdTgfGyKDx1ekLQs8XPhIt+p+SfBz38PvANI9jR3562JLl7ENkkS8xt0MmO4\n\tJ6dqpwZtHr6OSiPA69epTEh7kwK+F+AmhvUiiSCtK9NuSx4jE6sQza7Q3LQRufAdK7\n\t4qFgQP6VmdAWw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700733412;\n\tbh=zu+YKG1eLQB61N9kp9wMdhOULCJ790GbuT20VzO3Pa0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QhhwQyxapMqTwpvBaYyY7kwPQqTCKnzspVwyaxArmAhPn9pPSlIW3o7jryIZBtPjQ\n\tkbl/JH9Y/ISuZiTTYUYRymkaybgiOW/RIFQ5YGmc96x3CxLIBeBFGmHtO2BESzXZZM\n\tDsE3mn2bpBXZuVPsLONQMIiHl5bzkmZqMp8UE8t4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"QhhwQyxa\"; dkim-atps=neutral","Date":"Thu, 23 Nov 2023 11:57:30 +0200","To":"Naushir Patuck ","Message-ID":"<20231123095730.GI15697@pendragon.ideasonboard.com>","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231121145350.5956-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/6] controls: Update argument\n\thandling for controls generation scripts","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\" "}},{"id":28159,"web_url":"https://patchwork.libcamera.org/comment/28159/","msgid":"","date":"2023-11-23T13:26:42","subject":"Re: [libcamera-devel] [PATCH v2 2/6] controls: Update argument\n\thandling for controls generation scripts","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Thu, 23 Nov 2023 at 09:57, Laurent Pinchart\n wrote:\n>\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Tue, Nov 21, 2023 at 02:53:46PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > The template file to the gen-controls.py and gen-py-controls.py is now\n> > passed in through the '-t' command line argument instead of being a\n> > positional argument. This will allow multiple input files to be\n> > provided to the scripts in a future commit.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > include/libcamera/meson.build | 7 ++++---\n> > src/libcamera/meson.build | 7 ++++---\n> > src/py/libcamera/gen-py-controls.py | 2 +-\n> > src/py/libcamera/meson.build | 18 ++++++++----------\n> > utils/gen-controls.py | 2 +-\n> > 5 files changed, 18 insertions(+), 18 deletions(-)\n> >\n> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > index 2c8c0258c95e..5fb772e6dd14 100644\n> > --- a/include/libcamera/meson.build\n> > +++ b/include/libcamera/meson.build\n> > @@ -41,12 +41,13 @@ control_source_files = {\n> > control_headers = []\n> >\n> > foreach header, mode : control_source_files\n> > - input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')\n> > + input_files = files('../../src/libcamera/' + header +'.yaml')\n> > + template_file = files(header + '.h.in')\n> > control_headers += custom_target(header + '_h',\n> > input : input_files,\n> > output : header + '.h',\n> > - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> > - '--mode', mode],\n> > + command : [gen_controls, '-o', '@OUTPUT@',\n> > + '--mode', mode, '-t', template_file, '@INPUT@'],\n> > install : true,\n> > install_dir : libcamera_headers_install_dir)\n> > endforeach\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index e49bf850b355..05ee38daf22b 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -128,12 +128,13 @@ endif\n> > control_sources = []\n> >\n> > foreach source, mode : control_source_files\n> > - input_files = files(source +'.yaml', source + '.cpp.in')\n> > + input_files = files(source +'.yaml')\n> > + template_file = files(source + '.cpp.in')\n>\n> Hmmm... I think we have a problem here. The custom_target() input\n> argument is used to track dependencies, and cause the target to be\n> rebuild if any of the dependencies has changed. Passing the template\n> file through '-t' is fine, but dropping it from the inputs means that\n> dependencies won't be tracked properly. You can test this by modifying\n> the template file (without touching anything else) and seeing if all the\n> source files get rebuilt.\n\nThis seems logical, but surprisingly it works! If do a full build,\nmake an edit to any .in template file and rebuild, the output does get\nregenerated. Could it be working because I wrap the template in a\nfile() construct?\n\nRegards,\nNaush\n\n\n>\n> > control_sources += custom_target(source + '_cpp',\n> > input : input_files,\n> > output : source + '.cpp',\n> > - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> > - '--mode', mode])\n> > + command : [gen_controls, '-o', '@OUTPUT@',\n> > + '--mode', mode, '-t', template_file, '@INPUT@'])\n> > endforeach\n> >\n> > libcamera_sources += control_sources\n> > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > index 8f14cdafe313..ea28f0139f23 100755\n> > --- a/src/py/libcamera/gen-py-controls.py\n> > +++ b/src/py/libcamera/gen-py-controls.py\n> > @@ -93,7 +93,7 @@ def main(argv):\n> > help='Output file name. Defaults to standard output if not specified.')\n> > parser.add_argument('input', type=str,\n> > help='Input file name.')\n> > - parser.add_argument('template', type=str,\n> > + parser.add_argument('-t', dest='template', type=str, required=True,\n> > help='Template file name.')\n>\n> While at it, could you make the arguments sorted alphabetically ? As we\n> have '--mode', I would make the template argument\n>\n> parser.add_argument('--template', '-', dest='template', type=str,\n> required=True, help='Template file name')\n>\n> and change '--mode' to '--mode', '-m' for consistency.\n>\n> A \"while at it\" comment in the commit message is fine with me, unless\n> you insist splitting it in a separate patch :-)\n>\n> > parser.add_argument('--mode', type=str, required=True,\n> > help='Mode is either \"controls\" or \"properties\"')\n> > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> > index f58c7198ee9e..1c3ea1843ac0 100644\n> > --- a/src/py/libcamera/meson.build\n> > +++ b/src/py/libcamera/meson.build\n> > @@ -28,29 +28,27 @@ pycamera_sources = files([\n> >\n> > # Generate controls\n> >\n> > -gen_py_controls_input_files = files([\n> > - '../../libcamera/control_ids.yaml',\n> > - 'py_controls_generated.cpp.in',\n> > -])\n> > +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> > +gen_py_controls_template = files('py_controls_generated.cpp.in')\n> >\n> > gen_py_controls = files('gen-py-controls.py')\n> >\n> > pycamera_sources += custom_target('py_gen_controls',\n> > input : gen_py_controls_input_files,\n> > output : ['py_controls_generated.cpp'],\n> > - command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])\n> > + command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@',\n> > + '-t', gen_py_controls_template, '@INPUT@'])\n> >\n> > # Generate properties\n> >\n> > -gen_py_property_enums_input_files = files([\n> > - '../../libcamera/property_ids.yaml',\n> > - 'py_properties_generated.cpp.in',\n> > -])\n> > +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > +gen_py_properties_template = files('py_properties_generated.cpp.in')\n> >\n> > pycamera_sources += custom_target('py_gen_properties',\n> > input : gen_py_property_enums_input_files,\n> > output : ['py_properties_generated.cpp'],\n> > - command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])\n> > + command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@',\n> > + '-t', gen_py_properties_template, '@INPUT@'])\n> >\n> > # Generate formats\n> >\n> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index 8f2f8fdb02c3..8af33d29cd07 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -361,7 +361,7 @@ def main(argv):\n> > help='Output file name. Defaults to standard output if not specified.')\n> > parser.add_argument('input', type=str,\n> > help='Input file name.')\n> > - parser.add_argument('template', type=str,\n> > + parser.add_argument('-t', dest='template', type=str, required=True,\n> > help='Template file name.')\n>\n> Same comment as above.\n>\n> > parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],\n> > help='Mode of operation')\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 F0BE6C3220\n\tfor ;\n\tThu, 23 Nov 2023 13:27:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F78B629AF;\n\tThu, 23 Nov 2023 14:27:11 +0100 (CET)","from mail-yb1-xb35.google.com (mail-yb1-xb35.google.com\n\t[IPv6:2607:f8b0:4864:20::b35])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8C95061DAC\n\tfor ;\n\tThu, 23 Nov 2023 14:27:09 +0100 (CET)","by mail-yb1-xb35.google.com with SMTP id\n\t3f1490d57ef6-d9b9adaf291so867349276.1\n\tfor ;\n\tThu, 23 Nov 2023 05:27:09 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700746031;\n\tbh=rcUxWpM+KZxtl3pf5BSfDaRoFWEk8VkkJTmNlI7DgtM=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=wjgXszxrkZFAQucjdd/+TAAPyZ81Z8ZV6Og6ZooSncKYjDgoUi5adboJvKzm9oJa5\n\ti43v33C32DwO/SEoD1HM+rjpcO6qBCDdjdtXIM6Pt081THb1dn/owfC9sFZ41Giz8d\n\ttqIkeAkZT+4Zppe2qsrCYagwRuk+zaLcAN4UKjWosxGP5G0b967UMe2KfCIGnAuS2j\n\tiwDjCmLWt/kwgymBP+mXuQrfgFR7p8FGOE/SZEwzmy7tbCxSx2Jz5vAn6gLCaGd0Qg\n\t/ixDWETC23COuLm0i+HokmXiurRSlw9719ZxYk+ArwSFtZjdXKcvorliJB6mDNAWzn\n\typiihqsH4I6bQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1700746028; x=1701350828;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=iwNz/9tTEmfET6qs1pkKixfUURx+fG2VgM/iZmP55W8=;\n\tb=kHMgP3bGPOoJrbVXafqLN05kj0dufCQZ3KzupFJUBuBmhlaVQRortTFa4i1sJ1SkiO\n\togXiFw7R0FJjiuPuClQ1hze1wEffR+YSti+CDaekCWUPcD7F2Vz6TRtrNvBrX6OrBYhN\n\t3aj/JBjZ4CtsBgPbhygWHDb54W80FjCPovQqOsxOxmsT33SQIUOk8BMngU+1xjW0uocx\n\tioB9q2b155nT9NuAGbrNZ1rBHEF7AEJ6jBbuhLQZo5hsIsILUeLvTAYQ9FWsuFgK5WGX\n\te3HiRFlnIAXjXWPV81NnQ+QLKz77ekc0C9JPUNwnTuiUUvZUMuUgV8y0FtvXdnJboTJR\n\tooJQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"kHMgP3bG\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1700746028; x=1701350828;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=iwNz/9tTEmfET6qs1pkKixfUURx+fG2VgM/iZmP55W8=;\n\tb=gjITsbfoEkJy1LauzeYAWIOjBWDewhcI1Js3SRE9SlNIug7TV++tpM2HKzG56CSh94\n\tnmOjwDzSdHataAqYkr/fUUvGDabfRZF+MjG3Oie0otJLOQW8WNNYAkBiSCWCHauSOj0Y\n\tLC26jf+QgpQR+6EQXMFATReiGKn0+EuOcyUB4RWDd7PVix8ssfa1UHhTXdykTWA7Thn3\n\toLzM6w2AlYb5LleIFZRQj0RGrKmB18s7I7iIt+iJzzSCTOvDB7OZTy4OOLDhfR6dALTT\n\tz6qlPar+P3i80Dp8lvzdhxqkGdu2lIebIh50e5/aKFoxG4gvvupleI8j899RI15XplQY\n\tNRew==","X-Gm-Message-State":"AOJu0YyKc512jUqWW9DMejeqVWzhimttqshB8N0z3bJwdQ0clPcByypB\n\t8rFrLqYFT38VQH6zFYDzg0EJm/moxiGi9zPvLLCITBkiUQbxtsarYug=","X-Google-Smtp-Source":"AGHT+IFFgN/nNaKPx14KY36hiSS9I8IHNVMSiayV+WcdPZQdSBhj6KZsCX/O/nxrKnq4DKlNc0GTnBi6xo/yE4njYyE=","X-Received":"by 2002:a0d:e501:0:b0:5cb:fedc:2b8d with SMTP id\n\to1-20020a0de501000000b005cbfedc2b8dmr5610927ywe.38.1700746028279;\n\tThu, 23 Nov 2023 05:27:08 -0800 (PST)","MIME-Version":"1.0","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-3-naush@raspberrypi.com>\n\t<20231123095730.GI15697@pendragon.ideasonboard.com>","In-Reply-To":"<20231123095730.GI15697@pendragon.ideasonboard.com>","Date":"Thu, 23 Nov 2023 13:26:42 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v2 2/6] controls: Update argument\n\thandling for controls generation scripts","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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":28164,"web_url":"https://patchwork.libcamera.org/comment/28164/","msgid":"<20231123140828.GH16377@pendragon.ideasonboard.com>","date":"2023-11-23T14:08:28","subject":"Re: [libcamera-devel] [PATCH v2 2/6] controls: Update argument\n\thandling for controls generation scripts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nOn Thu, Nov 23, 2023 at 01:26:42PM +0000, Naushir Patuck wrote:\n> On Thu, 23 Nov 2023 at 09:57, Laurent Pinchart wrote:\n> > On Tue, Nov 21, 2023 at 02:53:46PM +0000, Naushir Patuck via libcamera-devel wrote:\n> > > The template file to the gen-controls.py and gen-py-controls.py is now\n> > > passed in through the '-t' command line argument instead of being a\n> > > positional argument. This will allow multiple input files to be\n> > > provided to the scripts in a future commit.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > include/libcamera/meson.build | 7 ++++---\n> > > src/libcamera/meson.build | 7 ++++---\n> > > src/py/libcamera/gen-py-controls.py | 2 +-\n> > > src/py/libcamera/meson.build | 18 ++++++++----------\n> > > utils/gen-controls.py | 2 +-\n> > > 5 files changed, 18 insertions(+), 18 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> > > index 2c8c0258c95e..5fb772e6dd14 100644\n> > > --- a/include/libcamera/meson.build\n> > > +++ b/include/libcamera/meson.build\n> > > @@ -41,12 +41,13 @@ control_source_files = {\n> > > control_headers = []\n> > >\n> > > foreach header, mode : control_source_files\n> > > - input_files = files('../../src/libcamera/' + header +'.yaml', header + '.h.in')\n> > > + input_files = files('../../src/libcamera/' + header +'.yaml')\n> > > + template_file = files(header + '.h.in')\n> > > control_headers += custom_target(header + '_h',\n> > > input : input_files,\n> > > output : header + '.h',\n> > > - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> > > - '--mode', mode],\n> > > + command : [gen_controls, '-o', '@OUTPUT@',\n> > > + '--mode', mode, '-t', template_file, '@INPUT@'],\n> > > install : true,\n> > > install_dir : libcamera_headers_install_dir)\n> > > endforeach\n> > > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > > index e49bf850b355..05ee38daf22b 100644\n> > > --- a/src/libcamera/meson.build\n> > > +++ b/src/libcamera/meson.build\n> > > @@ -128,12 +128,13 @@ endif\n> > > control_sources = []\n> > >\n> > > foreach source, mode : control_source_files\n> > > - input_files = files(source +'.yaml', source + '.cpp.in')\n> > > + input_files = files(source +'.yaml')\n> > > + template_file = files(source + '.cpp.in')\n> >\n> > Hmmm... I think we have a problem here. The custom_target() input\n> > argument is used to track dependencies, and cause the target to be\n> > rebuild if any of the dependencies has changed. Passing the template\n> > file through '-t' is fine, but dropping it from the inputs means that\n> > dependencies won't be tracked properly. You can test this by modifying\n> > the template file (without touching anything else) and seeing if all the\n> > source files get rebuilt.\n> \n> This seems logical, but surprisingly it works! If do a full build,\n> make an edit to any .in template file and rebuild, the output does get\n> regenerated. Could it be working because I wrap the template in a\n> file() construct?\n\nIndeed, meson seems to handle this automatically. It's magic :-) Please\nignore my comment.\n\n> > > control_sources += custom_target(source + '_cpp',\n> > > input : input_files,\n> > > output : source + '.cpp',\n> > > - command : [gen_controls, '-o', '@OUTPUT@', '@INPUT@',\n> > > - '--mode', mode])\n> > > + command : [gen_controls, '-o', '@OUTPUT@',\n> > > + '--mode', mode, '-t', template_file, '@INPUT@'])\n> > > endforeach\n> > >\n> > > libcamera_sources += control_sources\n> > > diff --git a/src/py/libcamera/gen-py-controls.py b/src/py/libcamera/gen-py-controls.py\n> > > index 8f14cdafe313..ea28f0139f23 100755\n> > > --- a/src/py/libcamera/gen-py-controls.py\n> > > +++ b/src/py/libcamera/gen-py-controls.py\n> > > @@ -93,7 +93,7 @@ def main(argv):\n> > > help='Output file name. Defaults to standard output if not specified.')\n> > > parser.add_argument('input', type=str,\n> > > help='Input file name.')\n> > > - parser.add_argument('template', type=str,\n> > > + parser.add_argument('-t', dest='template', type=str, required=True,\n> > > help='Template file name.')\n> >\n> > While at it, could you make the arguments sorted alphabetically ? As we\n> > have '--mode', I would make the template argument\n> >\n> > parser.add_argument('--template', '-', dest='template', type=str,\n> > required=True, help='Template file name')\n> >\n> > and change '--mode' to '--mode', '-m' for consistency.\n> >\n> > A \"while at it\" comment in the commit message is fine with me, unless\n> > you insist splitting it in a separate patch :-)\n> >\n> > > parser.add_argument('--mode', type=str, required=True,\n> > > help='Mode is either \"controls\" or \"properties\"')\n> > > diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\n> > > index f58c7198ee9e..1c3ea1843ac0 100644\n> > > --- a/src/py/libcamera/meson.build\n> > > +++ b/src/py/libcamera/meson.build\n> > > @@ -28,29 +28,27 @@ pycamera_sources = files([\n> > >\n> > > # Generate controls\n> > >\n> > > -gen_py_controls_input_files = files([\n> > > - '../../libcamera/control_ids.yaml',\n> > > - 'py_controls_generated.cpp.in',\n> > > -])\n> > > +gen_py_controls_input_files = files('../../libcamera/control_ids.yaml')\n> > > +gen_py_controls_template = files('py_controls_generated.cpp.in')\n> > >\n> > > gen_py_controls = files('gen-py-controls.py')\n> > >\n> > > pycamera_sources += custom_target('py_gen_controls',\n> > > input : gen_py_controls_input_files,\n> > > output : ['py_controls_generated.cpp'],\n> > > - command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@', '@INPUT@'])\n> > > + command : [gen_py_controls, '--mode', 'controls', '-o', '@OUTPUT@',\n> > > + '-t', gen_py_controls_template, '@INPUT@'])\n> > >\n> > > # Generate properties\n> > >\n> > > -gen_py_property_enums_input_files = files([\n> > > - '../../libcamera/property_ids.yaml',\n> > > - 'py_properties_generated.cpp.in',\n> > > -])\n> > > +gen_py_property_enums_input_files = files('../../libcamera/property_ids.yaml')\n> > > +gen_py_properties_template = files('py_properties_generated.cpp.in')\n> > >\n> > > pycamera_sources += custom_target('py_gen_properties',\n> > > input : gen_py_property_enums_input_files,\n> > > output : ['py_properties_generated.cpp'],\n> > > - command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@', '@INPUT@'])\n> > > + command : [gen_py_controls, '--mode', 'properties', '-o', '@OUTPUT@',\n> > > + '-t', gen_py_properties_template, '@INPUT@'])\n> > >\n> > > # Generate formats\n> > >\n> > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > index 8f2f8fdb02c3..8af33d29cd07 100755\n> > > --- a/utils/gen-controls.py\n> > > +++ b/utils/gen-controls.py\n> > > @@ -361,7 +361,7 @@ def main(argv):\n> > > help='Output file name. Defaults to standard output if not specified.')\n> > > parser.add_argument('input', type=str,\n> > > help='Input file name.')\n> > > - parser.add_argument('template', type=str,\n> > > + parser.add_argument('-t', dest='template', type=str, required=True,\n> > > help='Template file name.')\n> >\n> > Same comment as above.\n> >\n> > > parser.add_argument('--mode', type=str, required=True, choices=['controls', 'properties'],\n> > > help='Mode of operation')","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 26A21BDE6B\n\tfor ;\n\tThu, 23 Nov 2023 14:08:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7E7B5629AF;\n\tThu, 23 Nov 2023 15:08:23 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BB7D7629AF\n\tfor ;\n\tThu, 23 Nov 2023 15:08:21 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B6E12A06;\n\tThu, 23 Nov 2023 15:07:49 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1700748503;\n\tbh=7M+mvUIBeRg7THrtmjlO9FvfDqdWmbeDavHHjvPQjYU=;\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=mqP4Q0/Sq9hZwTgjy3ocIhrAOqkp1ovz6xq2kkeY88rgCru7WxnbiMQqiFTK5y7A2\n\tIJOuHLr2z0OEcJeBB9pzyAIp6je81TTOs04wjr2Oh2y6LwUsg5KssmVhu2oDHeCsRv\n\tmkshSWHPkF51nU08hnvO7Ac4luIsJYxw+fmi3dlYC93DfLSPT/FQG7XO9ZjbfGJc3y\n\tyGIjGN+r5HRUbzcG9eNDPhGsSSUAnxD0g57Y1JSYUw66xH9vLrk0Wh0X9MJSJ1X58X\n\tIpHpgGDaLJqScE8L1RQpm/df2B1hYW4e+WbEwhd4nELZy3VEhEkLvLvU/m6qG/V5d+\n\tkSvivxs8eTFxw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1700748469;\n\tbh=7M+mvUIBeRg7THrtmjlO9FvfDqdWmbeDavHHjvPQjYU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EmY9edzF535CDoOw/jKqN3gjopQXyIahJ02BWJI0Xt8SPXc9e2n4CmK+Rpuw84528\n\tppIjODEWFbWQJ0ilCrts5oAAt64MtN29A8D2FFe2yoA0t+i2DsjQxWlkhyKm9LlP4F\n\tmHx9aHolxNMHvrKTrvvxS9ZIWzoWYOSDu0lrpocI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EmY9edzF\"; dkim-atps=neutral","Date":"Thu, 23 Nov 2023 16:08:28 +0200","To":"Naushir Patuck ","Message-ID":"<20231123140828.GH16377@pendragon.ideasonboard.com>","References":"<20231121145350.5956-1-naush@raspberrypi.com>\n\t<20231121145350.5956-3-naush@raspberrypi.com>\n\t<20231123095730.GI15697@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 2/6] controls: Update argument\n\thandling for controls generation scripts","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\" "}}]