* [Buildroot] [PATCH v2 01/14] utils/check-package: decouple adding rules from fixing all intree files
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-02-06 21:22 ` Thomas Petazzoni via buildroot
2022-07-31 19:35 ` [Buildroot] [PATCH v2 02/14] support/testing: test check-package ignore list Ricardo Martincoski
` (13 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Sen Hastings, Ricardo Martincoski
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.
Add also a new option --ignore-list that can be used:
- by pkg-stats
- by developers willing to reduce the intree ignore list
- by developers of br2-external trees that want its own ignore list
- for debug and tests purposes
When using for a br2-external each entry in the ignore file is relative
to the directory that contains the ignore file. Since calling
check-package -b already uses relative paths to the directory it was
called from, when a developer wants a ignore file for a br2-external
he/she must call check-package from the directory that contains it.
For instance, with these files:
/path/to/br2-external/.checkpackageignore
/path/to/br2-external/package/mypackage/mypackage.mk
The file .checkpackageignore must contain entries in the format:
package/mypackage/mypackage.mk EmptyLastLine
and the developer must call check-package this way:
$ cd /path/to/br2-external/ && \
/otherpath/to/check-package \
--ignore-list=.checkpackageignore \
-b package/mypackage/mypackage.mk
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>
---
Changes v1 -> v2:
- rename the patch
Is is intentional to NOT add .checkpackageignore to my DEVELOPER entry,
I don't want to be notified for every check-package warning that once
was ignored and then becomes fixed.
---
.checkpackageignore | 0
support/scripts/pkg-stats | 2 +-
utils/check-package | 87 ++++++++++++++++++++++++++++++++-------
3 files changed, 72 insertions(+), 17 deletions(-)
create mode 100644 .checkpackageignore
diff --git a/.checkpackageignore b/.checkpackageignore
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/support/scripts/pkg-stats b/support/scripts/pkg-stats
index d795131cef..8886fb30bc 100755
--- a/support/scripts/pkg-stats
+++ b/support/scripts/pkg-stats
@@ -267,7 +267,7 @@ class Package:
"""
Fills in the .warnings and .status['pkg-check'] fields
"""
- cmd = [os.path.join(brpath, "utils/check-package")]
+ cmd = [os.path.join(brpath, "utils/check-package"), "--ignore-list="]
pkgdir = os.path.dirname(os.path.join(brpath, self.path))
self.status['pkg-check'] = ("error", "Missing")
for root, dirs, files in os.walk(pkgdir):
diff --git a/utils/check-package b/utils/check-package
index f64daed84c..00be29d02a 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -15,10 +15,37 @@ import checkpackagelib.lib_mk
import checkpackagelib.lib_patch
import checkpackagelib.lib_sysv
+IGNORE_FILENAME = ".checkpackageignore"
VERBOSE_LEVEL_TO_SHOW_IGNORED_FILES = 3
flags = None # Command line arguments.
+def get_ignored_parsers_per_file(intree_only, ignore_filename):
+ ignored = dict()
+ entry_base_dir = ''
+
+ if ignore_filename == '':
+ return ignored # the user explicitly asked to not use any ignore list
+
+ if ignore_filename:
+ filename = os.path.abspath(ignore_filename)
+ if not intree_only:
+ # for br2-external the entries are relative to the path were the ignore list is
+ entry_base_dir = os.path.join(os.path.dirname(filename))
+ else:
+ if not intree_only:
+ return ignored # do not use the intree ignore list when testing files in a br2-external
+ base_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
+ filename = os.path.join(base_dir, IGNORE_FILENAME)
+
+ with open(filename, "r") as f:
+ for line in f.readlines():
+ filename, warnings_str = line.split(' ', 1)
+ warnings = warnings_str.split()
+ ignored[os.path.join(entry_base_dir, filename)] = warnings
+ return ignored
+
+
def parse_args():
parser = argparse.ArgumentParser()
@@ -29,6 +56,8 @@ def parse_args():
parser.add_argument("--br2-external", "-b", dest='intree_only', action="store_false",
help="do not apply the pathname filters used for intree files")
+ parser.add_argument("--ignore-list", dest='ignore_filename', action="store",
+ help='override the default list of ignored warnings')
parser.add_argument("--manual-url", action="store",
default="http://nightly.buildroot.org/",
@@ -44,7 +73,11 @@ def parse_args():
parser.add_argument("--dry-run", action="store_true", help="print the "
"functions that would be called for each file (debug)")
- return parser.parse_args()
+ flags = parser.parse_args()
+
+ flags.ignore_list = get_ignored_parsers_per_file(flags.intree_only, flags.ignore_filename)
+
+ return flags
CONFIG_IN_FILENAME = re.compile(r"Config\.\S*$")
@@ -120,21 +153,25 @@ def is_external_tool(m):
return common_inspect_rules(m)
-def print_warnings(warnings):
+def print_warnings(warnings, xfail):
# Avoid the need to use 'return []' at the end of every check function.
if warnings is None:
- return 0 # No warning generated.
+ return 0, 0 # No warning generated.
+ if xfail:
+ return 0, 1 # Warning not generated, fail expected for this file.
for level, message in enumerate(warnings):
if flags.verbose >= level:
print(message.replace("\t", "< tab >").rstrip())
- return 1 # One more warning to count.
+ return 1, 1 # One more warning to count.
def check_file_using_lib(fname):
# Count number of warnings generated and lines processed.
nwarnings = 0
nlines = 0
+ xfail = flags.ignore_list.get(fname, [])
+ failed = set()
lib = get_lib_from_filename(fname)
if not lib:
@@ -150,10 +187,13 @@ def check_file_using_lib(fname):
print("{}: would run: {}".format(fname, functions_to_run))
return nwarnings, nlines
- objects = [c[1](fname, flags.manual_url) for c in internal_functions]
+ objects = [[c[0], c[1](fname, flags.manual_url)] for c in internal_functions]
- for cf in objects:
- nwarnings += print_warnings(cf.before())
+ for name, cf in objects:
+ warn, fail = print_warnings(cf.before(), name in xfail)
+ if fail > 0:
+ failed.add(name)
+ nwarnings += warn
if six.PY3:
f = open(fname, "r", errors="surrogateescape")
else:
@@ -161,19 +201,34 @@ def check_file_using_lib(fname):
lastline = ""
for lineno, text in enumerate(f.readlines()):
nlines += 1
- for cf in objects:
+ for name, cf in objects:
if cf.disable.search(lastline):
continue
- nwarnings += print_warnings(cf.check_line(lineno + 1, text))
+ warn, fail = print_warnings(cf.check_line(lineno + 1, text), name in xfail)
+ if fail > 0:
+ failed.add(name)
+ nwarnings += warn
lastline = text
f.close()
- for cf in objects:
- nwarnings += print_warnings(cf.after())
-
- tools = [c[1](fname) for c in external_tools]
-
- for tool in tools:
- nwarnings += print_warnings(tool.run())
+ for name, cf in objects:
+ warn, fail = print_warnings(cf.after(), name in xfail)
+ if fail > 0:
+ failed.add(name)
+ nwarnings += warn
+
+ tools = [[c[0], c[1](fname)] for c in external_tools]
+
+ for name, tool in tools:
+ warn, fail = print_warnings(tool.run(), name in xfail)
+ if fail > 0:
+ failed.add(name)
+ nwarnings += warn
+
+ for should_fail in xfail:
+ if should_fail not in failed:
+ print("{}:0: {} was expected to fail, did you fixed the file and forgot to update {}?"
+ .format(fname, should_fail, IGNORE_FILENAME))
+ nwarnings += 1
return nwarnings, nlines
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [PATCH v2 01/14] utils/check-package: decouple adding rules from fixing all intree files
2022-07-31 19:35 ` [Buildroot] [PATCH v2 01/14] utils/check-package: decouple adding rules from fixing all intree files Ricardo Martincoski
@ 2023-02-06 21:22 ` Thomas Petazzoni via buildroot
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-06 21:22 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: Sen Hastings, buildroot
On Sun, 31 Jul 2022 16:35:08 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> 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.
>
> Add also a new option --ignore-list that can be used:
> - by pkg-stats
> - by developers willing to reduce the intree ignore list
> - by developers of br2-external trees that want its own ignore list
> - for debug and tests purposes
>
> When using for a br2-external each entry in the ignore file is relative
> to the directory that contains the ignore file. Since calling
> check-package -b already uses relative paths to the directory it was
> called from, when a developer wants a ignore file for a br2-external
> he/she must call check-package from the directory that contains it.
> For instance, with these files:
> /path/to/br2-external/.checkpackageignore
> /path/to/br2-external/package/mypackage/mypackage.mk
> The file .checkpackageignore must contain entries in the format:
> package/mypackage/mypackage.mk EmptyLastLine
> and the developer must call check-package this way:
> $ cd /path/to/br2-external/ && \
> /otherpath/to/check-package \
> --ignore-list=.checkpackageignore \
> -b package/mypackage/mypackage.mk
>
> 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>
> ---
> Changes v1 -> v2:
> - rename the patch
We finally applied this, but we did one change: now by default
check-package does not ignore any error. You have to explicitly pass it
an ignore list for it to ignore some errors. This way by default
check-package is noisy, and you need to do some action to silence some
of the warnings.
Thanks a lot!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH v2 02/14] support/testing: test check-package ignore list
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
2022-07-31 19:35 ` [Buildroot] [PATCH v2 01/14] utils/check-package: decouple adding rules from fixing all intree files Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-02-06 21:23 ` Thomas Petazzoni via buildroot
2022-07-31 19:35 ` [Buildroot] [PATCH v2 03/14] Makefile: add target to update .checkpackageignore Ricardo Martincoski
` (12 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
Extend test_check_package to also check the ignore list functionality.
Check:
- the entries in the ignore list use relative path;
- an entry in the ignore list actually ignores the warning;
- an outdated entry in the ignore list generates a warning by its own,
preventing the ignoring list to grow indefinitely.
For this to work, add 3 test fixtures, listing entries for an
pre-existing file in the br2-external used in the test.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- none
---
.../utils/br2-external/.checkpackageignore | 1 +
.../br2-external/package/.checkpackageignore | 1 +
.../package/.checkpackageignore_outdated | 1 +
.../testing/tests/utils/test_check_package.py | 31 +++++++++++++++++++
4 files changed, 34 insertions(+)
create mode 100644 support/testing/tests/utils/br2-external/.checkpackageignore
create mode 100644 support/testing/tests/utils/br2-external/package/.checkpackageignore
create mode 100644 support/testing/tests/utils/br2-external/package/.checkpackageignore_outdated
diff --git a/support/testing/tests/utils/br2-external/.checkpackageignore b/support/testing/tests/utils/br2-external/.checkpackageignore
new file mode 100644
index 0000000000..efb7680173
--- /dev/null
+++ b/support/testing/tests/utils/br2-external/.checkpackageignore
@@ -0,0 +1 @@
+package/external/external.mk PackageHeader
diff --git a/support/testing/tests/utils/br2-external/package/.checkpackageignore b/support/testing/tests/utils/br2-external/package/.checkpackageignore
new file mode 100644
index 0000000000..5f4a5e1187
--- /dev/null
+++ b/support/testing/tests/utils/br2-external/package/.checkpackageignore
@@ -0,0 +1 @@
+external/external.mk PackageHeader
diff --git a/support/testing/tests/utils/br2-external/package/.checkpackageignore_outdated b/support/testing/tests/utils/br2-external/package/.checkpackageignore_outdated
new file mode 100644
index 0000000000..1df59f3bed
--- /dev/null
+++ b/support/testing/tests/utils/br2-external/package/.checkpackageignore_outdated
@@ -0,0 +1 @@
+external/external.mk Indent NewlineAtEof PackageHeader
diff --git a/support/testing/tests/utils/test_check_package.py b/support/testing/tests/utils/test_check_package.py
index c70ba02324..f21b9e939f 100644
--- a/support/testing/tests/utils/test_check_package.py
+++ b/support/testing/tests/utils/test_check_package.py
@@ -75,6 +75,12 @@ class TestCheckPackage(unittest.TestCase):
generated = int(stderr[1].split()[0])
self.assertGreater(generated, 0)
+ def assert_no_warnings_generated_for_file(self, stderr):
+ """Infer from check-package stderr if no warning was generated and fail otherwise."""
+ self.assertIn("warnings generated", stderr[1], stderr)
+ generated = int(stderr[1].split()[0])
+ self.assertEqual(generated, 0)
+
def test_run(self):
"""Test the various ways the script can be called in a simple top to
bottom sequence."""
@@ -201,3 +207,28 @@ class TestCheckPackage(unittest.TestCase):
self.assert_file_was_processed(m)
self.assert_warnings_generated_for_file(m)
self.assertIn("{}:1: should be 80 hashes (http://nightly.buildroot.org/#writing-rules-mk)".format(abs_file), w)
+
+ # br2-external with ignore list
+ topdir_path = infra.filepath("tests/utils/br2-external")
+ topdir_file = os.path.join(topdir_path, "package/external/external.mk")
+ subdir_path = infra.filepath("tests/utils/br2-external/package")
+ subdir_file = os.path.join(subdir_path, "external/external.mk")
+
+ w, m = call_script(["check-package", "--ignore-list=./.checkpackageignore", "-b", topdir_file],
+ self.WITH_UTILS_IN_PATH, topdir_path)
+ self.assert_file_was_processed(m)
+ self.assert_no_warnings_generated_for_file(m)
+
+ w, m = call_script(["check-package", "--ignore-list=./.checkpackageignore", "-b", subdir_file],
+ self.WITH_UTILS_IN_PATH, subdir_path)
+ self.assert_file_was_processed(m)
+ self.assert_no_warnings_generated_for_file(m)
+
+ w, m = call_script(["check-package", "--ignore-list=./.checkpackageignore_outdated", "-b", subdir_file],
+ self.WITH_UTILS_IN_PATH, subdir_path)
+ self.assert_file_was_processed(m)
+ self.assert_warnings_generated_for_file(m)
+ self.assertIn("{}:0: Indent was expected to fail, did you fixed the file and forgot to update .checkpackageignore?"
+ .format(subdir_file), w)
+ self.assertIn("{}:0: NewlineAtEof was expected to fail, did you fixed the file and forgot to update .checkpackageignore?"
+ .format(subdir_file), w)
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [PATCH v2 02/14] support/testing: test check-package ignore list
2022-07-31 19:35 ` [Buildroot] [PATCH v2 02/14] support/testing: test check-package ignore list Ricardo Martincoski
@ 2023-02-06 21:23 ` Thomas Petazzoni via buildroot
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-06 21:23 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: buildroot
On Sun, 31 Jul 2022 16:35:09 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> Extend test_check_package to also check the ignore list functionality.
> Check:
> - the entries in the ignore list use relative path;
> - an entry in the ignore list actually ignores the warning;
> - an outdated entry in the ignore list generates a warning by its own,
> preventing the ignoring list to grow indefinitely.
>
> For this to work, add 3 test fixtures, listing entries for an
> pre-existing file in the br2-external used in the test.
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
We adjusted the test cases to the changes we did on PATCH 01/14, and
applied. Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH v2 03/14] Makefile: add target to update .checkpackageignore
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
2022-07-31 19:35 ` [Buildroot] [PATCH v2 01/14] utils/check-package: decouple adding rules from fixing all intree files Ricardo Martincoski
2022-07-31 19:35 ` [Buildroot] [PATCH v2 02/14] support/testing: test check-package ignore list Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-02-06 21:23 ` Thomas Petazzoni via buildroot
2022-07-31 19:35 ` [Buildroot] [PATCH v2 04/14] Makefile: make check-package assume a git tree Ricardo Martincoski
` (11 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
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>
---
Changes v1 -> v2:
- on check-package, do not return error code when --failed-only is
used
- add a target to the makefile
- completely rewrite commit message, in order to clarify why it is
needed
---
Makefile | 5 +++++
utils/check-package | 13 ++++++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index a743e42f91..e49080b110 100644
--- a/Makefile
+++ b/Makefile
@@ -1244,6 +1244,11 @@ check-package:
find $(TOPDIR) -type f \( -name '*.mk' -o -name '*.hash' -o -name 'Config.*' -o -name '*.patch' \) \
-exec ./utils/check-package --exclude=Sob {} +
+.PHONY: .checkpackageignore
+.checkpackageignore:
+ find $(TOPDIR) -type f \( -name '*.mk' -o -name '*.hash' -o -name 'Config.*' -o -name '*.patch' \) \
+ -exec ./utils/check-package --exclude=Sob --failed-only {} > .checkpackageignore +
+
include docs/manual/manual.mk
-include $(foreach dir,$(BR2_EXTERNAL_DIRS),$(sort $(wildcard $(dir)/docs/*/*.mk)))
diff --git a/utils/check-package b/utils/check-package
index 00be29d02a..d896e8b117 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -72,11 +72,17 @@ def parse_args():
help="do not run the specified functions (debug)")
parser.add_argument("--dry-run", action="store_true", help="print the "
"functions that would be called for each file (debug)")
+ parser.add_argument("--failed-only", action="store_true", help="print only"
+ " the name of the functions that failed (debug)")
flags = parser.parse_args()
flags.ignore_list = get_ignored_parsers_per_file(flags.intree_only, flags.ignore_filename)
+ if flags.failed_only:
+ flags.dry_run = False
+ flags.verbose = -1
+
return flags
@@ -230,6 +236,11 @@ def check_file_using_lib(fname):
.format(fname, should_fail, IGNORE_FILENAME))
nwarnings += 1
+ if flags.failed_only:
+ if len(failed) > 0:
+ f = " ".join(sorted(failed))
+ print("{} {}".format(fname, f))
+
return nwarnings, nlines
@@ -268,7 +279,7 @@ def __main__():
print("{} lines processed".format(total_lines), file=sys.stderr)
print("{} warnings generated".format(total_warnings), file=sys.stderr)
- if total_warnings > 0:
+ if total_warnings > 0 and not flags.failed_only:
sys.exit(1)
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [PATCH v2 03/14] Makefile: add target to update .checkpackageignore
2022-07-31 19:35 ` [Buildroot] [PATCH v2 03/14] Makefile: add target to update .checkpackageignore Ricardo Martincoski
@ 2023-02-06 21:23 ` Thomas Petazzoni via buildroot
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-06 21:23 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: buildroot
On Sun, 31 Jul 2022 16:35:10 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> 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>
> ---
> Changes v1 -> v2:
> - on check-package, do not return error code when --failed-only is
> used
> - add a target to the makefile
> - completely rewrite commit message, in order to clarify why it is
> needed
> ---
> Makefile | 5 +++++
> utils/check-package | 13 ++++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
Applied to master, thanks.
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH v2 04/14] Makefile: make check-package assume a git tree
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (2 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 03/14] Makefile: add target to update .checkpackageignore Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-02-06 21:23 ` Thomas Petazzoni via buildroot
2022-07-31 19:35 ` [Buildroot] [PATCH v2 05/14] docs/manual: check-package before submitting patch Ricardo Martincoski
` (10 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
... just like check-flake8 already does.
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.
Since now check-package has an ignore file to list all warnings in the
tree, that will eventually be fixed, there is no need to filter the
files passed to check-package.
So test all files in the tree when 'make check-package' is called.
It brings following advantages;
- any new check_function added to check-package takes place immediately
for new files;
- adding new check_functions is less traumatic to the developer doing
this, since he/she does not need anymore to fix all warnings in the
tree before the new check_function takes effect;
- prevent regressions, e.g. ANY new .patch file must have SoB;
- as a side-effect, print a single statistics line as output of
'make ckeck-package'.
But just enabling the check would generate many warnings when
'make check-package' is called, so update the ignore file by using:
$ ./utils/docker-run make .checkpackageignore
Notice: in order to ensure reproducible results, one should run 'make
check-package' and 'make .checkpackageignore' inside the docker image,
otherwise a variation in shellcheck version (installed in the host) can
produce different results.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- use 'make .checkpackageignore' created on previous patch
- update the ignored list of warnings for current master branch
- use Sob as example in the commit message, since the Hashes were fixed
NOTE to the maintainer applying this patch: please re-generate the list
of ignored warnings while applying:
$ ./utils/docker-run make .checkpackageignore
---
.checkpackageignore | 185 ++++++++++++++++++++++++++++++++++++++++++++
Makefile | 7 +-
2 files changed, 188 insertions(+), 4 deletions(-)
diff --git a/.checkpackageignore b/.checkpackageignore
index e69de29bb2..d8947e89fc 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -0,0 +1,185 @@
+package/alsamixergui/0001-misc-fixes.patch Sob
+package/am33x-cm3/S93-am335x-pm-firmware-load Variables
+package/android-tools/0008-Include-sysmacros.h-to-compile-with-glibc-2.28.patch Sob
+package/apache/S50apache Indent Shellcheck Variables
+package/at/S99at Indent Variables
+package/audit/S02auditd Shellcheck Variables
+package/avahi/S05avahi-setup.sh Indent Variables
+package/avahi/S50avahi-daemon Indent Variables
+package/babeld/S50babeld Indent Shellcheck Variables
+package/bind/S81named Indent Shellcheck Variables
+package/bluez5_utils/S40bluetooth NotExecutable Variables
+package/boinc/S99boinc-client Indent Shellcheck Variables
+package/brickd/S70brickd Indent Shellcheck Variables
+package/brltty/S10brltty Indent Shellcheck Variables
+package/busybox/S02sysctl Variables
+package/busybox/S10mdev ConsecutiveEmptyLines Indent Shellcheck
+package/busybox/S15watchdog Indent Variables
+package/busybox/S50telnet Indent Shellcheck Variables
+package/c-icap/S96cicap Indent Shellcheck Variables
+package/cfm/S65cfm Indent Variables
+package/cgroupfs-mount/S30cgroupfs Indent Shellcheck Variables
+package/chrony/S49chrony Indent Shellcheck Variables
+package/connman/S45connman Variables
+package/curlftpfs/0001-fix-CURLOPT_INFILESIZE.patch Sob
+package/curlftpfs/0002-free_ftpfs_file-memleak-fix.patch Sob
+package/curlftpfs/0003-nocache-memleak-fix.patch Sob
+package/dante/S50dante Indent Shellcheck Variables
+package/darkhttpd/S50darkhttpd Indent Shellcheck Variables
+package/dbus/S30dbus Indent Shellcheck TrailingSpace Variables
+package/dcron/S90dcron Variables
+package/dhcp/S80dhcp-relay Shellcheck Variables
+package/dhcp/S80dhcp-server Shellcheck Variables
+package/dhcpcd/S41dhcpcd Indent Variables
+package/dhrystone/0001-cmdline-nruns.patch Sob
+package/dhrystone/0002-HZ.patch Sob
+package/dhrystone/0003-exit.patch Sob
+package/dhrystone/0004-headers.patch Sob
+package/dhrystone/0005-prototypes.patch Sob
+package/directfb-examples/0001-remove-bzero.patch Sob
+package/dmraid/S20dmraid Variables
+package/dnsmasq/S80dnsmasq Shellcheck Variables
+package/docker-engine/S60dockerd Indent Shellcheck Variables
+package/domoticz/S99domoticz Shellcheck
+package/dropbear/S50dropbear Indent Shellcheck Variables
+package/earlyoom/S02earlyoom Indent Shellcheck
+package/ejabberd/S50ejabberd Indent Shellcheck Variables
+package/eudev/S10udev ConsecutiveEmptyLines Indent Shellcheck Variables
+package/exim/S86exim Indent Variables
+package/fail2ban/S60fail2ban Shellcheck Variables
+package/fbv/0001-cross.patch Sob
+package/fbv/0002-fix-24bpp-support-on-big-endian.patch Sob
+package/fbv/0005-include.patch Sob
+package/freescale-imx/imx-uuc/S80imx-uuc Indent Shellcheck Variables
+package/frr/S50frr Shellcheck
+package/gamin/0002-no-const-return.patch Sob
+package/gcc/arc-2020.09-release/0002-libsanitizer-Remove-cyclades-from-libsanitizer.patch Sob
+package/genromfs/0001-build-system.patch Sob
+package/gerbera/S99gerbera Indent
+package/go/0002-cmd-dist-use-gohostarch-for-ssa-rewrite-check.patch Sob
+package/gpsd/S50gpsd Indent Shellcheck Variables
+package/haveged/S21haveged Shellcheck Variables
+package/htpdate/S43htpdate Shellcheck
+package/i2pd/S99i2pd Indent Shellcheck Variables
+package/ifplugd/0001-cross.patch Sob
+package/ifplugd/0002-fix-headers.patch Sob
+package/ifupdown-scripts/S40network EmptyLastLine Indent Shellcheck Variables
+package/igd2-for-linux/S99upnpd Indent Shellcheck Variables
+package/inadyn/S70inadyn Indent NotExecutable
+package/input-event-daemon/S99input-event-daemon ConsecutiveEmptyLines Indent Variables
+package/iptables/S35iptables Shellcheck
+package/irda-utils/0001-daemon.patch Sob
+package/irda-utils/0002-nommu.patch Sob
+package/irda-utils/0003-subdir.patch Sob
+package/irqbalance/S13irqbalance Indent Shellcheck Variables
+package/iucode-tool/S00iucode-tool Variables
+package/iwd/S40iwd Shellcheck Variables
+package/keyutils/0002-cifs.patch Sob
+package/kodi/S50kodi Shellcheck Variables
+package/libart/0001-art-config-cross.patch Sob
+package/libcgicc/0001-disable-documentation-option.patch Sob
+package/libfcgi/0002-disable-examples.patch Sob
+package/libftdi/0001-pkgconfig_libusb.patch Sob
+package/libftdi/0002-libftdi.pc-requires-libusb-fix-static-build.patch Sob
+package/libiio/S99iiod Shellcheck Variables
+package/libmad/0001-mips-h-constraint-removal.patch Sob
+package/lighttpd/S50lighttpd EmptyLastLine Indent Shellcheck Variables
+package/linux-tools/S10hyperv Indent Variables
+package/linuxptp/S65ptp4l Indent Shellcheck
+package/linuxptp/S66phc2sys Indent Shellcheck
+package/lirc-tools/S25lircd Indent Variables
+package/lite/0001-dfbspy-stat.patch Sob
+package/lite/0002-no-tests.patch Sob
+package/lite/0003-pkg-config.patch Sob
+package/lldpd/S60lldpd Indent Shellcheck Variables
+package/lockfile-progs/0001-sus3v-legacy.patch Sob
+package/madplay/0001-switch-to-new-alsa-api.patch Sob
+package/mariadb/S97mysqld Indent Shellcheck Variables
+package/mender-connect/S43mender-connect Shellcheck
+package/mii-diag/0001-strchr.patch Sob
+package/minidlna/S60minidlnad Indent Shellcheck Variables
+package/minissdpd/S50minissdpd Indent Shellcheck Variables
+package/modem-manager/S44modem-manager Shellcheck Variables
+package/mosquitto/S50mosquitto Indent Shellcheck Variables
+package/motion/S99motion Indent Shellcheck Variables
+package/mpd/S95mpd Variables
+package/mrouted/S41mrouted NotExecutable
+package/mrp/S65mrp Indent Variables
+package/multipath-tools/S60multipathd Shellcheck
+package/neard/S53neard Indent Shellcheck Variables
+package/netatalk/S50netatalk EmptyLastLine Indent Variables
+package/netcat/0001-signed-bit-counting.patch Sob
+package/netopeer2/S52netopeer2 Shellcheck Variables
+package/netplug/0001-makefile-flags.patch Sob
+package/netplug/S29netplug Indent Shellcheck Variables
+package/netsnmp/S59snmpd Indent Shellcheck Variables
+package/network-manager/S45network-manager ConsecutiveEmptyLines EmptyLastLine Shellcheck Variables
+package/nfs-utils/S60nfs ConsecutiveEmptyLines Shellcheck Variables
+package/nginx/S50nginx Indent Variables
+package/nodm/S90nodm Indent Shellcheck Variables
+package/nss-pam-ldapd/S45nslcd EmptyLastLine Indent Shellcheck Variables
+package/ntp/S49ntp.in Variables
+package/ofono/S46ofono Variables
+package/olsr/S50olsr Indent Shellcheck Variables
+package/openntpd/S49ntp Shellcheck Variables
+package/openssh/S50sshd EmptyLastLine Indent Variables
+package/openvpn/S60openvpn Indent Shellcheck Variables
+package/optee-client/S30optee Indent Shellcheck Variables
+package/oracle-mysql/S97mysqld Shellcheck Variables
+package/owfs/S55owserver Shellcheck Variables
+package/owfs/S60owfs Shellcheck Variables
+package/pigpio/S50pigpio Shellcheck Variables
+package/polkit/S50polkit NotExecutable Shellcheck Variables
+package/postgresql/S50postgresql Variables
+package/procps-ng/S02sysctl Variables
+package/proftpd/S50proftpd Indent Shellcheck Variables
+package/prosody/S50prosody Indent Shellcheck Variables
+package/ptpd/S65ptpd Indent Shellcheck Variables
+package/ptpd2/S65ptpd2 Indent Shellcheck Variables
+package/pulseaudio/S50pulseaudio ConsecutiveEmptyLines EmptyLastLine Indent Variables
+package/python-huepy/0001-fix-import-with-python3.patch Sob
+package/python-web2py/S51web2py Shellcheck Variables
+package/rabbitmq-server/S50rabbitmq-server Indent Shellcheck Variables
+package/rdesktop/0001-8bit-colors.patch Sob
+package/redis/S50redis Shellcheck Variables
+package/restorecond/S02restorecond Shellcheck
+package/rng-tools/S21rngd Shellcheck Variables
+package/rpcbind/S30rpcbind EmptyLastLine Indent Variables
+package/rubix/0002-misc-fixes.patch Sob
+package/rygel/S99rygel Indent Shellcheck Variables
+package/samba4/S91smb Indent Shellcheck Variables
+package/seatd/S70seatd NotExecutable Variables
+package/ser2net/S50ser2net Indent Shellcheck Variables
+package/shairport-sync/S99shairport-sync Indent Shellcheck Variables
+package/smcroute/S41smcroute Indent NotExecutable Variables
+package/smstools3/S50smsd Shellcheck Variables
+package/solarus/0002-Add-a-basic-FindOpenGLES2.cmake.patch Sob
+package/squid/S97squid Indent Shellcheck Variables
+package/ssdp-responder/S50ssdpd Indent NotExecutable Shellcheck Variables
+package/sshguard/S49sshguard Indent
+package/sslh/S35sslh Indent Shellcheck Variables
+package/stunnel/S50stunnel Indent Shellcheck Variables
+package/supervisor/S99supervisord Variables
+package/suricata/S99suricata Shellcheck
+package/sysrepo/S51sysrepo-plugind Indent Shellcheck
+package/targetcli-fb/S50target Shellcheck Variables
+package/tcf-agent/S55tcf-agent Shellcheck Variables
+package/tftpd/S80tftpd-hpa Indent Shellcheck Variables
+package/ti-gfx/S80ti-gfx Shellcheck Variables
+package/ti-sgx-um/S80ti-sgx Variables
+package/tpm2-abrmd/S80tpm2-abrmd Indent Shellcheck Variables
+package/transmission/S92transmission ConsecutiveEmptyLines Indent Shellcheck Variables
+package/triggerhappy/S10triggerhappy Indent Shellcheck Variables
+package/tvheadend/S99tvheadend Indent Shellcheck Variables
+package/unbound/S70unbound Shellcheck
+package/unscd/S46unscd Indent Shellcheck Variables
+package/upmpdcli/S99upmpdcli Indent Shellcheck Variables
+package/urandom-scripts/S20urandom Variables
+package/usbguard/S20usbguard Indent Shellcheck Variables
+package/vsftpd/S70vsftpd Indent Shellcheck Variables
+package/watchdogd/S01watchdogd Indent NotExecutable
+package/x11r7/xapp_xdm/S99xdm Indent Variables
+package/x11r7/xdriver_xf86-video-mach64/0001-cross-compile.patch Sob
+package/x11r7/xdriver_xf86-video-savage/0001-cross-compile.patch Sob
+package/x11r7/xdriver_xf86-video-tdfx/0001-cross.patch Sob
+package/x11r7/xserver_xorg-server/S40xorg Shellcheck Variables
diff --git a/Makefile b/Makefile
index e49080b110..c5dee409f1 100644
--- a/Makefile
+++ b/Makefile
@@ -1241,13 +1241,12 @@ check-flake8:
| xargs -- python3 -m flake8 --statistics
check-package:
- find $(TOPDIR) -type f \( -name '*.mk' -o -name '*.hash' -o -name 'Config.*' -o -name '*.patch' \) \
- -exec ./utils/check-package --exclude=Sob {} +
+ $(Q)./utils/check-package `git ls-tree -r --name-only HEAD`
.PHONY: .checkpackageignore
.checkpackageignore:
- find $(TOPDIR) -type f \( -name '*.mk' -o -name '*.hash' -o -name 'Config.*' -o -name '*.patch' \) \
- -exec ./utils/check-package --exclude=Sob --failed-only {} > .checkpackageignore +
+ $(Q)./utils/check-package --failed-only `git ls-tree -r --name-only HEAD` \
+ > .checkpackageignore
include docs/manual/manual.mk
-include $(foreach dir,$(BR2_EXTERNAL_DIRS),$(sort $(wildcard $(dir)/docs/*/*.mk)))
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [PATCH v2 04/14] Makefile: make check-package assume a git tree
2022-07-31 19:35 ` [Buildroot] [PATCH v2 04/14] Makefile: make check-package assume a git tree Ricardo Martincoski
@ 2023-02-06 21:23 ` Thomas Petazzoni via buildroot
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-06 21:23 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: buildroot
On Sun, 31 Jul 2022 16:35:11 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> ... just like check-flake8 already does.
>
> 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.
>
> Since now check-package has an ignore file to list all warnings in the
> tree, that will eventually be fixed, there is no need to filter the
> files passed to check-package.
> So test all files in the tree when 'make check-package' is called.
> It brings following advantages;
> - any new check_function added to check-package takes place immediately
> for new files;
> - adding new check_functions is less traumatic to the developer doing
> this, since he/she does not need anymore to fix all warnings in the
> tree before the new check_function takes effect;
> - prevent regressions, e.g. ANY new .patch file must have SoB;
> - as a side-effect, print a single statistics line as output of
> 'make ckeck-package'.
>
> But just enabling the check would generate many warnings when
> 'make check-package' is called, so update the ignore file by using:
> $ ./utils/docker-run make .checkpackageignore
>
> Notice: in order to ensure reproducible results, one should run 'make
> check-package' and 'make .checkpackageignore' inside the docker image,
> otherwise a variation in shellcheck version (installed in the host) can
> produce different results.
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Applied to master after regenerating the ignore file, as requested,
thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH v2 05/14] docs/manual: check-package before submitting patch
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (3 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 04/14] Makefile: make check-package assume a git tree Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-02-06 21:23 ` Thomas Petazzoni via buildroot
2022-07-31 19:35 ` [Buildroot] [PATCH v2 06/14] support/docker: add python3-magic Ricardo Martincoski
` (9 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Romain Naour, Thomas De Schampheleire, Ricardo Martincoski
Add 'utils/docker-run make check-package' to the default workflow of
submitting patches, just after the rebase and before using format-patch.
Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
Cc: Romain Naour <romain.naour@gmail.com>
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- since the automatic run inside docker was removed from the series,
use a 'good default' to newcomers, that ensures repdroducible
results, by mentioning to use docker-run
- add a small description of what check-package really does (Romain)
---
docs/manual/contribute.txt | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/docs/manual/contribute.txt b/docs/manual/contribute.txt
index e588c69be6..94f1c64496 100644
--- a/docs/manual/contribute.txt
+++ b/docs/manual/contribute.txt
@@ -294,6 +294,12 @@ $ git fetch --all --tags
$ git rebase origin/master
---------------------
+Now check the coding style for the changes you committed:
+
+---------------------
+$ utils/docker-run make check-package
+---------------------
+
Now, you are ready to generate then submit your patch set.
To generate it, run:
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [PATCH v2 05/14] docs/manual: check-package before submitting patch
2022-07-31 19:35 ` [Buildroot] [PATCH v2 05/14] docs/manual: check-package before submitting patch Ricardo Martincoski
@ 2023-02-06 21:23 ` Thomas Petazzoni via buildroot
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-06 21:23 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: Romain Naour, Thomas De Schampheleire, buildroot
On Sun, 31 Jul 2022 16:35:12 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> Add 'utils/docker-run make check-package' to the default workflow of
> submitting patches, just after the rebase and before using format-patch.
>
> Cc: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> Cc: Romain Naour <romain.naour@gmail.com>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> ---
> Changes v1 -> v2:
> - since the automatic run inside docker was removed from the series,
> use a 'good default' to newcomers, that ensures repdroducible
> results, by mentioning to use docker-run
> - add a small description of what check-package really does (Romain)
> ---
> docs/manual/contribute.txt | 6 ++++++
> 1 file changed, 6 insertions(+)
Applied to master, thanks.
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH v2 06/14] support/docker: add python3-magic
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (4 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 05/14] docs/manual: check-package before submitting patch Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-02-06 21:24 ` Thomas Petazzoni via buildroot
2022-07-31 19:35 ` [Buildroot] [PATCH v2 07/14] utils/check-package: check all shell scripts Ricardo Martincoski
` (8 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
It will be needed by check-package to run checks according to the file
type (the same determined by the command 'file').
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- none
NOTE to the maintainer applying this patch: all patches after this in
the series assume this patch was applied, a new docker image was
generated and its version was updated in .gitlab.yml
---
support/docker/Dockerfile | 1 +
1 file changed, 1 insertion(+)
diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile
index f54c31b54a..d775ae23e1 100644
--- a/support/docker/Dockerfile
+++ b/support/docker/Dockerfile
@@ -39,6 +39,7 @@ RUN apt-get install -y --no-install-recommends \
openssh-server \
python3 \
python3-flake8 \
+ python3-magic \
python3-nose2 \
python3-pexpect \
python3-pytest \
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [PATCH v2 06/14] support/docker: add python3-magic
2022-07-31 19:35 ` [Buildroot] [PATCH v2 06/14] support/docker: add python3-magic Ricardo Martincoski
@ 2023-02-06 21:24 ` Thomas Petazzoni via buildroot
0 siblings, 0 replies; 25+ messages in thread
From: Thomas Petazzoni via buildroot @ 2023-02-06 21:24 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: Yann E. MORIN, buildroot
On Sun, 31 Jul 2022 16:35:13 -0300
Ricardo Martincoski <ricardo.martincoski@gmail.com> wrote:
> It will be needed by check-package to run checks according to the file
> type (the same determined by the command 'file').
>
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> ---
> Changes v1 -> v2:
> - none
Applied to master, thanks. I stopped applying there, because we now
need to regenerate the Docker container image, push it on the Gitlab CI
registry, and update .gitlab-ci.yml. And I must say I have no idea how
to do this :-)
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH v2 07/14] utils/check-package: check all shell scripts
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (5 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 06/14] support/docker: add python3-magic Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-02-08 12:30 ` Arnout Vandecappelle
2022-07-31 19:35 ` [Buildroot] [PATCH v2 08/14] utils/check-package: check files in utils/ Ricardo Martincoski
` (7 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
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.
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>
---
Changes v1 -> v2:
- do not mention docker image in the commit message
NOTE to the maintainer applying this patch: please re-generate the list
of ignored warnings while applying:
$ utils/docker-run make .checkpackageignore
---
.checkpackageignore | 20 +++++++++++++++++++
.../utils/br2-external/utils/x-shellscript | 2 ++
.../testing/tests/utils/test_check_package.py | 17 ++++++++++++++++
utils/check-package | 13 +++++++++++-
utils/checkpackagelib/lib_shellscript.py | 5 +++++
5 files changed, 56 insertions(+), 1 deletion(-)
create mode 100755 support/testing/tests/utils/br2-external/utils/x-shellscript
create mode 100644 utils/checkpackagelib/lib_shellscript.py
diff --git a/.checkpackageignore b/.checkpackageignore
index d8947e89fc..a527254464 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -16,11 +16,13 @@ package/busybox/S02sysctl Variables
package/busybox/S10mdev ConsecutiveEmptyLines Indent Shellcheck
package/busybox/S15watchdog Indent Variables
package/busybox/S50telnet Indent Shellcheck Variables
+package/busybox/udhcpc.script Shellcheck
package/c-icap/S96cicap Indent Shellcheck Variables
package/cfm/S65cfm Indent Variables
package/cgroupfs-mount/S30cgroupfs Indent Shellcheck Variables
package/chrony/S49chrony Indent Shellcheck Variables
package/connman/S45connman Variables
+package/coremark-pro/coremark-pro.sh.in Shellcheck
package/curlftpfs/0001-fix-CURLOPT_INFILESIZE.patch Sob
package/curlftpfs/0002-free_ftpfs_file-memleak-fix.patch Sob
package/curlftpfs/0003-nocache-memleak-fix.patch Sob
@@ -30,6 +32,7 @@ package/dbus/S30dbus Indent Shellcheck TrailingSpace Variables
package/dcron/S90dcron Variables
package/dhcp/S80dhcp-relay Shellcheck Variables
package/dhcp/S80dhcp-server Shellcheck Variables
+package/dhcp/dhclient-script Shellcheck TrailingSpace
package/dhcpcd/S41dhcpcd Indent Variables
package/dhrystone/0001-cmdline-nruns.patch Sob
package/dhrystone/0002-HZ.patch Sob
@@ -44,9 +47,11 @@ package/domoticz/S99domoticz Shellcheck
package/dropbear/S50dropbear Indent Shellcheck Variables
package/earlyoom/S02earlyoom Indent Shellcheck
package/ejabberd/S50ejabberd Indent Shellcheck Variables
+package/ejabberd/check-erlang-lib Shellcheck
package/eudev/S10udev ConsecutiveEmptyLines Indent Shellcheck Variables
package/exim/S86exim Indent Variables
package/fail2ban/S60fail2ban Shellcheck Variables
+package/fakedate/fakedate Shellcheck
package/fbv/0001-cross.patch Sob
package/fbv/0002-fix-24bpp-support-on-big-endian.patch Sob
package/fbv/0005-include.patch Sob
@@ -57,6 +62,7 @@ package/gcc/arc-2020.09-release/0002-libsanitizer-Remove-cyclades-from-libsaniti
package/genromfs/0001-build-system.patch Sob
package/gerbera/S99gerbera Indent
package/go/0002-cmd-dist-use-gohostarch-for-ssa-rewrite-check.patch Sob
+package/google-breakpad/gen-syms.sh Shellcheck
package/gpsd/S50gpsd Indent Shellcheck Variables
package/haveged/S21haveged Shellcheck Variables
package/htpdate/S43htpdate Shellcheck
@@ -64,8 +70,12 @@ package/i2pd/S99i2pd Indent Shellcheck Variables
package/ifplugd/0001-cross.patch Sob
package/ifplugd/0002-fix-headers.patch Sob
package/ifupdown-scripts/S40network EmptyLastLine Indent Shellcheck Variables
+package/ifupdown-scripts/network/if-pre-up.d/wait_iface EmptyLastLine Shellcheck
+package/ifupdown-scripts/nfs_check Shellcheck
package/igd2-for-linux/S99upnpd Indent Shellcheck Variables
package/inadyn/S70inadyn Indent NotExecutable
+package/initscripts/init.d/rcK ConsecutiveEmptyLines EmptyLastLine Shellcheck
+package/initscripts/init.d/rcS ConsecutiveEmptyLines EmptyLastLine Shellcheck
package/input-event-daemon/S99input-event-daemon ConsecutiveEmptyLines Indent Variables
package/iptables/S35iptables Shellcheck
package/irda-utils/0001-daemon.patch Sob
@@ -95,6 +105,7 @@ package/lldpd/S60lldpd Indent Shellcheck Variables
package/lockfile-progs/0001-sus3v-legacy.patch Sob
package/madplay/0001-switch-to-new-alsa-api.patch Sob
package/mariadb/S97mysqld Indent Shellcheck Variables
+package/matchbox-keyboard/mb-applet-kbd-wrapper.sh Shellcheck TrailingSpace
package/mender-connect/S43mender-connect Shellcheck
package/mii-diag/0001-strchr.patch Sob
package/minidlna/S60minidlnad Indent Shellcheck Variables
@@ -112,6 +123,7 @@ package/netcat/0001-signed-bit-counting.patch Sob
package/netopeer2/S52netopeer2 Shellcheck Variables
package/netplug/0001-makefile-flags.patch Sob
package/netplug/S29netplug Indent Shellcheck Variables
+package/netplug/netplug-script ConsecutiveEmptyLines Shellcheck
package/netsnmp/S59snmpd Indent Shellcheck Variables
package/network-manager/S45network-manager ConsecutiveEmptyLines EmptyLastLine Shellcheck Variables
package/nfs-utils/S60nfs ConsecutiveEmptyLines Shellcheck Variables
@@ -123,12 +135,14 @@ package/ofono/S46ofono Variables
package/olsr/S50olsr Indent Shellcheck Variables
package/openntpd/S49ntp Shellcheck Variables
package/openssh/S50sshd EmptyLastLine Indent Variables
+package/openvmtools/shutdown Shellcheck
package/openvpn/S60openvpn Indent Shellcheck Variables
package/optee-client/S30optee Indent Shellcheck Variables
package/oracle-mysql/S97mysqld Shellcheck Variables
package/owfs/S55owserver Shellcheck Variables
package/owfs/S60owfs Shellcheck Variables
package/pigpio/S50pigpio Shellcheck Variables
+package/pkgconf/pkg-config.in Shellcheck
package/polkit/S50polkit NotExecutable Shellcheck Variables
package/postgresql/S50postgresql Variables
package/procps-ng/S02sysctl Variables
@@ -161,11 +175,14 @@ package/sslh/S35sslh Indent Shellcheck Variables
package/stunnel/S50stunnel Indent Shellcheck Variables
package/supervisor/S99supervisord Variables
package/suricata/S99suricata Shellcheck
+package/swupdate/swupdate.sh Shellcheck
package/sysrepo/S51sysrepo-plugind Indent Shellcheck
+package/systemd/fakeroot_tmpfiles.sh Shellcheck
package/targetcli-fb/S50target Shellcheck Variables
package/tcf-agent/S55tcf-agent Shellcheck Variables
package/tftpd/S80tftpd-hpa Indent Shellcheck Variables
package/ti-gfx/S80ti-gfx Shellcheck Variables
+package/ti-gfx/esrev.sh Shellcheck
package/ti-sgx-um/S80ti-sgx Variables
package/tpm2-abrmd/S80tpm2-abrmd Indent Shellcheck Variables
package/transmission/S92transmission ConsecutiveEmptyLines Indent Shellcheck Variables
@@ -176,10 +193,13 @@ package/unscd/S46unscd Indent Shellcheck Variables
package/upmpdcli/S99upmpdcli Indent Shellcheck Variables
package/urandom-scripts/S20urandom Variables
package/usbguard/S20usbguard Indent Shellcheck Variables
+package/vala/vala-wrapper Shellcheck
package/vsftpd/S70vsftpd Indent Shellcheck Variables
package/watchdogd/S01watchdogd Indent NotExecutable
+package/wpa_supplicant/ifupdown.sh Shellcheck
package/x11r7/xapp_xdm/S99xdm Indent Variables
package/x11r7/xdriver_xf86-video-mach64/0001-cross-compile.patch Sob
package/x11r7/xdriver_xf86-video-savage/0001-cross-compile.patch Sob
package/x11r7/xdriver_xf86-video-tdfx/0001-cross.patch Sob
package/x11r7/xserver_xorg-server/S40xorg Shellcheck Variables
+package/xl2tp/xl2tpd TrailingSpace
diff --git a/support/testing/tests/utils/br2-external/utils/x-shellscript b/support/testing/tests/utils/br2-external/utils/x-shellscript
new file mode 100755
index 0000000000..a7de4124bd
--- /dev/null
+++ b/support/testing/tests/utils/br2-external/utils/x-shellscript
@@ -0,0 +1,2 @@
+#!/bin/bash
+unused="text"
diff --git a/support/testing/tests/utils/test_check_package.py b/support/testing/tests/utils/test_check_package.py
index f21b9e939f..aeda0857e3 100644
--- a/support/testing/tests/utils/test_check_package.py
+++ b/support/testing/tests/utils/test_check_package.py
@@ -232,3 +232,20 @@ class TestCheckPackage(unittest.TestCase):
.format(subdir_file), w)
self.assertIn("{}:0: NewlineAtEof was expected to fail, did you fixed the file and forgot to update .checkpackageignore?"
.format(subdir_file), w)
+
+ # shell scripts are tested using shellcheck
+ rel_file = "utils/x-shellscript"
+ abs_path = infra.filepath("tests/utils/br2-external")
+ abs_file = os.path.join(abs_path, rel_file)
+
+ w, m = call_script(["check-package", "-b", rel_file],
+ self.WITH_UTILS_IN_PATH, abs_path)
+ self.assert_file_was_processed(m)
+ self.assert_warnings_generated_for_file(m)
+ self.assertIn("{}:0: run 'shellcheck' and fix the warnings".format(rel_file), w)
+
+ w, m = call_script(["check-package", "-b", abs_file],
+ self.WITH_UTILS_IN_PATH, infra.basepath())
+ self.assert_file_was_processed(m)
+ self.assert_warnings_generated_for_file(m)
+ self.assertIn("{}:0: run 'shellcheck' and fix the warnings".format(abs_file), w)
diff --git a/utils/check-package b/utils/check-package
index d896e8b117..732f1cf112 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -3,6 +3,7 @@
import argparse
import inspect
+import magic
import os
import re
import six
@@ -13,6 +14,7 @@ import checkpackagelib.lib_config
import checkpackagelib.lib_hash
import checkpackagelib.lib_mk
import checkpackagelib.lib_patch
+import checkpackagelib.lib_shellscript
import checkpackagelib.lib_sysv
IGNORE_FILENAME = ".checkpackageignore"
@@ -86,6 +88,15 @@ def parse_args():
return flags
+def get_lib_from_filetype(fname):
+ if not os.path.isfile(fname):
+ return None
+ filetype = magic.from_file(fname, mime=True)
+ if filetype == "text/x-shellscript":
+ return checkpackagelib.lib_shellscript
+ return None
+
+
CONFIG_IN_FILENAME = re.compile(r"Config\.\S*$")
DO_CHECK_INTREE = re.compile(r"|".join([
r"Config.in",
@@ -129,7 +140,7 @@ def get_lib_from_filename(fname):
return checkpackagelib.lib_patch
if SYSV_INIT_SCRIPT_FILENAME.search(fname):
return checkpackagelib.lib_sysv
- return None
+ return get_lib_from_filetype(fname)
def common_inspect_rules(m):
diff --git a/utils/checkpackagelib/lib_shellscript.py b/utils/checkpackagelib/lib_shellscript.py
new file mode 100644
index 0000000000..9b4f4aed58
--- /dev/null
+++ b/utils/checkpackagelib/lib_shellscript.py
@@ -0,0 +1,5 @@
+from checkpackagelib.lib import ConsecutiveEmptyLines # noqa: F401
+from checkpackagelib.lib import EmptyLastLine # noqa: F401
+from checkpackagelib.lib import NewlineAtEof # noqa: F401
+from checkpackagelib.lib import TrailingSpace # noqa: F401
+from checkpackagelib.tool import Shellcheck # noqa: F401
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [PATCH v2 07/14] utils/check-package: check all shell scripts
2022-07-31 19:35 ` [Buildroot] [PATCH v2 07/14] utils/check-package: check all shell scripts Ricardo Martincoski
@ 2023-02-08 12:30 ` Arnout Vandecappelle
0 siblings, 0 replies; 25+ messages in thread
From: Arnout Vandecappelle @ 2023-02-08 12:30 UTC (permalink / raw)
To: Ricardo Martincoski, buildroot
On 31/07/2022 21:35, Ricardo Martincoski wrote:
> 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.
Turns out there are (at least) two Python modules called "magic", and I
happened to have the other one installed... So I adapted the code to support both.
> 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>
> ---
> Changes v1 -> v2:
> - do not mention docker image in the commit message
>
> NOTE to the maintainer applying this patch: please re-generate the list
> of ignored warnings while applying:
> $ utils/docker-run make .checkpackageignore
Only one exception more than originally, so that's good I guess.
Applied to master, thanks.
Regards,
Arnout
> ---
> .checkpackageignore | 20 +++++++++++++++++++
> .../utils/br2-external/utils/x-shellscript | 2 ++
> .../testing/tests/utils/test_check_package.py | 17 ++++++++++++++++
> utils/check-package | 13 +++++++++++-
> utils/checkpackagelib/lib_shellscript.py | 5 +++++
> 5 files changed, 56 insertions(+), 1 deletion(-)
> create mode 100755 support/testing/tests/utils/br2-external/utils/x-shellscript
> create mode 100644 utils/checkpackagelib/lib_shellscript.py
>
> diff --git a/.checkpackageignore b/.checkpackageignore
> index d8947e89fc..a527254464 100644
> --- a/.checkpackageignore
> +++ b/.checkpackageignore
> @@ -16,11 +16,13 @@ package/busybox/S02sysctl Variables
> package/busybox/S10mdev ConsecutiveEmptyLines Indent Shellcheck
> package/busybox/S15watchdog Indent Variables
> package/busybox/S50telnet Indent Shellcheck Variables
> +package/busybox/udhcpc.script Shellcheck
> package/c-icap/S96cicap Indent Shellcheck Variables
> package/cfm/S65cfm Indent Variables
> package/cgroupfs-mount/S30cgroupfs Indent Shellcheck Variables
> package/chrony/S49chrony Indent Shellcheck Variables
> package/connman/S45connman Variables
> +package/coremark-pro/coremark-pro.sh.in Shellcheck
> package/curlftpfs/0001-fix-CURLOPT_INFILESIZE.patch Sob
> package/curlftpfs/0002-free_ftpfs_file-memleak-fix.patch Sob
> package/curlftpfs/0003-nocache-memleak-fix.patch Sob
> @@ -30,6 +32,7 @@ package/dbus/S30dbus Indent Shellcheck TrailingSpace Variables
> package/dcron/S90dcron Variables
> package/dhcp/S80dhcp-relay Shellcheck Variables
> package/dhcp/S80dhcp-server Shellcheck Variables
> +package/dhcp/dhclient-script Shellcheck TrailingSpace
> package/dhcpcd/S41dhcpcd Indent Variables
> package/dhrystone/0001-cmdline-nruns.patch Sob
> package/dhrystone/0002-HZ.patch Sob
> @@ -44,9 +47,11 @@ package/domoticz/S99domoticz Shellcheck
> package/dropbear/S50dropbear Indent Shellcheck Variables
> package/earlyoom/S02earlyoom Indent Shellcheck
> package/ejabberd/S50ejabberd Indent Shellcheck Variables
> +package/ejabberd/check-erlang-lib Shellcheck
> package/eudev/S10udev ConsecutiveEmptyLines Indent Shellcheck Variables
> package/exim/S86exim Indent Variables
> package/fail2ban/S60fail2ban Shellcheck Variables
> +package/fakedate/fakedate Shellcheck
> package/fbv/0001-cross.patch Sob
> package/fbv/0002-fix-24bpp-support-on-big-endian.patch Sob
> package/fbv/0005-include.patch Sob
> @@ -57,6 +62,7 @@ package/gcc/arc-2020.09-release/0002-libsanitizer-Remove-cyclades-from-libsaniti
> package/genromfs/0001-build-system.patch Sob
> package/gerbera/S99gerbera Indent
> package/go/0002-cmd-dist-use-gohostarch-for-ssa-rewrite-check.patch Sob
> +package/google-breakpad/gen-syms.sh Shellcheck
> package/gpsd/S50gpsd Indent Shellcheck Variables
> package/haveged/S21haveged Shellcheck Variables
> package/htpdate/S43htpdate Shellcheck
> @@ -64,8 +70,12 @@ package/i2pd/S99i2pd Indent Shellcheck Variables
> package/ifplugd/0001-cross.patch Sob
> package/ifplugd/0002-fix-headers.patch Sob
> package/ifupdown-scripts/S40network EmptyLastLine Indent Shellcheck Variables
> +package/ifupdown-scripts/network/if-pre-up.d/wait_iface EmptyLastLine Shellcheck
> +package/ifupdown-scripts/nfs_check Shellcheck
> package/igd2-for-linux/S99upnpd Indent Shellcheck Variables
> package/inadyn/S70inadyn Indent NotExecutable
> +package/initscripts/init.d/rcK ConsecutiveEmptyLines EmptyLastLine Shellcheck
> +package/initscripts/init.d/rcS ConsecutiveEmptyLines EmptyLastLine Shellcheck
> package/input-event-daemon/S99input-event-daemon ConsecutiveEmptyLines Indent Variables
> package/iptables/S35iptables Shellcheck
> package/irda-utils/0001-daemon.patch Sob
> @@ -95,6 +105,7 @@ package/lldpd/S60lldpd Indent Shellcheck Variables
> package/lockfile-progs/0001-sus3v-legacy.patch Sob
> package/madplay/0001-switch-to-new-alsa-api.patch Sob
> package/mariadb/S97mysqld Indent Shellcheck Variables
> +package/matchbox-keyboard/mb-applet-kbd-wrapper.sh Shellcheck TrailingSpace
> package/mender-connect/S43mender-connect Shellcheck
> package/mii-diag/0001-strchr.patch Sob
> package/minidlna/S60minidlnad Indent Shellcheck Variables
> @@ -112,6 +123,7 @@ package/netcat/0001-signed-bit-counting.patch Sob
> package/netopeer2/S52netopeer2 Shellcheck Variables
> package/netplug/0001-makefile-flags.patch Sob
> package/netplug/S29netplug Indent Shellcheck Variables
> +package/netplug/netplug-script ConsecutiveEmptyLines Shellcheck
> package/netsnmp/S59snmpd Indent Shellcheck Variables
> package/network-manager/S45network-manager ConsecutiveEmptyLines EmptyLastLine Shellcheck Variables
> package/nfs-utils/S60nfs ConsecutiveEmptyLines Shellcheck Variables
> @@ -123,12 +135,14 @@ package/ofono/S46ofono Variables
> package/olsr/S50olsr Indent Shellcheck Variables
> package/openntpd/S49ntp Shellcheck Variables
> package/openssh/S50sshd EmptyLastLine Indent Variables
> +package/openvmtools/shutdown Shellcheck
> package/openvpn/S60openvpn Indent Shellcheck Variables
> package/optee-client/S30optee Indent Shellcheck Variables
> package/oracle-mysql/S97mysqld Shellcheck Variables
> package/owfs/S55owserver Shellcheck Variables
> package/owfs/S60owfs Shellcheck Variables
> package/pigpio/S50pigpio Shellcheck Variables
> +package/pkgconf/pkg-config.in Shellcheck
> package/polkit/S50polkit NotExecutable Shellcheck Variables
> package/postgresql/S50postgresql Variables
> package/procps-ng/S02sysctl Variables
> @@ -161,11 +175,14 @@ package/sslh/S35sslh Indent Shellcheck Variables
> package/stunnel/S50stunnel Indent Shellcheck Variables
> package/supervisor/S99supervisord Variables
> package/suricata/S99suricata Shellcheck
> +package/swupdate/swupdate.sh Shellcheck
> package/sysrepo/S51sysrepo-plugind Indent Shellcheck
> +package/systemd/fakeroot_tmpfiles.sh Shellcheck
> package/targetcli-fb/S50target Shellcheck Variables
> package/tcf-agent/S55tcf-agent Shellcheck Variables
> package/tftpd/S80tftpd-hpa Indent Shellcheck Variables
> package/ti-gfx/S80ti-gfx Shellcheck Variables
> +package/ti-gfx/esrev.sh Shellcheck
> package/ti-sgx-um/S80ti-sgx Variables
> package/tpm2-abrmd/S80tpm2-abrmd Indent Shellcheck Variables
> package/transmission/S92transmission ConsecutiveEmptyLines Indent Shellcheck Variables
> @@ -176,10 +193,13 @@ package/unscd/S46unscd Indent Shellcheck Variables
> package/upmpdcli/S99upmpdcli Indent Shellcheck Variables
> package/urandom-scripts/S20urandom Variables
> package/usbguard/S20usbguard Indent Shellcheck Variables
> +package/vala/vala-wrapper Shellcheck
> package/vsftpd/S70vsftpd Indent Shellcheck Variables
> package/watchdogd/S01watchdogd Indent NotExecutable
> +package/wpa_supplicant/ifupdown.sh Shellcheck
> package/x11r7/xapp_xdm/S99xdm Indent Variables
> package/x11r7/xdriver_xf86-video-mach64/0001-cross-compile.patch Sob
> package/x11r7/xdriver_xf86-video-savage/0001-cross-compile.patch Sob
> package/x11r7/xdriver_xf86-video-tdfx/0001-cross.patch Sob
> package/x11r7/xserver_xorg-server/S40xorg Shellcheck Variables
> +package/xl2tp/xl2tpd TrailingSpace
> diff --git a/support/testing/tests/utils/br2-external/utils/x-shellscript b/support/testing/tests/utils/br2-external/utils/x-shellscript
> new file mode 100755
> index 0000000000..a7de4124bd
> --- /dev/null
> +++ b/support/testing/tests/utils/br2-external/utils/x-shellscript
> @@ -0,0 +1,2 @@
> +#!/bin/bash
> +unused="text"
> diff --git a/support/testing/tests/utils/test_check_package.py b/support/testing/tests/utils/test_check_package.py
> index f21b9e939f..aeda0857e3 100644
> --- a/support/testing/tests/utils/test_check_package.py
> +++ b/support/testing/tests/utils/test_check_package.py
> @@ -232,3 +232,20 @@ class TestCheckPackage(unittest.TestCase):
> .format(subdir_file), w)
> self.assertIn("{}:0: NewlineAtEof was expected to fail, did you fixed the file and forgot to update .checkpackageignore?"
> .format(subdir_file), w)
> +
> + # shell scripts are tested using shellcheck
> + rel_file = "utils/x-shellscript"
> + abs_path = infra.filepath("tests/utils/br2-external")
> + abs_file = os.path.join(abs_path, rel_file)
> +
> + w, m = call_script(["check-package", "-b", rel_file],
> + self.WITH_UTILS_IN_PATH, abs_path)
> + self.assert_file_was_processed(m)
> + self.assert_warnings_generated_for_file(m)
> + self.assertIn("{}:0: run 'shellcheck' and fix the warnings".format(rel_file), w)
> +
> + w, m = call_script(["check-package", "-b", abs_file],
> + self.WITH_UTILS_IN_PATH, infra.basepath())
> + self.assert_file_was_processed(m)
> + self.assert_warnings_generated_for_file(m)
> + self.assertIn("{}:0: run 'shellcheck' and fix the warnings".format(abs_file), w)
> diff --git a/utils/check-package b/utils/check-package
> index d896e8b117..732f1cf112 100755
> --- a/utils/check-package
> +++ b/utils/check-package
> @@ -3,6 +3,7 @@
>
> import argparse
> import inspect
> +import magic
> import os
> import re
> import six
> @@ -13,6 +14,7 @@ import checkpackagelib.lib_config
> import checkpackagelib.lib_hash
> import checkpackagelib.lib_mk
> import checkpackagelib.lib_patch
> +import checkpackagelib.lib_shellscript
> import checkpackagelib.lib_sysv
>
> IGNORE_FILENAME = ".checkpackageignore"
> @@ -86,6 +88,15 @@ def parse_args():
> return flags
>
>
> +def get_lib_from_filetype(fname):
> + if not os.path.isfile(fname):
> + return None
> + filetype = magic.from_file(fname, mime=True)
> + if filetype == "text/x-shellscript":
> + return checkpackagelib.lib_shellscript
> + return None
> +
> +
> CONFIG_IN_FILENAME = re.compile(r"Config\.\S*$")
> DO_CHECK_INTREE = re.compile(r"|".join([
> r"Config.in",
> @@ -129,7 +140,7 @@ def get_lib_from_filename(fname):
> return checkpackagelib.lib_patch
> if SYSV_INIT_SCRIPT_FILENAME.search(fname):
> return checkpackagelib.lib_sysv
> - return None
> + return get_lib_from_filetype(fname)
>
>
> def common_inspect_rules(m):
> diff --git a/utils/checkpackagelib/lib_shellscript.py b/utils/checkpackagelib/lib_shellscript.py
> new file mode 100644
> index 0000000000..9b4f4aed58
> --- /dev/null
> +++ b/utils/checkpackagelib/lib_shellscript.py
> @@ -0,0 +1,5 @@
> +from checkpackagelib.lib import ConsecutiveEmptyLines # noqa: F401
> +from checkpackagelib.lib import EmptyLastLine # noqa: F401
> +from checkpackagelib.lib import NewlineAtEof # noqa: F401
> +from checkpackagelib.lib import TrailingSpace # noqa: F401
> +from checkpackagelib.tool import Shellcheck # noqa: F401
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH v2 08/14] utils/check-package: check files in utils/
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (6 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 07/14] utils/check-package: check all shell scripts Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-02-08 14:29 ` Arnout Vandecappelle
2022-07-31 19:35 ` [Buildroot] [PATCH v2 09/14] utils/check-package: check files in board/ Ricardo Martincoski
` (6 subsequent siblings)
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- none
NOTE to the maintainer applying this patch: please re-generate the list
of ignored warnings while applying:
$ utils/docker-run make .checkpackageignore
---
.checkpackageignore | 4 ++++
utils/check-package | 1 +
2 files changed, 5 insertions(+)
diff --git a/.checkpackageignore b/.checkpackageignore
index a527254464..615fe22e52 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -203,3 +203,7 @@ package/x11r7/xdriver_xf86-video-savage/0001-cross-compile.patch Sob
package/x11r7/xdriver_xf86-video-tdfx/0001-cross.patch Sob
package/x11r7/xserver_xorg-server/S40xorg Shellcheck Variables
package/xl2tp/xl2tpd TrailingSpace
+utils/brmake Shellcheck
+utils/config Shellcheck
+utils/docker-run Shellcheck
+utils/test-pkg Shellcheck
diff --git a/utils/check-package b/utils/check-package
index 732f1cf112..094b7b9c07 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -107,6 +107,7 @@ DO_CHECK_INTREE = re.compile(r"|".join([
r"package/",
r"system/",
r"toolchain/",
+ r"utils/",
]))
DO_NOT_CHECK_INTREE = re.compile(r"|".join([
r"boot/barebox/barebox\.mk$",
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [PATCH v2 08/14] utils/check-package: check files in utils/
2022-07-31 19:35 ` [Buildroot] [PATCH v2 08/14] utils/check-package: check files in utils/ Ricardo Martincoski
@ 2023-02-08 14:29 ` Arnout Vandecappelle
0 siblings, 0 replies; 25+ messages in thread
From: Arnout Vandecappelle @ 2023-02-08 14:29 UTC (permalink / raw)
To: Ricardo Martincoski, buildroot
On 31/07/2022 21:35, Ricardo Martincoski wrote:
> Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
> ---
> Changes v1 -> v2:
> - none
>
> NOTE to the maintainer applying this patch: please re-generate the list
> of ignored warnings while applying:
> $ utils/docker-run make .checkpackageignore
Instead, I pushed 4 other commits that fix the shellcheck issues.
Applied to master, thanks.
Regards,
Arnout
> ---
> .checkpackageignore | 4 ++++
> utils/check-package | 1 +
> 2 files changed, 5 insertions(+)
>
> diff --git a/.checkpackageignore b/.checkpackageignore
> index a527254464..615fe22e52 100644
> --- a/.checkpackageignore
> +++ b/.checkpackageignore
> @@ -203,3 +203,7 @@ package/x11r7/xdriver_xf86-video-savage/0001-cross-compile.patch Sob
> package/x11r7/xdriver_xf86-video-tdfx/0001-cross.patch Sob
> package/x11r7/xserver_xorg-server/S40xorg Shellcheck Variables
> package/xl2tp/xl2tpd TrailingSpace
> +utils/brmake Shellcheck
> +utils/config Shellcheck
> +utils/docker-run Shellcheck
> +utils/test-pkg Shellcheck
> diff --git a/utils/check-package b/utils/check-package
> index 732f1cf112..094b7b9c07 100755
> --- a/utils/check-package
> +++ b/utils/check-package
> @@ -107,6 +107,7 @@ DO_CHECK_INTREE = re.compile(r"|".join([
> r"package/",
> r"system/",
> r"toolchain/",
> + r"utils/",
> ]))
> DO_NOT_CHECK_INTREE = re.compile(r"|".join([
> r"boot/barebox/barebox\.mk$",
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH v2 09/14] utils/check-package: check files in board/
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (7 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 08/14] utils/check-package: check files in utils/ Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2022-07-31 19:35 ` [Buildroot] [PATCH v2 10/14] utils/check-package: check files in support/ Ricardo Martincoski
` (5 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
When a SysV init script is inside package/ it does not need to be
executable.
But this check does not apply in the case the script is in a fs_overlay.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- none
NOTE to the maintainer applying this patch: please re-generate the list
of ignored warnings while applying:
$ utils/docker-run make .checkpackageignore
---
.checkpackageignore | 91 +++++++++++++++++++++++++++++++
utils/check-package | 1 +
utils/checkpackagelib/lib_sysv.py | 3 +
utils/checkpackagelib/tool.py | 5 ++
4 files changed, 100 insertions(+)
diff --git a/.checkpackageignore b/.checkpackageignore
index 615fe22e52..d528b0122e 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -1,3 +1,94 @@
+board/aarch64-efi/post-image.sh Shellcheck
+board/amarula/a64-relic/post-build.sh Shellcheck
+board/amarula/vyasa/post-build.sh Shellcheck
+board/andes/ae350/post-build.sh Shellcheck
+board/arcturus/aarch64-ucls1012a/post-build.sh Shellcheck
+board/arcturus/aarch64-ucls1012a/post-image.sh Shellcheck
+board/aspeed/common/post-image.sh Shellcheck
+board/asus/tinker/post-build.sh Shellcheck
+board/atmel/flasher.sh Shellcheck
+board/beaglebone/post-build.sh Shellcheck
+board/beagleboneai/post-build.sh Shellcheck
+board/beaglev/post-build.sh Shellcheck
+board/beelink/gs1/post-build.sh Shellcheck
+board/boundarydevices/common/post-build.sh Shellcheck
+board/boundarydevices/common/post-image.sh Shellcheck
+board/canaan/k210-soc/post-build.sh Shellcheck
+board/canaan/k210-soc/rootfs_overlay/sbin/init Shellcheck
+board/chromebook/elm/sign.sh Shellcheck
+board/chromebook/mksd.sh Shellcheck
+board/chromebook/snow/linux-4.15-dts-tpm.patch ApplyOrder
+board/chromebook/snow/sign.sh Shellcheck
+board/cubietech/cubieboard2/post-image.sh Shellcheck
+board/firefly/roc-rk3399-pc/post-build.sh Shellcheck
+board/freescale/common/imx/imx8-bootloader-prepare.sh Shellcheck
+board/freescale/common/imx/post-image.sh Shellcheck
+board/freescale/common/mxs/post-image.sh Shellcheck
+board/friendlyarm/nanopc-t4/post-build.sh Shellcheck
+board/friendlyarm/nanopi-m4/post-build.sh Shellcheck
+board/friendlyarm/nanopi-neo-plus2/post-build.sh Shellcheck
+board/friendlyarm/nanopi-neo4/post-build.sh Shellcheck
+board/friendlyarm/nanopi-r2s/post-build.sh Shellcheck
+board/hardkernel/odroidc2/post-image.sh Shellcheck
+board/hardkernel/odroidc2/rootfs_overlay/etc/init.d/S09modload Shellcheck Variables
+board/hardkernel/odroidxu4/post-image.sh EmptyLastLine Shellcheck
+board/intel/galileo/post-build.sh Shellcheck
+board/intel/galileo/rootfs_overlay/etc/init.d/S09modload Shellcheck Variables
+board/kontron/bl-imx8mm/post-build.sh Shellcheck
+board/kontron/pitx-imx8m/patches/uboot/2022.04/0001-tools-mkeficapsule-use-pkg-config-to-get-luuid-and-l.patch NumberedSubject
+board/kontron/pitx-imx8m/post-build.sh Shellcheck
+board/kontron/smarc-sal28/post-build.sh Shellcheck
+board/lego/ev3/post-image.sh Shellcheck
+board/lemaker/bananapro/post-build.sh Shellcheck
+board/lemaker/bananapro/post-image.sh Shellcheck
+board/mender/x86_64/post-image-efi.sh ConsecutiveEmptyLines
+board/minnowboard/post-build.sh Shellcheck
+board/nexbox/a95x/post-build.sh Shellcheck
+board/nexbox/a95x/post-image.sh Shellcheck
+board/octavo/osd32mp1-red/patches/uboot/0001-Add-OSD32MP1-RED-Device-Tree-support.patch NumberedSubject
+board/octavo/osd32mp1-red/patches/uboot/0002-configs-stm32mp15_trusted_defconfig-disable-environm.patch NumberedSubject
+board/olimex/a13_olinuxino/post-build.sh Shellcheck
+board/olimex/a20_olinuxino/post-build.sh Shellcheck
+board/olimex/a33_olinuxino/post-build.sh Shellcheck
+board/olpc/post-build.sh Shellcheck
+board/orangepi/common/post-build.sh Shellcheck
+board/orangepi/orangepi-lite2/post-build.sh Shellcheck
+board/orangepi/orangepi-one-plus/post-build.sh Shellcheck
+board/orangepi/orangepi-rk3399/post-build.sh Shellcheck
+board/pine64/rock64/post-build.sh Shellcheck
+board/pine64/rockpro64/post-build.sh Shellcheck
+board/qemu/aarch64-sbsa/assemble-flash-images Shellcheck
+board/qemu/post-image.sh Shellcheck
+board/qemu/x86/post-build.sh Shellcheck
+board/qemu/x86_64/post-build.sh Shellcheck
+board/radxa/rockpi-4/post-build.sh Shellcheck
+board/radxa/rockpi-n10/post-build.sh Shellcheck
+board/radxa/rockpi-n8/post-build.sh Shellcheck
+board/raspberrypi/post-build.sh Shellcheck
+board/raspberrypi/post-image.sh Shellcheck
+board/roseapplepi/post-build.sh Shellcheck
+board/sifive/hifive-unleashed/post-build.sh Shellcheck
+board/sinovoip/m1-plus/post-build.sh Shellcheck
+board/solidrun/clearfog/post-build.sh Shellcheck
+board/solidrun/macchiatobin/post-build-mainline.sh Shellcheck
+board/solidrun/macchiatobin/post-build.sh Shellcheck
+board/stmicroelectronics/common/stm32f4xx/stm32-post-build.sh Shellcheck
+board/stmicroelectronics/common/stm32mp157/post-image.sh Shellcheck
+board/stmicroelectronics/stm32f429-disco/flash.sh Shellcheck
+board/stmicroelectronics/stm32f469-disco/flash_sd.sh Shellcheck
+board/stmicroelectronics/stm32f469-disco/flash_xip.sh Shellcheck
+board/stmicroelectronics/stm32f469-disco/post-build.sh Shellcheck
+board/synopsys/axs10x/post-build.sh Shellcheck
+board/technologic/ts4900/post-image.sh Shellcheck
+board/toradex/apalis-imx6/post-image.sh Shellcheck
+board/udoo/common/post-build.sh Shellcheck
+board/zynq/post-build.sh Shellcheck
+board/zynq/post-image.sh Shellcheck
+board/zynqmp/kria/kv260/kv260.sh Shellcheck TrailingSpace
+board/zynqmp/kria/patches/uboot/0001-arm64-zynqmp-zynqmp-sm-k26-revA-Fix-DP-PLL-configura.patch NumberedSubject
+board/zynqmp/post-build.sh Shellcheck
+board/zynqmp/post-image.sh Shellcheck
+board/zynqmp/zcu106/patches/uboot/0001-arm64-zynqmp-zynqmp-zcu102-revA-Fix-DP-PLL-configura.patch NumberedSubject
package/alsamixergui/0001-misc-fixes.patch Sob
package/am33x-cm3/S93-am335x-pm-firmware-load Variables
package/android-tools/0008-Include-sysmacros.h-to-compile-with-glibc-2.28.patch Sob
diff --git a/utils/check-package b/utils/check-package
index 094b7b9c07..004352261b 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -101,6 +101,7 @@ CONFIG_IN_FILENAME = re.compile(r"Config\.\S*$")
DO_CHECK_INTREE = re.compile(r"|".join([
r"Config.in",
r"arch/",
+ r"board/",
r"boot/",
r"fs/",
r"linux/",
diff --git a/utils/checkpackagelib/lib_sysv.py b/utils/checkpackagelib/lib_sysv.py
index 386d085afc..dc4afd71b8 100644
--- a/utils/checkpackagelib/lib_sysv.py
+++ b/utils/checkpackagelib/lib_sysv.py
@@ -21,6 +21,9 @@ class Indent(_CheckFunction):
class NotExecutable(checkpackagelib.tool.NotExecutable):
+ def ignore(self):
+ return 'etc/init.d/' in self.filename
+
def hint(self):
return ", just make sure you use '$(INSTALL) -D -m 0755' in the .mk file"
diff --git a/utils/checkpackagelib/tool.py b/utils/checkpackagelib/tool.py
index e719fdd407..632aaa9f9e 100644
--- a/utils/checkpackagelib/tool.py
+++ b/utils/checkpackagelib/tool.py
@@ -4,7 +4,12 @@ from checkpackagelib.base import _Tool
class NotExecutable(_Tool):
+ def ignore(self):
+ return False
+
def run(self):
+ if self.ignore():
+ return
if os.access(self.filename, os.X_OK):
return ["{}:0: This file does not need to be executable{}".format(self.filename, self.hint())]
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH v2 10/14] utils/check-package: check files in support/
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (8 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 09/14] utils/check-package: check files in board/ Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2022-07-31 19:35 ` [Buildroot] [PATCH v2 11/14] Makefile: merge check-flake8 into check-package Ricardo Martincoski
` (4 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
The .mk files inside both support/dependencies and support/misc are not
package recipes, they are makefiles, so check-package doesn't understand
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>
---
Changes v1 -> v2:
- none
NOTE to the maintainer applying this patch: please re-generate the list
of ignored warnings while applying:
$ utils/docker-run make .checkpackageignore
---
.checkpackageignore | 41 +++++++++++++++++++++++++++++++++++++++++
utils/check-package | 6 ++++++
2 files changed, 47 insertions(+)
diff --git a/.checkpackageignore b/.checkpackageignore
index d528b0122e..91efb7fa1a 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -294,6 +294,47 @@ package/x11r7/xdriver_xf86-video-savage/0001-cross-compile.patch Sob
package/x11r7/xdriver_xf86-video-tdfx/0001-cross.patch Sob
package/x11r7/xserver_xorg-server/S40xorg Shellcheck Variables
package/xl2tp/xl2tpd TrailingSpace
+support/dependencies/check-host-asciidoc.sh Shellcheck
+support/dependencies/check-host-cmake.sh Shellcheck
+support/dependencies/check-host-gzip.sh Shellcheck
+support/dependencies/check-host-lzip.sh Shellcheck
+support/dependencies/check-host-make.sh Shellcheck
+support/dependencies/check-host-python3.sh Shellcheck
+support/dependencies/check-host-tar.sh Shellcheck
+support/dependencies/check-host-xzcat.sh Shellcheck
+support/dependencies/dependencies.sh Shellcheck
+support/download/bzr ConsecutiveEmptyLines Shellcheck
+support/download/cargo-post-process Shellcheck
+support/download/check-hash Shellcheck
+support/download/cvs Shellcheck
+support/download/dl-wrapper Shellcheck
+support/download/file Shellcheck
+support/download/git Shellcheck
+support/download/go-post-process Shellcheck
+support/download/hg Shellcheck
+support/download/scp Shellcheck
+support/download/sftp Shellcheck
+support/download/svn Shellcheck
+support/download/wget Shellcheck
+support/gnuconfig/update Shellcheck
+support/libtool/buildroot-libtool-v1.5.patch ApplyOrder Sob
+support/libtool/buildroot-libtool-v2.2.patch ApplyOrder Sob
+support/libtool/buildroot-libtool-v2.4.4.patch ApplyOrder
+support/libtool/buildroot-libtool-v2.4.patch ApplyOrder Sob
+support/misc/relocate-sdk.sh Shellcheck
+support/scripts/apply-patches.sh Shellcheck
+support/scripts/br2-external Shellcheck
+support/scripts/check-bin-arch Shellcheck
+support/scripts/check-host-rpath Shellcheck
+support/scripts/fix-configure-powerpc64.sh EmptyLastLine
+support/scripts/fix-rpath Shellcheck
+support/scripts/generate-gitlab-ci-yml Shellcheck
+support/scripts/mkmakefile ConsecutiveEmptyLines Shellcheck
+support/scripts/mkusers Shellcheck
+support/scripts/setlocalversion Shellcheck
+support/testing/tests/core/post-build.sh Shellcheck
+support/testing/tests/package/test_opkg/post-build.sh Shellcheck
+support/testing/tests/utils/test_get_developers/0001-package-binutils-change-.mk.patch NumberedSubject
utils/brmake Shellcheck
utils/config Shellcheck
utils/docker-run Shellcheck
diff --git a/utils/check-package b/utils/check-package
index 004352261b..423b53ffb8 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -106,6 +106,7 @@ DO_CHECK_INTREE = re.compile(r"|".join([
r"fs/",
r"linux/",
r"package/",
+ r"support/",
r"system/",
r"toolchain/",
r"utils/",
@@ -115,6 +116,11 @@ DO_NOT_CHECK_INTREE = re.compile(r"|".join([
r"fs/common\.mk$",
r"package/doc-asciidoc\.mk$",
r"package/pkg-\S*\.mk$",
+ r"support/dependencies/[^/]+\.mk$",
+ r"support/gnuconfig/config\.",
+ r"support/kconfig/",
+ r"support/misc/[^/]+\.mk$",
+ r"support/testing/tests/.*br2-external/",
r"toolchain/helpers\.mk$",
r"toolchain/toolchain-external/pkg-toolchain-external\.mk$",
]))
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH v2 11/14] Makefile: merge check-flake8 into check-package
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (9 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 10/14] utils/check-package: check files in support/ Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2022-07-31 19:35 ` [Buildroot] [PATCH v2 12/14] utils/docker-run: fix shellcheck warnings Ricardo Martincoski
` (3 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
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>
---
Changes v1 -> v2:
- minor update only commit message (do not mention docker image)
Cc: Arnout Vandecappelle <arnout@mind.be>
---
Makefile | 9 +-----
support/misc/gitlab-ci.yml.in | 4 ---
support/scripts/generate-gitlab-ci-yml | 2 +-
.../tests/utils/br2-external/utils/x-python | 2 ++
.../testing/tests/utils/test_check_package.py | 17 +++++++++++
utils/check-package | 3 ++
utils/checkpackagelib/lib_python.py | 1 +
utils/checkpackagelib/test_tool.py | 28 +++++++++++++++++++
utils/checkpackagelib/tool.py | 15 ++++++++++
9 files changed, 68 insertions(+), 13 deletions(-)
create mode 100644 support/testing/tests/utils/br2-external/utils/x-python
create mode 100644 utils/checkpackagelib/lib_python.py
diff --git a/Makefile b/Makefile
index c5dee409f1..30a4eed31b 100644
--- a/Makefile
+++ b/Makefile
@@ -125,7 +125,7 @@ endif
noconfig_targets := menuconfig nconfig gconfig xconfig config oldconfig randconfig \
defconfig %_defconfig allyesconfig allnoconfig alldefconfig syncconfig release \
randpackageconfig allyespackageconfig allnopackageconfig \
- print-version olddefconfig distclean manual manual-% check-package check-flake8
+ print-version olddefconfig distclean manual manual-% check-package
# Some global targets do not trigger a build, but are used to collect
# metadata, or do various checks. When such targets are triggered,
@@ -1233,13 +1233,6 @@ release:
print-version:
@echo $(BR2_VERSION_FULL)
-check-flake8:
- $(Q)git ls-tree -r --name-only HEAD \
- | xargs file \
- | grep 'Python script' \
- | cut -d':' -f1 \
- | xargs -- python3 -m flake8 --statistics
-
check-package:
$(Q)./utils/check-package `git ls-tree -r --name-only HEAD`
diff --git a/support/misc/gitlab-ci.yml.in b/support/misc/gitlab-ci.yml.in
index 3ac988a519..2fde904006 100644
--- a/support/misc/gitlab-ci.yml.in
+++ b/support/misc/gitlab-ci.yml.in
@@ -6,10 +6,6 @@
script:
- utils/get-developers -v
-.check-flake8_base:
- script:
- - make check-flake8
-
.check-package_base:
script:
- make check-package
diff --git a/support/scripts/generate-gitlab-ci-yml b/support/scripts/generate-gitlab-ci-yml
index aa43aac019..536ae0f042 100755
--- a/support/scripts/generate-gitlab-ci-yml
+++ b/support/scripts/generate-gitlab-ci-yml
@@ -26,7 +26,7 @@ gen_tests() {
local do_basics do_defconfigs do_runtime do_testpkg
local defconfigs_ext cfg tst
- basics=( check-package DEVELOPERS flake8 package )
+ basics=( check-package DEVELOPERS package )
defconfigs=( $(cd configs; LC_ALL=C ls -1 *_defconfig) )
diff --git a/support/testing/tests/utils/br2-external/utils/x-python b/support/testing/tests/utils/br2-external/utils/x-python
new file mode 100644
index 0000000000..63f77b6be1
--- /dev/null
+++ b/support/testing/tests/utils/br2-external/utils/x-python
@@ -0,0 +1,2 @@
+#!/usr/bin/env python3
+
diff --git a/support/testing/tests/utils/test_check_package.py b/support/testing/tests/utils/test_check_package.py
index aeda0857e3..e655bff1ec 100644
--- a/support/testing/tests/utils/test_check_package.py
+++ b/support/testing/tests/utils/test_check_package.py
@@ -249,3 +249,20 @@ class TestCheckPackage(unittest.TestCase):
self.assert_file_was_processed(m)
self.assert_warnings_generated_for_file(m)
self.assertIn("{}:0: run 'shellcheck' and fix the warnings".format(abs_file), w)
+
+ # python scripts are tested using flake8
+ rel_file = "utils/x-python"
+ abs_path = infra.filepath("tests/utils/br2-external")
+ abs_file = os.path.join(abs_path, rel_file)
+
+ w, m = call_script(["check-package", "-vvv", "-b", rel_file],
+ self.WITH_UTILS_IN_PATH, abs_path)
+ self.assert_file_was_processed(m)
+ self.assert_warnings_generated_for_file(m)
+ self.assertIn("{}:0: run 'flake8' and fix the warnings".format(rel_file), w)
+
+ w, m = call_script(["check-package", "-b", abs_file],
+ self.WITH_UTILS_IN_PATH, infra.basepath())
+ self.assert_file_was_processed(m)
+ self.assert_warnings_generated_for_file(m)
+ self.assertIn("{}:0: run 'flake8' and fix the warnings".format(abs_file), w)
diff --git a/utils/check-package b/utils/check-package
index 423b53ffb8..3507709707 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -14,6 +14,7 @@ import checkpackagelib.lib_config
import checkpackagelib.lib_hash
import checkpackagelib.lib_mk
import checkpackagelib.lib_patch
+import checkpackagelib.lib_python
import checkpackagelib.lib_shellscript
import checkpackagelib.lib_sysv
@@ -94,6 +95,8 @@ def get_lib_from_filetype(fname):
filetype = magic.from_file(fname, mime=True)
if filetype == "text/x-shellscript":
return checkpackagelib.lib_shellscript
+ if filetype in ["text/x-python", "text/x-script.python"]:
+ return checkpackagelib.lib_python
return None
diff --git a/utils/checkpackagelib/lib_python.py b/utils/checkpackagelib/lib_python.py
new file mode 100644
index 0000000000..f8c17ddc10
--- /dev/null
+++ b/utils/checkpackagelib/lib_python.py
@@ -0,0 +1 @@
+from checkpackagelib.tool import Flake8 # noqa: F401
diff --git a/utils/checkpackagelib/test_tool.py b/utils/checkpackagelib/test_tool.py
index a0bf88001d..cfa826f57c 100644
--- a/utils/checkpackagelib/test_tool.py
+++ b/utils/checkpackagelib/test_tool.py
@@ -66,6 +66,34 @@ def test_NotExecutable_hint(testname, hint, filename, permissions, string, expec
assert warnings == expected
+Flake8 = [
+ ('empty',
+ 'empty.py',
+ '',
+ []),
+ ('W391',
+ 'blank-line.py',
+ '\n',
+ ["dir/blank-line.py:0: run 'flake8' and fix the warnings",
+ "dir/blank-line.py:1:1: W391 blank line at end of file"]),
+ ('more than one warning',
+ 'file',
+ 'import os\n'
+ 'import re\n'
+ '\n',
+ ["dir/file:0: run 'flake8' and fix the warnings",
+ "dir/file:1:1: F401 'os' imported but unused\n"
+ "dir/file:2:1: F401 're' imported but unused\n"
+ 'dir/file:3:1: W391 blank line at end of file']),
+ ]
+
+
+@pytest.mark.parametrize('testname,filename,string,expected', Flake8)
+def test_Flake8(testname, filename, string, expected):
+ warnings = check_file(m.Flake8, filename, string)
+ assert warnings == expected
+
+
Shellcheck = [
('missing shebang',
'empty.sh',
diff --git a/utils/checkpackagelib/tool.py b/utils/checkpackagelib/tool.py
index 632aaa9f9e..907ada704f 100644
--- a/utils/checkpackagelib/tool.py
+++ b/utils/checkpackagelib/tool.py
@@ -1,5 +1,7 @@
+import flake8.main.application
import os
import subprocess
+import tempfile
from checkpackagelib.base import _Tool
@@ -14,6 +16,19 @@ class NotExecutable(_Tool):
return ["{}:0: This file does not need to be executable{}".format(self.filename, self.hint())]
+class Flake8(_Tool):
+ def run(self):
+ with tempfile.NamedTemporaryFile() as output:
+ app = flake8.main.application.Application()
+ app.run(['--output-file={}'.format(output.name), self.filename])
+ stdout = output.readlines()
+ processed_output = [str(line.decode().rstrip()) for line in stdout if line]
+ if len(stdout) == 0:
+ return
+ return ["{}:0: run 'flake8' and fix the warnings".format(self.filename),
+ '\n'.join(processed_output)]
+
+
class Shellcheck(_Tool):
def run(self):
cmd = ['shellcheck', self.filename]
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH v2 12/14] utils/docker-run: fix shellcheck warnings
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (10 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 11/14] Makefile: merge check-flake8 into check-package Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2022-07-31 19:35 ` [Buildroot] [PATCH v2 13/14] utils/checkpackagelib: warn about $(HOST_DIR)/usr Ricardo Martincoski
` (2 subsequent siblings)
14 siblings, 0 replies; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
SC2046: Quote this to prevent word splitting.
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- none
---
.checkpackageignore | 1 -
utils/docker-run | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/.checkpackageignore b/.checkpackageignore
index 91efb7fa1a..e9d996008d 100644
--- a/.checkpackageignore
+++ b/.checkpackageignore
@@ -337,5 +337,4 @@ support/testing/tests/package/test_opkg/post-build.sh Shellcheck
support/testing/tests/utils/test_get_developers/0001-package-binutils-change-.mk.patch NumberedSubject
utils/brmake Shellcheck
utils/config Shellcheck
-utils/docker-run Shellcheck
utils/test-pkg Shellcheck
diff --git a/utils/docker-run b/utils/docker-run
index 5653764254..f42dc33d0e 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -7,7 +7,7 @@ IMAGE=$(grep ^image: "${MAIN_DIR}/.gitlab-ci.yml" | \
sed -e 's,^image: ,,g' | sed -e 's,\$CI_REGISTRY,registry.gitlab.com,g')
exec docker run -it --rm \
- --user $(id -u):$(id -g) \
+ --user "$(id -u)":"$(id -g)" \
--mount "type=bind,src=${MAIN_DIR},dst=${MAIN_DIR}" \
--workdir "${MAIN_DIR}" \
"${IMAGE}" "${@}"
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH v2 13/14] utils/checkpackagelib: warn about $(HOST_DIR)/usr
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (11 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 12/14] utils/docker-run: fix shellcheck warnings Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2022-07-31 19:35 ` [Buildroot] [RFC 14/14] utils/git_hooks: new script Ricardo Martincoski
2023-05-01 19:50 ` [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Yann E. MORIN
14 siblings, 0 replies; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Yann E . MORIN, Ricardo Martincoski
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>
---
Changes v1 -> v2:
- update commit message, the patch was applied
NOTE to the maintainer applying this patch: please re-generate the list
of ignored warnings while applying:
$ utils/docker-run make .checkpackageignore
---
utils/checkpackagelib/lib_mk.py | 12 ++++++++++++
utils/checkpackagelib/test_lib_mk.py | 23 +++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/utils/checkpackagelib/lib_mk.py b/utils/checkpackagelib/lib_mk.py
index b50a19ac62..eaef19b2ec 100644
--- a/utils/checkpackagelib/lib_mk.py
+++ b/utils/checkpackagelib/lib_mk.py
@@ -21,6 +21,18 @@ continue_conditional = ["elif", "else"]
end_conditional = ["endif"]
+class DoNotInstallToHostdirUsr(_CheckFunction):
+ INSTALL_TO_HOSTDIR_USR = re.compile(r"^[^#].*\$\(HOST_DIR\)/usr")
+
+ def check_line(self, lineno, text):
+ if self.INSTALL_TO_HOSTDIR_USR.match(text.rstrip()):
+ if self.filename in ['package/skeleton/skeleton.mk']:
+ return
+ return ["{}:{}: install files to $(HOST_DIR)/ instead of $(HOST_DIR)/usr/"
+ .format(self.filename, lineno),
+ text]
+
+
class Indent(_CheckFunction):
COMMENT = re.compile(r"^\s*#")
CONDITIONAL = re.compile(r"^\s*({})\s".format("|".join(start_conditional + end_conditional + continue_conditional)))
diff --git a/utils/checkpackagelib/test_lib_mk.py b/utils/checkpackagelib/test_lib_mk.py
index 49fa216fcd..db6641f14c 100644
--- a/utils/checkpackagelib/test_lib_mk.py
+++ b/utils/checkpackagelib/test_lib_mk.py
@@ -3,6 +3,29 @@ import checkpackagelib.test_util as util
import checkpackagelib.lib_mk as m
+DoNotInstallToHostdirUsr = [
+ ('real case',
+ 'libapparmor.mk',
+ 'LIBAPPARMOR_CONF_OPTS += \\\n'
+ '\t--with-python \\\n'
+ '\tPYTHON=$(HOST_DIR)/usr/bin/python3 \\\n'
+ '\tPYTHON_CONFIG=$(STAGING_DIR)/usr/bin/python3-config \\\n'
+ '\tSWIG=$(SWIG)\n',
+ [['libapparmor.mk:3: install files to $(HOST_DIR)/ instead of $(HOST_DIR)/usr/',
+ '\tPYTHON=$(HOST_DIR)/usr/bin/python3 \\\n']]),
+ ('ignore comment',
+ 'any',
+ '# following code do not install to $(HOST_DIR)/usr/\n',
+ []),
+ ]
+
+
+@pytest.mark.parametrize('testname,filename,string,expected', DoNotInstallToHostdirUsr)
+def test_DoNotInstallToHostdirUsr(testname, filename, string, expected):
+ warnings = util.check_file(m.DoNotInstallToHostdirUsr, filename, string)
+ assert warnings == expected
+
+
Indent = [
('ignore comment at beginning of line',
'any',
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [RFC 14/14] utils/git_hooks: new script
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (12 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [PATCH v2 13/14] utils/checkpackagelib: warn about $(HOST_DIR)/usr Ricardo Martincoski
@ 2022-07-31 19:35 ` Ricardo Martincoski
2023-04-23 19:41 ` Yann E. MORIN
2023-05-01 19:50 ` [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Yann E. MORIN
14 siblings, 1 reply; 25+ messages in thread
From: Ricardo Martincoski @ 2022-07-31 19:35 UTC (permalink / raw)
To: buildroot; +Cc: Ricardo Martincoski
Add a pre-commit hook that can be used by any buildroot developer to
prevent local commits that introduce warnings that check-package would
catch.
It can also be used by maintainers.
This hook checks only the files added/changed by the commit, so it runs
fast.
It also skips the check when working in branches that do not have this
infra.
But when there is a change to check-package itself in the commit being
performed, the shole tree is checked, since new warnings in the tree can
occur.
At same time, add an option -T to utils/docker-run so docker can run
without a tty, otherwise calling it from inside the commit hook would
lead to this error:
the input device is not a TTY
Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
---
Changes v1 -> v2:
- new patch, only a RFC!
Please someone do a thorough review on this patch.
Last time I wrote a pre-commit hook was more than 10 years ago. :)
Output from local tests:
$ git checkout origin/2022.05.x
$ echo >> utils/check-package
$ git add -u
$ git commit -m 'another branch'
check-package will run in the full tree...
the input device is not a TTY
the input device is not a TTY
check-package SKIPPED for commit.
[detached HEAD aaf6ab603a] another branch
1 file changed, 1 insertion(+)
$
$ git checkout make-check-package-standardize-flake8
$ echo >> utils/check-package
$ git add -u
$ git commit -m 'check full tree'
check-package will run in the full tree...
utils/check-package:0: run 'flake8' and fix the warnings
check-package FAILED for commit.
$
$ git checkout make-check-package-standardize-flake8
$ git rm support/libtool/buildroot-libtool-v2.4.patch
$ git commit -m 'check full tree'
check-package will run in the full tree...
Commit successfully checked with check-package.
[make-check-package-standardize-flake8 b761eccc68] check full tree
1 file changed, 89 deletions(-)
delete mode 100644 support/libtool/buildroot-libtool-v2.4.patch
$
# notice the entry on .checkpackageignore remains for deleted files
# but it can be removed from time to time by using:
# $ utils/docker-run make .checkpackageignore
$ git checkout make-check-package-standardize-flake8
$ sed -e 's, , ,g' -i package/atop/atop.hash
$ git add -u
$ git commit -m 'only changed'
package/atop/atop.hash:2: separation does not match expectation (http://nightly.buildroot.org/#adding-packages-hash)
package/atop/atop.hash:5: separation does not match expectation (http://nightly.buildroot.org/#adding-packages-hash)
check-package FAILED for commit.
$
$ git checkout make-check-package-standardize-flake8
$ cp package/busybox/S02sysctl package/busybox/S02sysctl2
$ git add package/busybox/S02sysctl2
$ git commit -m 'only added'
package/busybox/S02sysctl2:0: DAEMON variable not defined (http://nightly.buildroot.org/#adding-packages-start-script)
check-package FAILED for commit.
$
---
DEVELOPERS | 1 +
utils/docker-run | 13 ++++++++++-
utils/git_hooks/pre-commit | 46 ++++++++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 1 deletion(-)
create mode 100644 utils/git_hooks/pre-commit
diff --git a/DEVELOPERS b/DEVELOPERS
index 88e0d02c69..5908ca7f33 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -2499,6 +2499,7 @@ F: support/testing/tests/utils/test_check_package.py
F: utils/check-package
F: utils/checkpackagelib/
F: utils/docker-run
+F: utils/git_hooks/
N: Richard Braun <rbraun@sceen.net>
F: package/curlftpfs/
diff --git a/utils/docker-run b/utils/docker-run
index f42dc33d0e..bcddcd5a1b 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -1,12 +1,23 @@
#!/usr/bin/env bash
set -o errexit -o pipefail
+
+tty="-t"
+while getopts "T" OPT; do
+ case "${OPT}" in
+ T) tty=;;
+ *) exit 1;;
+ esac
+done
+shift $((OPTIND-1))
+
DIR=$(dirname "${0}")
MAIN_DIR=$(readlink -f "${DIR}/..")
# shellcheck disable=SC2016
IMAGE=$(grep ^image: "${MAIN_DIR}/.gitlab-ci.yml" | \
sed -e 's,^image: ,,g' | sed -e 's,\$CI_REGISTRY,registry.gitlab.com,g')
-exec docker run -it --rm \
+# shellcheck disable=SC2086
+exec docker run -i ${tty} --rm \
--user "$(id -u)":"$(id -g)" \
--mount "type=bind,src=${MAIN_DIR},dst=${MAIN_DIR}" \
--workdir "${MAIN_DIR}" \
diff --git a/utils/git_hooks/pre-commit b/utils/git_hooks/pre-commit
new file mode 100644
index 0000000000..c8764c4d57
--- /dev/null
+++ b/utils/git_hooks/pre-commit
@@ -0,0 +1,46 @@
+#!/bin/sh
+# In order to prevent commits with check-package warnings, do the following:
+# cp utils/git_hooks/pre-commit .git/hooks/pre-commit
+# chmod +x .git/hooks/pre-commit
+
+# shellcheck disable=SC1091
+. git-sh-setup
+cd_to_toplevel
+
+full_tree=0
+if git diff --cached --name-only HEAD | grep -q '^utils/\(checkpackagelib\|check-package\)'; then
+ full_tree=1
+else
+ removed_typechanged=$(git diff --cached --name-only --diff-filter=acmr HEAD | wc -l)
+ if [ "$removed_typechanged" != 0 ]; then
+ full_tree=1
+ fi
+fi
+
+if [ $full_tree = 1 ]; then
+ echo "check-package will run in the full tree..."
+ # shellcheck disable=SC2046 disable=SC2006
+ utils/docker-run -T utils/check-package -q `git ls-files`
+ result=$?
+else
+ added_changed_renamed=$(git diff --cached --name-only --diff-filter=ACMR HEAD | wc -l)
+ if [ "$added_changed_renamed" = 0 ]; then
+ echo "No files in commit to be passed to check-package."
+ exit 0
+ fi
+ git diff --cached --name-only --diff-filter=ACMR -z HEAD | \
+ xargs -0 utils/docker-run -T utils/check-package -q
+ result=$?
+fi
+
+if [ $result = 0 ]; then
+ echo "Commit successfully checked with check-package."
+ exit 0
+fi
+if utils/docker-run -T true; then
+ echo "check-package FAILED for commit."
+ exit $result
+else
+ # probably in a branch without 'utils/docker-run -T'
+ echo "check-package SKIPPED for commit."
+fi
--
2.25.1
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [Buildroot] [RFC 14/14] utils/git_hooks: new script
2022-07-31 19:35 ` [Buildroot] [RFC 14/14] utils/git_hooks: new script Ricardo Martincoski
@ 2023-04-23 19:41 ` Yann E. MORIN
0 siblings, 0 replies; 25+ messages in thread
From: Yann E. MORIN @ 2023-04-23 19:41 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: buildroot
Ricardo, All,
On 2022-07-31 16:35 -0300, Ricardo Martincoski spake thusly:
> Add a pre-commit hook that can be used by any buildroot developer to
> prevent local commits that introduce warnings that check-package would
> catch.
The thing is, most users will not add that hook before comitting and
submitting patches.
> It can also be used by maintainers.
In fact, as maintainers, what we are more insterested in, is a
post-applypatch hook, not a pre-commit one. However, your script
does not work as a post-applypatch hook.
Furthermore, maintainers already have their hooks. For example, I have
been maintaining and publishing mines:
https://git.buildroot.org/~ymorin/git/buildroot.git-hooks
Even if the history only dates back to 2021-11-14, I had those hooks
even before, and they are an aggregation of what Arnout and Peter
already had before and shared with me.
My hooks are a bit slow to run, as they run on all commits, even during
a rebase, or when applying a series of patches (which I do with a helper
around patchwork API). I am OK with the extra time it takes, but that is
not necessarily the case for other maintainers.
> This hook checks only the files added/changed by the commit, so it runs
> fast.
> It also skips the check when working in branches that do not have this
> infra.
> But when there is a change to check-package itself in the commit being
> performed, the shole tree is checked, since new warnings in the tree can
> occur.
Ah, this last part is nice, indeed.
But as a maintainer, when I see changes to, say, check-package, I do run
it to validate it, so changes in the ignore list, or new issues, or
similar, would get caught.
And if check-package et al. add new checks, either the issues it reports
were previously fixed (so we can run that new check unconditionally), or
the offenders have been added to the ignore list (or will be in the next
patch).
So, I am not sure this is important.
> At same time, add an option -T to utils/docker-run so docker can run
> without a tty, otherwise calling it from inside the commit hook would
> lead to this error:
> the input device is not a TTY
I already applied a better patch, that automatically detects if a tty is
needed or not, see 3d8212c4b29d (utils/docker-run: allow running without
a tty).
In the end, I am not convinced we should have this hook in the tree...
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2
2022-07-31 19:35 [Buildroot] [PATCH v2 00/14] Preventing style regressions using check-package v2 Ricardo Martincoski
` (13 preceding siblings ...)
2022-07-31 19:35 ` [Buildroot] [RFC 14/14] utils/git_hooks: new script Ricardo Martincoski
@ 2023-05-01 19:50 ` Yann E. MORIN
14 siblings, 0 replies; 25+ messages in thread
From: Yann E. MORIN @ 2023-05-01 19:50 UTC (permalink / raw)
To: Ricardo Martincoski; +Cc: Romain Naour, buildroot
Ricardo, All,
(I'm not sure which series introduced the ignore list for
check-package, so I'm hjijacking this one.)
When there is a file listed in .checkpackageignore, and that file is no
longer present, that does not trigger an error.
This hapens for example when a patch is removed, or when it is renamed.
For example 60d8e5257608 (package/lua: bump to version 5.4.5) renamed
the three lua 5.4.4 patches, but their entries in the ignore list were
not updated.
Would it be possible/easy to have check-package verify that all files in
the ignore list do exist?
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 25+ messages in thread