From patchwork Wed Dec 12 11:09:35 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 38 Return-Path: Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B566060B13 for ; Wed, 12 Dec 2018 12:09:54 +0100 (CET) Received: from w540.lan (2-224-242-101.ip172.fastwebnet.it [2.224.242.101]) (Authenticated sender: jacopo@jmondi.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id 3C77E24000E; Wed, 12 Dec 2018 11:09:54 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Wed, 12 Dec 2018 12:09:35 +0100 Message-Id: <1544612976-27101-2-git-send-email-jacopo@jmondi.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1544612976-27101-1-git-send-email-jacopo@jmondi.org> References: <1544612976-27101-1-git-send-email-jacopo@jmondi.org> Subject: [libcamera-devel] [PATCH v4 1/2] Documentation: Add coding style document 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: Wed, 12 Dec 2018 11:09:54 -0000 Add document to summarize the coding style adopted by libcamera. Signed-off-by: Jacopo Mondi Reviewed-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Documentation/coding-style.rst | 80 ++++++++++++++++++++++++++++++++++++++++++ Documentation/index.rst | 1 + Documentation/meson.build | 1 + 3 files changed, 82 insertions(+) create mode 100644 Documentation/coding-style.rst diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst new file mode 100644 index 0000000..4747927 --- /dev/null +++ b/Documentation/coding-style.rst @@ -0,0 +1,80 @@ +Coding Style Guidelines +======================= + +The libcamera project has high standards of stability, efficiency and +reliability. To achieve those, the project goes to great length to produce +code that is as easy to read, understand and maintain as possible. + +These coding guidelines are meant to ensure code quality. As a contributor +you are expected to follow them in all code submitted to the project. While +strict compliance is desired, exceptions are tolerated when justified with +good reasons. Please read the whole coding guidelines and use common sense +to decide when departing from them is appropriate. + +libcamera is written in C++, a language that has seen many revisions and +offers an extensive set of features that are easy to abuse. These coding +guidelines establish the subset of C++ used by the project. + + +Coding Style +------------ + +Even if the programming language in use is different, the project embraces the +`Linux Kernel Coding Style`_ with a few exception and some C++ specificities. + +.. _Linux Kernel Coding Style: https://www.kernel.org/doc/html/latest/process/coding-style.html + +In particular, from the kernel style document, the following section are adopted: + + * 1 "Indentation" + * 2 "Breaking Long Lines" with the maximum line length set to 120 columns + * 3 "Placing Braces and Spaces" + * 3.1 "Spaces" + * 8 "Commenting" with the exception that in-function comments are not + always un-welcome. + +While libcamera uses the kernel coding style for all typographic matters, the +project is a user space library, developed in a different programming language, +and the kernel guidelines fall short for this use case. + +For this reason, rules and guidelines from the `Google C++ Style Guide`_ have +been adopted as well as most coding principles specified therein, with a +few exceptions and relaxed limitations on some subjects. + +.. _Google C++ Style Guide: https://google.github.io/styleguide/cppguide.html + +The following exceptions apply to the naming conventions specified in the +document: + + * File names: libcamera uses the .cpp extensions for C++ source files and + the .h extension for header files. + * Variables, function parameters, function names and class members use + camel case style, with the first letter in lower-case (as in 'camelCase' + and not 'CamelCase') + * Types (classes, structs, type aliases, and type template parameters) use + camel case, with the first letter in capital case (as in 'CamelCase' and + not 'camelCase'). + * Enum members use 'CamelCase', while macros are in capital case with + underscores in between. + * All formatting rules specified in the selected sections of the Linux kernel + Code Style for indentation, braces, spacing, etc + * Header guards are formatted as '__LIBCAMERA_FILE_NAME_H__' + + +C++ Specific Rules +------------------ + +The code shall be implemented in C++03, extended with the following +C++-11-specific features: + + * Initializer lists + * Type inference (auto and decltype) + Type inference shall be used with caution, to avoid drifting towards an + untyped language. + * Range-based for loop + * Lambda functions + * Explicit overrides and final + * Null pointer constant + * 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. diff --git a/Documentation/index.rst b/Documentation/index.rst index c9b7c1c..ba6c6c6 100644 --- a/Documentation/index.rst +++ b/Documentation/index.rst @@ -20,6 +20,7 @@ systems, including traditional Linux distributions, ChromeOS and Android. :maxdepth: 2 :caption: Contents: + coding-style contributing diff --git a/Documentation/meson.build b/Documentation/meson.build index 578c1ca..3b87619 100644 --- a/Documentation/meson.build +++ b/Documentation/meson.build @@ -5,6 +5,7 @@ endif if sphinx.found() docs_sources = [ + 'coding-style.rst', 'conf.py', 'contributing.rst', 'index.rst', From patchwork Wed Dec 12 11:09:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 39 Return-Path: Received: from relay10.mail.gandi.net (relay10.mail.gandi.net [217.70.178.230]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4FD9260B13 for ; Wed, 12 Dec 2018 12:09:55 +0100 (CET) Received: from w540.lan (2-224-242-101.ip172.fastwebnet.it [2.224.242.101]) (Authenticated sender: jacopo@jmondi.org) by relay10.mail.gandi.net (Postfix) with ESMTPSA id DB09124001D; Wed, 12 Dec 2018 11:09:54 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Wed, 12 Dec 2018 12:09:36 +0100 Message-Id: <1544612976-27101-3-git-send-email-jacopo@jmondi.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1544612976-27101-1-git-send-email-jacopo@jmondi.org> References: <1544612976-27101-1-git-send-email-jacopo@jmondi.org> Subject: [libcamera-devel] [PATCH v4 2/2] Documentation: Add style checker tool 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: Wed, 12 Dec 2018 11:09:55 -0000 Add the style checker tool 'checkstyle.sh' and add tool documentation section to 'coding-style.rst'. The script is in a very early development stage, and it has been tested locally only. Use this a starting point, as we might later consider re-implementing it in something that is not shell scripting (as Python, in example). Signed-off-by: Jacopo Mondi --- Documentation/coding-style.rst | 45 +++++++++++++ utils/checkstyle.sh | 141 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100755 utils/checkstyle.sh diff --git a/Documentation/coding-style.rst b/Documentation/coding-style.rst index 4747927..c2f95c7 100644 --- a/Documentation/coding-style.rst +++ b/Documentation/coding-style.rst @@ -78,3 +78,48 @@ 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 +----- + +Libcamera provides a style checker scripts that uses 'astyle', to ease +identification of style errors before patches gets submitted for review. + +Right now, these are the basic astyle options used by the project's code base: + +| --style=linux +| --indent=force-tab=8 +| --attach-namespaces +| --attach-extern-c +| --pad-oper +| --align-pointer=name +| --align-reference=name +| --max-code-length=120 + +Astyle works on full files, and modifies files in place unless instructed to +do differently. It can't serve directly as a style validator by operating +directly on patches. The libcamera project thus provides a 'checkstyle.sh' +script that wraps around git and astyle to get the changes recorded in the +top-most commit in the working tree and detect style errors. + +Here is a simple usage example: + + * Do your file editing, then "git add" and "git commit" as usual. + * Run 'checkstyle.sh' on your latest commit: be aware that 'checkstyle.sh' + works on commits, so make sure your index is clean. + +To use the script simply run: + +.. code-block:: bash + + $ ./utils/checkstyle.sh + +The tool outputs the differences between the code added by the last commit +and its astyled version, for all source files modified by the commit. + +Once the script doesn't report any difference, or when the reported +differences are false positives according to your best judgment, the patches +are ready to be submitted for review. + +Happy hacking, libcamera awaits your patches! diff --git a/utils/checkstyle.sh b/utils/checkstyle.sh new file mode 100755 index 0000000..7f52975 --- /dev/null +++ b/utils/checkstyle.sh @@ -0,0 +1,141 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0-or-later +# Copyright (C) 2018, Google Inc. +# +# Author: Jacopo Mondi +# +# checkstyle.sh - A style checker script using astyle for the libcamera project +# +# The scripts makes use of the following tools, which are expected to be +# found in $PATH: +# - astyle +# - git +# - diff +# - colordiff + +ASTYLE=astyle +ASTYLE_OPT='-n --style=linux --indent=force-tab=8 --attach-namespaces +--attach-extern-c --pad-oper --align-pointer=name --align-reference=name +--max-code-length=120' +EXTDIFF=colordiff +INTDIFF=diff +GIT=git +TMP=/tmp/ + +# Check for tools to be installed and available in $PATH +TOOL_LIST="$ASTYLE $EXTDIFF $INTDIFF $GIT" +for T in $TOOL_LIST; do + if [ _$(which $T) = '_' ]; then + echo $T "missing or not in \$PATH; please install it" + exit 1 + fi +done + +COMMIT_MSG=$($GIT log --format=%s -n1) +FLIST=$($GIT diff-index --name-only HEAD^) + +echo "Running $0 on commit: \"$COMMIT_MSG\"" +echo +echo "The commit modifies the following files:" +for F in $FLIST; do echo $F; done +echo + +# Loop on every file modified by the last commit +for F in $FLIST; do + rm $TMP/chstyle.* &> /dev/null + BASENAME=$(basename $F) + DIRNAME=$(dirname $F) + + echo + # Skip style check on meson files + if [[ $BASENAME == "meson.build" ]]; then + echo "=================================================================" + echo "skip checks on:" $F + echo "it's a meson build file" + echo "=================================================================" + continue; + fi + + # Skip style check on hidden files + if [[ $BASENAME == '.'* ]]; then + echo "=================================================================" + echo "skip checks on:" $F + echo "it's an hidden file" + echo "=================================================================" + continue; + fi + + # Skip Documentation patches + if [[ $DIRNAME == 'Documentation' ]]; then + echo "=================================================================" + echo "skip checks on:" $F + echo "it's a Documentation file" + echo "=================================================================" + continue; + fi + + # Skip patches on utils/ + if [[ $DIRNAME == 'utils' ]]; then + echo "=================================================================" + echo "skip checks on:" $F + echo "it's a utils/ script" + echo "=================================================================" + continue; + fi + + echo "=================================================================" + echo "Checking style on file: " $F + + $GIT show $F > $TMP/chstyle.patch + patch -p1 -R < $TMP/chstyle.patch > /dev/null + if [[ ! -f "$F" ]]; then + # If the file has been added by the last commit, run astyle + # on it and report the differences + echo + echo "Is $BASENAME introduced by this commit? It seems so..." + echo "Now running astyle on the whole file: $BASENAME" + echo + patch -p1 < $TMP/chstyle.patch > /dev/null + $ASTYLE $ASTYLE_OPT < $F > $TMP/chstyle.astylepre + echo "-----------------------------------------------------------------" + $EXTDIFF $F $TMP/chstyle.astylepre + echo "-----------------------------------------------------------------" + echo + echo "If you see nothing here, it means the patch on this file is good!" + echo "Otherwise, you may want to check what's wrong with this change" + echo "=================================================================" + continue + fi + + # Run astyle on the file -before- this commit, and then -after- + # Record the differences between the two to have the astyled version + # of the latest changes + $ASTYLE $ASTYLE_OPT < $F > $TMP/chstyle.astylepre + patch -p1 < $TMP/chstyle.patch > /dev/null + $ASTYLE $ASTYLE_OPT < $F > $TMP/chstyle.astylepost + $INTDIFF $TMP/chstyle.astylepre $TMP/chstyle.astylepost \ + | grep -e '^[\>\<]' \ + | sed s/^[\>\<]\ // > $TMP/chstyle.astylediff + + # Sanitize the content of the patch as recorded in the commit + grep ^[+-] $TMP/chstyle.patch \ + | sed s/^[+-]// | sed /^[+-]\.*/d > $TMP/chstyle.patchpost + + # Now compare the two: raw commit and its astyled version + echo "-----------------------------------------------------------------" + $EXTDIFF -u $TMP/chstyle.patchpost /tmp/chstyle.astylediff + echo "-----------------------------------------------------------------" + echo + echo "If you see nothing here, it means the patch on this file is good!" + echo "Otherwise, you may want to check what's wrong with this change" + echo "=================================================================" +done + +cat << _END_MSG + +----------------------------------------------------------------- +If checkstyle reports any difference you may want to check what's +wrong in your patch before submitting it, otherwise the patch +is ready to be sent out! +----------------------------------------------------------------- +_END_MSG