Patch Detail
Show a patch.
GET /api/1.1/patches/18730/?format=api
{ "id": 18730, "url": "https://patchwork.libcamera.org/api/1.1/patches/18730/?format=api", "web_url": "https://patchwork.libcamera.org/patch/18730/", "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": "<20230612224751.4437-5-laurent.pinchart@ideasonboard.com>", "date": "2023-06-12T22:47:51", "name": "[libcamera-devel,v1,4/4] utils: checkstyle: Add trailers checker", "commit_ref": "d06ed87d49ca3d734fd1c2f1409280abb499c625", "pull_url": null, "state": "accepted", "archived": false, "hash": "c3cc59e91618fef97ede34d10e156a735d9c657e", "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/18730/mbox/", "series": [ { "id": 3920, "url": "https://patchwork.libcamera.org/api/1.1/series/3920/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=3920", "date": "2023-06-12T22:47:47", "name": "utils: checkstyle: Add a commit message trailers checker", "version": 1, "mbox": "https://patchwork.libcamera.org/series/3920/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/18730/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/18730/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 A91F1C31E9\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 Jun 2023 22:47:59 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6950C61E50;\n\tTue, 13 Jun 2023 00:47:59 +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 9019461E51\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 13 Jun 2023 00:47:57 +0200 (CEST)", "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 D5915D80\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 13 Jun 2023 00:47:27 +0200 (CEST)" ], "DKIM-Signature": [ "v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686610079;\n\tbh=ndBRtEmIBZGjH91lpJCqt/EeeCr4N7TvaJHIhQx5eL4=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=BCwDPhz7pTkFVaue5iHz9w7pfe8WPmmOL54f3A/M6eD7UsZuDGBYCFI08KrCpAVeS\n\tsA7WKhsALeym8nOGrFGpO3Uvda4DpULbZ/yQ+rvadHwS/hViFGQmq4mbPODIwN2TGx\n\t/faoSP38IanX9NJTZvv3EJ17jd2hk/anRx72DSFN9nZZMkEHqjpoJyIb7Cu9IR0K0V\n\tkc56WiAtjjwzfODOORDjjyhnkp6yjmVviyHZVk8hXWEV6Vf2i5Y4p8A1HGX2Beo+W0\n\tJdw4m2NKOP5XV2v6CpX2OjO2V4O4Ye60HCHLNdkCyJ9LpQPEMywqTGCzlqDKHiAl83\n\tidZv2wL4urKoA==", "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686610048;\n\tbh=ndBRtEmIBZGjH91lpJCqt/EeeCr4N7TvaJHIhQx5eL4=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=exv113ctSynJl/h/mqr08CCJSyPp029GwwlekcjdAQc/voDo8KvUxcTqX1iBmg/ay\n\tMahXo2h11UeJg0SkZMvt0/fxMAKkKJ7x0OyExZzrMqHhPDRZWgw359NYWp7PmMau9h\n\t4yQh27Cs7Jid6UBiPgnuq7dXbBSEX8WY00S8VmvI=" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"exv113ct\"; dkim-atps=neutral", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 13 Jun 2023 01:47:51 +0300", "Message-Id": "<20230612224751.4437-5-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.39.3", "In-Reply-To": "<20230612224751.4437-1-laurent.pinchart@ideasonboard.com>", "References": "<20230612224751.4437-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v1 4/4] utils: checkstyle: Add trailers\n\tchecker", "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>", "From": "Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>", "Reply-To": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The libcamera git history contains numerous examples of incorrect commit\nmessage trailers due to invalid trailer types (e.g. Change-Id), typos\nand other small issues. Those went unnoticed through reviews, which\nshows that an automated checker is required.\n\nAdd a trailers checker to checkstyle.py to catch invalid or malformed\ntrailers, with a set of supported trailers that match libcamera's commit\nmessage practices. New trailer keys can easily be added later as new\nneeds arise.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nThis reports a total of 42 issues through the project's history. 37 of\nthem should not be controversial:\n\n- Trailer keys not valid for libcamera ('Cc', 'Change-Id', 'Inspired-by'\n and 'Reported-on')\n\n- Typos in trailer keys ('Fixed' instead of 'Fixes', 'Reviewed' instead\n of 'Reviewed-by' and 'Signed-off-By' instead of 'Signed-off-by')\n\n- Typos in e-mail address (missing display name, missing space before\n '<' and missing trailing '>')\n\n- Link in 'Fixes' trailer (should be a commit)\n\n- Too short commit ID in 'Fixes' trailer\n\n- Typos in 'Fixes' trailer (extra 'commit' before commit ID, missing\n space or extra ':' after commit ID, missing '\"' around commit subject)\n\nThe five remaining issues may benefit from discussions:\n\n- Invalid trailer key 'Co-developed-by' (one instance). This is a\n trailer key commonly used in the kernel, but git..b both recommend\n Co-authored-by. I'm sure which option would be best, so I haven't\n included either for now.\n\n- Typo in 'Reported-by' trailer for issues reported by Coverity (one\n instance). 'Reported-by' usually has an e-mail address value, but we\n have commonly used 'Coverity CID=<CID>' for issues reported by\n Coverity. I've tentatively added support for this (feedback is\n welcome), and one commit still got flagged as its 'Reported-by'\n trailer has a space instead of an '=' after 'CID'.\n\n- Usage of a github user URL in 'Reported-by' (one instance). Our policy\n is to be able to identify users by name and e-mail address for\n 'Signed-off-by' trailers, and I would prefer covering 'Reported-by'\n trailers too. If someone *really* doesn't want their name included in\n the git log when reporting an issue, we can simply omit the\n 'Reported-by' trailer.\n\n- Usage of URLs in 'Reported-by' to point to buildbot.libcamera.org (two\n instances). This should use a 'Link' trailer instead.\n---\n utils/checkstyle.py | 80 +++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 78 insertions(+), 2 deletions(-)", "diff": "diff --git a/utils/checkstyle.py b/utils/checkstyle.py\nindex e68c874609bc..3558740d389d 100755\n--- a/utils/checkstyle.py\n+++ b/utils/checkstyle.py\n@@ -210,13 +210,23 @@ class Commit:\n \n def _parse(self):\n # Get the commit title and list of files.\n- ret = subprocess.run(['git', 'show', '--format=%s', '--name-status',\n+ ret = subprocess.run(['git', 'show', '--format=%s%n%(trailers:only,unfold)', '--name-status',\n self.commit],\n stdout=subprocess.PIPE).stdout.decode('utf-8')\n lines = ret.splitlines()\n- self._files = [CommitFile(f) for f in lines[1:] if f]\n+\n self._title = lines[0]\n \n+ self._trailers = []\n+ for index in range(1, len(lines)):\n+ line = lines[index]\n+ if not line:\n+ break\n+\n+ self._trailers.append(line)\n+\n+ self._files = [CommitFile(f) for f in lines[index:] if f]\n+\n def files(self, filter='AMR'):\n return [f.filename for f in self._files if f.status in filter]\n \n@@ -224,6 +234,10 @@ class Commit:\n def title(self):\n return self._title\n \n+ @property\n+ def trailers(self):\n+ return self._trailers\n+\n def get_diff(self, top_level, filename):\n diff = subprocess.run(['git', 'diff', '%s~..%s' % (self.commit, self.commit),\n '--', '%s/%s' % (top_level, filename)],\n@@ -424,6 +438,68 @@ class TitleChecker(CommitChecker):\n 'possible candidates are ' + candidates)]\n \n \n+class TrailersChecker(CommitChecker):\n+ commit_regex = re.compile(r'[0-9a-f]{12}[0-9a-f]* \\(\".*\"\\)')\n+\n+ coverity_regex = re.compile(r'Coverity CID=.*')\n+\n+ # Simple e-mail address validator regex, with an additional trailing\n+ # comment. The complexity of a full RFC6531 validator isn't worth the\n+ # additional invalid addresses it would reject.\n+ email_regex = re.compile(r'[^<]+ <[^@>]+@[^>]+>( # .*)?')\n+\n+ link_regex = re.compile(r'https?://.*')\n+\n+ @staticmethod\n+ def validate_reported_by(value):\n+ if TrailersChecker.email_regex.fullmatch(value):\n+ return True\n+ if TrailersChecker.coverity_regex.fullmatch(value):\n+ return True\n+ return False\n+\n+ known_trailers = {\n+ 'Acked-by': email_regex,\n+ 'Bug': link_regex,\n+ 'Fixes': commit_regex,\n+ 'Link': link_regex,\n+ 'Reported-by': validate_reported_by,\n+ 'Reviewed-by': email_regex,\n+ 'Signed-off-by': email_regex,\n+ 'Suggested-by': email_regex,\n+ 'Tested-by': email_regex,\n+ }\n+\n+ trailer_regex = re.compile(r'([A-Z][a-zA-Z-]*)\\s*:\\s*(.*)')\n+\n+ @classmethod\n+ def check(cls, commit, top_level):\n+ issues = []\n+\n+ for trailer in commit.trailers:\n+ match = TrailersChecker.trailer_regex.fullmatch(trailer)\n+ if not match:\n+ raise RuntimeError(f\"Malformed commit trailer '{trailer}'\")\n+\n+ key, value = match.groups()\n+\n+ validator = TrailersChecker.known_trailers.get(key)\n+ if not validator:\n+ issues.append(CommitIssue(f\"Invalid commit trailer key '{key}'\"))\n+ continue\n+\n+ if isinstance(validator, re.Pattern):\n+ valid = bool(validator.fullmatch(value))\n+ else:\n+ valid = validator(value)\n+\n+ if not valid:\n+ issues.append(CommitIssue(f\"Malformed value '{value}' for commit trailer '{key}'\"))\n+ continue\n+\n+ return issues\n+\n+\n # ------------------------------------------------------------------------------\n # Style Checkers\n #\n", "prefixes": [ "libcamera-devel", "v1", "4/4" ] }