Patch Detail
Show a patch.
GET /api/1.1/patches/21700/?format=api
{ "id": 21700, "url": "https://patchwork.libcamera.org/api/1.1/patches/21700/?format=api", "web_url": "https://patchwork.libcamera.org/patch/21700/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20241018193246.805-5-laurent.pinchart@ideasonboard.com>", "date": "2024-10-18T19:32:46", "name": "[4/4] utils: checkstyle.py: Centralize dependency handling for checkers", "commit_ref": "7ff6fd9774e5697eebddc28170d8cb36b1ad4c37", "pull_url": null, "state": "accepted", "archived": false, "hash": "7a48e33c7ac256c10a0f3379bdaaab780917ec8d", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/21700/mbox/", "series": [ { "id": 4723, "url": "https://patchwork.libcamera.org/api/1.1/series/4723/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4723", "date": "2024-10-18T19:32:42", "name": "utils: checkstyle.py: Improve dependency handling", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4723/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/21700/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/21700/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "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 5EB9BC3300\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 18 Oct 2024 19:33:06 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9B8C65396;\n\tFri, 18 Oct 2024 21:33:04 +0200 (CEST)", "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 D71646538B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 18 Oct 2024 21:32:57 +0200 (CEST)", "from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DE56C21C;\n\tFri, 18 Oct 2024 21:31:13 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"RDWyhN7F\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729279874;\n\tbh=W6dqA0GHMyI4YC4hIoVftEIx0pRv9/FVT+RZFyaxKAM=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=RDWyhN7F0uim1i/XHeLMFyB0XrfIJzoe3CUEeTe2Hb2KWyEaoJAoaDl8VYRwbmGcH\n\tPKP8EKjlYzebBSAhk6dAMmW5q1TdVZFZNftYEzsP56/CkgDqKE6oldRmUagt+JTUUK\n\tv4X/rccuCI1E2M+Va45PKb7uj5gAapCDxKuhSRgg=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Stefan Klug <stefan.klug@ideasonboard.com>", "Subject": "[PATCH 4/4] utils: checkstyle.py: Centralize dependency handling for\n\tcheckers", "Date": "Fri, 18 Oct 2024 22:32:46 +0300", "Message-ID": "<20241018193246.805-5-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.45.2", "In-Reply-To": "<20241018193246.805-1-laurent.pinchart@ideasonboard.com>", "References": "<20241018193246.805-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The checkstyle.py script depends on external tools. Those dependencies\nare handled in different ways in different parts of the code. Centralize\nthe management of checker-specific dependencies to simplify the checkers\nand output more consistent error messages.\n\nThis fixes a crash in the Pep8Formatter class when the autopep8\ndependency is not found.\n\nFixes: 8ffaf376bb53 (\"utils: checkstyle: Add a python formatter\")\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n utils/checkstyle.py | 81 ++++++++++++++++++++++++++++++---------------\n 1 file changed, 55 insertions(+), 26 deletions(-)", "diff": "diff --git a/utils/checkstyle.py b/utils/checkstyle.py\nindex 3f841a54d54a..f6229bbd86ce 100755\n--- a/utils/checkstyle.py\n+++ b/utils/checkstyle.py\n@@ -23,7 +23,6 @@ import subprocess\n import sys\n \n dependencies = {\n- 'clang-format': True,\n 'git': True,\n }\n \n@@ -346,9 +345,6 @@ class ClassRegistry(type):\n \n \n class CheckerBase(metaclass=ClassRegistry):\n- #\n- # Class methods\n- #\n @classmethod\n def instances(cls, obj, names):\n for instance in cls.subclasses:\n@@ -378,6 +374,22 @@ class CheckerBase(metaclass=ClassRegistry):\n \n return patterns\n \n+ @classmethod\n+ def check_dependencies(cls):\n+ if not hasattr(cls, 'dependencies'):\n+ return []\n+\n+ issues = []\n+\n+ for command in cls.dependencies:\n+ if command not in dependencies:\n+ dependencies[command] = shutil.which(command)\n+\n+ if not dependencies[command]:\n+ issues.append(CommitIssue(f'Missing {command} to run {cls.__name__}'))\n+\n+ return issues\n+\n \n # ------------------------------------------------------------------------------\n # Commit Checkers\n@@ -710,6 +722,7 @@ class MesonChecker(StyleChecker):\n \n \n class ShellChecker(StyleChecker):\n+ dependencies = ('shellcheck',)\n patterns = ('*.sh',)\n results_line_regex = re.compile(r'In - line ([0-9]+):')\n \n@@ -718,12 +731,8 @@ class ShellChecker(StyleChecker):\n issues = []\n data = ''.join(content).encode('utf-8')\n \n- try:\n- ret = subprocess.run(['shellcheck', '-Cnever', '-'],\n- input=data, stdout=subprocess.PIPE)\n- except FileNotFoundError:\n- issues.append(StyleIssue(0, None, None, 'Please install shellcheck to validate shell script additions'))\n- return issues\n+ ret = subprocess.run(['shellcheck', '-Cnever', '-'],\n+ input=data, stdout=subprocess.PIPE)\n \n results = ret.stdout.decode('utf-8').splitlines()\n for nr, item in enumerate(results):\n@@ -750,6 +759,7 @@ class Formatter(CheckerBase):\n \n \n class CLangFormatter(Formatter):\n+ dependencies = ('clang-format',)\n patterns = ('*.c', '*.cpp', '*.h')\n priority = -1\n \n@@ -879,17 +889,13 @@ class IncludeOrderFormatter(Formatter):\n \n \n class Pep8Formatter(Formatter):\n+ dependencies = ('autopep8',)\n patterns = ('*.py',)\n \n @classmethod\n def format(cls, filename, data):\n- try:\n- ret = subprocess.run(['autopep8', '--ignore=E501', '-'],\n- input=data.encode('utf-8'), stdout=subprocess.PIPE)\n- except FileNotFoundError:\n- issues.append(StyleIssue(0, None, None, 'Please install autopep8 to format python additions'))\n- return issues\n-\n+ ret = subprocess.run(['autopep8', '--ignore=E501', '-'],\n+ input=data.encode('utf-8'), stdout=subprocess.PIPE)\n return ret.stdout.decode('utf-8')\n \n \n@@ -908,6 +914,24 @@ class StripTrailingSpaceFormatter(Formatter):\n # Style checking\n #\n \n+def check_commit(top_level, commit, checkers):\n+ issues = []\n+\n+ # Apply the commit checkers first.\n+ for checker in CommitChecker.instances(commit, checkers):\n+ issues_ = checker.check_dependencies()\n+ if issues_:\n+ issues += issues_\n+ continue\n+\n+ issues += checker.check(commit, top_level)\n+\n+ for issue in issues:\n+ print(issue)\n+\n+ return len(issues)\n+\n+\n def check_file(top_level, commit, filename, checkers):\n # Extract the line numbers touched by the commit.\n commit_diff = commit.get_diff(top_level, filename)\n@@ -923,9 +947,15 @@ def check_file(top_level, commit, filename, checkers):\n # Format the file after the commit with all formatters and compute the diff\n # between the unformatted and formatted contents.\n after = commit.get_file(filename)\n+ issues = []\n \n formatted = after\n for formatter in Formatter.instances(filename, checkers):\n+ issues_ = formatter.check_dependencies()\n+ if issues_:\n+ issues += issues_\n+ continue\n+\n formatted = formatter.format(filename, formatted)\n \n after = after.splitlines(True)\n@@ -938,8 +968,12 @@ def check_file(top_level, commit, filename, checkers):\n formatted_diff = [hunk for hunk in formatted_diff if hunk.intersects(lines)]\n \n # Check for code issues not related to formatting.\n- issues = []\n for checker in StyleChecker.instances(filename, checkers):\n+ issues_ = checker.check_dependencies()\n+ if issues_:\n+ issues += issues_\n+ continue\n+\n for hunk in commit_diff:\n issues += checker.check(after, hunk.side('to').touched)\n \n@@ -955,7 +989,7 @@ def check_file(top_level, commit, filename, checkers):\n print(hunk)\n \n if len(issues):\n- issues = sorted(issues, key=lambda i: i.line_number)\n+ issues = sorted(issues, key=lambda i: getattr(i, 'line_number', -1))\n for issue in issues:\n print(issue)\n \n@@ -969,13 +1003,8 @@ def check_style(top_level, commit, checkers):\n print(title)\n print(separator)\n \n- issues = 0\n-\n # Apply the commit checkers first.\n- for checker in CommitChecker.instances(commit, checkers):\n- for issue in checker.check(commit, top_level):\n- print(issue)\n- issues += 1\n+ issues = check_commit(top_level, commit, checkers)\n \n # Filter out files we have no checker for.\n patterns = set()\n@@ -1047,7 +1076,7 @@ def main(argv):\n if args.checkers:\n args.checkers = args.checkers.split(',')\n \n- # Check for required dependencies.\n+ # Check for required common dependencies.\n for command, mandatory in dependencies.items():\n found = shutil.which(command)\n if mandatory and not found:\n", "prefixes": [ "4/4" ] }