It's been ages (5 years at the next release) that we've not installed
host packages in $(HOST_DIR)/usr, but we still have a few packages that
reference it or install things in there. See [1]
Add a new check_function that warns when a file is added installing to
or referencing $(HOST_DIR)/usr .
[1] "d9ff62c4cd pacakge: drop remnants of $(HOST_DIR)/usr"
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Arnout: exclude skeleton.mk with disable comment instead of explicit
code]
Signed-off-by: Arnout Vandecappelle <arnout@mind.be>
There are two legitimate cases to prefer ifdef over ifeq in package
recipes: command-line overrides are allowed for busybox and uclibc
configs.
Except for that, all package in tree already use ifeq, so warn the
developer adding/changing a package to use ifeq instead of ifdef, in
order to keep consistence across packages.
file.mk:2: use ifeq ($(SYMBOL),y) instead of ifdef SYMBOL
file.mk:5: use ifneq ($(SYMBOL),y) instead of ifndef SYMBOL
The difference between ifeq and ifdef is that ifdef doesn't expand
recursively.
Add comments to busybox and uclibc packages to avoid a warning in such
special cases.
Cc: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
As already done for {FOO}_DEPENDENCIES in commit
4910a175b3, check that {FOO}_CONF_OPTS are
never overridden in a conditional
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Currently there are no .mk, Config.in, .patch or .hash files with
executable permissions in the tree.
But we don't want to have that.
So warn when a file checked by check-package has executable permission.
This check will be reused when testing SysV init scripts in the tree.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Arnout: use context manager for temp dir so it gets deleted]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Currently this .mk snippet results in unexpected behavior from
check-package:
|VAR_1 = VALUE1
|ifeq (condition)
|VAR_1 := $(VAR_1), VALUE2
|endif
Fix commit "163f160a8e utils/{check-package, checkpackagelib}:
consistently use raw strings for re.compile" that ended up doing this:
- CONCATENATING = re.compile("^([A-Z0-9_]+)\s*(\+|:|)=\s*\$\(\\1\)")
+ CONCATENATING = re.compile(r"^([A-Z0-9_]+)\s*(\+|:|)=\s*\$\(\\1\)")
But raw strings do not expect escaping when referencing \1 and the
pattern ends up searching for a raw '\\1' instead of an occurrence of
the first pattern inside parenthesis.
|$ python3
|Python 3.8.10 (default, Sep 28 2021, 16:10:42)
|[GCC 9.3.0] on linux
|Type "help", "copyright", "credits" or "license" for more information.
|>>> import re
|>>> p1 = re.compile('(foo)bar\\1')
|>>> p2 = re.compile(r'(foo)bar\\1')
|>>> p3 = re.compile(r'(foo)bar\1')
|>>> s1 = 'foobarfoo'
|>>> s2 = 'foobar\\1'
|>>> print(p1.search(s1))
|<re.Match object; span=(0, 9), match='foobarfoo'>
|>>> print(p2.search(s1))
|None
|>>> print(p3.search(s1))
|<re.Match object; span=(0, 9), match='foobarfoo'>
|>>> print(p1.search(s2))
|None
|>>> print(p2.search(s2))
|<re.Match object; span=(0, 8), match='foobar\\1'>
|>>> print(p3.search(s2))
|None
|>>>
So use '\1' instead of '\\1' in the raw string.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Titouan Christophe <titouan.christophe@railnova.eu>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Check that {FOO}_DEPENDENCIES are never overriden in a conditional
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Check typo in define to detect SMAKE_LINUX_CONFIG_FIXUPS in smack
(fixed by 41e2132fbe)
The new expression will catch "SMAKE_CONF_OPTS" as well as
"define SMAKE_LINUX_CONFIG_FIXUPS"
Two modifications were made:
- add (define\s+)? which will match "define " but also an empty value.
Thanks to this, the second group will always contain the variable
name.
- remove \s*(\+|)= which seems superfluous
Also, add GCC_TARGET in ALLOWED variable to avoid the following
warnings:
arch/arch.mk:12: possible typo: GCC_TARGET_ARCH -> *ARCH*
arch/arch.mk:13: possible typo: GCC_TARGET_ABI -> *ARCH*
arch/arch.mk:14: possible typo: GCC_TARGET_NAN -> *ARCH*
arch/arch.mk:15: possible typo: GCC_TARGET_FP32_MODE -> *ARCH*
arch/arch.mk:16: possible typo: GCC_TARGET_CPU -> *ARCH*
arch/arch.mk:17: possible typo: GCC_TARGET_FPU -> *ARCH*
arch/arch.mk:18: possible typo: GCC_TARGET_FLOAT_ABI -> *ARCH*
arch/arch.mk:19: possible typo: GCC_TARGET_MODE -> *ARCH*
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
An 'else' or 'elif' clause inside a make conditional should not be indented
in the same way as the if/endif clause. check-package did not recognize the
else statement and expected an indentation.
For example:
ifdef FOOBAR
interesting
else
more interesting
endif
would, according to check-package, need to become:
ifdef FOOBAR
interesting
else
more interesting
endif
Treat 'else' and 'elif' the same as if-like keywords in the Indent test, but
take into account that 'else' is also valid shell, so we need to correctly
handle line continuation to prevent complaining about the 'else' in:
ifdef FOOBAR
if true; \
... \
else \
... \
fi
endif
We don't add the 'else' and 'elif' statements to start_conditional, because
it would cause incorrect nesting counting in class OverriddenVariable.
Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
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>
The aclocal program is provided by the automake package, so it makes
sense to define aclocal-related variables in automake.mk.
Add an exception to check-package to ignore that variable.
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
One of the possible usages of check-package is to first cd to the
directory that contains the files to test (e.g. a package directory) and
then call the script passing the files in the current dir.
It already works when used for intree files, but for files in a
br2-external it throws an exception because some check functions (from
utils/checkpackagelib/lib_*.py) do need the name of the file being
processed and assume there will be a slash before the name.
Fix all check functions that assume that the full filename being checked
contains a slash. Do not use regexps to extract the filename, use
os.path functions instead.
Notice RemoveDefaultPackageSourceVariable and TypoInPackageVariable lead
to an exception in this case, but ApplyOrder instead generates a false
warning.
Fixes 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>
Cc: Vitaliy Lotorev <lotorev@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
