* [PATCH 1/8] verify-bashisms: fix typo
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
@ 2017-01-31 12:50 ` Patrick Ohly
2017-01-31 12:50 ` [PATCH 2/8] verify-bashisms: point out where to get checkbashisms.pl Patrick Ohly
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-01-31 12:50 UTC (permalink / raw)
To: openembedded-core
Variable was renamed, it's now called "output".
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
scripts/verify-bashisms | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/verify-bashisms b/scripts/verify-bashisms
index a8f761d..1bda60c 100755
--- a/scripts/verify-bashisms
+++ b/scripts/verify-bashisms
@@ -41,7 +41,7 @@ def process(recipe, function, script):
# Replace the temporary filename with the function and split it
output = e.output.replace(fn.name, function).splitlines()
- if len(results) % 2 != 0:
+ if len(output) % 2 != 0:
print("Unexpected output from checkbashism: %s" % str(output))
return
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/8] verify-bashisms: point out where to get checkbashisms.pl
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
2017-01-31 12:50 ` [PATCH 1/8] verify-bashisms: fix typo Patrick Ohly
@ 2017-01-31 12:50 ` Patrick Ohly
2017-01-31 12:50 ` [PATCH 3/8] verify-bashisms: explicitly shut down server Patrick Ohly
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-01-31 12:50 UTC (permalink / raw)
To: openembedded-core
The current SourceForge project seems to be unmaintained (last release
2.0.0.2 from 2015) while the copy used by Debian is quite active (last
commit 2016-09-30).
Ideally, checkbashisms.pl should get installed automatically via a
recipe, but for now at least provide the link for manual installation.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
scripts/verify-bashisms | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/verify-bashisms b/scripts/verify-bashisms
index 1bda60c..28795f4 100755
--- a/scripts/verify-bashisms
+++ b/scripts/verify-bashisms
@@ -68,7 +68,7 @@ def get_tinfoil():
if __name__=='__main__':
import shutil
if shutil.which("checkbashisms.pl") is None:
- print("Cannot find checkbashisms.pl on $PATH")
+ print("Cannot find checkbashisms.pl on $PATH, get it from https://anonscm.debian.org/cgit/collab-maint/devscripts.git/plain/scripts/checkbashisms.pl")
sys.exit(1)
tinfoil = get_tinfoil()
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 3/8] verify-bashisms: explicitly shut down server
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
2017-01-31 12:50 ` [PATCH 1/8] verify-bashisms: fix typo Patrick Ohly
2017-01-31 12:50 ` [PATCH 2/8] verify-bashisms: point out where to get checkbashisms.pl Patrick Ohly
@ 2017-01-31 12:50 ` Patrick Ohly
2017-01-31 12:50 ` [PATCH 4/8] verify-bashisms: fix problems with tinfoil2 Patrick Ohly
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-01-31 12:50 UTC (permalink / raw)
To: openembedded-core
Current tinfoil2 requires manually shutting down the server.
Without that, the script hangs during exit. This might change
in the future.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
scripts/verify-bashisms | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/verify-bashisms b/scripts/verify-bashisms
index 28795f4..ed0a563 100755
--- a/scripts/verify-bashisms
+++ b/scripts/verify-bashisms
@@ -114,3 +114,4 @@ if __name__=='__main__':
for message,source in results:
print(" %s\n %s" % (message, source))
print()
+ tinfoil.shutdown()
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 4/8] verify-bashisms: fix problems with tinfoil2
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
` (2 preceding siblings ...)
2017-01-31 12:50 ` [PATCH 3/8] verify-bashisms: explicitly shut down server Patrick Ohly
@ 2017-01-31 12:50 ` Patrick Ohly
2017-01-31 12:50 ` [PATCH 5/8] verify-bashisms: revise update-rc.d whitelist entry Patrick Ohly
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-01-31 12:50 UTC (permalink / raw)
To: openembedded-core
tinfoil2 is based on a client/server architecture, which broke the
verify-bashisms script:
- The tinfoil instance and its data proxies can't be pickled, so
all interaction with the bitbake server has to run in the main
script process and only processing of the plain scripts can
be done with multiprocessing:
_pickle.PicklingError: Can't pickle <class 'bb.tinfoil.TinfoilCookerAdapter.TinfoilRecipeCacheAdapter'>: attribute lookup TinfoilRecipeCacheAdapter on bb.tinfoil failed
- The multiprocessing pool has to be created before initializing
tinfoil, otherwise the pool workers end up trying to communicate
with the bitbake server during shutdown:
ERROR: UI received SIGTERM
Process ForkPoolWorker-2:
Traceback (most recent call last):
File "/usr/lib/python3.4/multiprocessing/process.py", line 257, in _bootstrap
util._exit_function()
File "/usr/lib/python3.4/multiprocessing/util.py", line 286, in _exit_function
_run_finalizers(0)
...
File "/usr/lib/python3.4/multiprocessing/process.py", line 131, in is_alive
assert self._parent_pid == os.getpid(), 'can only test a child process'
AssertionError: can only test a child process
- func() needs to defined before creating the pool to avoid:
AttributeError: Can't get attribute 'func' on <module '__main__' from '/work/openembedded-core/scripts/verify-bashisms'>
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
scripts/verify-bashisms | 44 +++++++++++++++++++++++-------------------
1 file changed, 25 insertions(+), 19 deletions(-)
diff --git a/scripts/verify-bashisms b/scripts/verify-bashisms
index ed0a563..613f174 100755
--- a/scripts/verify-bashisms
+++ b/scripts/verify-bashisms
@@ -71,6 +71,20 @@ if __name__=='__main__':
print("Cannot find checkbashisms.pl on $PATH, get it from https://anonscm.debian.org/cgit/collab-maint/devscripts.git/plain/scripts/checkbashisms.pl")
sys.exit(1)
+ # The order of defining the worker function,
+ # initializing the pool and connecting to the
+ # bitbake server is crucial, don't change it.
+ def func(item):
+ fn, scripts = item
+ result = []
+ for key, script in scripts:
+ r = process(fn, key, script)
+ if r: result.extend(r)
+ return fn, result
+
+ import multiprocessing
+ pool = multiprocessing.Pool()
+
tinfoil = get_tinfoil()
# This is only the default configuration and should iterate over
@@ -83,32 +97,24 @@ if __name__=='__main__':
else:
initial_pns = sorted(pkg_pn)
- pns = []
- print("Generating file list...")
+ pns = {}
+ print("Generating scripts...")
for pn in initial_pns:
for fn in pkg_pn[pn]:
# There's no point checking multiple BBCLASSEXTENDed variants of the same recipe
realfn, _, _ = bb.cache.virtualfn2realfn(fn)
if realfn not in pns:
- pns.append(realfn)
-
-
- def func(fn):
- result = []
- data = tinfoil.parse_recipe_file(fn)
- for key in data.keys():
- if data.getVarFlag(key, "func") and not data.getVarFlag(key, "python"):
- script = data.getVar(key, False)
- if not script: continue
- #print ("%s:%s" % (fn, key))
- r = process(fn, key, script)
- if r: result.extend(r)
- return fn, result
+ data = tinfoil.parse_recipe_file(realfn)
+ scripts = []
+ for key in data.keys():
+ if data.getVarFlag(key, "func") and not data.getVarFlag(key, "python"):
+ script = data.getVar(key, False)
+ if script:
+ scripts.append((key, script))
+ pns[realfn] = scripts
print("Scanning scripts...\n")
- import multiprocessing
- pool = multiprocessing.Pool()
- for pn,results in pool.imap(func, pns):
+ for pn, results in pool.imap(func, pns.items()):
if results:
print(pn)
for message,source in results:
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/8] verify-bashisms: revise update-rc.d whitelist entry
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
` (3 preceding siblings ...)
2017-01-31 12:50 ` [PATCH 4/8] verify-bashisms: fix problems with tinfoil2 Patrick Ohly
@ 2017-01-31 12:50 ` Patrick Ohly
2017-01-31 12:50 ` [PATCH 6/8] verify-bashisms: check scripts only once, include original file and line Patrick Ohly
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-01-31 12:50 UTC (permalink / raw)
To: openembedded-core
The actual code recently changed to:
if ${@use_updatercd(d)} && type update-rc.d >/dev/null 2>/dev/null; then
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
scripts/verify-bashisms | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/verify-bashisms b/scripts/verify-bashisms
index 613f174..df071e3 100755
--- a/scripts/verify-bashisms
+++ b/scripts/verify-bashisms
@@ -6,7 +6,7 @@ whitelist = (
# type is supported by dash
'if type systemctl >/dev/null 2>/dev/null; then',
'if type systemd-tmpfiles >/dev/null 2>/dev/null; then',
- 'if type update-rc.d >/dev/null 2>/dev/null; then',
+ 'type update-rc.d >/dev/null 2>/dev/null; then',
'command -v',
# HOSTNAME is set locally
'buildhistory_single_commit "$CMDLINE" "$HOSTNAME"',
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 6/8] verify-bashisms: check scripts only once, include original file and line
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
` (4 preceding siblings ...)
2017-01-31 12:50 ` [PATCH 5/8] verify-bashisms: revise update-rc.d whitelist entry Patrick Ohly
@ 2017-01-31 12:50 ` Patrick Ohly
2017-01-31 12:50 ` [PATCH 7/8] populate_sdk_ext: fix == bashism Patrick Ohly
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-01-31 12:50 UTC (permalink / raw)
To: openembedded-core
Several scripts that are defined in .bbclass files end up in multiple
different recipes. It's better (faster, less repetitive error reports)
to check them only once.
In addition, the real information for the developer is where he can
find the script, not which recipe file uses it. verify-bashisms now
prints the original file instead of the recipe whenever possible
(i.e. 'filename' is set) and also bumps the line number so that it is
relative to the file and not the script.
Example with one real error and one added just for testing:
$ verify-bashisms core-image-minimal core-image-sato
Loading cache: 100% |#################################################################################| Time: 0:00:00
Loaded 2935 entries from dependency cache.
Parsing recipes: 100% |###############################################################################| Time: 0:00:01
Parsing of 2137 .bb files complete (2101 cached, 36 parsed). 2935 targets, 412 skipped, 0 masked, 0 errors.
Generating scripts...
Scanning scripts...
/.../openembedded-core/meta/classes/populate_sdk_ext.bbclass
possible bashism in install_tools line 515 (should be 'b = a'):
if [ "${SDK_INCLUDE_TOOLCHAIN}" == "1" -a ! -e $unfsd_path ] ; then
possible bashism in install_tools line 521 (type):
type fixme
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
scripts/verify-bashisms | 56 +++++++++++++++++++++++++++---------------
1 file changed, 36 insertions(+), 20 deletions(-)
diff --git a/scripts/verify-bashisms b/scripts/verify-bashisms
index df071e3..7283980 100755
--- a/scripts/verify-bashisms
+++ b/scripts/verify-bashisms
@@ -22,7 +22,9 @@ def is_whitelisted(s):
return True
return False
-def process(recipe, function, script):
+SCRIPT_LINENO_RE = re.compile(r' line (\d+) ')
+
+def process(filename, function, lineno, script):
import tempfile
if not script.startswith("#!"):
@@ -45,13 +47,25 @@ def process(recipe, function, script):
print("Unexpected output from checkbashism: %s" % str(output))
return
- # Turn the output into a list of (message, source) values
+ # Turn the output into a single string like this:
+ # /.../foobar.bb
+ # possible bashism in updatercd_postrm line 2 (type):
+ # if ${@use_updatercd(d)} && type update-rc.d >/dev/null 2>/dev/null; then
+ # ...
+ # ...
result = []
# Check the results against the whitelist
for message, source in zip(output[0::2], output[1::2]):
if not is_whitelisted(source):
- result.append((message, source))
- return result
+ if lineno is not None:
+ message = SCRIPT_LINENO_RE.sub(lambda m: ' line %d ' % (int(m.group(1)) + int(lineno) - 1),
+ message)
+ result.extend([' ' + message, ' ' + source])
+ if result:
+ result.insert(0, filename)
+ return '\n'.join(result)
+ else:
+ return None
def get_tinfoil():
scripts_path = os.path.dirname(os.path.realpath(__file__))
@@ -75,12 +89,8 @@ if __name__=='__main__':
# initializing the pool and connecting to the
# bitbake server is crucial, don't change it.
def func(item):
- fn, scripts = item
- result = []
- for key, script in scripts:
- r = process(fn, key, script)
- if r: result.extend(r)
- return fn, result
+ (filename, key, lineno), script = item
+ return process(filename, key, lineno, script)
import multiprocessing
pool = multiprocessing.Pool()
@@ -97,27 +107,33 @@ if __name__=='__main__':
else:
initial_pns = sorted(pkg_pn)
- pns = {}
+ pns = set()
+ scripts = {}
print("Generating scripts...")
for pn in initial_pns:
for fn in pkg_pn[pn]:
# There's no point checking multiple BBCLASSEXTENDed variants of the same recipe
+ # (at least in general - there is some risk that the variants contain different scripts)
realfn, _, _ = bb.cache.virtualfn2realfn(fn)
if realfn not in pns:
+ pns.add(realfn)
data = tinfoil.parse_recipe_file(realfn)
- scripts = []
for key in data.keys():
if data.getVarFlag(key, "func") and not data.getVarFlag(key, "python"):
script = data.getVar(key, False)
if script:
- scripts.append((key, script))
- pns[realfn] = scripts
+ filename = data.getVarFlag(key, "filename")
+ lineno = data.getVarFlag(key, "lineno")
+ # There's no point in checking a function multiple
+ # times just because different recipes include it.
+ # We identify unique scripts by file, name, and (just in case)
+ # line number.
+ attributes = (filename or realfn, key, lineno)
+ scripts.setdefault(attributes, script)
+
print("Scanning scripts...\n")
- for pn, results in pool.imap(func, pns.items()):
- if results:
- print(pn)
- for message,source in results:
- print(" %s\n %s" % (message, source))
- print()
+ for result in pool.imap(func, scripts.items()):
+ if result:
+ print(result)
tinfoil.shutdown()
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 7/8] populate_sdk_ext: fix == bashism
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
` (5 preceding siblings ...)
2017-01-31 12:50 ` [PATCH 6/8] verify-bashisms: check scripts only once, include original file and line Patrick Ohly
@ 2017-01-31 12:50 ` Patrick Ohly
2017-01-31 12:50 ` [PATCH 8/8] verify-bashisms: support warnings with more than one line of source code Patrick Ohly
2017-02-01 9:17 ` verify-bashisms + shellcheck (was: Re: [PATCH 0/8] verify-bashisms: fix script and one issue found by it) Patrick Ohly
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-01-31 12:50 UTC (permalink / raw)
To: openembedded-core
Found via verify-bashisms.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
meta/classes/populate_sdk_ext.bbclass | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meta/classes/populate_sdk_ext.bbclass b/meta/classes/populate_sdk_ext.bbclass
index 39e0c83..9517111 100644
--- a/meta/classes/populate_sdk_ext.bbclass
+++ b/meta/classes/populate_sdk_ext.bbclass
@@ -512,7 +512,7 @@ install_tools() {
# We can't use the same method as above because files in the sysroot won't exist at this point
# (they get populated from sstate on installation)
unfsd_path="${SDK_OUTPUT}/${SDKPATHNATIVE}${bindir_nativesdk}/unfsd"
- if [ "${SDK_INCLUDE_TOOLCHAIN}" == "1" -a ! -e $unfsd_path ] ; then
+ if [ "${SDK_INCLUDE_TOOLCHAIN}" = "1" -a ! -e $unfsd_path ] ; then
binrelpath=${@os.path.relpath(d.getVar('STAGING_BINDIR_NATIVE'), d.getVar('TMPDIR'))}
lnr ${SDK_OUTPUT}/${SDKPATH}/tmp/$binrelpath/unfsd $unfsd_path
fi
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 8/8] verify-bashisms: support warnings with more than one line of source code
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
` (6 preceding siblings ...)
2017-01-31 12:50 ` [PATCH 7/8] populate_sdk_ext: fix == bashism Patrick Ohly
@ 2017-01-31 12:50 ` Patrick Ohly
2017-02-01 9:17 ` verify-bashisms + shellcheck (was: Re: [PATCH 0/8] verify-bashisms: fix script and one issue found by it) Patrick Ohly
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-01-31 12:50 UTC (permalink / raw)
To: openembedded-core
All warnings start with "possible bashism in", followed by one or more
(in the case of line continuation) lines of source code. To support
more than one line, we now split by matching against the known intro
text.
Example:
$ verify-bashisms guile
...
/.../openembedded-core/meta/recipes-devtools/guile/guile_2.0.13.bb
possible bashism in guile_cross_config line 94 ($'...' should be "$(printf '...')"):
echo '#!'`which ${BUILD_SYS}-guile`$' \\\n--no-auto-compile -e main -s\n!#\n(define %guile-build-info '\'\( \
> ${B}/guile-config.cross
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
scripts/verify-bashisms | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/scripts/verify-bashisms b/scripts/verify-bashisms
index 7283980..dab64ef 100755
--- a/scripts/verify-bashisms
+++ b/scripts/verify-bashisms
@@ -23,6 +23,7 @@ def is_whitelisted(s):
return False
SCRIPT_LINENO_RE = re.compile(r' line (\d+) ')
+BASHISM_WARNING = re.compile(r'^(possible bashism in.*)$', re.MULTILINE)
def process(filename, function, lineno, script):
import tempfile
@@ -42,11 +43,18 @@ def process(filename, function, lineno, script):
# TODO check exit code is 1
# Replace the temporary filename with the function and split it
- output = e.output.replace(fn.name, function).splitlines()
- if len(output) % 2 != 0:
- print("Unexpected output from checkbashism: %s" % str(output))
- return
-
+ output = e.output.replace(fn.name, function)
+ if not output or not output.startswith('possible bashism'):
+ # Probably starts with or contains only warnings. Dump verbatim
+ # with one space indention. Can't do the splitting and whitelist
+ # checking below.
+ return '\n'.join([filename,
+ ' Unexpected output from checkbashisms.pl'] +
+ [' ' + x for x in output.splitlines()])
+
+ # We know that the first line matches and that therefore the first
+ # list entry will be empty - skip it.
+ output = BASHISM_WARNING.split(output)[1:]
# Turn the output into a single string like this:
# /.../foobar.bb
# possible bashism in updatercd_postrm line 2 (type):
@@ -60,7 +68,8 @@ def process(filename, function, lineno, script):
if lineno is not None:
message = SCRIPT_LINENO_RE.sub(lambda m: ' line %d ' % (int(m.group(1)) + int(lineno) - 1),
message)
- result.extend([' ' + message, ' ' + source])
+ result.append(' ' + message.strip())
+ result.extend([' %s' % x for x in source.splitlines()])
if result:
result.insert(0, filename)
return '\n'.join(result)
--
git-series 0.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* verify-bashisms + shellcheck (was: Re: [PATCH 0/8] verify-bashisms: fix script and one issue found by it)
2017-01-31 12:50 [PATCH 0/8] verify-bashisms: fix script and one issue found by it Patrick Ohly
` (7 preceding siblings ...)
2017-01-31 12:50 ` [PATCH 8/8] verify-bashisms: support warnings with more than one line of source code Patrick Ohly
@ 2017-02-01 9:17 ` Patrick Ohly
8 siblings, 0 replies; 10+ messages in thread
From: Patrick Ohly @ 2017-02-01 9:17 UTC (permalink / raw)
To: openembedded-core
On Tue, 2017-01-31 at 13:50 +0100, Patrick Ohly wrote:
> The script broke when introducing tinfoil2. The patches also include
> several usability enhancements, like making the output less redundant
> and including information about the actual file and line where the
> offending script can be found.
I've further modified verify-bashisms so that it can optionally run the
scripts through shellcheck (https://www.shellcheck.net/).
I'm quite impressed with how much real issues shellcheck finds and I
also found the recommendations useful. However, it is too verbose to be
applied to all scripts in OE. For example, it warns about missing
quotation marks around variables a lot.
There is no simple "check for bashisms" command line flag or "enable
just these checks" mode. One can disable warnings, so perhaps excluding
a long list of know "harmless" checks would be a way how shellcheck
could replace checkbashisms.pl?
My current solution always calls checkbashisms.pl, and shellcheck only
when a function is annotated:
foobar () {
echo hello world
}
foobar[shellcheck] = "sh"
This sets "sh" as expected shell flavor. I did it this way because I can
imagine that someone might want to have some functions with bash
features and then ensure that bash is used to execute the code.
I can also imagine that this varflag could be used to exclude certain
checks with "sh SC2001 SC2002 ...", although the same can also be done
with inline comments for specific lines:
foobar () {
# shellcheck disable=SC2003
i=$( expr $i + 1 )
}
Using expr triggers a warning because usually $(( )) is a better
alternative. However, that currently can't be used in bitbake functions
because the shell parser chokes on it:
NotImplementedError: $((
So I've disabled that check by default.
Any suggestions how to proceed with this?
Note that shellcheck is written in Haskell. Getting it installed
automatically via a recipe would imply adding Haskell support to
OE-core. OTOH it seems to be packaged quite widely, so assuming it to be
installed on the host seems okay.
The "verify-bashisms" name of the script no longer quite matches the
actual functionality when using shellcheck, but that's minor (?).
FWIW, current additional patch is here:
diff --git a/scripts/verify-bashisms b/scripts/verify-bashisms
index dab64ef5019..000ed3f1764 100755
--- a/scripts/verify-bashisms
+++ b/scripts/verify-bashisms
@@ -24,8 +24,9 @@ def is_whitelisted(s):
SCRIPT_LINENO_RE = re.compile(r' line (\d+) ')
BASHISM_WARNING = re.compile(r'^(possible bashism in.*)$', re.MULTILINE)
+SHELLCHECK_LINENO_RE = re.compile(r'^(In .* line )(\d+):$', re.MULTILINE)
-def process(filename, function, lineno, script):
+def process(filename, function, lineno, script, shellcheck):
import tempfile
if not script.startswith("#!"):
@@ -35,10 +36,10 @@ def process(filename, function, lineno, script):
fn.write(script)
fn.flush()
+ results = []
try:
subprocess.check_output(("checkbashisms.pl", fn.name), universal_newlines=True, stderr=subprocess.STDOUT)
- # No bashisms, so just return
- return
+ # No bashisms, so just continue
except subprocess.CalledProcessError as e:
# TODO check exit code is 1
@@ -48,33 +49,53 @@ def process(filename, function, lineno, script):
# Probably starts with or contains only warnings. Dump verbatim
# with one space indention. Can't do the splitting and whitelist
# checking below.
- return '\n'.join([filename,
- ' Unexpected output from checkbashisms.pl'] +
- [' ' + x for x in output.splitlines()])
-
- # We know that the first line matches and that therefore the first
- # list entry will be empty - skip it.
- output = BASHISM_WARNING.split(output)[1:]
- # Turn the output into a single string like this:
- # /.../foobar.bb
- # possible bashism in updatercd_postrm line 2 (type):
- # if ${@use_updatercd(d)} && type update-rc.d >/dev/null 2>/dev/null; then
- # ...
- # ...
- result = []
- # Check the results against the whitelist
- for message, source in zip(output[0::2], output[1::2]):
- if not is_whitelisted(source):
- if lineno is not None:
- message = SCRIPT_LINENO_RE.sub(lambda m: ' line %d ' % (int(m.group(1)) + int(lineno) - 1),
- message)
- result.append(' ' + message.strip())
- result.extend([' %s' % x for x in source.splitlines()])
- if result:
- result.insert(0, filename)
- return '\n'.join(result)
+ results.extend([' Unexpected output from checkbashisms.pl'] +
+ [' ' + x for x in output.splitlines()])
else:
- return None
+ # We know that the first line matches and that therefore the first
+ # list entry will be empty - skip it.
+ output = BASHISM_WARNING.split(output)[1:]
+ # Turn the output into a single string like this:
+ # /.../foobar.bb
+ # possible bashism in updatercd_postrm line 2 (type):
+ # if ${@use_updatercd(d)} && type update-rc.d >/dev/null 2>/dev/null; then
+ # ...
+ # ...
+ # Check the results against the whitelist
+ for message, source in zip(output[0::2], output[1::2]):
+ if not is_whitelisted(source):
+ if lineno is not None:
+ message = SCRIPT_LINENO_RE.sub(lambda m: ' line %d ' % (int(m.group(1)) + int(lineno) - 1),
+ message)
+ results.append(' ' + message.strip())
+ results.extend([' %s' % x for x in source.splitlines()])
+
+ if shellcheck:
+ try:
+ excluded = []
+ # SC2003 warns about using expr instead of $(( )).
+ # bitbake's shell parser chokes on $(( )), so expr
+ # is what embedded scripts have to use - ignore the warning.
+ excluded.append("SC2003")
+ subprocess.check_output(["shellcheck", "--shell=%s" % shellcheck] +
+ ["-e%s" % x for x in excluded] +
+ [fn.name],
+ universal_newlines=True, stderr=subprocess.STDOUT)
+ # No shell warnings, so just continue.
+ except subprocess.CalledProcessError as e:
+ # Replace the temporary filename with the function,
+ # update line numbers and indent the output.
+ output = e.output.replace(fn.name, function)
+ results.extend([' ' +
+ (x if lineno is None else
+ SHELLCHECK_LINENO_RE.sub(lambda m: m.group(1) + str(int(m.group(2)) + int(lineno) - 1), x))
+ for x in output.splitlines()])
+
+ if results:
+ results.insert(0, filename)
+ return '\n'.join(results)
+ else:
+ return None
def get_tinfoil():
scripts_path = os.path.dirname(os.path.realpath(__file__))
@@ -94,12 +115,16 @@ if __name__=='__main__':
print("Cannot find checkbashisms.pl on $PATH, get it from https://anonscm.debian.org/cgit/collab-maint/devscripts.git/plain/scripts/checkbashisms.pl")
sys.exit(1)
+ if shutil.which("shellcheck") is None:
+ print("Cannot find shellcheck on $PATH, get it from your distro or https://www.shellcheck.net/")
+ sys.exit(1)
+
# The order of defining the worker function,
# initializing the pool and connecting to the
# bitbake server is crucial, don't change it.
def func(item):
- (filename, key, lineno), script = item
- return process(filename, key, lineno, script)
+ (filename, key, lineno), (script, shellcheck) = item
+ return process(filename, key, lineno, script, shellcheck)
import multiprocessing
pool = multiprocessing.Pool()
@@ -129,16 +154,22 @@ if __name__=='__main__':
data = tinfoil.parse_recipe_file(realfn)
for key in data.keys():
if data.getVarFlag(key, "func") and not data.getVarFlag(key, "python"):
+ # Do not expand by default, it could change line numbers.
script = data.getVar(key, False)
+ if script and '${@' in script:
+ # Embedded Python code confuses the checkers too much,
+ # we have to expand.
+ script = data.getVar(key, True)
if script:
filename = data.getVarFlag(key, "filename")
lineno = data.getVarFlag(key, "lineno")
+ shellcheck = data.getVarFlag(key, "shellcheck")
# There's no point in checking a function multiple
# times just because different recipes include it.
# We identify unique scripts by file, name, and (just in case)
# line number.
attributes = (filename or realfn, key, lineno)
- scripts.setdefault(attributes, script)
+ scripts.setdefault(attributes, (script, shellcheck))
print("Scanning scripts...\n")
--
Best Regards, Patrick Ohly
The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.
^ permalink raw reply related [flat|nested] 10+ messages in thread