[Buildroot] [PATCH v2 2/9] support/scripts/check-package: new script
Ricardo Martincoski
ricardo.martincoski at gmail.com
Sun Feb 19 22:17:17 UTC 2017
Create the infra to check the style of new packages before submitting.
The overall function of the script is described inside a txt file.
It is designed to process the actual files and NOT the patch files
generated by git format-patch.
Also add the first check function, to warn if a file (Config.*, *.mk,
*.hash, *.patch) has no newline at the last line of the file, see [1].
Basic usage for simple packages:
support/scripts/check-package -vvv package/newpackage/*
Basic usage for packages with subdirs:
support/scripts/check-package -vvv $(find package/newpackage/ -type f)
See "checkpackage" in [2].
[1] http://patchwork.ozlabs.org/patch/631129/
[2] http://elinux.org/Buildroot#Todo_list
Signed-off-by: Ricardo Martincoski <ricardo.martincoski at gmail.com>
Cc: Thomas De Schampheleire <patrickdepinguin at gmail.com>
---
Changes v1 -> v2:
- support packages with different config filenames (Thomas DS);
- define named constants instead of magic '3' (Thomas DS);
- use classes instead of functions to declare each check (Thomas DS);
- assign to self (using classes) instead of using static variables
(Thomas DS);
- the only command line argument the check functions need now is the
url to the manual, so pass only this as argument instead of all
flags;
- add entry do DEVELOPERS file;
- hold command line arguments as a global (to allow below changes);
- use a custom function as predicate to inspect.getmembers
(Thomas DS);
- do not pass command line arguments everywhere;
---
Notes:
$ time support/scripts/check-package $(find package -type f) >/dev/null 2>/dev/null
real 0m0.391s
user 0m0.344s
sys 0m0.044s
NewlineAtEof:
support/scripts/check-package --include-only NewlineAtEof \
$(find package -type f) 2>/dev/null | wc -l
0
(cd support/scripts/check-package-example && \
../check-package --include-only NewlineAtEof -vv package/*/*)
package/package1/Config.something:10: missing newline at end of file
endmenu
package/package1/wrong-name.patch:14: missing newline at end of file
#include <curses.h>
180 lines processed
2 warnings generated
DEVELOPERS | 3 +
support/scripts/check-package | 144 ++++++++++++++++++++++++++++++
support/scripts/check-package.txt | 76 ++++++++++++++++
support/scripts/checkpackagebase.py | 16 ++++
support/scripts/checkpackagelib.py | 19 ++++
support/scripts/checkpackagelib_config.py | 7 ++
support/scripts/checkpackagelib_hash.py | 7 ++
support/scripts/checkpackagelib_mk.py | 8 ++
support/scripts/checkpackagelib_patch.py | 7 ++
9 files changed, 287 insertions(+)
create mode 100755 support/scripts/check-package
create mode 100644 support/scripts/check-package.txt
create mode 100644 support/scripts/checkpackagebase.py
create mode 100644 support/scripts/checkpackagelib.py
create mode 100644 support/scripts/checkpackagelib_config.py
create mode 100644 support/scripts/checkpackagelib_hash.py
create mode 100644 support/scripts/checkpackagelib_mk.py
create mode 100644 support/scripts/checkpackagelib_patch.py
diff --git a/DEVELOPERS b/DEVELOPERS
index 09a0a6e85..4dda13ce1 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -1260,6 +1260,9 @@ F: package/atop/
N: Rhys Williams <github at wilberforce.co.nz>
F: package/lirc-tools/
+N: Ricardo Martincoski <ricardo.martincoski at gmail.com>
+F: support/scripts/check*package*
+
N: Richard Braun <rbraun at sceen.net>
F: package/curlftpfs/
F: package/tzdata/
diff --git a/support/scripts/check-package b/support/scripts/check-package
new file mode 100755
index 000000000..2add4712c
--- /dev/null
+++ b/support/scripts/check-package
@@ -0,0 +1,144 @@
+#!/usr/bin/env python
+# See support/scripts/check-package.txt before editing this file.
+
+from __future__ import print_function
+import argparse
+import inspect
+import re
+import sys
+
+import checkpackagelib_config
+import checkpackagelib_hash
+import checkpackagelib_mk
+import checkpackagelib_patch
+
+VERBOSE_LEVEL_TO_SHOW_IGNORED_FILES = 3
+flags = None # Command line arguments.
+
+
+def parse_args():
+ parser = argparse.ArgumentParser()
+
+ # Do not use argparse.FileType("r") here because only files with known
+ # format will be open based on the filename.
+ parser.add_argument("files", metavar="F", type=str, nargs="*",
+ help="list of files")
+
+ parser.add_argument("--manual-url", action="store",
+ default="http://nightly.buildroot.org/",
+ help="default: %(default)s")
+ parser.add_argument("--verbose", "-v", action="count", default=0)
+
+ # Now the debug options in the order they are processed.
+ parser.add_argument("--include-only", dest="include_list", action="append",
+ help="run only the specified functions (debug)")
+ parser.add_argument("--exclude", dest="exclude_list", action="append",
+ 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)")
+
+ return parser.parse_args()
+
+
+CONFIG_IN_FILENAME = re.compile("/Config\.\S*$")
+FILE_IS_FROM_A_PACKAGE = re.compile("package/[^/]*/")
+
+
+def get_lib_from_filename(fname):
+ if FILE_IS_FROM_A_PACKAGE.search(fname) is None:
+ return None
+ if CONFIG_IN_FILENAME.search(fname):
+ return checkpackagelib_config
+ if fname.endswith(".hash"):
+ return checkpackagelib_hash
+ if fname.endswith(".mk"):
+ return checkpackagelib_mk
+ if fname.endswith(".patch"):
+ return checkpackagelib_patch
+ return None
+
+
+def is_a_check_function(m):
+ if not inspect.isclass(m):
+ return False
+ # do not call the base class
+ if m.__name__.startswith("_"):
+ return False
+ if flags.include_list and m.__name__ not in flags.include_list:
+ return False
+ if flags.exclude_list and m.__name__ in flags.exclude_list:
+ return False
+ return True
+
+
+def print_warnings(warnings):
+ # Avoid the need to use 'return []' at the end of every check function.
+ if warnings is None:
+ return 0 # No warning generated.
+
+ for level, message in enumerate(warnings):
+ if flags.verbose >= level:
+ print(message.replace("\t", "< tab >").rstrip())
+ return 1 # One more warning to count.
+
+
+def check_file_using_lib(fname):
+ # Count number of warnings generated and lines processed.
+ nwarnings = 0
+ nlines = 0
+
+ lib = get_lib_from_filename(fname)
+ if not lib:
+ if flags.verbose >= VERBOSE_LEVEL_TO_SHOW_IGNORED_FILES:
+ print("{}: ignored".format(fname))
+ return nwarnings, nlines
+ classes = inspect.getmembers(lib, is_a_check_function)
+
+ if flags.dry_run:
+ functions_to_run = [c[0] for c in classes]
+ print("{}: would run: {}".format(fname, functions_to_run))
+ return nwarnings, nlines
+
+ objects = [c[1](fname, flags.manual_url) for c in classes]
+
+ for cf in objects:
+ nwarnings += print_warnings(cf.before())
+ for lineno, text in enumerate(open(fname, "r").readlines()):
+ nlines += 1
+ for cf in objects:
+ nwarnings += print_warnings(cf.check_line(lineno + 1, text))
+ for cf in objects:
+ nwarnings += print_warnings(cf.after())
+
+ return nwarnings, nlines
+
+
+def __main__():
+ global flags
+ flags = parse_args()
+
+ if len(flags.files) == 0:
+ print("No files to check style")
+ sys.exit(1)
+
+ # Accumulate number of warnings generated and lines processed.
+ total_warnings = 0
+ total_lines = 0
+
+ for fname in flags.files:
+ nwarnings, nlines = check_file_using_lib(fname)
+ total_warnings += nwarnings
+ total_lines += nlines
+
+ # The warning messages are printed to stdout and can be post-processed
+ # (e.g. counted by 'wc'), so for stats use stderr. Wait all warnings are
+ # printed, for the case there are many of them, before printing stats.
+ sys.stdout.flush()
+ print("{} lines processed".format(total_lines), file=sys.stderr)
+ print("{} warnings generated".format(total_warnings), file=sys.stderr)
+
+ if total_warnings > 0:
+ sys.exit(1)
+
+
+__main__()
diff --git a/support/scripts/check-package.txt b/support/scripts/check-package.txt
new file mode 100644
index 000000000..630cd04f0
--- /dev/null
+++ b/support/scripts/check-package.txt
@@ -0,0 +1,76 @@
+How the scripts are structured:
+- check-package is the main engine, called by the user.
+ For each input file, this script decides which parser should be used and it
+ collects all classes declared in the library file and instantiates them.
+ The main engine opens the input files and it serves each raw line (including
+ newline!) to the method check_line() of every check object.
+ Two special methods before() and after() are used to call the initialization
+ of variables (for the case it needs to keep data across calls) and the
+ equivalent finalization (e.g. for the case a warning must be issued if some
+ pattern is not in the input file).
+- checkpackagebase.py contains the base class for all check functions.
+- checkpackagelib.py contains the classes for common check functions.
+ Each check function is explicitly included in a given type-parsing library.
+ Do not include every single check function in this file, a class that will
+ only parse hash files should be implemented in the hash-parsing library.
+ When a warning must be issued, the check function returns an array of strings.
+ Each string is a warning message and is displayed if the corresponding verbose
+ level is active. When the script is called without --verbose only the first
+ warning in the returned array is printed; when called with --verbose both
+ first and second warnings are printed; when called with -vv until the third
+ warning is printed; an so on.
+ Helper functions can be defined and will not be called by the main script.
+- checkpackagelib_type.py contains check functions specific to files of this
+ type.
+
+Some hints when changing this code:
+- prefer O(n) algorithms, where n is the total number of lines in the files
+ processed.
+- when there is no other reason for ordering, use alphabetical order (e.g. keep
+ the check functions in alphabetical order, keep the imports in alphabetical
+ order, and so on).
+- use pyflakes to detect and fix potential problems.
+- use pep8 formatting.
+- keep in mind that for every class the method before() will be called before
+ any line is served to be checked by the method check_line(). A class that
+ checks the filename should only implement the method before(). A function that
+ needs to keep data across calls (e.g. keep the last line before the one being
+ processed) should initialize all variables using this method.
+- keep in mind that for every class the method after() will be called after all
+ lines were served to be checked by the method check_line(). A class that
+ checks the absence of a pattern in the file will need to use this method.
+- try to avoid false warnings. It's better to not issue a warning message to a
+ corner case than have too many false warnings. The second can make users stop
+ using the script.
+- do not check spacing in the input line in every single function. Trailing
+ whitespace and wrong indentation should be checked by separate functions.
+- avoid duplicate tests. Try to test only one thing in each function.
+- in the warning message, include the url to a section from the manual, when
+ applicable. It potentially will make more people know the manual.
+- use short sentences in the warning messages. A complete explanation can be
+ added to show when --verbose is used.
+- when testing, verify the error message is displayed when the error pattern is
+ found, but also verify the error message is not displayed for few
+ well-formatted packages... there are many of these, just pick your favorite
+ as golden package that should not trigger any warning message.
+- check the url displayed by the warning message works.
+
+Usage examples:
+- to get a list of check functions that would be called without actually
+ calling them you can use the --dry-run option:
+$ support/scripts/check-package --dry-run package/yourfavorite/*
+
+- when you just added a new check function, e.g. Something, check how it behaves
+ for all current packages:
+$ support/scripts/check-package --include-only Something $(find package -type f)
+
+- the effective processing time (when the .pyc were already generated and all
+ files to be processed are cached in the RAM) should stay in the order of few
+ seconds:
+$ support/scripts/check-package $(find package -type f) >/dev/null ; \
+ time support/scripts/check-package $(find package -type f) >/dev/null
+
+- vim users can navigate the warnings (most editors probably have similar
+ function) since warnings are generated in the form 'path/file:line: warning':
+$ find package/ -name 'Config.*' > filelist && vim -c \
+ 'set makeprg=support/scripts/check-package\ $(cat\ filelist)' -c make -c copen
diff --git a/support/scripts/checkpackagebase.py b/support/scripts/checkpackagebase.py
new file mode 100644
index 000000000..e4c664d24
--- /dev/null
+++ b/support/scripts/checkpackagebase.py
@@ -0,0 +1,16 @@
+# See support/scripts/check-package.txt before editing this file.
+
+
+class _CheckFunction(object):
+ def __init__(self, filename, url_to_manual):
+ self.filename = filename
+ self.url_to_manual = url_to_manual
+
+ def before(self):
+ pass
+
+ def check_line(self, lineno, text):
+ pass
+
+ def after(self):
+ pass
diff --git a/support/scripts/checkpackagelib.py b/support/scripts/checkpackagelib.py
new file mode 100644
index 000000000..1a4904183
--- /dev/null
+++ b/support/scripts/checkpackagelib.py
@@ -0,0 +1,19 @@
+# See support/scripts/check-package.txt before editing this file.
+
+from checkpackagebase import _CheckFunction
+
+
+class NewlineAtEof(_CheckFunction):
+ def before(self):
+ self.lastlineno = 0
+ self.lastline = "\n"
+
+ def check_line(self, lineno, text):
+ self.lastlineno = lineno
+ self.lastline = text
+
+ def after(self):
+ if self.lastline == self.lastline.rstrip("\r\n"):
+ return ["{}:{}: missing newline at end of file"
+ .format(self.filename, self.lastlineno),
+ self.lastline]
diff --git a/support/scripts/checkpackagelib_config.py b/support/scripts/checkpackagelib_config.py
new file mode 100644
index 000000000..f546d173e
--- /dev/null
+++ b/support/scripts/checkpackagelib_config.py
@@ -0,0 +1,7 @@
+# See support/scripts/check-package.txt before editing this file.
+# Kconfig generates errors if someone introduces a typo like "boool" instead of
+# "bool", so below check functions don't need to check for things already
+# checked by running "make menuconfig".
+
+# Notice: ignore 'imported but unused' from pyflakes for check functions.
+from checkpackagelib import NewlineAtEof
diff --git a/support/scripts/checkpackagelib_hash.py b/support/scripts/checkpackagelib_hash.py
new file mode 100644
index 000000000..8c0337fc9
--- /dev/null
+++ b/support/scripts/checkpackagelib_hash.py
@@ -0,0 +1,7 @@
+# See support/scripts/check-package.txt before editing this file.
+# The validity of the hashes itself is checked when building, so below check
+# functions don't need to check for things already checked by running
+# "make package-dirclean package-source".
+
+# Notice: ignore 'imported but unused' from pyflakes for check functions.
+from checkpackagelib import NewlineAtEof
diff --git a/support/scripts/checkpackagelib_mk.py b/support/scripts/checkpackagelib_mk.py
new file mode 100644
index 000000000..84eeef889
--- /dev/null
+++ b/support/scripts/checkpackagelib_mk.py
@@ -0,0 +1,8 @@
+# See support/scripts/check-package.txt before editing this file.
+# There are already dependency checks during the build, so below check
+# functions don't need to check for things already checked by exploring the
+# menu options using "make menuconfig" and by running "make" with appropriate
+# packages enabled.
+
+# Notice: ignore 'imported but unused' from pyflakes for check functions.
+from checkpackagelib import NewlineAtEof
diff --git a/support/scripts/checkpackagelib_patch.py b/support/scripts/checkpackagelib_patch.py
new file mode 100644
index 000000000..131e42347
--- /dev/null
+++ b/support/scripts/checkpackagelib_patch.py
@@ -0,0 +1,7 @@
+# See support/scripts/check-package.txt before editing this file.
+# The format of the patch files is tested during the build, so below check
+# functions don't need to check for things already checked by running
+# "make package-dirclean package-patch".
+
+# Notice: ignore 'imported but unused' from pyflakes for check functions.
+from checkpackagelib import NewlineAtEof
--
2.11.0
More information about the buildroot
mailing list