support/scripts/check-package: new script
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@gmail.com> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
This commit is contained in:
parent
6e432d5ecb
commit
111132903d
@ -1312,6 +1312,9 @@ F: package/atop/
|
||||
N: Rhys Williams <github@wilberforce.co.nz>
|
||||
F: package/lirc-tools/
|
||||
|
||||
N: Ricardo Martincoski <ricardo.martincoski@gmail.com>
|
||||
F: support/scripts/check*package*
|
||||
|
||||
N: Richard Braun <rbraun@sceen.net>
|
||||
F: package/curlftpfs/
|
||||
F: package/tzdata/
|
||||
|
144
support/scripts/check-package
Executable file
144
support/scripts/check-package
Executable file
@ -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__()
|
76
support/scripts/check-package.txt
Normal file
76
support/scripts/check-package.txt
Normal file
@ -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
|
16
support/scripts/checkpackagebase.py
Normal file
16
support/scripts/checkpackagebase.py
Normal file
@ -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
|
19
support/scripts/checkpackagelib.py
Normal file
19
support/scripts/checkpackagelib.py
Normal file
@ -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]
|
7
support/scripts/checkpackagelib_config.py
Normal file
7
support/scripts/checkpackagelib_config.py
Normal file
@ -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
|
7
support/scripts/checkpackagelib_hash.py
Normal file
7
support/scripts/checkpackagelib_hash.py
Normal file
@ -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
|
8
support/scripts/checkpackagelib_mk.py
Normal file
8
support/scripts/checkpackagelib_mk.py
Normal file
@ -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
|
7
support/scripts/checkpackagelib_patch.py
Normal file
7
support/scripts/checkpackagelib_patch.py
Normal file
@ -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
|
Loading…
Reference in New Issue
Block a user