* [PATCH 0/2] V3 replace os.system and os.popen with subbprocess module
@ 2012-05-16 5:55 Robert Yang
2012-05-16 5:55 ` [PATCH 1/2] replace os.system with subprocess.call Robert Yang
2012-05-16 5:55 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang
0 siblings, 2 replies; 6+ messages in thread
From: Robert Yang @ 2012-05-16 5:55 UTC (permalink / raw)
To: bitbake-devel; +Cc: Zhenfeng.Zhao
* 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(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] replace os.system with subprocess.call
2012-05-16 5:55 [PATCH 0/2] V3 replace os.system and os.popen with subbprocess module Robert Yang
@ 2012-05-16 5:55 ` Robert Yang
2012-05-16 5:55 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang
1 sibling, 0 replies; 6+ messages in thread
From: Robert Yang @ 2012-05-16 5:55 UTC (permalink / raw)
To: bitbake-devel; +Cc: Zhenfeng.Zhao
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]
Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
---
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(-)
diff --git a/bitbake/lib/bb/fetch2/perforce.py b/bitbake/lib/bb/fetch2/perforce.py
index cbdc848..6abf15d 100644
--- a/bitbake/lib/bb/fetch2/perforce.py
+++ b/bitbake/lib/bb/fetch2/perforce.py
@@ -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:
diff --git a/bitbake/lib/bb/shell.py b/bitbake/lib/bb/shell.py
index 1dd8d54..a83dedd 100644
--- a/bitbake/lib/bb/shell.py
+++ b/bitbake/lib/bb/shell.py
@@ -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 ):
diff --git a/bitbake/lib/bb/ui/crumbs/imagedetailspage.py b/bitbake/lib/bb/ui/crumbs/imagedetailspage.py
index 1cfef80..2453dbc 100755
--- a/bitbake/lib/bb/ui/crumbs/imagedetailspage.py
+++ b/bitbake/lib/bb/ui/crumbs/imagedetailspage.py
@@ -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)
diff --git a/bitbake/lib/bb/ui/ncurses.py b/bitbake/lib/bb/ui/ncurses.py
index 8524446..1425bbd 100644
--- a/bitbake/lib/bb/ui/ncurses.py
+++ b/bitbake/lib/bb/ui/ncurses.py
@@ -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:
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] replace os.popen with subprocess.Popen
2012-05-16 5:55 [PATCH 0/2] V3 replace os.system and os.popen with subbprocess module Robert Yang
2012-05-16 5:55 ` [PATCH 1/2] replace os.system with subprocess.call Robert Yang
@ 2012-05-16 5:55 ` Robert Yang
2012-05-16 9:35 ` GOPIKRISHNAN S
2012-05-16 14:14 ` Chris Larson
1 sibling, 2 replies; 6+ messages in thread
From: Robert Yang @ 2012-05-16 5:55 UTC (permalink / raw)
To: bitbake-devel; +Cc: Zhenfeng.Zhao
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 <liezhi.yang@windriver.com>
---
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(-)
diff --git a/bitbake/lib/bb/fetch2/perforce.py b/bitbake/lib/bb/fetch2/perforce.py
index 6abf15d..e07afdd 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, 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)
diff --git a/bitbake/lib/bb/fetch2/svk.py b/bitbake/lib/bb/fetch2/svk.py
index 9d34abf..157e487 100644
--- a/bitbake/lib/bb/fetch2/svk.py
+++ b/bitbake/lib/bb/fetch2/svk.py
@@ -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()
diff --git a/bitbake/lib/bb/ui/crumbs/builddetailspage.py b/bitbake/lib/bb/ui/crumbs/builddetailspage.py
index 51e6a4a..3f54437 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,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))
diff --git a/bitbake/lib/bb/ui/crumbs/hig.py b/bitbake/lib/bb/ui/crumbs/hig.py
index 4baf960..7d109e5 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.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)
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen
2012-05-16 5:55 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang
@ 2012-05-16 9:35 ` GOPIKRISHNAN S
2012-05-16 14:14 ` Chris Larson
1 sibling, 0 replies; 6+ messages in thread
From: GOPIKRISHNAN S @ 2012-05-16 9:35 UTC (permalink / raw)
To: bitbake-devel
[-- Attachment #1: Type: text/plain, Size: 337 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 368 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen
2012-05-16 5:55 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang
2012-05-16 9:35 ` GOPIKRISHNAN S
@ 2012-05-16 14:14 ` Chris Larson
2012-05-17 1:59 ` Robert Yang
1 sibling, 1 reply; 6+ messages in thread
From: Chris Larson @ 2012-05-16 14:14 UTC (permalink / raw)
To: Robert Yang; +Cc: bitbake-devel, Zhenfeng.Zhao
On Tue, May 15, 2012 at 10:55 PM, Robert Yang <liezhi.yang@windriver.com> wrote:
> 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 <liezhi.yang@windriver.com>
> ---
> 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(-)
>
> diff --git a/bitbake/lib/bb/fetch2/perforce.py b/bitbake/lib/bb/fetch2/perforce.py
> index 6abf15d..e07afdd 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, 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()
> @@ -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.
--
Christopher Larson
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen
2012-05-16 14:14 ` Chris Larson
@ 2012-05-17 1:59 ` Robert Yang
0 siblings, 0 replies; 6+ messages in thread
From: Robert Yang @ 2012-05-17 1:59 UTC (permalink / raw)
To: Chris Larson; +Cc: bitbake-devel, Zhenfeng.Zhao
On 05/16/2012 10:14 PM, Chris Larson wrote:
> On Tue, May 15, 2012 at 10:55 PM, Robert Yang<liezhi.yang@windriver.com> wrote:
>> 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<liezhi.yang@windriver.com>
>> ---
>> 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(-)
>>
>> diff --git a/bitbake/lib/bb/fetch2/perforce.py b/bitbake/lib/bb/fetch2/perforce.py
>> index 6abf15d..e07afdd 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, 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()
>
>> @@ -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
> to the list. Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-05-17 2:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-16 5:55 [PATCH 0/2] V3 replace os.system and os.popen with subbprocess module Robert Yang
2012-05-16 5:55 ` [PATCH 1/2] replace os.system with subprocess.call Robert Yang
2012-05-16 5:55 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang
2012-05-16 9:35 ` GOPIKRISHNAN S
2012-05-16 14:14 ` Chris Larson
2012-05-17 1:59 ` Robert Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.