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>
Fix an issue introduced by Arnout while committing. Jerzy originally
initialized the menu_of_packages, package and print_package_warning
members like they should be, but Arnout thought it wasn't needed and
removed that.
It is actually needed, to make sure the top level (level 0) works.
Fixes: https://gitlab.com/buildroot.org/buildroot/-/jobs/264383157
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
The 'source' strings identify which package is incorrectly ordered. We
need to extract the actual package name from that string, which is
currently done with constants that assume the file is package/Config.in.
In addition, only 'source' lines that are indented with a tab are
checked. This kind of indentation is done in package/Config.in, but not
e.g. boot/Config.in.
Therefore, use a regular expression to match the 'source' lines, and to
extract the directory part from it.
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
In the future, the nesting level of menus, comments and conditions may
increase. The fixed array length used now is not appropriate. Therefore,
append elements to the arrays if needed.
Also change order of variables.
Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
The CommentsMenusPackagesOrder check builds the 'state' to track the
depth of menus and conditions. However, a menuconfig doesn't create a
menu by itself - it is always followed by a condition that implies the
menu. As a result, when unwinding the 'state', the level will be wrong.
Fix this by checking for menu followed by a space, so it no longer
matches menuconfig. For consistency, do the same for comment and if
as well.
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
The CommentsMenusPackagesOrder test is broken in various ways for files
other than package/Config.in and package/Config.in.host. Therefore, the
script gives bogus errors for various other Config.in files.
However, we don't really want to check those other files. Indeed, many
of them have a non-alphabetical ordering for good reasons.
Therefore, skip the check for files other than package/Config.in and
package/Config.in.host.
Fixes: https://gitlab.com/buildroot.org/buildroot/-/jobs/251214899
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Cc: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Acked-by: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
[Arnout:
- calculate level by counting - instead of with a static array;
- new_package is only used locally, so don't make it a class member;
- do indentation according to length of prefix;
- don't split string in the middle of a line;
- report first wrong package per menu;
- do replace() only once;
- add comment why we do replace().
]
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>
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>
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>
package/Config.in, package/Config.in.host, package/x11r7/Config.in and
package/kodi/Config.in do not comply with the normal Config.in
indentation rules. However, this violation of the rule is legitimate, so
let's skip them in check-package for this specific indentation check.
This removes the last 2197 remaining warnings on Config.in files.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
[Ricardo: rebase patch to use relative paths passed by the main script,
fix flake8 warnings, add package/Config.* to the list]
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>
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>
Currently, we check that the filenames in hash lists do not contain
a slash '/' character, because all we are checking so far are the
downloaded archives, and we explicitly need the filename to not contain
a directory component at all.
However, we're soon to also check the hashes of the license files in
packages sources, and those license files may be at any arbitrary
directory-depth in the packages source tree.
[Peter: Remove reference to files with same basename]
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Rahul Bedarkar <rahulbedarkar89@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Acked-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Peter Korsgaard <peter@korsgaard.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>
