From patchwork Fri Dec 14 13:32:15 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 53 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6B05060B17 for ; Fri, 14 Dec 2018 14:31:36 +0100 (CET) Received: from avalon.bb.dnainternet.fi (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id EAD9F549 for ; Fri, 14 Dec 2018 14:31:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1544794293; bh=MlPIxC82VBT5x3T9lmHHtGs04IfSmdPobNdRKFO7UAU=; h=From:To:Subject:Date:From; b=q6OecqPRgAZPyVR+1S0RXtoetWNV0VG6/QR26epxPjX5r9NkBkWxCjasVoHpnv0fu Da4r+OUpbJ7SVlmmk1eC3eufBgANNW9O2FsILpCQ9P1AFddhEjqnQprDGGlb5oDMVQ AmD626sS2ZmGibDmcO4ANaMDYD145kUiu+mBtolc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 14 Dec 2018 15:32:15 +0200 Message-Id: <20181214133216.18375-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/2] utils: Add Python-based commit style checker script X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Dec 2018 13:31:36 -0000 checkstyle.py is a reimplementation of checkstyle.sh in Python, that should be easier to extend with additional features. Three additional features and enhancements are already implemented: - While retaining the default behaviour of operating on the HEAD commit, a list of commits can also be specified on the command line. - Correct line numbers are printed in the diff output. - The index and working tree are not touched, they can be dirty. Signed-off-by: Laurent Pinchart --- utils/checkstyle.py | 267 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 267 insertions(+) create mode 100755 utils/checkstyle.py diff --git a/utils/checkstyle.py b/utils/checkstyle.py new file mode 100755 index 000000000000..6e07ffdc19e6 --- /dev/null +++ b/utils/checkstyle.py @@ -0,0 +1,267 @@ +#!/usr/bin/python3 +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (C) 2018, Google Inc. +# +# Author: Laurent Pinchart +# +# checkstyle.py - A patch style checker script based on astyle +# +# TODO: +# +# - Support other formatting tools (clang-format, ...) +# - Split large hunks to minimize context noise +# - Improve style issues counting +# + +import argparse +import difflib +import re +import shutil +import subprocess +import sys + +astyle_options = ( + '-n', + '--style=linux', + '--indent=force-tab=8', + '--attach-namespaces', + '--attach-extern-c', + '--pad-oper', + '--align-pointer=name', + '--align-reference=name', + '--max-code-length=120' +) + +source_extensions = ( + '.c', + '.cpp', + '.h' +) + +class Colours: + Default = 0 + Red = 31 + Green = 32 + Cyan = 36 + +for attr in Colours.__dict__.keys(): + if attr.startswith('_'): + continue + + if sys.stdout.isatty(): + setattr(Colours, attr, '\033[0;%um' % getattr(Colours, attr)) + else: + setattr(Colours, attr, '') + + +class DiffHunkSide(object): + """A side of a diff hunk, recording line numbers""" + def __init__(self, start): + self.start = start + self.touched = [] + self.untouched = [] + + def __len__(self): + return len(self.touched) + len(self.untouched) + + +class DiffHunk(object): + diff_header_regex = re.compile('@@ -([0-9]+),([0-9]+) \+([0-9]+),([0-9]+) @@') + + def __init__(self, line): + match = DiffHunk.diff_header_regex.match(line) + if not match: + raise RuntimeError("Malformed diff hunk header '%s'" % line) + + self.__from_line = int(match.group(1)) + self.__to_line = int(match.group(3)) + self.__from = DiffHunkSide(self.__from_line) + self.__to = DiffHunkSide(self.__to_line) + + self.lines = [] + + def __repr__(self): + s = '%s@@ -%u,%u +%u,%u @@\n' % \ + (Colours.Cyan, + self.__from.start, len(self.__from), + self.__to.start, len(self.__to)) + + for line in self.lines: + if line[0] == '-': + s += Colours.Red + elif line[0] == '+': + s += Colours.Green + else: + s += Colours.Default + s += line + + s += Colours.Default + return s + + def append(self, line): + if line[0] == ' ': + self.__from.untouched.append(self.__from_line) + self.__from_line += 1 + self.__to.untouched.append(self.__to_line) + self.__to_line += 1 + elif line[0] == '-': + self.__from.touched.append(self.__from_line) + self.__from_line += 1 + elif line[0] == '+': + self.__to.touched.append(self.__to_line) + self.__to_line += 1 + + self.lines.append(line) + + def intersects(self, lines): + for line in lines: + if line in self.__from.touched: + return True + return False + + def side(self, side): + if side == 'from': + return self.__from + else: + return self.__to + + +def parse_diff(diff): + hunks = [] + hunk = None + for line in diff: + if line.startswith('@@'): + if hunk: + hunks.append(hunk) + hunk = DiffHunk(line) + + elif hunk is not None: + hunk.append(line) + + if hunk: + hunks.append(hunk) + + return hunks + + +def check_file(commit, filename): + # Extract the line numbers touched by the commit. + diff = subprocess.run(['git', 'diff', '%s~..%s' % (commit, commit), '--', filename], + stdout=subprocess.PIPE).stdout + diff = diff.decode('utf-8').splitlines(True) + commit_diff = parse_diff(diff) + + lines = [] + for hunk in commit_diff: + lines.extend(hunk.side('to').touched) + + # Skip commits that don't add any line. + if len(lines) == 0: + return 0 + + # Format the file after the commit with astyle and compute the diff between + # the two files. + after = subprocess.run(['git', 'show', '%s:%s' % (commit, filename)], + stdout=subprocess.PIPE).stdout + formatted = subprocess.run(['astyle', *astyle_options], + input=after, stdout=subprocess.PIPE).stdout + + after = after.decode('utf-8').splitlines(True) + formatted = formatted.decode('utf-8').splitlines(True) + + diff = difflib.unified_diff(after, formatted) + + # Split the diff in hunks, recording line number ranges for each hunk. + formatted_diff = parse_diff(diff) + + # Filter out hunks that are not touched by the commit. + formatted_diff = [hunk for hunk in formatted_diff if hunk.intersects(lines)] + if len(formatted_diff) == 0: + return 0 + + print('%s---' % Colours.Red, filename) + print('%s+++' % Colours.Green, filename) + for hunk in formatted_diff: + print(hunk) + + return len(formatted_diff) + + +def check_style(commit): + # Get the commit title and list of files. + ret = subprocess.run(['git', 'show', '--pretty=oneline','--name-only', commit], + stdout=subprocess.PIPE) + files = ret.stdout.decode('utf-8').splitlines() + title = files[0] + files = files[1:] + + separator = '-' * len(title) + print(separator) + print(title) + print(separator) + + # Filter out non C/C++ files. + files = [f for f in files if f.endswith(source_extensions)] + if len(files) == 0: + print("Commit doesn't touch source files, skipping") + return + + issues = 0 + for f in files: + issues += check_file(commit, f) + + if issues == 0: + print("No style issue detected") + else: + print('---') + print("%u potential style %s detected, please review" % \ + (issues, 'issue' if issues == 1 else 'issues')) + + +def extract_revlist(revs): + """Extract a list of commits on which to operate from a revision or revision + range. + """ + ret = subprocess.run(['git', 'rev-parse', revs], stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + if ret.returncode != 0: + print(ret.stderr.decode('utf-8').splitlines()[0]) + return [] + + revlist = ret.stdout.decode('utf-8').splitlines() + + # If the revlist contains more than one item, pass it to git rev-list to list + # each commit individually. + if len(revlist) > 1: + ret = subprocess.run(['git', 'rev-list', *revlist], stdout=subprocess.PIPE) + revlist = ret.stdout.decode('utf-8').splitlines() + revlist.reverse() + + return revlist + + +def main(argv): + + # Parse command line arguments + parser = argparse.ArgumentParser() + parser.add_argument('revision_range', type=str, default='HEAD', nargs='?', + help='Revision range (as defined by git rev-parse). Defaults to HEAD if not specified.') + args = parser.parse_args(argv[1:]) + + # Check for required dependencies. + dependencies = ('astyle', 'git') + + for dependency in dependencies: + if not shutil.which(dependency): + print("Executable %s not found" % dependency) + return 1 + + revlist = extract_revlist(args.revision_range) + + for commit in revlist: + check_style(commit) + print('') + + +if __name__ == '__main__': + sys.exit(main(sys.argv)) From patchwork Fri Dec 14 13:32:16 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 52 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B371260B0C for ; Fri, 14 Dec 2018 14:31:34 +0100 (CET) Received: from avalon.bb.dnainternet.fi (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 46F7D5A9 for ; Fri, 14 Dec 2018 14:31:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1544794293; bh=cP+2C8H9AZ/zmwhGKlnuoAHHUlSa5dueA/5Y2hm6PtI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=o7dgMff9S3QOrKa/APPs5xKOI4kCekSptWA2oWGgpUlRpHFGMVGAdQA18uBM5Nas7 vdPq3Ykx7nXgUYvnQrEL+83pSLVxaYtz6LMXeIoq9clVJQXKnuBk4kvdlKLd755Jsj h6S5GAG/+FG9VRQSCa14xrZq1P8g+eIH0b0y07Rs= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 14 Dec 2018 15:32:16 +0200 Message-Id: <20181214133216.18375-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.19.2 In-Reply-To: <20181214133216.18375-1-laurent.pinchart@ideasonboard.com> References: <20181214133216.18375-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/2] Documentation: Document the style check script X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 14 Dec 2018 13:31:34 -0000 From: Jacopo Mondi Add a section to the coding style documentation to explain usage of the checkstyle.py script. Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart --- Documentation/coding-style.rst | 113 +++++++++++++++++++++++++++++++++ 1 file changed, 113 insertions(+) diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst index 1b67abed3a53..55e195cfc368 100644 --- a/Documentation/coding-style.rst +++ b/Documentation/coding-style.rst @@ -79,3 +79,116 @@ C++-11-specific features: * General-purpose smart pointers (std::unique_ptr), deprecating std::auto_ptr. Smart pointers, as well as shared pointers and weak pointers, shall not be overused. + + +Tools +----- + +The 'astyle' code formatting tool can be used to reformat source files with the +libcamera coding style, defined by the following arguments. + +:: + + --style=linux + --indent=force-tab=8 + --attach-namespaces + --attach-extern-c + --pad-oper + --align-pointer=name + --align-reference=name + --max-code-length=120 + +As astyle is a code formatter, it operates on full files outputs reformatted +source code. While it can be used to reformat code before sending patches, it +may generate unrelated changes. To avoid this, libcamera provides a +'checkstyle.py' script wrapping astyle to only retain related changes. This +should be used to validate modifications before submitting them for review. + +The script operates on one or multiple git commits specified on the command +line. It does not modify the git tree, the index or the working directory and +is thus safe to run at any point. + +Commits are specified using the same revision range syntax as 'git log'. The +most usual use cases are to specify a single commit by sha1, branch name or tag +name, or a commit range with the .. syntax. When no arguments are +given, the topmost commit of the current branch is selected. + +:: + + $ ./utils/checkstyle.py cc7d204b2c51 + ---------------------------------------------------------------------------------- + cc7d204b2c51853f7d963d144f5944e209e7ea29 libcamera: Use the logger instead of cout + ---------------------------------------------------------------------------------- + No style issue detected + +When operating on a range of commits, style checks are performed on each commit +from oldest to newest. + +:: + + $ ../utils/checkstyle.py 3b56ddaa96fb~3..3b56ddaa96fb + ---------------------------------------------------------------------------------- + b4351e1a6b83a9cfbfc331af3753602a02dbe062 libcamera: log: Fix Doxygen documentation + ---------------------------------------------------------------------------------- + No style issue detected + + -------------------------------------------------------------------------------------- + 6ab3ff4501fcfa24db40fcccbce35bdded7cd4bc libcamera: log: Document the LogMessage class + -------------------------------------------------------------------------------------- + No style issue detected + + --------------------------------------------------------------------------------- + 3b56ddaa96fbccf4eada05d378ddaa1cb6209b57 build: Add 'std=c++11' cpp compiler flag + --------------------------------------------------------------------------------- + Commit doesn't touch source files, skipping + +Commits that do not touch any .c, .cpp or .h files are skipped. + +:: + + $ ./utils/checkstyle.py edbd2059d8a4 + ---------------------------------------------------------------------- + edbd2059d8a4bd759302ada4368fa4055638fd7f libcamera: Add initial logger + ---------------------------------------------------------------------- + --- src/libcamera/include/log.h + +++ src/libcamera/include/log.h + @@ -21,11 +21,14 @@ + { + public: + LogMessage(const char *fileName, unsigned int line, + - LogSeverity severity); + - LogMessage(const LogMessage&) = delete; + + LogSeverity severity); + + LogMessage(const LogMessage &) = delete; + ~LogMessage(); + + - std::ostream& stream() { return msgStream; } + + std::ostream &stream() + + { + + return msgStream; + + } + + private: + std::ostringstream msgStream; + + --- src/libcamera/log.cpp + +++ src/libcamera/log.cpp + @@ -42,7 +42,7 @@ + + static const char *log_severity_name(LogSeverity severity) + { + - static const char * const names[] = { + + static const char *const names[] = { + "INFO", + "WARN", + " ERR", + + --- + 2 potential style issues detected, please review + +When potential style issues are detected, they are displayed in the form of a +diff that fixes the issues, on top of the corresponding commit. As the script is +in early development false positive are expected. The flagged issues should be +reviewed, but the diff doesn't need to be applied blindly. + +Happy hacking, libcamera awaits your patches!