[OE-core] [poky] [PATCH v3] meta: Fix return value checks from subprocess.call()'s

[Burton, Ross]

Thanks Mikko, applied to staging.

Ross

On 22 June 2017 at 14:23, Mikko Rapeli <mikko.rapeli at bmw.de> wrote:

> Python function subprocess.call() returns the return value of the
> executed process. If return values are not checked, errors may
> go unnoticed and bad things can happen.
>
> Change all callers of subprocess.call() which do not check for
> the return value to use subprocess.check_call() which raises
> CalledProcessError if the subprocess returns with non-zero value.
>
> https://docs.python.org/2/library/subprocess.html#using-
> the-subprocess-module
>
> All users of the function were found with:
>
> $ git grep "subprocess\.call" | \
>   egrep -v 'if.*subprocess\.call|=\ +subprocess\.call|return.*
> subprocess\.call'
>
> Tested similar patch on top of yocto jethro. Only compile tested
> core-image-minimal on poky master branch.
>
> Signed-off-by: Mikko Rapeli <mikko.rapeli at bmw.de>
> ---
>  meta/classes/cml1.bbclass                | 2 +-
>  meta/classes/kernel-module-split.bbclass | 2 +-
>  meta/classes/sstate.bbclass              | 4 ++--
>  meta/lib/oeqa/utils/buildproject.py      | 2 +-
>  meta/lib/oeqa/utils/targetbuild.py       | 4 ++--
>  meta/recipes-extended/cups/cups.inc      | 2 +-
>  6 files changed, 8 insertions(+), 8 deletions(-)
>
> v3:
> rebased
>
> v2:
> split patches to bitbake, meta and scripts
>
> v1:
> http://lists.openembedded.org/pipermail/openembedded-core/
> 2017-May/136901.html
>
> diff --git a/meta/classes/cml1.bbclass b/meta/classes/cml1.bbclass
> index 38e6613..eb8e790 100644
> --- a/meta/classes/cml1.bbclass
> +++ b/meta/classes/cml1.bbclass
> @@ -63,7 +63,7 @@ python do_diffconfig() {
>
>      if isdiff:
>          statement = 'diff --unchanged-line-format= --old-line-format=
> --new-line-format="%L" ' + configorig + ' ' + config + '>' + fragment
> -        subprocess.call(statement, shell=True)
> +        subprocess.check_call(statement, shell=True)
>
>          shutil.copy(configorig, config)
>
> diff --git a/meta/classes/kernel-module-split.bbclass
> b/meta/classes/kernel-module-split.bbclass
> index 5e10dcf..1035525 100644
> --- a/meta/classes/kernel-module-split.bbclass
> +++ b/meta/classes/kernel-module-split.bbclass
> @@ -47,7 +47,7 @@ python split_kernel_module_packages () {
>          tf = tempfile.mkstemp()
>          tmpfile = tf[1]
>          cmd = "%sobjcopy -j .modinfo -O binary %s %s" %
> (d.getVar("HOST_PREFIX") or "", file, tmpfile)
> -        subprocess.call(cmd, shell=True)
> +        subprocess.check_call(cmd, shell=True)
>          f = open(tmpfile)
>          l = f.read().split("\000")
>          f.close()
> diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> index 0a12935..f446c3d 100644
> --- a/meta/classes/sstate.bbclass
> +++ b/meta/classes/sstate.bbclass
> @@ -404,7 +404,7 @@ python sstate_hardcode_path_unpack () {
>              return
>
>          bb.note("Replacing fixme paths in sstate package: %s" %
> (sstate_hardcode_cmd))
> -        subprocess.call(sstate_hardcode_cmd, shell=True)
> +        subprocess.check_call(sstate_hardcode_cmd, shell=True)
>
>          # Need to remove this or we'd copy it into the target directory
> and may
>          # conflict with another writer
> @@ -453,7 +453,7 @@ def sstate_clean_manifest(manifest, d, prefix=None):
>      if os.path.exists(manifest + ".postrm"):
>          import subprocess
>          os.chmod(postrm, 0o755)
> -        subprocess.call(postrm, shell=True)
> +        subprocess.check_call(postrm, shell=True)
>          oe.path.remove(postrm)
>
>      oe.path.remove(manifest)
> diff --git a/meta/lib/oeqa/utils/buildproject.py b/meta/lib/oeqa/utils/
> buildproject.py
> index 487f08b..721f35d 100644
> --- a/meta/lib/oeqa/utils/buildproject.py
> +++ b/meta/lib/oeqa/utils/buildproject.py
> @@ -52,4 +52,4 @@ class BuildProject(metaclass=ABCMeta):
>
>      def clean(self):
>          self._run('rm -rf %s' % self.targetdir)
> -        subprocess.call('rm -f %s' % self.localarchive, shell=True)
> +        subprocess.check_call('rm -f %s' % self.localarchive, shell=True)
> diff --git a/meta/lib/oeqa/utils/targetbuild.py b/meta/lib/oeqa/utils/
> targetbuild.py
> index 9249fa2..1202d57 100644
> --- a/meta/lib/oeqa/utils/targetbuild.py
> +++ b/meta/lib/oeqa/utils/targetbuild.py
> @@ -69,7 +69,7 @@ class BuildProject(metaclass=ABCMeta):
>
>      def clean(self):
>          self._run('rm -rf %s' % self.targetdir)
> -        subprocess.call('rm -f %s' % self.localarchive, shell=True)
> +        subprocess.check_call('rm -f %s' % self.localarchive, shell=True)
>          pass
>
>  class TargetBuildProject(BuildProject):
> @@ -136,4 +136,4 @@ class SDKBuildProject(BuildProject):
>
>      def _run(self, cmd):
>          self.log("Running . %s; " % self.sdkenv + cmd)
> -        return subprocess.call(". %s; " % self.sdkenv + cmd, shell=True)
> +        return subprocess.check_call(". %s; " % self.sdkenv + cmd,
> shell=True)
> diff --git a/meta/recipes-extended/cups/cups.inc
> b/meta/recipes-extended/cups/cups.inc
> index c3fa459..699a34a 100644
> --- a/meta/recipes-extended/cups/cups.inc
> +++ b/meta/recipes-extended/cups/cups.inc
> @@ -83,7 +83,7 @@ python do_package_append() {
>      import subprocess
>      # Change permissions back the way they were, they probably had a
> reason...
>      workdir = d.getVar('WORKDIR')
> -    subprocess.call('chmod 0511 %s/install/cups/var/run/cups/certs' %
> workdir, shell=True)
> +    subprocess.check_call('chmod 0511 %s/install/cups/var/run/cups/certs'
> % workdir, shell=True)
>  }
>
>  PACKAGES =+ "${PN}-lib ${PN}-libimage"
> --
> 1.9.1
>
> --
> _______________________________________________
> poky mailing list
> poky at yoctoproject.org
> https://lists.yoctoproject.org/listinfo/poky
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openembedded.org/pipermail/openembedded-core/attachments/20170622/7f60102d/attachment-0002.html>