{"id":18730,"url":"https://patchwork.libcamera.org/api/patches/18730/?format=json","web_url":"https://patchwork.libcamera.org/patch/18730/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","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/people/2/?format=json","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/series/3920/?format=json","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":"","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 ;\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 ;\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 ;\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":"","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"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 \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=' 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"]}