Re: [PATCH 2/2] replace os.popen with subprocess.Popen

["Wang, Shane" <shane.wang@intel.com>]

Robert, be careful. Your patch 094742bed2fc01d55f572da946fcfa7a48521401 has broke Hob.

I am going to assign the bug https://bugzilla.yoctoproject.org/show_bug.cgi?id=2511 to you and Kai.

The error is:
Traceback (most recent call last):
  File "/home/yocto-build5/poky-contrib/bitbake/lib/bb/ui/crumbs/builder.py", line 821, in handler_build_started_cb
    self.build_details_page.show_configurations(self.configuration, self.parameters)
  File "/home/yocto-build5/poky-contrib/bitbake/lib/bb/ui/crumbs/builddetailspage.py", line 336, in show_configurations
    self.config_tv.show(configurations, params)
  File "/home/yocto-build5/poky-contrib/bitbake/lib/bb/ui/crumbs/builddetailspage.py", line 105, in show
    f.close()
AttributeError: 'str' object has no attribute 'close'

--
Shane

Robert Yang wrote on 2012-05-20:

> Replace os.popen with subprocess.Popen since the older function would
> fail (more or less) silently if the executed program cannot be found
> 
> There is a bb.process.run() which will invoke the Popen to run command,
> use it for simplify the code.
> 
> For the:
> p4file = os.popen("%s%s files %s" % (p4cmd, p4opt, depot))
> ...
> for file in p4file:
>     list = file.split()
> in bitbake/lib/bb/fetch2/perforce.py, it should be an error in the past,
> since it didn't use readline() to read the pipe, but directly used the
> split() for the pipe. Use the bb.process.run would fix the problem since
> bb.process.run will return strings.
> 
> More info:
> http://docs.python.org/library/subprocess.html#subprocess-replacements
> 
> [YOCTO #2075]
> 
> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> ---
>  bitbake/lib/bb/fetch2/perforce.py            |   11 ++++++-----
>  bitbake/lib/bb/fetch2/svk.py                 |    4 ++--
>  bitbake/lib/bb/ui/crumbs/builddetailspage.py |    5 +++--
>  bitbake/lib/bb/ui/crumbs/hig.py              |    7 ++++---
>  4 files changed, 15 insertions(+), 12 deletions(-)
> diff --git a/bitbake/lib/bb/fetch2/perforce.py
> b/bitbake/lib/bb/fetch2/perforce.py index 6abf15d..df3a3a3 100644 ---
> a/bitbake/lib/bb/fetch2/perforce.py +++
> b/bitbake/lib/bb/fetch2/perforce.py @@ -91,8 +91,8 @@ class
> Perforce(FetchMethod):
> 
>          p4cmd = data.getVar('FETCHCOMMAND_p4', d, True)
>          logger.debug(1, "Running %s%s changes -m 1 %s", p4cmd, p4opt,
> depot) -        p4file = os.popen("%s%s changes -m 1 %s" % (p4cmd,
> p4opt, depot)) -        cset = p4file.readline().strip() +       
> p4file, errors = bb.process.run("%s%s changes -m 1 %s" % (p4cmd, p4opt,
> depot)) +        cset = p4file.strip()
>          logger.debug(1, "READ %s", cset)
>          if not cset:
>              return -1
> @@ -155,8 +155,8 @@ class Perforce(FetchMethod):
>          logger.debug(2, "Fetch: creating temporary directory")
>          bb.utils.mkdirhier(data.expand('${WORKDIR}', localdata))
>          data.setVar('TMPBASE', data.expand('${WORKDIR}/oep4.XXXXXX',
> localdata), localdata)
> -        tmppipe = os.popen(data.getVar('MKTEMPDIRCMD', localdata,
> True) or "false")
> -        tmpfile = tmppipe.readline().strip()
> +        tmpfile, errors = bb.process.run(data.getVar('MKTEMPDIRCMD',
> localdata, True) or "false")
> +        tmpfile = tmpfile.strip()
>          if not tmpfile:
>              raise FetchError("Fetch: unable to create temporary
> directory.. make sure 'mktemp' is in the PATH.", loc)
> 
> @@ -169,7 +169,8 @@ class Perforce(FetchMethod):
>          os.chdir(tmpfile)
>          logger.info("Fetch " + loc)
>          logger.info("%s%s files %s", p4cmd, p4opt, depot)
> -        p4file = os.popen("%s%s files %s" % (p4cmd, p4opt, depot)) +   
>     p4file, errors = bb.process.run("%s%s files %s" % (p4cmd, p4opt,
> depot)) +        p4file = p4file.strip()
> 
>          if not p4file:
>              raise FetchError("Fetch: unable to get the P4 files from %s" %
> depot, loc)
> diff --git a/bitbake/lib/bb/fetch2/svk.py b/bitbake/lib/bb/fetch2/svk.py
> index 9d34abf..ee3823f 100644
> --- a/bitbake/lib/bb/fetch2/svk.py
> +++ b/bitbake/lib/bb/fetch2/svk.py
> @@ -77,8 +77,8 @@ class Svk(FetchMethod):
>          logger.debug(2, "Fetch: creating temporary directory")
>          bb.utils.mkdirhier(data.expand('${WORKDIR}', localdata))
>          data.setVar('TMPBASE', data.expand('${WORKDIR}/oesvk.XXXXXX',
> localdata), localdata)
> -        tmppipe = os.popen(data.getVar('MKTEMPDIRCMD', localdata,
> True) or "false")
> -        tmpfile = tmppipe.readline().strip()
> +        tmpfile, errors = bb.process.run(data.getVar('MKTEMPDIRCMD',
> localdata, True) or "false")
> +        tmpfile = tmpfile.strip()
>          if not tmpfile:
>              logger.error()
>              raise FetchError("Fetch: unable to create temporary
> directory.. make sure 'mktemp' is in the PATH.", loc) diff --git
> a/bitbake/lib/bb/ui/crumbs/builddetailspage.py
> b/bitbake/lib/bb/ui/crumbs/builddetailspage.py index c2d5abc..3ec15d4
> 100755 --- a/bitbake/lib/bb/ui/crumbs/builddetailspage.py +++
> b/bitbake/lib/bb/ui/crumbs/builddetailspage.py @@ -23,6 +23,7 @@
>  import gtk import pango import gobject +import bb.process from
>  bb.ui.crumbs.progressbar import HobProgressBar from
>  bb.ui.crumbs.hobwidget import hic, HobNotebook, HobAltButton,
>  HobWarpCellRendererText from bb.ui.crumbs.runningbuild import
>  RunningBuildTreeView
> @@ -96,9 +97,9 @@ class BuildConfigurationTreeView(gtk.TreeView):
>          for path in src_config_info.layers:
>              import os, os.path
>              if os.path.exists(path):
> -                f = os.popen('cd %s; git branch 2>&1 | grep "^* " | tr -d
> "* "' % path)
> +                f, errors = bb.process.run('cd %s; git branch 2>&1 | grep
> "^* " | tr -d "* "' % path)
>                  if f:
> -                    branch = f.readline().lstrip('\n').rstrip('\n')
> +                    branch = f.strip('\n')
>                      vars.append(self.set_vars("Branch:", branch))
>                      f.close()
>                  break
> diff --git a/bitbake/lib/bb/ui/crumbs/hig.py b/bitbake/lib/bb/ui/crumbs/hig.py
> index 721d145..b3936f8 100644
> --- a/bitbake/lib/bb/ui/crumbs/hig.py
> +++ b/bitbake/lib/bb/ui/crumbs/hig.py
> @@ -25,12 +25,12 @@ import gobject
>  import hashlib import os import re -import subprocess import shlex from
>  bb.ui.crumbs.hobcolor import HobColors from bb.ui.crumbs.hobwidget
>  import hcc, hic, HobViewTable, HobInfoButton, HobButton, HobAltButton,
>  HobIconChecker from bb.ui.crumbs.progressbar import HobProgressBar
>  import bb.ui.crumbs.utils
> +import bb.process
> 
>  """
>  The following are convenience classes for implementing GNOME HIG
> compliant
> @@ -726,7 +726,8 @@ class DeployImageDialog (CrumbsDialog):
>          self.progress_bar.hide()
>      def popen_read(self, cmd):
> -        return os.popen("%s 2>/dev/null" % cmd).read().strip()
> +        tmpout, errors = bb.process.run("%s" % cmd)
> +        return tmpout.strip()
> 
>      def find_all_usb_devices(self):
>          usb_devs = [ os.readlink(u)
> @@ -755,7 +756,7 @@ class DeployImageDialog (CrumbsDialog):
>                  cmdline = bb.ui.crumbs.utils.which_terminal()
>                  if cmdline:
>                      cmdline += "\"sudo dd if=" + self.image_path + "
> of=" + combo_item + "\""
> -                    subprocess.Popen(args=shlex.split(cmdline))
> +                    bb.process.run(shlex.split(cmdline))
> 
>      def update_progress_bar(self, title, fraction, status=None):
>          self.progress_bar.update(fraction)