{"id":20869,"url":"https://patchwork.libcamera.org/api/1.1/patches/20869/?format=json","web_url":"https://patchwork.libcamera.org/patch/20869/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20240810005840.20841-1-laurent.pinchart@ideasonboard.com>","date":"2024-08-10T00:58:38","name":"[1/3] utils: checkstyle.py: Rework commit message parsing","commit_ref":"eeb435bbda98da9f03d56e23722e4ac741d6107c","pull_url":null,"state":"accepted","archived":false,"hash":"315fcd59a1aa0ecc0a5996c4d0ce17d2360ebc20","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/20869/mbox/","series":[{"id":4508,"url":"https://patchwork.libcamera.org/api/1.1/series/4508/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4508","date":"2024-08-10T00:58:38","name":"[1/3] utils: checkstyle.py: Rework commit message parsing","version":1,"mbox":"https://patchwork.libcamera.org/series/4508/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/20869/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/20869/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 B439ABE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 10 Aug 2024 00:59:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E256633B9;\n\tSat, 10 Aug 2024 02:59:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 80F1D61946\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 10 Aug 2024 02:59:04 +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 ADB044AB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 10 Aug 2024 02:58:09 +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=\"ppG8BifN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723251489;\n\tbh=6uQLveOZaSo57cK3t2/+QvCi4+q5TM0/+upo74l1a6o=;\n\th=From:To:Subject:Date:From;\n\tb=ppG8BifNBPo6qXKQiY2hw7qmw3I7GwUYqokX5GnALCVHk/9rVIKBG8fLrBqtszv2v\n\tgapyZ/RiTUJFyocAi7r1nJX/ClFiC6t9UO9e1qozu0c/vrcUbMCOTqea2E7uT5scZ0\n\teMRYFDpC06wec7KEsg+6JuuNMbwRyJzF2H77J9Xk=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Subject":"[PATCH 1/3] utils: checkstyle.py: Rework commit message parsing","Date":"Sat, 10 Aug 2024 03:58:38 +0300","Message-ID":"<20240810005840.20841-1-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.44.2","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":"When parsing commit messages, the Commit class tries to optimize the\nprocess by invoking git-show once only, extracting both the commit\nauthor, title and modified files in one go. As a result, the derived\nAmendment class needs to implement the commit parsing separately, as the\nmodified files need to be extracted differently for amendments (and the\ncommit ID also needs to be retrieved differently). Furthermore, because\nof the list of named files, extracting the trailers needs to invoke\ngit-show separately.\n\nImprove the situation by reworking the commit message parsing in three\nsteps. In the first step, use git-show to extract the commit ID, author,\ntitle and body. In the second step, invoke git-interpret-trailers to\nextract the trailers from the body that was previously extracted. The\nthird and final step extracts the list of modified files, using\ndifferent methods for regular commits and amendments.\n\nThis allows sharing code for the first two steps between the Commit and\nAmendment classes, making the code simpler. The Commit class still\ninvokes git three times, while the Amendment class runs it three times\ninstead of four, improving performance slightly.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n utils/checkstyle.py | 60 +++++++++++++++++++++++++--------------------\n 1 file changed, 33 insertions(+), 27 deletions(-)\n\n\nbase-commit: 62760bd2605a83e663b9003244ff42f8946f8955","diff":"diff --git a/utils/checkstyle.py b/utils/checkstyle.py\nindex dae5d518920a..aa0731abdb5a 100755\n--- a/utils/checkstyle.py\n+++ b/utils/checkstyle.py\n@@ -211,33 +211,42 @@ class CommitFile:\n \n class Commit:\n     def __init__(self, commit):\n-        self.commit = commit\n+        self._commit = commit\n         self._author = None\n         self._trailers = []\n         self._parse()\n \n-    def _parse_trailers(self):\n-        proc_show = subprocess.run(['git', 'show', '--format=%b',\n-                                    '--no-patch', self.commit],\n-                                   stdout=subprocess.PIPE)\n+    def _parse_commit(self):\n+        # Get and parse the commit message.\n+        ret = subprocess.run(['git', 'show', '--format=%H%n%an <%ae>%n%s%n%b',\n+                              '--no-patch', self.commit],\n+                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n+        lines = ret.splitlines()\n+\n+        self._commit = lines[0]\n+        self._author = lines[1]\n+        self._title = lines[2]\n+        self._body = lines[3:]\n+\n+        # Parse the trailers. Feed git-interpret-trailers with a full commit\n+        # message that includes both the title and the body, as it otherwise\n+        # fails to find trailers when the body contains trailers only.\n+        message = self._title + '\\n\\n' + '\\n'.join(self._body)\n         trailers = subprocess.run(['git', 'interpret-trailers', '--parse'],\n-                                  input=proc_show.stdout,\n+                                  input=message.encode('utf-8'),\n                                   stdout=subprocess.PIPE).stdout.decode('utf-8')\n \n         self._trailers = trailers.splitlines()\n \n     def _parse(self):\n-        # Get the commit title and list of files.\n-        ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%s',\n-                              '--name-status', self.commit],\n+        self._parse_commit()\n+\n+        # Get the list of files. Use an empty format specifier to suppress the\n+        # commit message completely.\n+        ret = subprocess.run(['git', 'show', '--format=', '--name-status',\n+                              self.commit],\n                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n-        lines = ret.splitlines()\n-\n-        self._author = lines[0]\n-        self._title = lines[1]\n-        self._files = [CommitFile(f) for f in lines[2:] if f]\n-\n-        self._parse_trailers()\n+        self._files = [CommitFile(f) for f in ret.splitlines()]\n \n     def files(self, filter='AMR'):\n         return [f.filename for f in self._files if f.status in filter]\n@@ -246,6 +255,10 @@ class Commit:\n     def author(self):\n         return self._author\n \n+    @property\n+    def commit(self):\n+        return self._commit\n+\n     @property\n     def title(self):\n         return self._title\n@@ -284,21 +297,14 @@ class StagedChanges(Commit):\n \n class Amendment(Commit):\n     def __init__(self):\n-        Commit.__init__(self, '')\n+        Commit.__init__(self, 'HEAD')\n \n     def _parse(self):\n-        # Create a title using HEAD commit and parse the trailers.\n-        ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%H %s',\n-                             '--no-patch'],\n-                             stdout=subprocess.PIPE).stdout.decode('utf-8')\n-        lines = ret.splitlines()\n+        self._parse_commit()\n \n-        self._author = lines[0]\n-        self._title = 'Amendment of ' + lines[1]\n+        self._title = f'Amendment of \"{self.title}\"'\n \n-        self._parse_trailers()\n-\n-        # Extract the list of modified files\n+        # Extract the list of modified files.\n         ret = subprocess.run(['git', 'diff', '--staged', '--name-status', 'HEAD~'],\n                              stdout=subprocess.PIPE).stdout.decode('utf-8')\n         self._files = [CommitFile(f) for f in ret.splitlines()]\n","prefixes":["1/3"]}