From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Martincoski Date: Sun, 19 Feb 2017 19:17:17 -0300 Subject: [Buildroot] [PATCH v2 2/9] support/scripts/check-package: new script In-Reply-To: <20170219221724.27298-1-ricardo.martincoski@gmail.com> References: <20170219221724.27298-1-ricardo.martincoski@gmail.com> Message-ID: <20170219221724.27298-3-ricardo.martincoski@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: buildroot@busybox.net 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 Cc: Thomas De Schampheleire --- 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 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 F: package/lirc-tools/ +N: Ricardo Martincoski +F: support/scripts/check*package* + N: Richard Braun 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