Patchwork [bitbake-devel,2/2] replace os.popen with subprocess.Popen

login
mail settings
Submitter Robert Yang
Date May 14, 2012, 8:07 a.m.
Message ID <a072dd7795f44603322db6583ac748f0418684fd.1336974361.git.liezhi.yang@windriver.com>
Download mbox | patch
Permalink /patch/27549/
State New
Headers show

Comments

Robert Yang - May 14, 2012, 8:07 a.m.
Replace os.popen with subprocess.Popen since the older function would
fail (more or less) silently if the executed program cannot be found

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            |    6 +++---
 bitbake/lib/bb/fetch2/svk.py                 |    3 ++-
 bitbake/lib/bb/ui/crumbs/builddetailspage.py |    3 ++-
 bitbake/lib/bb/ui/crumbs/hig.py              |    2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)
Chris Larson - May 14, 2012, 1:59 p.m.
On Mon, May 14, 2012 at 1:07 AM, Robert Yang <liezhi.yang@windriver.com> wrote:
>     def popen_read(self, cmd):
> -        return os.popen("%s 2>/dev/null" % cmd).read().strip()
> +        return subprocess.Popen("%s 2>/dev/null" % cmd, shell=True, stdout=subprocess.PIPE).stdout.read().strip()

The 2>/dev/null is irrelevant when we're only inspecting the stdout
anyway. Also, we already have wrappers around Popen for convenience in
the bb.process python module.
Robert Yang - May 15, 2012, 6:49 a.m.
On 05/14/2012 09:59 PM, Chris Larson wrote:
> On Mon, May 14, 2012 at 1:07 AM, Robert Yang<liezhi.yang@windriver.com>  wrote:
>>      def popen_read(self, cmd):
>> -        return os.popen("%s 2>/dev/null" % cmd).read().strip()
>> +        return subprocess.Popen("%s 2>/dev/null" % cmd, shell=True, stdout=subprocess.PIPE).stdout.read().strip()
>
> The 2>/dev/null is irrelevant when we're only inspecting the stdout
> anyway. Also, we already have wrappers around Popen for convenience in
> the bb.process python module.

Thanks, I will remove the 2>/dev/null and use:

from bb.process import Popen

to simplify the code, I will send the V2 sooner.

// Robert

Patch

diff --git a/bitbake/lib/bb/fetch2/perforce.py b/bitbake/lib/bb/fetch2/perforce.py
index 6abf15d..ad7df5f 100644
--- a/bitbake/lib/bb/fetch2/perforce.py
+++ b/bitbake/lib/bb/fetch2/perforce.py
@@ -91,7 +91,7 @@  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))
+        p4file = subprocess.Popen("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot), shell=True, stdout=subprocess.PIPE).stdout
         cset = p4file.readline().strip()
         logger.debug(1, "READ %s", cset)
         if not cset:
@@ -155,7 +155,7 @@  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")
+        tmppipe = subprocess.Popen(data.getVar('MKTEMPDIRCMD', localdata, True) or "false", shell=True, stdout=subprocess.PIPE).stdout
         tmpfile = tmppipe.readline().strip()
         if not tmpfile:
             raise FetchError("Fetch: unable to create temporary directory.. make sure 'mktemp' is in the PATH.", loc)
@@ -169,7 +169,7 @@  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 = subprocess.Popen("%s%s files %s" % (p4cmd, p4opt, depot), shell=True, stdout=subprocess.PIPE).stdout
 
         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..cfca713 100644
--- a/bitbake/lib/bb/fetch2/svk.py
+++ b/bitbake/lib/bb/fetch2/svk.py
@@ -26,6 +26,7 @@  This implementation is for svk. It is based on the svn implementation
 # Based on functions from the base bb module, Copyright 2003 Holger Schurig
 
 import os
+import subprocess
 import logging
 import bb
 from   bb import data
@@ -77,7 +78,7 @@  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")
+        tmppipe = subprocess.Popen(data.getVar('MKTEMPDIRCMD', localdata, True) or "false", shell=True, stdout=subprocess.PIPE).stdout
         tmpfile = tmppipe.readline().strip()
         if not tmpfile:
             logger.error()
diff --git a/bitbake/lib/bb/ui/crumbs/builddetailspage.py b/bitbake/lib/bb/ui/crumbs/builddetailspage.py
index 51e6a4a..30f8249 100755
--- a/bitbake/lib/bb/ui/crumbs/builddetailspage.py
+++ b/bitbake/lib/bb/ui/crumbs/builddetailspage.py
@@ -28,6 +28,7 @@  from bb.ui.crumbs.hobwidget import hic, HobNotebook, HobAltButton, HobWarpCellRe
 from bb.ui.crumbs.runningbuild import RunningBuildTreeView
 from bb.ui.crumbs.runningbuild import BuildFailureTreeView
 from bb.ui.crumbs.hobpages import HobPage
+import subprocess
 
 class BuildConfigurationTreeView(gtk.TreeView):
     def __init__ (self):
@@ -96,7 +97,7 @@  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 = subprocess.Popen('cd %s; git branch 2>&1 | grep "^* " | tr -d "* "' % path, shell=True, stdout=subprocess.PIPE).stdout
                 if f:
                     branch = f.readline().lstrip('\n').rstrip('\n')
                     vars.append(self.set_vars("Branch:", branch))
diff --git a/bitbake/lib/bb/ui/crumbs/hig.py b/bitbake/lib/bb/ui/crumbs/hig.py
index 4baf960..e7650f2 100644
--- a/bitbake/lib/bb/ui/crumbs/hig.py
+++ b/bitbake/lib/bb/ui/crumbs/hig.py
@@ -726,7 +726,7 @@  class DeployImageDialog (CrumbsDialog):
         self.progress_bar.hide()
 
     def popen_read(self, cmd):
-        return os.popen("%s 2>/dev/null" % cmd).read().strip()
+        return subprocess.Popen("%s 2>/dev/null" % cmd, shell=True, stdout=subprocess.PIPE).stdout.read().strip()
 
     def find_all_usb_devices(self):
         usb_devs = [ os.readlink(u)