V3 replace os.system and os.popen with subbprocess module
cover V3 replace os.system and os.popen with subbprocess module
* Changes of V3:
Use bb.process.run() rather than bb.process.Popen() to replace os.popen()
* Changes of V2:
- Remove the 2>/dev/null since we dont' need it.
- Use the wrapped Popen from bb.process, which is simpler than
subprocess.Popen(....).
* Original message of V1:
Replace os.popen and os.system with subprocess.Popen and subprocess.call, since the older functions would fail (more or less) if the executed program cannot be found, this would cause potential errors since we don't know whether the problem executed well or not.
For the performance issue, I've done the testing before the patches and after with the oe-core layer (also the oe-core have applied the similar patches):
# The sources are on local disk
1) Before applied these pacthes to bitbake and similar patches to oe-core:
$ time bitbake core-image-sato real 177m50.723s user 436m1.551s sys 71m29.588s
2) After applied the pathes:
$ time bitbake core-image-sato real 176m26.194s user 436m7.931s sys 71m1.994s
After applied these patches, the time has reduced 84 seconds, this is very slight, I think that we can assume this is just a deviation, it doesn't cause performance problems.
// Robert
The following changes since commit f3ba3cb6af96aebf5167bb1565edf11cceb7897f:
gdk-pixbuf: Fix lsb builds where dependency may be missing (2012-05-15 19:44:37 +0100)
are available in the git repository at:
git://git.pokylinux.org/poky-contrib robert/subprocess
http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=robert/subprocess
Robert Yang (2):
replace os.system with subprocess.call
replace os.popen with subprocess.Popen
bitbake/lib/bb/fetch2/perforce.py | 9 +++++---- bitbake/lib/bb/fetch2/svk.py | 2 +- bitbake/lib/bb/shell.py | 6 +++--- bitbake/lib/bb/ui/crumbs/builddetailspage.py | 3 ++- bitbake/lib/bb/ui/crumbs/hig.py | 7 ++++--- bitbake/lib/bb/ui/crumbs/imagedetailspage.py | 3 ++- bitbake/lib/bb/ui/ncurses.py | 4 ++-- 7 files changed, 19 insertions(+), 15 deletions(-)
PATCH 1/2 replace os.system with subprocess.call
Replace os.system with subprocess.call 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]
--- bitbake/lib/bb/fetch2/perforce.py | 3 ++- bitbake/lib/bb/shell.py | 6 +++--- bitbake/lib/bb/ui/crumbs/imagedetailspage.py | 3 ++- bitbake/lib/bb/ui/ncurses.py | 4 ++-- 4 files changed, 9 insertions(+), 7 deletions(-)
bitbake/lib/bb/fetch2/perforce.py +2 -1
@@ -27,6 +27,7 @@ BitBake build tools. from future_builtins import zip import os +import subprocess import logging import bb from bb import data
@@ -184,7 +185,7 @@ class Perforce(FetchMethod): dest = list[0][len(path)+1:] where = dest.find("#") - os.system("%s%s print -o %s/%s %s" % (p4cmd, p4opt, module, dest[:where], list[0])) + subprocess.call("%s%s print -o %s/%s %s" % (p4cmd, p4opt, module, dest[:where], list[0]), shell=True) count = count + 1 if count == 0:
bitbake/lib/bb/shell.py +3 -3
@@ -214,7 +214,7 @@ class BitBakeShellCommands: name = params[0] bbfile = self._findProvider( name ) if bbfile is not None: - os.system( "%s %s" % ( os.environ.get( "EDITOR", "vi" ), bbfile ) ) + subprocess.call( "%s %s" % ( os.environ.get( "EDITOR", "vi" ), bbfile ) , shell=True) else: print("ERROR: Nothing provides '%s'" % name) edit.usage = "<providee>"
@@ -259,7 +259,7 @@ class BitBakeShellCommands: def fileEdit( self, params ): """Call $EDITOR on a .bb file""" name = params[0] - os.system( "%s %s" % ( os.environ.get( "EDITOR", "vi" ), completeFilePath( name ) ) ) + subprocess.call( "%s %s" % ( os.environ.get( "EDITOR", "vi" ), completeFilePath( name ) ) , shell=True) fileEdit.usage = "<bbfile>" def fileRebuild( self, params ):
@@ -370,7 +370,7 @@ SRC_URI = "" #} """, file=newpackage) newpackage.close() - os.system( "%s %s/%s" % ( os.environ.get( "EDITOR" ), fulldirname, filename ) ) + subprocess.call( "%s %s/%s" % ( os.environ.get( "EDITOR" ), fulldirname, filename ) , shell=True) new.usage = "<directory> <filename>" def package( self, params ):
bitbake/lib/bb/ui/crumbs/imagedetailspage.py +2 -1
@@ -25,6 +25,7 @@ import gtk from bb.ui.crumbs.hobcolor import HobColors from bb.ui.crumbs.hobwidget import hic, HobViewTable, HobAltButton, HobButton from bb.ui.crumbs.hobpages import HobPage +import subprocess # # ImageDetailsPage
@@ -299,7 +300,7 @@ class ImageDetailsPage (HobPage): self.show_all() def view_files_clicked_cb(self, button, image_addr): - os.system("xdg-open /%s" % image_addr) + subprocess.call("xdg-open /%s" % image_addr, shell=True) def refresh_package_detail_box(self, image_size): self.package_detail.update_line_widgets("Total image size: ", image_size)
bitbake/lib/bb/ui/ncurses.py +2 -2
@@ -47,7 +47,7 @@ from __future__ import division import logging -import os, sys, curses, itertools, time +import os, sys, curses, itertools, time, subprocess import bb import xmlrpclib from bb import ui
@@ -286,7 +286,7 @@ class NCursesUI: # bb.error("log data follows (%s)" % logfile) # number_of_lines = data.getVar("BBINCLUDELOGS_LINES", d) # if number_of_lines: -# os.system('tail -n%s %s' % (number_of_lines, logfile)) +# subprocess.call('tail -n%s %s' % (number_of_lines, logfile), shell=True) # else: # f = open(logfile, "r") # while True:
Signature · 1 line
1.7.1
PATCH 2/2 replace os.popen with subprocess.Popen
In reply to Robert YangReplace 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.
More info:
http://docs.python.org/library/subprocess.html#subprocess-replacements
[YOCTO #2075]
--- bitbake/lib/bb/fetch2/perforce.py | 6 +++--- bitbake/lib/bb/fetch2/svk.py | 2 +- bitbake/lib/bb/ui/crumbs/builddetailspage.py | 3 ++- bitbake/lib/bb/ui/crumbs/hig.py | 7 ++++--- 4 files changed, 10 insertions(+), 8 deletions(-)
bitbake/lib/bb/fetch2/perforce.py +3 -3
@@ -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, errors) = bb.process.run("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot)) 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, errors) = bb.process.run(data.getVar('MKTEMPDIRCMD', localdata, True) or "false") 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, errors) = bb.process.run("%s%s files %s" % (p4cmd, p4opt, depot)) if not p4file: raise FetchError("Fetch: unable to get the P4 files from %s" % depot, loc)
bitbake/lib/bb/fetch2/svk.py +1 -1
@@ -77,7 +77,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, errors) = bb.process.run(data.getVar('MKTEMPDIRCMD', localdata, True) or "false") tmpfile = tmppipe.readline().strip() if not tmpfile: logger.error()
bitbake/lib/bb/ui/crumbs/builddetailspage.py +2 -1
@@ -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,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, errors) = bb.process.run('cd %s; git branch 2>&1 | grep "^* " | tr -d "* "' % path) if f: branch = f.readline().lstrip('\n').rstrip('\n') vars.append(self.set_vars("Branch:", branch))
bitbake/lib/bb/ui/crumbs/hig.py +4 -3
@@ -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.read().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(args=shlex.split(cmdline)) def update_progress_bar(self, title, fraction, status=None): self.progress_bar.update(fraction)
Signature · 1 line
1.7.1
Re: [PATCH 2/2] replace os.popen with subprocess.Popen
Hi,
I am a newbie in bitbake. I am stuggling with customizing the recipe
script. Whenever i customize
bindir, sbindir, libexecdir, libdir, datarootdir; My
package build (BB -c build <pkg>) is successful. But when creating rootfs; (bb -c build devel-image) , I get error opkg can install <pkg>
Reg,
Gopi
Re: [PATCH 2/2] replace os.popen with subprocess.Popen
In reply to Robert YangQuoted 24 lines · Robert Yang
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.More info:
http://docs.python.org/library/subprocess.html#subprocess-replacements[YOCTO #2075]
Signed-off-by Robert Yang <[email protected]>---
bitbake/lib/bb/fetch2/perforce.py | 6 +++---bitbake/lib/bb/fetch2/svk.py| 2 +-bitbake/lib/bb/ui/crumbs/builddetailspage.py| 3 ++-bitbake/lib/bb/ui/crumbs/hig.py| 7 ++++---4 files changed, 10 insertions(+), 8 deletions(-)bitbake/lib/bb/fetch2/perforce.py +0
-0
bitbake/lib/bb/fetch2/perforce.py class Perforce(FetchMethod): · line 95
Quoted 6 lines
bitbake/lib/bb/fetch2/perforce.py +1
-1@@ -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, errors) = bb.process.run("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot)) cset = p4file.readline().strip()
This is wrong, and will actually fail badly if you attempt to use this. run() returns the values from Popen.communicate(), which returns the data as strings, not pipes, so read() and readline() will not be available on that object, and aren't needed. Also, the parens aren't needed. "foo, bar =" is just as valid as "(foo, bar) =" (minor). This should do it:
output, errors = bb.process.run("%s%s changes -m 1 %s" % (p4cmd,
p4opt, depot))
cset = output.strip()
Quoted 6 lines
bitbake/lib/bb/fetch2/perforce.py +0
-0@@ -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(args=shlex.split(cmdline))
This is wrong, and will fail. run() has no 'args' named argument. Try not to get discouraged -- we do appreciate the work you're doing, but checking this in would break bitbake for a great number of people. I'd suggest actually testing bitbake with your changes before sending them to the list. Thanks.
Signature · 1 line
Christopher Larson
Re: [PATCH 2/2] replace os.popen with subprocess.Popen
Quoted 43 lines · Chris Larson
Quoted 24 lines · Robert Yang
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.More info:
http://docs.python.org/library/subprocess.html#subprocess-replacements[YOCTO #2075]
Signed-off-by Robert Yang<[email protected]>--- bitbake/lib/bb/fetch2/perforce.py | 6 +++--- bitbake/lib/bb/fetch2/svk.py | 2 +- bitbake/lib/bb/ui/crumbs/builddetailspage.py | 3 ++- bitbake/lib/bb/ui/crumbs/hig.py | 7 ++++--- 4 files changed, 10 insertions(+), 8 deletions(-)bitbake/lib/bb/fetch2/perforce.py +0
-0bitbake/lib/bb/fetch2/perforce.py class Perforce(FetchMethod): · line 95
Quoted 6 lines
bitbake/lib/bb/fetch2/perforce.py +1
-1@@ -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, errors) = bb.process.run("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot)) cset = p4file.readline().strip()This is wrong, and will actually fail badly if you attempt to use this.
run()returns the values fromPopen.communicate(), which returns the data as strings, not pipes, soread()andreadline()will not be available on that object, and aren't needed. Also, the parens aren't needed. "foo, bar =" is just as valid as "(foo, bar) =" (minor). This should do it:output, errors = bb.process.run("%s%s changes -m 1 %s" % (p4cmd,
p4opt, depot))
cset =output.strip()
Quoted 11 lines
bitbake/lib/bb/fetch2/perforce.py class DeployImageDialog (CrumbsDialog): · line 759
Quoted 6 lines
bitbake/lib/bb/fetch2/perforce.py +1
-1@@ -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(args=shlex.split(cmdline))This is wrong, and will fail.
run()has no 'args' named argument. Try not to get discouraged -- we do appreciate the work you're doing, but checking this in would break bitbake for a great number of people. I'd suggest actually testing bitbake with your changes before sending them
Yes, I had tested the "bitbake core-image-sato" before sent the patch V3,
that's fine, I will send the V4 tomorrow since I'm out of office today:-).
// Robert
Quoted 1 line
to the list. Thanks.