Commit Graph

51 Commits

Author SHA1 Message Date
Fabrice Fontaine
4dc48e8b9d utils/checkpackagelib/lib_mk.py: check CONF_OPTS
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>
2022-03-07 23:15:46 +01:00
Arnout Vandecappelle (Essensium/Mind)
b42e87772a utils/checkpackagelib/test_tool.py: fix expectation
While committing the shellcheck feature, it was changed to output the
full shellcheck output even at verbosity level 1. However, the
expectation of the shellcheck test was not updated accordingly.

Do that now, simply merging all the shellcheck output in a single
string.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2022-02-06 19:31:44 +01:00
Ricardo Martincoski
d409fe0856 utils/checkpackagelib/lib_sysv: run shellcheck
For simplicity, when shellcheck returns one or more warnings, count it
as a single check-package warning.
The developer can get the full output either by running shellcheck
directly or by calling check-package with -v.

Examples:

|$ ./utils/docker-run utils/check-package --include Shellcheck package/polkit/S50polkit
|package/polkit/S50polkit:0: run 'shellcheck' and fix the warnings
|51 lines processed
|1 warnings generated

|$ ./utils/docker-run utils/check-package --include Shellcheck -v package/polkit/S50polkit
|package/polkit/S50polkit:0: run 'shellcheck' and fix the warnings
|In package/polkit/S50polkit line 43:
|< tab  >start|stop|restart|reload)
|                           ^----^ SC2221: This pattern always overrides a later one on line 45.
|In package/polkit/S50polkit line 45:
|< tab  >reload)
|        ^----^ SC2222: This pattern never matches because of a previous pattern on line 43.
|For more information:
|  https://www.shellcheck.net/wiki/SC2221 -- This pattern always overrides a l...
|  https://www.shellcheck.net/wiki/SC2222 -- This pattern never matches becaus...
|51 lines processed
|1 warnings generated

NOTICE: shellcheck results depends on the version of the tool.
This is why the examples above run inside the docker image.

Also update .gitlab-ci.yml with the docker image after the change of
the previous commit. We don't actually use shellcheck in CI, but the
image from .gitlab-ci.yml is used by the docker-run script and we use
that to run shellcheck.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Arnout: make sure a single -v is enough to get shellcheck output;
 update docker image tag in .gitlab-ci.yml]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2022-02-06 18:27:03 +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
7947328de4 utils/checkpackagelib: warn about executable files
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>
2022-02-06 18:26:20 +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
Ricardo Martincoski
fc254881e6 utils/checkpackagelib: add unit tests
So anyone willing to contribute to check-package can run all tests in
less than 1 second by using:
$ python3 -m pytest -v utils/checkpackagelib/

Most test cases are in the form:
@pytest.mark.parametrize('testname,filename,string,expected', function)
 - testname: a short description of the scenario tested, added in order
   to improve readability of the log when some tests fail
 - filename: the filename the check-package function being tested thinks
   it is testing
 - string: the content of the file being sent to the function under test
 - expected: all expected warnings that a given function from
   check-package should generate for a given file named filename and
   with string as its content.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Romain Naour <romain.naour@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2022-02-06 15:35:19 +01:00
Yann E. MORIN
5da8218184 support/download: drop support for the 'none' hash
Commit 1ba85b7f87 (support/download: add explicit no-hash support)
introduced the 'none' hash type, in an attempt to make hash files
mandatory, but not failing on archives localy generated, like those
for git or svn repositories, especially for those packages where a
version choice was present, which would allow for either remote
archives for which we'd have a hash or VCS trees for which we could
not have a hash for the localy generated archive.

Indeed, back in the time, we did not have a mean to generate
reproducible archives, so having a hash file without a hash for
thosel ocally generated archives would trigger an error in the
hash-checking machinery.

But now, low-and-behold, we do know how to generate those archives,
and we have a mechanism to explicitly exclude some archives from being
hash-checked (e.g. when the version string itself can be user-provided).

As such, the 'none' hash type no longer has any raison d'être, we do not
use it in-tree, and its use in a br2-external tree is most probably
inexistent (as is the use of hash files alotgether most probably).

So we simply drop the support for that.

Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
[Thomas: drop support in checkpackagelib, as reported by Ricardo.]
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2022-01-11 21:58:01 +01:00
Heiko Thiery
f35a4b4ae2 utils/check-package: add a check for the new spacing convention
The seperation of the fields in the hash file should be 2 spaces for
consistency.

Since a large number of hash files still violate this rule, exclude it
from "make check-package" (and thus from CI).

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
[Arnout:
 - Move it to a separate class, so it can be excluded.
 - Exclude it from "make check-package"
]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2022-01-09 23:01:53 +01:00
Ricardo Martincoski
5bbedea9c2 utils/checkpackagelib/lib_mk.py: fix check for overridden variable
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>
2021-12-10 20:01:40 +01:00
Fabrice Fontaine
4910a175b3 utils/checkpackagelib/lib_mk.py: check DEPENDENCIES
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>
2021-12-10 19:26:30 +01:00
Fabrice Fontaine
fba568a16e utils/checkpackagelib/lib_mk.py: check typo in define
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>
2021-12-06 22:12:34 +01:00
Thomas De Schampheleire
a1bb132a81 utils/checkpackagelib/lib_mk.py: handle 'else' and 'elif' statements
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>
2021-01-02 13:54:59 +01: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
Yann E. MORIN
76772cefd8 utils/check-package: ignore ACLOCAL_PATH
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>
2020-02-04 17:15:19 +01:00
Yann E. MORIN
1e03a2e89e utils/check-package: report := that appends to variables
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-10-27 10:35:06 +01:00
Jerzy Grzegorek
31fc6a1fe4 utils/checkpackagelib: CommentsMenusPackagesOrder: add more Config.in files to check
Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Arnout: remove boot/Config.in, it is not ordered correctly yet.]
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-10-07 23:39:21 +02:00
Jerzy Grzegorek
16271ec356 utils/checkpackagelib: CommentsMenusPackagesOrder: initialize 'menu_of_packages' array
'source' without a previous 'menu' is common in package/Config.in in
br2-externals.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-10-07 23:39:21 +02:00
Jerzy Grzegorek
b47590fdb0 utils/checkpackagelib: CommentsMenusPackagesOrder: initialize in before()
This makes sure the state from a previous run (previous file) can never
leak over into the next file.

Also order the initializations alphabetically.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Acked-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-10-07 23:39:21 +02:00
Jerzy Grzegorek
6a5f6ea7e2 utils/checkpackagelib: CommentsMenusPackagesOrder: use '-' to describe state
Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-10-07 23:39:21 +02:00
Jerzy Grzegorek
0625899e3b utils/checkpackagelib: CommentsMenusPackagesOrder: separate comment/if/menu cases
The handling of 'comment...', 'if ...' and 'menu ...' lines have almost
nothing in common, and subsequent patches will give them even less in
common. Therefore, completely separate their handling in top-level
conditions. The only code that gets duplicated in the different branches
is the 'self.initialize_level_elements(text)' call.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-10-07 23:39:21 +02:00
Jerzy Grzegorek
dd251d68e5 utils/checkpackagelib: CommentsMenusPackagesOrder: remove '-comment' state before the '-menu' one
A comment is considered an alternative delimiter like a menu. I.e.,
a menu that comes after a comment should not be considered a submenu of
that comment. Therefore, remove the '-comment' state before adding the
'-menu' one.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
2019-10-07 23:39:21 +02:00
Jerzy Grzegorek
c82e0092f5 utils/checkpackagelib: CommentsMenusPackagesOrder: change the type of variable 'new_package'
Change the type of variable 'new_package' to make it a class member.
It will be used not only locally. Also initialize it.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2019-09-25 22:24:42 +02:00
Jerzy Grzegorek
7197d3cb80 utils/checkpackagelib: CommentsMenusPackagesOrder: add functions to initialize arrays elements
Factor out two functions to initialize arrays elements. They will be
reused by followup patches.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2019-09-25 22:24:42 +02:00
Jerzy Grzegorek
4e88b37274 utils/checkpackagelib: CommentsMenusPackagesOrder: get value of variable 'level'
Get value of variable 'level' only just after the state change.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2019-09-25 22:24:42 +02:00
Jerzy Grzegorek
6748e42ace utils/checkpackagelib: CommentsMenusPackagesOrder: change the type of variable 'level'
Change the type of variable "level" to make it a class member.
It will be used not only locally.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
[Thomas: initialize self.level in the before() method, as suggested by
Ricardo]
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2019-09-25 22:24:42 +02:00
Jerzy Grzegorek
dd99dc5763 utils/checkpackagelib: CommentsMenusPackagesOrder: rename variable 'm'
Rename variable 'm' for better readability.

Signed-off-by: Jerzy Grzegorek <jerzy.m.grzegorek@gmail.com>
Cc: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2019-09-25 22:24:39 +02:00
Ricardo Martincoski
1842bb1470 check-package: fix check of file in current dir with -b
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>
2019-08-03 17:17:29 +02:00
Arnout Vandecappelle (Essensium/Mind)
d5990601ba utils/checkpackagelib: CommentsMenusPackagesOrder: properly initialize levels
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>
2019-08-02 18:43:52 +02:00
Arnout Vandecappelle (Essensium/Mind)
0a4e1fc054 utils/checkpackagelib: CommentsMenusPackagesOrder: use regex for source
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>
2019-08-01 22:48:59 +02:00
Jerzy Grzegorek
83a34f7705 utils/checkpackagelib: CommentsMenusPackagesOrder: append elements to arrays if needed
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>
2019-08-01 22:48:59 +02:00
Arnout Vandecappelle (Essensium/Mind)
c62a282920 utils/checkpackagelib: CommentsMenusPackagesOrder: fix 'menuconfig' handling
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>
2019-08-01 22:48:59 +02:00
Arnout Vandecappelle (Essensium/Mind)
5603406bd1 utils/checkpackagelib: CommentsMenusPackagesOrder: only apply to top-level
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>
2019-08-01 09:58:15 +02:00
Jerzy Grzegorek
e36a63cf6b checkpackagelib/lib_config.py: check packages alphabetical order in {Config.in, Config.in.host}
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>
2019-07-14 00:21:26 +02:00
Peter Seiderer
b9b081d53a utils/check-package: fix flake8 warning
Fix flake 8 warning (introduced with commit [1]):

  utils/checkpackagelib/lib.py:56:1: E302 expected 2 blank lines, found 1

[1] https://git.buildroot.net/buildroot/commit/?id=8e352c32b0beded97a8a5c1e9edc9d618514ee7b

Reported-by: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
2019-05-20 10:20:38 +02:00
Peter Seiderer
8e352c32b0 utils/check-package: warn about utf-8 characters in .mk files
Signed-off-by: Peter Seiderer <ps.report@gmx.net>
Tested-by: Titouan Christophe <titouan.christophe@railnova.eu>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
2019-05-18 23:34:59 +02:00
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
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
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
Thomas Petazzoni
73dcbc19e9 utils/checkpackagelib: exclude four files from Config.in indentation check
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>
2018-04-01 10:16:21 +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
Yann E. MORIN
efb61ae07b support/check-package: don't check filenames of hashes
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>
2017-07-03 17:33:22 +02:00