Commit Graph

29 Commits

Author SHA1 Message Date
Yann E. MORIN
dfed5acb56 utils/check-package: use https for the manual URL
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2024-02-11 16:28:10 +01:00
James Knight
2be2ccb487 utils/check-package: cleanup line reading
Cleanup the implementation for reading lines by having files processed
in context managers and utilizing the iterable file object for line
reading (instead of needing to call `readlines()`).

Signed-off-by: James Knight <james.d.knight@live.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2023-09-02 17:47:48 +02:00
Yann E. MORIN
5eac4f81eb utils/check-package: check ignored files exist
When an ignored file is removed (e.g. a package patch is no longer
needed after a version bump), the corresponding entry in the ignore list
is no longer needed.

However, we currently only validate that an ignored *test* still fails,
not that a ignore files is now missing.

Add a new test to check-package that does that check, and add a
test-case for that check.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
2023-05-13 12:08:25 +02:00
Yann E. MORIN
a3e796b74e utils/check-package: fix grammar
The past participle for "to fix" is "fix". The "did you forget" got
eluded into "forget", so again a past participle.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
2023-04-23 21:49:46 +02:00
Vincent Fazio
b7d4ae98fa utils/check-package: drop six usage
The shebang in check-package now defines python3. There is no longer a
need to maintain support with python 2.x.

See-also: 02b165dc71 (check-package: fix Python3 support)

Signed-off-by: Vincent Fazio <vfazio@gmail.com>
Tested-by: James Knight <james.d.knight@live.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
2023-04-10 21:02:35 +02:00
Ricardo Martincoski
9df6503ed0 Makefile: merge check-flake8 into check-package
Teach check-package to detect python files by type and check them using
flake8.
Do not use subprocess to call 'python3 -m flake8' in order to avoid too
many spawned shells, which in its turn would slow down the check for
multiple files. (make check-package takes twice the time using a shell
for each flake8 call, when compared of importing the main application)

Expand the runtime test and the unit tests for check-package.

Remove check-flake8 from the makefile and also from the GitLab CI
because the exact same checks become part of check-package.

Suggested-by: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Arnout: add a comment to x-python to explain its purpose]
Signed-off-by: Arnout Vandecappelle <arnout@mind.be>
2023-04-09 22:33:24 +02:00
Ricardo Martincoski
60fdaf56fe utils/check-package: check files in support/
The .mk files inside both support/dependencies and support/misc are not
package recipes, similar to package/pkg-*.mk. The check-package don't
apply to them. Therefore ignore such files.

In the test infra, some br2-externals are used as fixtures to provide
(sometimes) failure cases, so ignore files in these directories.

Files inside support/kconfig are files copied from linux upstream, so do
not generate warnings for them.

support/gnuconfig contains auto-generated config.{guess,sub} files,
so do not generate shellcheck warnings for them.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle <arnout@mind.be>
2023-04-09 21:59:04 +02:00
Ricardo Martincoski
29d6f319a0 utils/check-package: check files in board/
When a SysV init script is inside package/ it doesn't need to be
executable. However, when an init script is inside a fs_overlay, it
*does* need to be executable. Therefore, skip the NotExecutable test for
init scripts. We detect them based on the directory /etc/init.d

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Arnout: update .checkpackageignore]
Signed-off-by: Arnout Vandecappelle <arnout@mind.be>
2023-04-09 21:51:11 +02:00
Ricardo Martincoski
2b021bc1d7 utils/check-package: check files in utils/
All the errors in existing scripts in utils/ have been fixed, so nothing
needs to be added to .checkpackageignore.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle <arnout@mind.be>
2023-02-08 15:28:33 +01:00
Ricardo Martincoski
c5a3dfb9a4 utils/check-package: check all shell scripts
Currently only SysV init scripts are checked using shellcheck and a few
other rules (e.g. variable naming, file naming).

Extend the check using shellcheck to all shell scripts in the tree.
This is actually limited to the list of directories that check-package
knows that can check, but that list can be expanded later.

In order to apply the check to all shell scripts, use python3-magic to
determine the file type. Unfortunately, there are two different python
modules called "magic". Support both by detecting which one is installed
and defining get_filetype accordingly.

Keep testing first for name pattern, and only in the case there is no
match, check the file type. This ensures, for instance, that SysV
init scripts follow specific rules.

Apply these checks for shell scripts:
 - shellcheck;
 - trailing space;
 - consecutive empty lines;
 - empty last line on file;
 - newline at end of file.

Update the list of ignored warnings.

Do not add unit tests since no function was added, they were just
reused.
But expand the runtime test for check-package using as fixture a file
that generates a shellcheck warning.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Arnout: support both variants of the "magic" module]
Signed-off-by: Arnout Vandecappelle <arnout@mind.be>
2023-02-08 13:31:06 +01:00
Ricardo Martincoski
dd28acc1e8 Makefile: add target to update .checkpackageignore
When a developer fixes an ignored warning from check-package, he/she
needs to update .checkpackageignore
By running './utils/docker-run make check-package' the developer
receives a warning about this.
Make that change easier to make, by adding a helper target on Makefile.

Add an option --failed-only to check-package that generates output in
the format:
<filename> <check_function> [<check_function> ...]
This is the very same format used by check-package ignore file.

Add the phony target .checkpackageignore

So one can update the ignore file using:
$ ./utils/docker-run make .checkpackageignore

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2023-02-06 18:54:18 +01:00
Ricardo Martincoski
ddf4586b00 utils/check-package: decouple adding rules from fixing all intree files
When a new check_function is added to check-package, often there are
files in the tree that would generate warnings.

An example is the Sob check_function for patch files:
| $ ./utils/check-package --i Sob $(git ls-files) >/dev/null
| 369301 lines processed
| 46 warnings generated
Currently these warnings are listed when calling check-package directly,
and also at the output of pkg-stats, but the check_function does not run
on 'make check-package' (that is used to catch regressions on GitLab CI
'check-package' job) until all warnings in the tree are fixed.
This (theoretically) allows new .patch files be added without SoB,
without the GitLab CI catching it.

So add a way to check-package itself ignore current warnings, while
still catching new files that do not follow that new check_function.

Add a file named .checkpackageignore to the buildroot topdir.
It contains the list of check_functions that are expected to fail for
each given intree file tested by check-package.
Each entries is in the format:
<filename> <check_function> [<check_function> ...]

These are 2 examples of possible entries:
package/initscripts/init.d/rcK ConsecutiveEmptyLines EmptyLastLine Shellcheck
utils/test-pkg Shellcheck

Keeping such a list allows us to have fine-grained control over which
warning to ignore.

In order to avoid this list to grow indefinitely, containing entries for
files that are already fixed, make each entry an 'expected to fail'
instead of just an 'ignore', and generate a warning if a check_function
that was expect to fail for a given files does not generate that
warning.
Unfortunately one case that do not generate warning is an entry for a
file that is deleted in a later commit.

By default, all checks are applied. The --ignore-list option allows to
specify a file that contains the list of warnings that should be
ignored.

The paths in the ignore file must be relative to the location of the
ignore file itself, which means:

 - in the main Buildroot tree, the paths in the ignore file are
   relative to the root of the main Buildroot tree

 - in a BR2_EXTERNAL tree, if the ignore file is at the root of the
   BR2_EXTERNAL, the paths it contains must be relative to that root
   of the BR2_EXTERNAL

This is one more step towards standardizing the use of just 'make
check-package' before submitting patches to the list.

Cc: Sen Hastings <sen@phobosdpl.com>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2023-02-06 18:54:18 +01:00
Ricardo Martincoski
d020368eea utils/checkpackagelib/lib_sysv: check SysV init scripts
Enable the common checks:
 - consecutive empty lines
 - empty last line
 - missing new line at end of file
 - trailing space
 - warn for executable files, with the hint to instead use
   '$(INSTALL) -D -m 0755' in the .mk file

Check indent with tabs:
 - add a simple check function to warn only when the indent is done
   using spaces or a mix of tabs and spaces. It does not check indenting
   levels, but it already makes the review easier, since it
   diferentiates spaces and tabs.

Check variables:
 - check DAEMON is defined
 - when DAEMON is defined, check the filename is in the form S01daemon
 - when PIDFILE is defined, expect it to be in /var/run and defined
   using $DAEMON.

Also add unit test for this.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Arnout: avoid 'del NotExecutable_base' by importing the module instead
 of the class; refer to manual in warnings]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2022-02-06 18:27:00 +01:00
Ricardo Martincoski
1734127e59 utils/check-package: prepare to run external tools
Some file formats have well-established syntax checkers.
One example of this is the tool 'shellcheck' that can analyse shell
scripts for common mistakes.

There is no reason to reimplement such tools in check-package, when we
can just call them.

Add the ability to check-package to call external tools that will run
once for each file to be analysed.
For simplicity, when the tool generated one or more warnings, count it
as a single warning from check-package, that can display something like
this:

|$ ./utils/check-package  package/unscd/S46unscd
|package/unscd/S46unscd:0: run 'shellcheck' and fix the warnings
|25 lines processed
|1 warnings generated

|$ ./utils/check-package -vvvvvvvvvvvvvvvv package/unscd/S46unscd
|package/unscd/S46unscd:0: run 'shellcheck' and fix the warnings
|In package/unscd/S46unscd line 9:
|        printf "Starting ${NAME}: "
|               ^------------------^ SC2059: Don't use variables in the printf format string. Use printf "..%s.." "$foo".
|In package/unscd/S46unscd line 11:
|        [ $? -eq 0 ] && echo "OK" || echo "FAIL"
|          ^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
|In package/unscd/S46unscd line 14:
|        printf "Stopping ${NAME}: "
|               ^------------------^ SC2059: Don't use variables in the printf format string. Use printf "..%s.." "$foo".
|In package/unscd/S46unscd line 16:
|        [ $? -eq 0 ] && echo "OK" || echo "FAIL"
|          ^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
|For more information:
|  https://www.shellcheck.net/wiki/SC2059 -- Don't use variables in the printf...
|  https://www.shellcheck.net/wiki/SC2181 -- Check exit code directly with e.g...
|25 lines processed
|1 warnings generated

In this first commit, add only the ability for check-package to call
external tools and not an example of such tool, as adding each tool to
call may need update to the docker image and can lead to it's own
discussion on how to implement.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2022-02-06 16:38:52 +01:00
Petr Vorel
35f15db30a support: utils: use python3 explicitly
Python 2 is EOL sice 2020 [1], it's still available on distros, but may not
be installed by default (as being replaced by python3).

Thus remove compatibility imports:
from __future__ import print_function
from __future__ import absolute_import

Tested with python3 -m py_compile.

[1] https://www.python.org/doc/sunset-python-2/

Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2021-09-22 21:30:24 +02:00
Thomas Petazzoni
163f160a8e utils/{check-package, checkpackagelib}: consistently use raw strings for re.compile
Raw strings need to be used when calling re.compile() otherwise Python
3.x flake8 complains with:

W605 invalid escape sequence '\s'

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Reviewed-by: Titouan Christophe <titouan.christophe@railnova.eu>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2020-08-14 21:55:59 +02:00
Ricardo Martincoski
75d6c2ba96 check-package: ignore external.mk
The external.mk file in a br2-external usually contains raw makefile
targets. This file is common code and not a package recipe so it should
not be tested against the code-style of a package .mk file.

When using this script to check files in a br2-external tree, usually
the user is responsible for not passing files that check-package do not
understand. But external.mk is special because it is part of the
br2-external structure, so it is likely someone expects it to be
checkable by an in-tree script.

Instead of adding another blob to the manual, just ignore this file.
Only do that when a br2-external is being tested (so with option -b
passed to the script) and also check that it is on the root path of the
br2-external to allow someone to have a package called external.

Reported on bug #11271.

Reported-by: Vitaliy Lotorev <lotorev@gmail.com>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
[Arnout: wrap at 80 columns]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-08-03 17:17:29 +02:00
Brandon Maier
44af8386f8 utils/check-package: Add a quiet flag
When running in a CI system, stat messages become white noise. Introduce
an option to suppress non-error, non-warning, messages.

Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-07-03 23:34:58 +02:00
Ricardo Martincoski
4a6c5ab2c3 utils/check-package: allow to disable warning for a line
Currently any exceptions for a check function need to be coded into the
check-package script itself.

Create a pattern that can be used in a comment to make check-package
ignore one or more warning types in the line immediately below:
 # check-package Indent, VariableWithBraces

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
2019-01-29 16:37:47 +01:00
Ricardo Martincoski
02b165dc71 check-package: fix Python3 support
This script currently uses "/usr/bin/env python" as shebang but it does
not really support Python3. Instead of limiting the script to Python2,
fix it to support both versions.

So change all imports to absolute imports because Python3 follows PEP328
and dropped implicit relative imports.

In order to avoid errors when decoding files with the default 'utf-8'
codec, use errors="surrogateescape" when opening files, the docs for
open() states: "This is useful for processing files in an unknown
encoding.". This argument is not compatible with Python2 open() so
import 'six' to use it only when running in Python3.
As a consequence the file handler becomes explicit, so use it to close()
the file after it got processed.

This "surrogateescape" is a simple alternative to the complete solution
of opening files with "rb" and changing all functions in the lib*.py
files to use bytes objects instead of strings. The only case we can have
non-ascii/non-utf-8 files being checked by the script are for patch
files when the upstream file to be patched is not ascii or utf-8. There
is currently one case in the tree:
package/urg/0002-urg-gcc6-fix-narrowing-conversion.patch.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Reviewed-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Tested-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
2019-01-16 23:14:25 +01:00
Arnout Vandecappelle (Essensium/Mind)
b3a5ab4c7a check-package: support symlinks to the script
The check-package script finds base_dir (= the Buildroot directory) and
cd's into it. To be able to support relative paths as arguments, it
first recalculates the arguments relative to base_dir.

However, if there is a symlink anywhere on the path to the
check-package script, the relative paths will be wrong. To solve this,
use realpath() instead of abspath(), so symlinks are resolved before
calculating the relative path.

Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Acked-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
2018-04-01 22:41:54 +02:00
Ricardo Martincoski
e2c4a687c7 check-package: enable for Config.in* in base dir
These files can benefit from this script to prevent common mistakes when
submitting patches.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-04-01 09:12:00 +02:00
Ricardo Martincoski
bfc8dce467 check-package: enable for toolchain/
The toolchain directory can benefit from this script to prevent common
mistakes when submitting patches.

In order to accomplish this:
Do not ignore anymore files from the toolchain/ directory.
Ignore this symbol:
 - BR_LIBC: defined by the buildroot toolchain, used by gcc-final.mk.

Ignore toolchain/toolchain-external/pkg-toolchain-external.mk as it
declares a package infra and not a package itself.
Ignore toolchain/helpers.mk as it contains only helper functions.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Romain Naour <romain.naour@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-04-01 09:09:01 +02:00
Ricardo Martincoski
4d39f5876d check-package: enable for linux/
This directory can benefit from this script to prevent common mistakes
when submitting patches.

In order to accomplish this:
Do not ignore anymore files from the linux/ directory.
Ignore missing LINUX_EXT_ prefix as the variables for linux extensions
do not use it.
Ignore this symbol:
 - LINUX_EXTENSIONS: defined by each linux extension, used by
   linux/linux.mk.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-04-01 08:54:07 +02:00
Ricardo Martincoski
0c83673638 check-package: enable for fs/
The filesystem types can benefit from this script to prevent common
mistakes when submitting patches.

In order to accomplish this:
Do not ignore anymore files from the fs/ directory.
Ignore fs/common.mk as it declares a package infra and not a package itself.
Register the ROOTFS_ as a valid prefix for variables.
Ignore these symbols:
 - PACKAGES_PERMISSIONS_TABLE: defined either by packages through
   pkg-generic or by filesystem types, used by fs/common.mk;
 - SUMTOOL: defined by package mtd, used by filesystem jffs2;
 - TARGETS_ROOTFS: defined by filesystem types, used in the main
   Makefile.

Keep using loose checks that warn about common mistakes while keep the
code simple.
As a consequence the check functions do not differentiate between
packages and filesystems so the symbol PACKAGE_UBI would not generate a
warning for the ubi filesystem neither the symbol ROOTFS_MTD would
generate a warning for the mtd package. But those kind of mistakes are
not common and are obvious in the code review, unlike typos i.e.
ROOTFS_UBl or PACKAGE_MID that would be hard to see in the code review.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-04-01 08:52:45 +02:00
Ricardo Martincoski
79936be11a check-package: enable for boot/
This directory can benefit from this script to prevent common mistakes
when submitting patches.

In order to accomplish this:
Do not ignore anymore files from the boot/ directory.
Ignore boot/barebox/barebox.mk as it declares a package infra and not a
package itself.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-04-01 08:49:18 +02:00
Ricardo Martincoski
560ab2cb95 check-package: enable for arch/ and system/
These directories can benefit from this script to prevent common
mistakes when submitting patches.

In order to accomplish this:
Do not ignore anymore files from these directories.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-04-01 08:01:05 +02:00
Ricardo Martincoski
a7b4bbfc14 check-package: prepare to extend to other directories
Currently the script only checks files inside the package/ directory.
Upcoming patches will enable it for other directories.

In order to reliably test for file names, i.e. the Config.in in the base
directory, normalize the path of files to check to a relative path to
the base directory.

Rename the variable that holds the compiled regexp to better represent
its content and rearrange how it is declared to make easy to later add
new directories to check. As a consequence the files that declare
package infra types would not be ignored anymore, so create a new
variable to list the files intree to be ignored during the check. The
same variable will be used by upcoming patches to ignore other files.
Ignore pkg-*.mk and doc-asciidoc.mk since they are package infra files.

In order to not produce weird results when used for files outside the
tree (i.e. in a private br2-external) add an explicit command line
option (-b) that bypasses any checks that would make a file be ignored
by the path that contains it.
When in this out-of-tree mode, the user is responsible for providing a
list of files to check that do not contain files the script does not
understand, e.g. package infra files.

As a result of this patch, besides the known use:
$ ./utils/check-package package/new-package/*
someone with the utils/ directory in the path can now also run:
$ cd package/new-package/
$ check-package *
or
$ check-package -b /path/to/br2-ext-tree/package/staging-package/*

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-04-01 07:58:07 +02:00
Thomas Petazzoni
7ca9fc3170 tools: rename to 'utils'
After some discussion, we found out that "tools" has the four first
letters identical to the "toolchain" subfolder, which makes it a bit
unpractical with tab-completion. So, this commit renames "tools" to
"utils", which is more tab-completion-friendly.

This has been discussed with Arnout and Yann.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
2017-07-01 18:07:00 +02:00