[PATCH 0/8] verify-bashisms: fix script and one issue found by it

[PATCH 0/8] verify-bashisms: fix script and one issue found by it

[Patrick Ohly]

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.

Patrick Ohly (8):
  verify-bashisms: fix typo
  verify-bashisms: point out where to get checkbashisms.pl
  verify-bashisms: explicitly shut down server
  verify-bashisms: fix problems with tinfoil2
  verify-bashisms: revise update-rc.d whitelist entry
  verify-bashisms: check scripts only once, include original file and line
  populate_sdk_ext: fix == bashism
  verify-bashisms: support warnings with more than one line of source code

 meta/classes/populate_sdk_ext.bbclass |   2 +-
 scripts/verify-bashisms               | 100 +++++++++++++++++----------
 2 files changed, 67 insertions(+), 35 deletions(-)

base-commit: 8d955d3486beca27fa23448f930d2c4df94b187a
-- 
git-series 0.9.1

[PATCH 1/8] verify-bashisms: fix typo

[Patrick Ohly]

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

[PATCH 2/8] verify-bashisms: point out where to get checkbashisms.pl

[Patrick Ohly]

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

[PATCH 3/8] verify-bashisms: explicitly shut down server

[Patrick Ohly]

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

[PATCH 4/8] verify-bashisms: fix problems with tinfoil2

[Patrick Ohly]

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

[PATCH 5/8] verify-bashisms: revise update-rc.d whitelist entry

[Patrick Ohly]

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

[PATCH 6/8] verify-bashisms: check scripts only once, include original file and line

[Patrick Ohly]

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

[PATCH 7/8] populate_sdk_ext: fix == bashism

[Patrick Ohly]

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

[PATCH 8/8] verify-bashisms: support warnings with more than one line of source code

[Patrick Ohly]

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

verify-bashisms + shellcheck (was: Re: [PATCH 0/8] verify-bashisms: fix script and one issue found by it)

[Patrick Ohly]

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.