From patchwork Sat Aug 10 00:58:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 20869 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id B439ABE173 for ; Sat, 10 Aug 2024 00:59:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8E256633B9; Sat, 10 Aug 2024 02:59:06 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ppG8BifN"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 80F1D61946 for ; Sat, 10 Aug 2024 02:59:04 +0200 (CEST) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id ADB044AB for ; Sat, 10 Aug 2024 02:58:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1723251489; bh=6uQLveOZaSo57cK3t2/+QvCi4+q5TM0/+upo74l1a6o=; h=From:To:Subject:Date:From; b=ppG8BifNBPo6qXKQiY2hw7qmw3I7GwUYqokX5GnALCVHk/9rVIKBG8fLrBqtszv2v gapyZ/RiTUJFyocAi7r1nJX/ClFiC6t9UO9e1qozu0c/vrcUbMCOTqea2E7uT5scZ0 eMRYFDpC06wec7KEsg+6JuuNMbwRyJzF2H77J9Xk= From: Laurent Pinchart 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 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When parsing commit messages, the Commit class tries to optimize the process by invoking git-show once only, extracting both the commit author, title and modified files in one go. As a result, the derived Amendment class needs to implement the commit parsing separately, as the modified files need to be extracted differently for amendments (and the commit ID also needs to be retrieved differently). Furthermore, because of the list of named files, extracting the trailers needs to invoke git-show separately. Improve the situation by reworking the commit message parsing in three steps. In the first step, use git-show to extract the commit ID, author, title and body. In the second step, invoke git-interpret-trailers to extract the trailers from the body that was previously extracted. The third and final step extracts the list of modified files, using different methods for regular commits and amendments. This allows sharing code for the first two steps between the Commit and Amendment classes, making the code simpler. The Commit class still invokes git three times, while the Amendment class runs it three times instead of four, improving performance slightly. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal Reviewed-by: Daniel Scally --- utils/checkstyle.py | 60 +++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 27 deletions(-) base-commit: 62760bd2605a83e663b9003244ff42f8946f8955 diff --git a/utils/checkstyle.py b/utils/checkstyle.py index dae5d518920a..aa0731abdb5a 100755 --- a/utils/checkstyle.py +++ b/utils/checkstyle.py @@ -211,33 +211,42 @@ class CommitFile: class Commit: def __init__(self, commit): - self.commit = commit + self._commit = commit self._author = None self._trailers = [] self._parse() - def _parse_trailers(self): - proc_show = subprocess.run(['git', 'show', '--format=%b', - '--no-patch', self.commit], - stdout=subprocess.PIPE) + def _parse_commit(self): + # Get and parse the commit message. + ret = subprocess.run(['git', 'show', '--format=%H%n%an <%ae>%n%s%n%b', + '--no-patch', self.commit], + stdout=subprocess.PIPE).stdout.decode('utf-8') + lines = ret.splitlines() + + self._commit = lines[0] + self._author = lines[1] + self._title = lines[2] + self._body = lines[3:] + + # Parse the trailers. Feed git-interpret-trailers with a full commit + # message that includes both the title and the body, as it otherwise + # fails to find trailers when the body contains trailers only. + message = self._title + '\n\n' + '\n'.join(self._body) trailers = subprocess.run(['git', 'interpret-trailers', '--parse'], - input=proc_show.stdout, + input=message.encode('utf-8'), stdout=subprocess.PIPE).stdout.decode('utf-8') self._trailers = trailers.splitlines() def _parse(self): - # Get the commit title and list of files. - ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%s', - '--name-status', self.commit], + self._parse_commit() + + # Get the list of files. Use an empty format specifier to suppress the + # commit message completely. + ret = subprocess.run(['git', 'show', '--format=', '--name-status', + self.commit], stdout=subprocess.PIPE).stdout.decode('utf-8') - lines = ret.splitlines() - - self._author = lines[0] - self._title = lines[1] - self._files = [CommitFile(f) for f in lines[2:] if f] - - self._parse_trailers() + self._files = [CommitFile(f) for f in ret.splitlines()] def files(self, filter='AMR'): return [f.filename for f in self._files if f.status in filter] @@ -246,6 +255,10 @@ class Commit: def author(self): return self._author + @property + def commit(self): + return self._commit + @property def title(self): return self._title @@ -284,21 +297,14 @@ class StagedChanges(Commit): class Amendment(Commit): def __init__(self): - Commit.__init__(self, '') + Commit.__init__(self, 'HEAD') def _parse(self): - # Create a title using HEAD commit and parse the trailers. - ret = subprocess.run(['git', 'show', '--format=%an <%ae>%n%H %s', - '--no-patch'], - stdout=subprocess.PIPE).stdout.decode('utf-8') - lines = ret.splitlines() + self._parse_commit() - self._author = lines[0] - self._title = 'Amendment of ' + lines[1] + self._title = f'Amendment of "{self.title}"' - self._parse_trailers() - - # Extract the list of modified files + # Extract the list of modified files. ret = subprocess.run(['git', 'diff', '--staged', '--name-status', 'HEAD~'], stdout=subprocess.PIPE).stdout.decode('utf-8') self._files = [CommitFile(f) for f in ret.splitlines()] From patchwork Sat Aug 10 00:58:39 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 20870 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 1BE18BE173 for ; Sat, 10 Aug 2024 00:59:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C61A7633BE; Sat, 10 Aug 2024 02:59:08 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="PeoYNR+B"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B6AD563398 for ; Sat, 10 Aug 2024 02:59:05 +0200 (CEST) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 14D274AB for ; Sat, 10 Aug 2024 02:58:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1723251491; bh=TeT5iMEHtALqBdeywfOnjCzCMO6MbgIkPLlDMyxfhvs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=PeoYNR+BzGwzpCkVB3AMkmEU6GEHxd+kaodkQMaX2FYIQKv+6pPgCu3LOAkBhkPqu Dm+7M2Wh3uikg2jS2gvvYYwAp0gK5eobJnMlVlHyOkUs9MgEBVDyMnu0kZguM6AbJy YJmkyScTiN/kwlu2WKDp2KGm/mntNyXiUQUte4Bo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH 2/3] utils: checkstyle.py: Skip title and trailers checkers for pre-commit Date: Sat, 10 Aug 2024 03:58:39 +0300 Message-ID: <20240810005840.20841-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.44.2 In-Reply-To: <20240810005840.20841-1-laurent.pinchart@ideasonboard.com> References: <20240810005840.20841-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When running checkstyle.py in a pre-commit hook, there is either no commit message at all (when committing staged changes as a new commit), or the commit message comes from a previous commit being amended. In either case, there's no new commit message yet, and thus nothing to validate for the title and trailers checkers. The trailers checker, in particular, will always flag an error, making all commits fail. To fix that, just skip the two checkers during pre-commit. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal Reviewed-by: Daniel Scally --- utils/checkstyle.py | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/utils/checkstyle.py b/utils/checkstyle.py index aa0731abdb5a..2b1e1f6c1b9e 100755 --- a/utils/checkstyle.py +++ b/utils/checkstyle.py @@ -344,11 +344,16 @@ class CommitChecker(metaclass=ClassRegistry): # Class methods # @classmethod - def checkers(cls, names): + def checkers(cls, commit, names): for checker in cls.subclasses: if names and checker.__name__ not in names: continue - yield checker + if checker.supports(commit): + yield checker + + @classmethod + def supports(cls, commit): + return type(commit) in cls.commit_types class CommitIssue(object): @@ -357,6 +362,8 @@ class CommitIssue(object): class HeaderAddChecker(CommitChecker): + commit_types = (Commit, StagedChanges, Amendment) + @classmethod def check(cls, commit, top_level): issues = [] @@ -401,6 +408,8 @@ class HeaderAddChecker(CommitChecker): class TitleChecker(CommitChecker): + commit_types = (Commit,) + prefix_regex = re.compile(r'^([a-zA-Z0-9_.-]+: )+') release_regex = re.compile(r'libcamera v[0-9]+\.[0-9]+\.[0-9]+') @@ -408,11 +417,6 @@ class TitleChecker(CommitChecker): def check(cls, commit, top_level): title = commit.title - # Skip the check when validating staged changes (as done through a - # pre-commit hook) as there is no title to check in that case. - if isinstance(commit, StagedChanges): - return [] - # Ignore release commits, they don't need a prefix. if TitleChecker.release_regex.fullmatch(title): return [] @@ -468,6 +472,8 @@ class TitleChecker(CommitChecker): class TrailersChecker(CommitChecker): + commit_types = (Commit,) + commit_regex = re.compile(r'[0-9a-f]{12}[0-9a-f]* \(".*"\)') coverity_regex = re.compile(r'Coverity CID=.*') @@ -1020,7 +1026,7 @@ def check_style(top_level, commit, checkers): issues = 0 # Apply the commit checkers first. - for checker in CommitChecker.checkers(checkers): + for checker in CommitChecker.checkers(commit, checkers): for issue in checker.check(commit, top_level): print('%s%s%s' % (Colours.fg(Colours.Yellow), issue.msg, Colours.reset())) issues += 1 From patchwork Sat Aug 10 00:58:40 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 20871 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 24870BE173 for ; Sat, 10 Aug 2024 00:59:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BAD32633C0; Sat, 10 Aug 2024 02:59:11 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="NBdX1VB+"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 80B2563398 for ; Sat, 10 Aug 2024 02:59:07 +0200 (CEST) Received: from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi [81.175.209.231]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 753E24AB for ; Sat, 10 Aug 2024 02:58:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1723251492; bh=Pv76YY4/yvthH5nVweMWTrDdG29zVzp/juhGQSvDS7A=; h=From:To:Subject:Date:In-Reply-To:References:From; b=NBdX1VB+KNQua93+tymcDbW3S4aiEHqCWx3X7wgTqK8v0hpJCvb88QH2qn4dfUN84 OjIPrL/78450ffmVqJpro9RwDIM8VzwJ9dMVo0Aqb8JlogUcs9GmpTTOfmum5SjEaZ vNlWDMFFvH/+ODpEifwv1ymlyCDIem+METRKHhfA= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Subject: [PATCH 3/3] utils: checkstyle.py: Add __repr__ method to Commit class Date: Sat, 10 Aug 2024 03:58:40 +0300 Message-ID: <20240810005840.20841-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.44.2 In-Reply-To: <20240810005840.20841-1-laurent.pinchart@ideasonboard.com> References: <20240810005840.20841-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When debugging issues with the Commit class, a __repr__ method proved to be useful to quickly print all the parsed information about a commit. To avoid reimplementing the method over and over again in the future, add it to the class, even if it is not actually used. Signed-off-by: Laurent Pinchart Reviewed-by: Milan Zamazal Reviewed-by: Daniel Scally --- utils/checkstyle.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/utils/checkstyle.py b/utils/checkstyle.py index 2b1e1f6c1b9e..c9e41d4149f7 100755 --- a/utils/checkstyle.py +++ b/utils/checkstyle.py @@ -248,6 +248,17 @@ class Commit: stdout=subprocess.PIPE).stdout.decode('utf-8') self._files = [CommitFile(f) for f in ret.splitlines()] + def __repr__(self): + return '\n'.join([ + f'commit {self.commit}', + f'Author: {self.author}', + f'', + f' {self.title}', + '', + '\n'.join([line and f' {line}' or '' for line in self._body]), + 'Trailers:', + ] + self.trailers) + def files(self, filter='AMR'): return [f.filename for f in self._files if f.status in filter]