Commit Graph

12 Commits

Author SHA1 Message Date
Ricardo Martincoski
b03fa5d96f utils/check-package: warn about overridden variables
For the general case, appending values to variables is OK and also a
good practice, like this:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR += value2

or this, when the above is not possible:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR := $(PACKAGE_VAR), value2

But this override is an error:
|PACKAGE_VAR = value1
|PACKAGE_VAR = value2

as well this one:
|ifeq ...
|PACKAGE_VAR += value1
|endif
|PACKAGE_VAR = value2

And this override is error-prone:
|PACKAGE_VAR = value1
|ifeq ...
|PACKAGE_VAR = value2

Create a check function to warn about overridden variables.

Some variables are likely to have a default value that gets overridden
in a conditional, so ignore them. The name of such variables end in
_ARCH, _CPU, _SITE, _SOURCE or _VERSION.

After ignoring these variable names, there are a few exceptions to this
rule in the tree. For them use the comment that disables the check.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Simon Dawson <spdawson@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Tested-by: Titouan Christophe <titouan.christophe@railnova.eu>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
2019-02-05 20:24:57 +01:00
Ricardo Martincoski
fe7a5d7120 utils/check-package: handle ifdef/ifndef in .mk files
Currently check-package only knows about ifeq/ifneq.
Add code to handle ifdef/ifndef as well.

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:38:41 +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
Ricardo Martincoski
910ee6f7bd utils/check-package: detect the use of ${} in .mk files
And warn to use $() instead.
For examples see [1] and [2].

In the regexp, search for ${VARIABLE} but:
 - ignore comments;
 - ignore variables to be expanded by the shell "$${}".

[1] http://lists.busybox.net/pipermail/buildroot/2018-July/225211.html
[2] 36305380db

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-09-21 00:05:55 +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
Nicolas Cavallari
42be6b6153 check-package: Flag usage of '\t \\$'.
check-package would flag tabs before a backslash ('\t\\'),
two spaces before a backslash ('  \\') but would not flag a tab before space
before backslash ('\t \\'), allowing someone to bypass the check.

Signed-off-by: Nicolas Cavallari <nicolas.cavallari@green-communications.fr>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2018-03-31 23:02:14 +02:00
Ricardo Martincoski
deb31a979a check-package: fix code style
Ignore these warnings:
F401 'lib.ConsecutiveEmptyLines' imported but unused

And remove comments that are not needed anymore.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
2018-01-29 23:14:24 +01:00
Jerzy Grzegorek
2f5421cb2c utils/checkpackagelib: add function to check of the default package source variable
Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Tested-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
2018-01-15 23:27:02 +01:00
Ricardo Martincoski
29d9f2f014 check-package: avoid false warning of useless flag
Just AUTORECONF = NO is redundant.
Just HOST_AUTORECONF = NO is redundant.
But the combination of AUTORECONF = YES + HOST_AUTORECONF = NO is valid.

So basically for all variables that have inheritance between target and
host, having the host variant of the variable set the variable value
back to its default is correct if the target variable is set.

Instead of increasing complexity of the script to fully detect this
case, ignore the host flag set to its default value as it can be
overriding a non-default value inherited from the equivalent target
flag.

Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Reported-by: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@datacom.ind.br>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
2017-12-02 14:51:27 +01: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