[{"id":27185,"web_url":"https://patchwork.libcamera.org/comment/27185/","msgid":"<20230530173048.GF22516@pendragon.ideasonboard.com>","date":"2023-05-30T17:30:48","subject":"Re: [libcamera-devel] [PATCH 3/2] libcamera: deprecated: Add new\n\tdeprecated header","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Tue, May 23, 2023 at 12:19:48PM +0100, Kieran Bingham via libcamera-devel wrote:\n> Provide a new header that will be included when applications\n> use to bring in additional context and warnings\n> to generate when the libcamera API is modified where possible.\n> \n> Introduce a deprecated implemenation of StreamRoles to report\n\ns/implemenation/implementation/\n\n> that StreamRoles is no longer a libcamera definition.\n> \n> This allows the compiler to report the following when the header is\n> available:\n> \n> ../../src/apps/qcam/main_window.cpp: In member function ‘int MainWindow::startCapture()’:\n> ../../src/apps/qcam/main_window.cpp:366:21: error: ‘using StreamRoles = class std::vector’ is deprecated: Use a span, array or vector directly [-Werror=deprecated-declarations]\n> 366 | StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);\n> | ^~~~~\n> In file included from include/libcamera/libcamera.h:16,\n> from ../../src/apps/qcam/main_window.cpp:14:\n> ../../include/libcamera/deprecated.h:21:7: note: declared here\n> 21 | using StreamRoles [[deprecated(\"Use a span, array or vector directly\")]]\n> | ^~~~~~~~~~~\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> \n> This is a proposal to support reporting deprecated functionality to\n> applications at compile time.\n\nThis is an interesting indeed, I quite like it. I have a few comments\nthough (which I'm sure doesn't surprise you :-)).\n\nFirst, I don't think we will be able to easily move all deprecated\nfeatures to a separate header. It's easy in this case, but it won't work\nfor a class member for instance. Maybe a macro that simplifies usage of\nthe [[deprecated]] attribute would be an alternative, especially if that\nmacro could take the version number in which the feature will be dropped\nto enforce proper scheduling of feature deprecation.\n\nThen, I wonder if application developers may complain, as a deprecation\nwarning would break builds that use -Werror. This could be considered as\nbreaking applications. Of course we could argue that deprecation\nwarnings should be treated as warnings by our users, not as errors, but\nwe can't enforce or control that. I'm not thinking here about the short\nterm, as breaking application build due to a deprecation warning isn't\nworse than breaking it due to removal of the feature, but once we'll\nguarantee ABI stability I expect us to accumulate deprecated features\nover the lifetime of a major release. Maybe making the deprecation\nhelper macro I mentioned above conditional upon a type of build (debug\nvs. release) or a configuration parameter could solve this problem. We\ncould also check how this is handled by other projects.\n\nFinally, do we want to introduce deprecation warnings already ? I expect\nlots of ABI changes, and this may slow us down.\n\n> include/libcamera/deprecated.h | 24 ++++++++++++++++++++++++\n> include/libcamera/meson.build | 1 +\n> 2 files changed, 25 insertions(+)\n> create mode 100644 include/libcamera/deprecated.h\n> \n> diff --git a/include/libcamera/deprecated.h b/include/libcamera/deprecated.h\n> new file mode 100644\n> index 000000000000..ba48d5a142dc\n> --- /dev/null\n> +++ b/include/libcamera/deprecated.h\n> @@ -0,0 +1,24 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2023, Ideas on Board Oy.\n> + *\n> + * deprecated.h - Deprecated API reporting\n> + *\n> + * Deprecated features of the API will be exposed here for at least one release\n> + * iteration to ease reporting of API adjustments for applications.\n> + */\n> +\n> +#pragma once\n> +\n> +namespace libcamera {\n> +\n> +/*\n> + * Deprectated following v0.0.5\n\ns/Deprectated/Deprecated/\n\n> + *\n> + * The use of StreamRoles indicates applications to use dynamic allocations\n> + * of the StreamRole when this is not always required.\n> + */\n> +using StreamRoles [[deprecated(\"Use a span, array or vector directly\")]]\n> +\t= std::vector;\n> +\n> +} /* namespace libcamera */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 408b7acf152c..2f470d2ac61e 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -7,6 +7,7 @@ libcamera_public_headers = files([\n> 'camera_manager.h',\n> 'color_space.h',\n> 'controls.h',\n> + 'deprecated.h',\n> 'fence.h',\n> 'framebuffer.h',\n> 'framebuffer_allocator.h',","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 1E09AC31E9\n\tfor ;\n\tTue, 30 May 2023 17:30:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91CB3626F8;\n\tTue, 30 May 2023 19:30:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1155960595\n\tfor ;\n\tTue, 30 May 2023 19:30:49 +0200 (CEST)","from pendragon.ideasonboard.com (om126205198071.34.openmobile.ne.jp\n\t[126.205.198.71])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4E5457EC;\n\tTue, 30 May 2023 19:30:27 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685467850;\n\tbh=rQrRCLe0Wte5HevBCDlXsqsCc2x7hiax6T8VrBt6TOs=;\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=eFZPv+KmjSwaN1kP9jmcHDMP/eLo0X+YqKi74lCCvp4CkcPDgvP9PK0wSDK10YRxE\n\tEjugzvLqIJWkua07DZlbP5IX9DmYC1RdAe/g7P0K9q8/quXjWRzzdSJSXqmi8CbkVn\n\trhArTTK4+oYDGqKa4aqaWR4kQFj88rrtyfPCyI0hMiBOiRrU9bsuRCvkfGxfwFJaey\n\tPV679bEBLwmMn+zD+J9HiXlZa3IrqQCYTaoqNhPWNL/G1Hox5vAuVonA4/BHRE8KzC\n\tEgkusAhaJtPWLqAsywv3o35YBwMgSDZ8+MBxw+NR0Db8Un65o3No67WuerdhB7UTj6\n\tawOVuhATSzkFw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685467828;\n\tbh=rQrRCLe0Wte5HevBCDlXsqsCc2x7hiax6T8VrBt6TOs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aOFyWW7lQUGsFyyYQt6MqcjzFqVMYELDnkmq5ORMSgLGm4Pd6Y21dwWY9041Xd3y+\n\tXbK+Qrnj7Zv/2YuY+9EAsnNuZm5lA/s2DZyFkv25r24z6dKf73mTiz1A7p/cemAmzt\n\t9/ck/2eJq3RR158I1ptBhuI53+emXIzLjMgATdug="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"aOFyWW7l\"; dkim-atps=neutral","Date":"Tue, 30 May 2023 20:30:48 +0300","To":"Kieran Bingham ","Message-ID":"<20230530173048.GF22516@pendragon.ideasonboard.com>","References":"<20230509231542.1123025-1-pobrn@protonmail.com>\n\t<20230523111948.2832224-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20230523111948.2832224-1-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/2] libcamera: deprecated: Add new\n\tdeprecated header","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]