[{"id":14335,"web_url":"https://patchwork.libcamera.org/comment/14335/","msgid":"","date":"2020-12-27T10:11:16","subject":"Re: [libcamera-devel] [PATCH v2] utils: checkstyle.py: Drop astyle\n\tsupport","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your cleanup work.\n\nOn 2020-12-24 15:52:30 +0200, Laurent Pinchart wrote:\n> Formatting code using astyle doesn't lead to results as good as with\n> clang-format, and doesn't receive much test coverage as most developers\n> use clang-format. The code is thus bitrotting. Drop it.\n> \n> Signed-off-by: Laurent Pinchart \n\nI like it, if it's not used drop it :-)\n\nReviewed-by: Niklas Söderlund \n\n> ---\n> Documentation/coding-style.rst | 32 ++++----------------\n> utils/checkstyle.py | 54 ++--------------------------------\n> 2 files changed, 8 insertions(+), 78 deletions(-)\n> \n> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n> index 71d5c0b2e842..30c1778ed8bf 100644\n> --- a/Documentation/coding-style.rst\n> +++ b/Documentation/coding-style.rst\n> @@ -283,28 +283,12 @@ The 'clang-format' code formatting tool can be used to reformat source files\n> with the libcamera coding style, defined in the .clang-format file at the root\n> of the source tree.\n> \n> -Alternatively the 'astyle' tool can also be used, with the following arguments.\n> -\n> -::\n> -\n> - --style=linux\n> - --indent=force-tab=8\n> - --attach-namespaces\n> - --attach-extern-c\n> - --pad-oper\n> - --align-pointer=name\n> - --align-reference=name\n> - --max-code-length=120\n> -\n> -Use of astyle is discouraged as clang-format better matches the libcamera coding\n> -style.\n> -\n> -As both astyle and clang-format are code formatters, they operate on full files\n> -and output reformatted source code. While they can be used to reformat code\n> -before sending patches, it may generate unrelated changes. To avoid this,\n> -libcamera provides a 'checkstyle.py' script wrapping the formatting tools to\n> -only retain related changes. This should be used to validate modifications\n> -before submitting them for review.\n> +As clang-format is a code formatter, it operates on full files and outputs\n> +reformatted source code. While it can be used to reformat code before sending\n> +patches, it may generate unrelated changes. To avoid this, libcamera provides a\n> +'checkstyle.py' script wrapping the formatting tools to only retain related\n> +changes. This should be used to validate modifications before submitting them\n> +for review.\n> \n> The script operates on one or multiple git commits specified on the command\n> line. It does not modify the git tree, the index or the working directory and\n> @@ -393,8 +377,4 @@ diff that fixes the issues, on top of the corresponding commit. As the script is\n> in early development false positive are expected. The flagged issues should be\n> reviewed, but the diff doesn't need to be applied blindly.\n> \n> -The checkstyle.py script uses clang-format by default if found, and otherwise\n> -falls back to astyle. The formatter can be manually selected with the\n> -'--formatter' argument.\n> -\n> Happy hacking, libcamera awaits your patches!\n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index e618db937c2b..0e9659e98518 100755\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -4,7 +4,7 @@\n> #\n> # Author: Laurent Pinchart \n> #\n> -# checkstyle.py - A patch style checker script based on astyle or clang-format\n> +# checkstyle.py - A patch style checker script based on clang-format\n> #\n> # TODO:\n> #\n> @@ -22,22 +22,8 @@ import shutil\n> import subprocess\n> import sys\n> \n> -astyle_options = (\n> - '-n',\n> - '--style=linux',\n> - '--indent=force-tab=8',\n> - '--attach-namespaces',\n> - '--attach-extern-c',\n> - '--pad-oper',\n> - '--align-pointer=name',\n> - '--align-reference=name',\n> - '--keep-one-line-blocks',\n> - '--max-code-length=120'\n> -)\n> -\n> dependencies = {\n> - 'astyle': False,\n> - 'clang-format': False,\n> + 'clang-format': True,\n> 'git': True,\n> }\n> \n> @@ -550,7 +536,6 @@ class ShellChecker(StyleChecker):\n> #\n> \n> class Formatter(metaclass=ClassRegistry):\n> - enabled = True\n> subclasses = []\n> \n> def __init__(self):\n> @@ -562,15 +547,11 @@ class Formatter(metaclass=ClassRegistry):\n> @classmethod\n> def formatters(cls, filename):\n> for formatter in cls.subclasses:\n> - if not cls.enabled:\n> - continue\n> if formatter.supports(filename):\n> yield formatter\n> \n> @classmethod\n> def supports(cls, filename):\n> - if not cls.enabled:\n> - return False\n> for pattern in cls.patterns:\n> if fnmatch.fnmatch(os.path.basename(filename), pattern):\n> return True\n> @@ -580,26 +561,12 @@ class Formatter(metaclass=ClassRegistry):\n> def all_patterns(cls):\n> patterns = set()\n> for formatter in cls.subclasses:\n> - if not cls.enabled:\n> - continue\n> patterns.update(formatter.patterns)\n> \n> return patterns\n> \n> \n> -class AStyleFormatter(Formatter):\n> - enabled = False\n> - patterns = ('*.c', '*.cpp', '*.h')\n> -\n> - @classmethod\n> - def format(cls, filename, data):\n> - ret = subprocess.run(['astyle', *astyle_options],\n> - input=data.encode('utf-8'), stdout=subprocess.PIPE)\n> - return ret.stdout.decode('utf-8')\n> -\n> -\n> class CLangFormatter(Formatter):\n> - enabled = False\n> patterns = ('*.c', '*.cpp', '*.h')\n> \n> @classmethod\n> @@ -854,8 +821,6 @@ def main(argv):\n> \n> # Parse command line arguments\n> parser = argparse.ArgumentParser()\n> - parser.add_argument('--formatter', '-f', type=str, choices=['astyle', 'clang-format'],\n> - help='Code formatter. Default to clang-format if not specified.')\n> parser.add_argument('--staged', '-s', action='store_true',\n> help='Include the changes in the index. Defaults to False')\n> parser.add_argument('--amend', '-a', action='store_true',\n> @@ -873,21 +838,6 @@ def main(argv):\n> \n> dependencies[command] = found\n> \n> - if args.formatter:\n> - if not args.formatter in dependencies or \\\n> - not dependencies[args.formatter]:\n> - print(\"Formatter %s not available\" % args.formatter)\n> - return 1\n> - formatter = args.formatter\n> - else:\n> - if dependencies['clang-format']:\n> - CLangFormatter.enabled = True\n> - elif dependencies['astyle']:\n> - AStyleFormatter.enabled = True\n> - else:\n> - print(\"No formatter found, please install clang-format or astyle\")\n> - return 1\n> -\n> # Get the top level directory to pass absolute file names to git diff\n> # commands, in order to support execution from subdirectories of the git\n> # tree.\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 4BB2FC0F1B\n\tfor ;\n\tSun, 27 Dec 2020 10:11:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1A6C260525;\n\tSun, 27 Dec 2020 11:11:23 +0100 (CET)","from mail-lf1-x144.google.com (mail-lf1-x144.google.com\n\t[IPv6:2a00:1450:4864:20::144])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 037D36031F\n\tfor ;\n\tSun, 27 Dec 2020 11:11:22 +0100 (CET)","by mail-lf1-x144.google.com with SMTP id l11so17975467lfg.0\n\tfor ;\n\tSun, 27 Dec 2020 02:11:21 -0800 (PST)","from localhost ([185.224.57.161]) by smtp.gmail.com with ESMTPSA id\n\tc1sm6023962ljd.117.2020.12.27.02.11.20\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSun, 27 Dec 2020 02:11:20 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"yZ3nV9tD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=mpQ0+0ktwhpwc6c6F2NDKVyoeJPUyIdjERln7KpYtUU=;\n\tb=yZ3nV9tDX3K/W7uvgPavJ1+xnkgAymucHH0pZTKuxVlXd+eQC2c39q80eAd0DX72l6\n\tUIeg3j5hMCRiziWvyFGqEeoAULY67E4zFGxDW+e1iiuBBxieUcXjhUHTPmA1swPWJM6n\n\tX2LwyYY1I6ESlqTzyegaUvwxcMVz846vgbhAptIAYZZnF+VplxXF7y+JJ8BkL81yA90R\n\tRyfY2Xy0lbZhZViEtjvHZlLvV6C+9bc001Wz/11J/6o2lmwrnuqodL7vgi4eHD5lQQ4k\n\tUsUOeqyqrL9vjPcegdHOvJTSJVPxKesp1joXLq6YflA35wTAuQgSOLDF9C6tB7q7Gizu\n\tBQGg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=mpQ0+0ktwhpwc6c6F2NDKVyoeJPUyIdjERln7KpYtUU=;\n\tb=O++ZJzDI0svB+6VdnnyYJg4LyZ16481KanDPQGyTXT9grJhf+3Y9tLoyBez5+FX7oD\n\tJVrez8dGhGvdnSvdae+Tbh8gn/M2Nf3v7ROQjp9Sc4wAIuAdwWXyt+pWGtmCBQ93mZmz\n\tYMcG6XLAIx6B/HCaD9AKlPD1k6m5eKiRTNusNhgT8MR8fVgjIp8s2aFZa9N67BN2CPwm\n\t6QkrzycFK/ewo5cfxJWjCIXWj9ZUZ2dZA7a+kog++bfVaZ9g0T70PjTU0FL+B8dH/Blm\n\tXFLuxDJw26RcQ5E0pbAoSlvGRX14rWbknO4idT+NeM1Vr1lG6MX7jhUVd63Q+038Axth\n\twSHA==","X-Gm-Message-State":"AOAM530AjrLsRmNn6r+n2RnB+iB7p6cqGs2ULT4Z13LWMyc2syZ9TZIj\n\tMp8TsOnldJBU3xK4vNt+zfRuGA==","X-Google-Smtp-Source":"ABdhPJzDJwS5QyLA5zLYe1gicKbcKoLFYvsR21y7GC7RsFYRliBgnJvSsjW6K6ZfYsASnn1cytUgRQ==","X-Received":"by 2002:a2e:584:: with SMTP id\n\t126mr18805042ljf.485.1609063881439; \n\tSun, 27 Dec 2020 02:11:21 -0800 (PST)","Date":"Sun, 27 Dec 2020 11:11:16 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20201224135230.24392-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201224135230.24392-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] utils: checkstyle.py: Drop astyle\n\tsupport","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14365,"web_url":"https://patchwork.libcamera.org/comment/14365/","msgid":"<20201228052038.GD1933@pyrite.rasen.tech>","date":"2020-12-28T05:20:38","subject":"Re: [libcamera-devel] [PATCH v2] utils: checkstyle.py: Drop astyle\n\tsupport","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Laurent,\n\nOn Thu, Dec 24, 2020 at 03:52:30PM +0200, Laurent Pinchart wrote:\n> Formatting code using astyle doesn't lead to results as good as with\n> clang-format, and doesn't receive much test coverage as most developers\n> use clang-format. The code is thus bitrotting. Drop it.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> Documentation/coding-style.rst | 32 ++++----------------\n> utils/checkstyle.py | 54 ++--------------------------------\n> 2 files changed, 8 insertions(+), 78 deletions(-)\n> \n> diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst\n> index 71d5c0b2e842..30c1778ed8bf 100644\n> --- a/Documentation/coding-style.rst\n> +++ b/Documentation/coding-style.rst\n> @@ -283,28 +283,12 @@ The 'clang-format' code formatting tool can be used to reformat source files\n> with the libcamera coding style, defined in the .clang-format file at the root\n> of the source tree.\n> \n> -Alternatively the 'astyle' tool can also be used, with the following arguments.\n> -\n> -::\n> -\n> - --style=linux\n> - --indent=force-tab=8\n> - --attach-namespaces\n> - --attach-extern-c\n> - --pad-oper\n> - --align-pointer=name\n> - --align-reference=name\n> - --max-code-length=120\n> -\n> -Use of astyle is discouraged as clang-format better matches the libcamera coding\n> -style.\n> -\n> -As both astyle and clang-format are code formatters, they operate on full files\n> -and output reformatted source code. While they can be used to reformat code\n> -before sending patches, it may generate unrelated changes. To avoid this,\n> -libcamera provides a 'checkstyle.py' script wrapping the formatting tools to\n> -only retain related changes. This should be used to validate modifications\n> -before submitting them for review.\n> +As clang-format is a code formatter, it operates on full files and outputs\n> +reformatted source code. While it can be used to reformat code before sending\n> +patches, it may generate unrelated changes. To avoid this, libcamera provides a\n> +'checkstyle.py' script wrapping the formatting tools to only retain related\n> +changes. This should be used to validate modifications before submitting them\n> +for review.\n> \n> The script operates on one or multiple git commits specified on the command\n> line. It does not modify the git tree, the index or the working directory and\n> @@ -393,8 +377,4 @@ diff that fixes the issues, on top of the corresponding commit. As the script is\n> in early development false positive are expected. The flagged issues should be\n> reviewed, but the diff doesn't need to be applied blindly.\n> \n> -The checkstyle.py script uses clang-format by default if found, and otherwise\n> -falls back to astyle. The formatter can be manually selected with the\n> -'--formatter' argument.\n> -\n> Happy hacking, libcamera awaits your patches!\n> diff --git a/utils/checkstyle.py b/utils/checkstyle.py\n> index e618db937c2b..0e9659e98518 100755\n> --- a/utils/checkstyle.py\n> +++ b/utils/checkstyle.py\n> @@ -4,7 +4,7 @@\n> #\n> # Author: Laurent Pinchart \n> #\n> -# checkstyle.py - A patch style checker script based on astyle or clang-format\n> +# checkstyle.py - A patch style checker script based on clang-format\n> #\n> # TODO:\n> #\n> @@ -22,22 +22,8 @@ import shutil\n> import subprocess\n> import sys\n> \n> -astyle_options = (\n> - '-n',\n> - '--style=linux',\n> - '--indent=force-tab=8',\n> - '--attach-namespaces',\n> - '--attach-extern-c',\n> - '--pad-oper',\n> - '--align-pointer=name',\n> - '--align-reference=name',\n> - '--keep-one-line-blocks',\n> - '--max-code-length=120'\n> -)\n> -\n> dependencies = {\n> - 'astyle': False,\n> - 'clang-format': False,\n> + 'clang-format': True,\n> 'git': True,\n> }\n> \n> @@ -550,7 +536,6 @@ class ShellChecker(StyleChecker):\n> #\n> \n> class Formatter(metaclass=ClassRegistry):\n> - enabled = True\n> subclasses = []\n> \n> def __init__(self):\n> @@ -562,15 +547,11 @@ class Formatter(metaclass=ClassRegistry):\n> @classmethod\n> def formatters(cls, filename):\n> for formatter in cls.subclasses:\n> - if not cls.enabled:\n> - continue\n> if formatter.supports(filename):\n> yield formatter\n> \n> @classmethod\n> def supports(cls, filename):\n> - if not cls.enabled:\n> - return False\n> for pattern in cls.patterns:\n> if fnmatch.fnmatch(os.path.basename(filename), pattern):\n> return True\n> @@ -580,26 +561,12 @@ class Formatter(metaclass=ClassRegistry):\n> def all_patterns(cls):\n> patterns = set()\n> for formatter in cls.subclasses:\n> - if not cls.enabled:\n> - continue\n> patterns.update(formatter.patterns)\n> \n> return patterns\n> \n> \n> -class AStyleFormatter(Formatter):\n> - enabled = False\n> - patterns = ('*.c', '*.cpp', '*.h')\n> -\n> - @classmethod\n> - def format(cls, filename, data):\n> - ret = subprocess.run(['astyle', *astyle_options],\n> - input=data.encode('utf-8'), stdout=subprocess.PIPE)\n> - return ret.stdout.decode('utf-8')\n> -\n> -\n> class CLangFormatter(Formatter):\n> - enabled = False\n> patterns = ('*.c', '*.cpp', '*.h')\n> \n> @classmethod\n> @@ -854,8 +821,6 @@ def main(argv):\n> \n> # Parse command line arguments\n> parser = argparse.ArgumentParser()\n> - parser.add_argument('--formatter', '-f', type=str, choices=['astyle', 'clang-format'],\n> - help='Code formatter. Default to clang-format if not specified.')\n> parser.add_argument('--staged', '-s', action='store_true',\n> help='Include the changes in the index. Defaults to False')\n> parser.add_argument('--amend', '-a', action='store_true',\n> @@ -873,21 +838,6 @@ def main(argv):\n> \n> dependencies[command] = found\n> \n> - if args.formatter:\n> - if not args.formatter in dependencies or \\\n> - not dependencies[args.formatter]:\n> - print(\"Formatter %s not available\" % args.formatter)\n> - return 1\n> - formatter = args.formatter\n> - else:\n> - if dependencies['clang-format']:\n> - CLangFormatter.enabled = True\n> - elif dependencies['astyle']:\n> - AStyleFormatter.enabled = True\n> - else:\n> - print(\"No formatter found, please install clang-format or astyle\")\n> - return 1\n> -\n> # Get the top level directory to pass absolute file names to git diff\n> # commands, in order to support execution from subdirectories of the git\n> # tree.\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 C1AE2C0F1B\n\tfor ;\n\tMon, 28 Dec 2020 05:20:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 503AC6159A;\n\tMon, 28 Dec 2020 06:20:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 53EDB6031B\n\tfor ;\n\tMon, 28 Dec 2020 06:20:46 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AF9E925C;\n\tMon, 28 Dec 2020 06:20:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Lsek+OT+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1609132845;\n\tbh=7yW2QUGDXX+3CXr0+c+c+1ZLZJBlxZjtRsq5V4JsvH0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Lsek+OT+Y708xkh1XsOBEJqgU8Hr12ccA4LrR/btTOFWKpR8dfs23Ac57HioYwMWb\n\tevWMjuCKMLU7s/tQ5Ryif17EaNwh7sR8y7UaH7stD38KwkcrhICIezQU8yZ89qPz6t\n\t8RX6vowlM+vjKqOJBd7XAY+kmcAyflsHDgSRhyP8=","Date":"Mon, 28 Dec 2020 14:20:38 +0900","From":"paul.elder@ideasonboard.com","To":"Laurent Pinchart ","Message-ID":"<20201228052038.GD1933@pyrite.rasen.tech>","References":"<20201224135230.24392-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201224135230.24392-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] utils: checkstyle.py: Drop astyle\n\tsupport","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]