The Amendment class calls `git show` twice, once to extract the commit
title, and a second time to extract the trailers. This can be combined
in a single command, which is more efficient. Do so.
While at it, centralize initialization of self._trailers in the
Commit.__init__() function.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The commit trailers are checked as part of processing the commit message
with the newly introduced TrailersChecker.
This relies on the trailers property being correctly exposed by the
Commit object, and is implemented for the base Commit but not processed
for Amendment commits.
Refactor the trailer property handling to a helper function in the base
Commit class and make use of it with a newly added call to obtain the
existing Trailers from the most recent commit when using Amendment.
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The Amendment commit class is derived from the StagedChanges class
(which in turn derives from the Commit base class), however there is no
code sharing between Amendment and StagedChanges other than the call to
initalise through the base Commit class.
Refactor the inheritance to make an Amendment derive directly from
Commit.
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
There are no possible Trailers for staged changes as the commit message
has not yet been written.
Initialise the empty trailers when the commit object is initialised.
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
If a Malformed trailer is identified the checkstyle script triggers a
RuntimeError and stops processing the rest of the commit.
A malformed trailer can be regarded as an issue in the commit and
reported as such using the same method as other faults identified by the
tool.
Convert the RuntimeError into a CommitIssue and continue processing
other trailers.
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The libcamera git history contains numerous examples of incorrect commit
message trailers due to invalid trailer types (e.g. Change-Id), typos
and other small issues. Those went unnoticed through reviews, which
shows that an automated checker is required.
Add a trailers checker to checkstyle.py to catch invalid or malformed
trailers, with a set of supported trailers that match libcamera's commit
message practices. New trailer keys can easily be added later as new
needs arise.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The commit title and commit ID are two different pieces of information.
Don't include the latter in the former, to simplify code that only needs
the commit title. Constructing a string from the ID and title is easier
than splitting the combined string back into its elements.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
During development of the checkstyle.py script, it can be useful to run
only a subset of the checker. Add the ability to do so with a
'--checkers' command line argument.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Add a custom representation to the CommitFile class in order to
facilitate debugging.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
When creating a new commit, there is no title, so the title checker
complains that the title isn't compliant and the commit cannot be
created if checkstyle is run as a pre-commit hook. Fix this by skipping
the title checker when run on staged changes.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Add a commit checker to ensure that commit titles start with a prefix.
The commit issue message lists prefix candidates retrieved from the git
log.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Declaration of new header file to the build system are only checked against
modified meson.build file. Therefore, this raises a false positive warning in
case the meson.build is added or renamed.
Add the new and renamed meson.build files to the list of files to check header
file inclusion.
Signed-off-by: Xavier Roumegue <xavier.roumegue@oss.nxp.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
If issue.line is None, the the terminal color is never reset back to
normal. This causes the yellow color to bleed.
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
libcamera uses the "..." include style for internal headers. Enforce it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
The IncludeOrderFormatter will be extended with more formatting rules
that should be applied before clang-format gets run, as it will
influence its output.
Add a priority mechanism for formatters, and give a negative priority to
the CLangFormatter to make it run last.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
checkstyle.py uses single-quoted strings in most locations already.
There are a few locations where this wouldn't be convenient (when the
string itself contains a single quote, which would then require
escaping), but there are also a few other locations where double quotes
are used when single quotes would work fine. Change those to standardize
on single-quoted strings.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The header add checker only handles added header, which makes it miss
issues when a header is renamed. Fix it.
Fixes: 8fffab46b8 ("utils: checkstyle.py: Add header add checker")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Commit fc91951250 ("utils: checkstyle.py: Add ability to filter files
by status in a commit") caused all renamed files to be ignored by the
checker. Fix it.
Fixes: fc91951250 ("utils: checkstyle.py: Add ability to filter files by status in a commit")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The Commit class and subclasses were reworked in commit 4f5d17f3a4
("utils: checkstyle.py: Make title and files properties of commit
class") with the introduction of members of the base class that were
meant to be protected (not used externally, but accessible by
subclasses). They have been named with a '__' prefix for this purpose,
which was a bad choice as Python effectively replaces a leading '__'
with a literal '__classname__' prefix to make them private
(https://docs.python.org/3/tutorial/classes.html#private-variables). The
members accessed in the derived classes are thus different from the ones
in the base class.
Fix this by replacing the double underscore prefix with a single
underscore, which is a "weak internal use indicator" (as specified in
https://www.python.org/dev/peps/pep-0008/), closer to the protected
access specifier of C++.
Reported-by: Umang Jain <email@uajain.com>
Reported-by: Naushir Patuck <naush@raspberrypi.com>
Fixes: 4f5d17f3a4 ("utils: checkstyle.py: Make title and files properties of commit class")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Tested-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Formatting code using astyle doesn't lead to results as good as with
clang-format, and doesn't receive much test coverage as most developers
use clang-format. The code is thus bitrotting. Drop it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Add a commit checker that ensures that all header files added to the
libcamera includes (public or internal) are accompanied by a
corresponding update of the meson.build file in the same directory.
Here's the output of the new checker when run against a commit that
forgot to update meson.build.
$ ./utils/checkstyle.py b3383da79f
---------------------------------------------------------------------------------
b3383da79f libcamera: buffer: Create a MappedBuffer
---------------------------------------------------------------------------------
Header include/libcamera/internal/buffer.h added without corresponding update to include/libcamera/internal/meson.build
---
1 potential issue detected, please review
In theory we could extend the checker to cover .cpp files too, but the
issue will be quite noticeable as meson won't build the file if
meson.build isn't updated. Header files are more tricky as problems
would only occur at when installing the headers (for public headers), or
would result in race conditions in the build. Both of those issues are
harder to catch.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Add a new category of checkers that operate on a whole commit.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
To avoid duplicating diff parsing in commit checkers, move it to the
Commit class.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
A commit can perform different operations on a file. Record the file
status (added, modified, renamed, deleted, ...) and add the ability to
filter files by status when listing the files touched by a commit.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Make the API of the Commit class more explicit by exposing the title and
files as properties instead of through a get_info() method.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
To prepare for checkers that operate directly on commits, move the
related classes to a separate section. No functional change is included.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
The style checkers and formatters duplicate automatic class registry
code. Factor it out to a common ClassRegistry helper class. The list of
subclasses is moved to a class member variable of the auto-registered
base class type.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
The super() call is a shortcut syntax for super(__class__, <first arg>).
Drop the arguments when they match the default.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Add a formatter to ensure consistent naming of 'd' and 'o' variables
related to the d-pointer design pattern, as implemented by the
Extensible class. The formatter also ensures that the pointer is always
const. const-correctness issues related to the data pointed to will be
caught by the compiler, and thus don't need to be checked here.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
While libcamera prefers usage of the C standard library headers (xxx.h)
over the C++ version (cxxx), we make an exception for cmath as the
overloaded versions of the math functions are convenient. Document this,
and adjust checkstyle.py accordingly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
This introduces a new argument "--amend" and a new special type of
commit "Amendment". It will check the style of changes that are in
the index combined with the changes of the last commit. So this is
the changes that would be applied by "git commit --amend" hence the
name of the argument.
This is needed to implement pre-commit hook.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
This introduces a new command line "--staged" and a new special type of
commit "StagedChanges". It will check the style of changes that are in
the index, so the changes that would be committed by "git commit".
"--staged" was chosen to match with "git diff --staged" command line.
Other valid name could have been "--index" or "--cached". This was
my personal preference, aliases can be added later. Note that we must
not confuse this with working tree changes, as these changes are not
picked by "git commit".
This feature is needed to implement pre-commit hook.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
This introduces a Commit class used in the final revlist list. All the
git commands are moved into that class. This class will be used to
introduce new types of commit (StagedChanges and Amendment) needed to implement
pre-commit hook support.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Makes the tool return 1 if there is any potential issues. This is
needed when using this tool for pre-commit hook in order to abort
the commit process.
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The tool has been renamed in 2016 to make it more obvious what it
is doing. There is no other changes needed on our side.
See https://github.com/PyCQA/pycodestyle/issues/466
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The checkstyle script expects hunks to be declared with a start line and
line count, however the unified diff format [0] declares that a single
line hunk will only have the start line:
> If a hunk contains just one line, only its start line number appears.
> Otherwise its line numbers look like ‘start,count’. An empty hunk is
> considered to start at the line that follows the hunk.
[0] https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html#Detailed-Unified
Attempting to parse a single line hunk results in the following error:
File "./utils/checkstyle.py", line 110, in __init__
raise RuntimeError("Malformed diff hunk header '%s'" % line)
RuntimeError: Malformed diff hunk header '@@ -1 +1,2 @@
The DiffHunk class only makes use of the start line, and does not
utilise the line count, thus update the regex to make the unused
groups optional.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The checkstyle.py patch has a fault which it identified in itself when
updating the regex string.
--- utils/checkstyle.py
+++ utils/checkstyle.py
#105: : W605 invalid escape sequence '\+'
+ diff_header_regex = re.compile('@@ -([0-9]+)(,[0-9]+)? \+([0-9]+)(,?[0-9]+)? @@')
---
1 potential style issue detected, please review
This is documented further at:
https://www.flake8rules.com/rules/W605.html
Update the string literal prefix to declare a raw byte string for the
regex parser.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Add an include checker to verify usage of the C compatibility headers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Process python additions with pep8 and report any errors that are added.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Add a formatter for doxygen comments. In its initial implementation the
formatter ensures that the first word of a \return statement starts with
an uppercase letter.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Introduce two new base classes for the code formatters and style
checkers, with an auto-registration mechanism that automatically uses
all derived classes. This will allow easier addition of new formatters
and checkers.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Add support for checkers not related to code formatting to the
checkstyle.py script, and create a first checker that catches usage of
the LOG() macro without an explicit category.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
clang-format produces better results than astyle as it can better match
the libcamera coding style. Default to clang-format over astyle, fall
back to astyle if clang-format isn't found, and add a --formatter
command line option to select a formatter manually.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
As astyle doesn't strip trailing white spaces, strip them manually.
Organize the code to allow for new additional formatting steps if
needed.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
In order to facilitate interpretation of diffs, highlight trailing white
space at end of lines with a red background.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The git diff command is invoked with relative paths, which causes git to
fail to locate files when the checkstyle.py script is run from
subdirectories of the git tree. Fix this by prepending the absolute path
to the git tree root directory to the file names.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
