* [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module @ 2012-05-20 12:36 Robert Yang 2012-05-20 12:36 ` [PATCH 1/2] replace os.system with subprocess.call Robert Yang ` (3 more replies) 0 siblings, 4 replies; 19+ messages in thread From: Robert Yang @ 2012-05-20 12:36 UTC (permalink / raw) To: bitbake-devel; +Cc: Zhenfeng.Zhao * Changes of V4: Fix use readline() for the return value of bb.process.run(), the return value is already a string, we can't (or don't have to) use readline() for it, the string is what we need. * 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 b4c8c74a45e386f99344cf9799eb5294ad6c9e3e: hob: update required pygtk to 2.22.0 and gtk+ to 2.20.0 (2012-05-20 09:24:26 +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 | 14 ++++++++------ bitbake/lib/bb/fetch2/svk.py | 4 ++-- bitbake/lib/bb/shell.py | 6 +++--- bitbake/lib/bb/ui/crumbs/builddetailspage.py | 5 +++-- 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, 24 insertions(+), 19 deletions(-) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/2] replace os.system with subprocess.call 2012-05-20 12:36 [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module Robert Yang @ 2012-05-20 12:36 ` Robert Yang 2012-05-20 12:36 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang ` (2 subsequent siblings) 3 siblings, 0 replies; 19+ messages in thread From: Robert Yang @ 2012-05-20 12:36 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] 19+ messages in thread
* [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-20 12:36 [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module Robert Yang 2012-05-20 12:36 ` [PATCH 1/2] replace os.system with subprocess.call Robert Yang @ 2012-05-20 12:36 ` Robert Yang 2012-05-29 14:58 ` Wang, Shane 2012-05-20 18:16 ` [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module Chris Larson 2012-05-23 10:30 ` Richard Purdie 3 siblings, 1 reply; 19+ messages in thread From: Robert Yang @ 2012-05-20 12:36 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. 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) -- 1.7.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-20 12:36 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang @ 2012-05-29 14:58 ` Wang, Shane 2012-05-29 15:17 ` Wang, Shane 2012-05-30 1:02 ` Robert Yang 0 siblings, 2 replies; 19+ messages in thread From: Wang, Shane @ 2012-05-29 14:58 UTC (permalink / raw) To: Robert Yang, bitbake-devel; +Cc: Kang Kai, Zhenfeng.Zhao 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) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-29 14:58 ` Wang, Shane @ 2012-05-29 15:17 ` Wang, Shane 2012-05-30 1:02 ` Robert Yang 1 sibling, 0 replies; 19+ messages in thread From: Wang, Shane @ 2012-05-29 15:17 UTC (permalink / raw) To: Wang, Shane, Robert Yang, bitbake-devel; +Cc: Kang Kai, Zhenfeng.Zhao Again, be careful to check whether "deploy image" and "run qemu" work. Because previously I used "os.popen" and found the frontend process (i.e. the Hob) hangs when the backend process (i.e. the deployment process and the qemu process) is running. So, I chose "subprocess.Popen", so please have a test with "deploy image" and "run qemu" after you replaced it. -- Shane Wang, Shane wrote on 2012-05-29: > 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' > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-29 14:58 ` Wang, Shane 2012-05-29 15:17 ` Wang, Shane @ 2012-05-30 1:02 ` Robert Yang 2012-05-30 1:10 ` Wang, Shane 1 sibling, 1 reply; 19+ messages in thread From: Robert Yang @ 2012-05-30 1:02 UTC (permalink / raw) To: Wang, Shane; +Cc: Kang Kai, bitbake-devel, Zhenfeng.Zhao On 05/29/2012 10:58 PM, Wang, Shane wrote: > Robert, be careful. Your patch 094742bed2fc01d55f572da946fcfa7a48521401 has broke Hob. > Sorry, in bitbake/lib/bb/ui/crumbs/builddetailspage.py: 103 branch = f.strip('\n') 104 vars.append(self.set_vars("Branch:", branch)) 105 f.close() 106 break Remove the f.close should be OK, I will check the patch again to see whether there is such an error in other corner, and then send a pull request today. // Robert > 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) > > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-30 1:02 ` Robert Yang @ 2012-05-30 1:10 ` Wang, Shane 0 siblings, 0 replies; 19+ messages in thread From: Wang, Shane @ 2012-05-30 1:10 UTC (permalink / raw) To: Robert Yang; +Cc: Kang Kai, bitbake-devel, Zhenfeng.Zhao Sounds good. Thanks. -- Shane Robert Yang wrote on 2012-05-30: > > > On 05/29/2012 10:58 PM, Wang, Shane wrote: >> Robert, be careful. Your patch 094742bed2fc01d55f572da946fcfa7a48521401 >> has broke Hob. >> > > Sorry, in bitbake/lib/bb/ui/crumbs/builddetailspage.py: > > 103 branch = f.strip('\n') > 104 vars.append(self.set_vars("Branch:", branch)) > 105 f.close() > 106 break > > Remove the f.close should be OK, I will check the patch again to see whether > there is such an error in other corner, and then send a pull request today. > > // Robert > >> 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) >> >> >> >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module 2012-05-20 12:36 [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module Robert Yang 2012-05-20 12:36 ` [PATCH 1/2] replace os.system with subprocess.call Robert Yang 2012-05-20 12:36 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang @ 2012-05-20 18:16 ` Chris Larson 2012-05-23 10:30 ` Richard Purdie 3 siblings, 0 replies; 19+ messages in thread From: Chris Larson @ 2012-05-20 18:16 UTC (permalink / raw) To: Robert Yang; +Cc: bitbake-devel, Zhenfeng.Zhao On Sun, May 20, 2012 at 5:36 AM, Robert Yang <liezhi.yang@windriver.com> wrote: > * Changes of V4: > Fix use readline() for the return value of bb.process.run(), the > return value is already a string, we can't (or don't have to) use > readline() for it, the string is what we need. > > * 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. This version looks good to me. -- Christopher Larson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module 2012-05-20 12:36 [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module Robert Yang ` (2 preceding siblings ...) 2012-05-20 18:16 ` [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module Chris Larson @ 2012-05-23 10:30 ` Richard Purdie 3 siblings, 0 replies; 19+ messages in thread From: Richard Purdie @ 2012-05-23 10:30 UTC (permalink / raw) To: Robert Yang; +Cc: bitbake-devel, Zhenfeng.Zhao On Sun, 2012-05-20 at 20:36 +0800, Robert Yang wrote: > * Changes of V4: > Fix use readline() for the return value of bb.process.run(), the > return value is already a string, we can't (or don't have to) use > readline() for it, the string is what we need. > > * 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 b4c8c74a45e386f99344cf9799eb5294ad6c9e3e: > > hob: update required pygtk to 2.22.0 and gtk+ to 2.20.0 (2012-05-20 09:24:26 +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 Merged to master, thanks. Richard ^ permalink raw reply [flat|nested] 19+ messages in thread
* [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 2/2] replace os.popen with subprocess.Popen Robert Yang 0 siblings, 1 reply; 19+ 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] 19+ messages in thread
* [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-16 5:55 [PATCH 0/2] V3 " Robert Yang @ 2012-05-16 5:55 ` Robert Yang 2012-05-16 9:35 ` GOPIKRISHNAN S 2012-05-16 14:14 ` Chris Larson 0 siblings, 2 replies; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ 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; 19+ 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] 19+ messages in thread
* [PATCH 0/2] V2 replace os.system and os.popen with subbprocess module @ 2012-05-15 9:53 Robert Yang 2012-05-15 9:53 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang 0 siblings, 1 reply; 19+ messages in thread From: Robert Yang @ 2012-05-15 9:53 UTC (permalink / raw) To: bitbake-devel; +Cc: Zhenfeng.Zhao * Change 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 12b4543ac9f54c0db0ee4a7ee546a71946a051f8: classes/rootfs_*: fix splitting package dependency strings (2012-05-13 20:09:56 +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 | 10 ++++++---- bitbake/lib/bb/fetch2/svk.py | 3 ++- bitbake/lib/bb/shell.py | 6 +++--- bitbake/lib/bb/ui/crumbs/builddetailspage.py | 3 ++- bitbake/lib/bb/ui/crumbs/hig.py | 6 +++--- bitbake/lib/bb/ui/crumbs/imagedetailspage.py | 3 ++- bitbake/lib/bb/ui/ncurses.py | 4 ++-- 7 files changed, 20 insertions(+), 15 deletions(-) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-15 9:53 [PATCH 0/2] V2 replace os.system and os.popen with subbprocess module Robert Yang @ 2012-05-15 9:53 ` Robert Yang 2012-05-15 14:43 ` Chris Larson 0 siblings, 1 reply; 19+ messages in thread From: Robert Yang @ 2012-05-15 9:53 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 wrappers around Popen for convenience in the bb.process python module, so use the Popen from bb.process to 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 | 7 ++++--- bitbake/lib/bb/fetch2/svk.py | 3 ++- bitbake/lib/bb/ui/crumbs/builddetailspage.py | 3 ++- bitbake/lib/bb/ui/crumbs/hig.py | 6 +++--- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/bitbake/lib/bb/fetch2/perforce.py b/bitbake/lib/bb/fetch2/perforce.py index 6abf15d..1c4ef25 100644 --- a/bitbake/lib/bb/fetch2/perforce.py +++ b/bitbake/lib/bb/fetch2/perforce.py @@ -35,6 +35,7 @@ from bb.fetch2 import FetchMethod from bb.fetch2 import FetchError from bb.fetch2 import logger from bb.fetch2 import runfetchcmd +from bb.process import Popen class Perforce(FetchMethod): def supports(self, url, ud, d): @@ -91,7 +92,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 = Popen("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot)).stdout cset = p4file.readline().strip() logger.debug(1, "READ %s", cset) if not cset: @@ -155,7 +156,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 = Popen(data.getVar('MKTEMPDIRCMD', localdata, True) or "false").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 +170,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 = Popen("%s%s files %s" % (p4cmd, p4opt, depot)).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..43d30a3 100644 --- a/bitbake/lib/bb/fetch2/svk.py +++ b/bitbake/lib/bb/fetch2/svk.py @@ -34,6 +34,7 @@ from bb.fetch2 import FetchError from bb.fetch2 import MissingParameterError from bb.fetch2 import logger from bb.fetch2 import runfetchcmd +from bb.process import Popen class Svk(FetchMethod): """Class to fetch a module or modules from svk repositories""" @@ -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 = Popen(data.getVar('MKTEMPDIRCMD', localdata, True) or "false").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..8387161 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 +from bb.process import Popen 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 = Popen('cd %s; git branch 2>&1 | grep "^* " | tr -d "* "' % path).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..d4fcb55 100644 --- a/bitbake/lib/bb/ui/crumbs/hig.py +++ b/bitbake/lib/bb/ui/crumbs/hig.py @@ -25,11 +25,11 @@ 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 +from bb.process import Popen import bb.ui.crumbs.utils """ @@ -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 Popen("%s" % cmd).stdout.read().strip() def find_all_usb_devices(self): usb_devs = [ os.readlink(u) @@ -755,7 +755,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)) + Popen(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] 19+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-15 9:53 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang @ 2012-05-15 14:43 ` Chris Larson 2012-05-16 1:29 ` Robert Yang 0 siblings, 1 reply; 19+ messages in thread From: Chris Larson @ 2012-05-15 14:43 UTC (permalink / raw) To: Robert Yang; +Cc: bitbake-devel, Zhenfeng.Zhao On Tue, May 15, 2012 at 2:53 AM, Robert Yang <liezhi.yang@windriver.com> wrote: > - p4file = os.popen("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot)) > + p4file = Popen("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot)).stdout > cset = p4file.readline().strip() When all that's being done is reading the output of the command, I'd think bb.process.run() would do the job, rather than manually using Popen, no? Once we require python 2.7, we can use subprocess.check_output() instead, of course. -- Christopher Larson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-15 14:43 ` Chris Larson @ 2012-05-16 1:29 ` Robert Yang 0 siblings, 0 replies; 19+ messages in thread From: Robert Yang @ 2012-05-16 1:29 UTC (permalink / raw) To: Chris Larson; +Cc: bitbake-devel, Zhenfeng.Zhao On 05/15/2012 10:43 PM, Chris Larson wrote: > On Tue, May 15, 2012 at 2:53 AM, Robert Yang<liezhi.yang@windriver.com> wrote: >> - p4file = os.popen("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot)) >> + p4file = Popen("%s%s changes -m 1 %s" % (p4cmd, p4opt, depot)).stdout >> cset = p4file.readline().strip() > > When all that's being done is reading the output of the command, I'd > think bb.process.run() would do the job, rather than manually using Thanks, the bb.process.run() would be better, I will send the V3 sooner. // Robert > Popen, no? Once we require python 2.7, we can use > subprocess.check_output() instead, of course. ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 0/2] replace os.system and os.popen with subbprocess module @ 2012-05-14 8:07 Robert Yang 2012-05-14 8:07 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang 0 siblings, 1 reply; 19+ messages in thread From: Robert Yang @ 2012-05-14 8:07 UTC (permalink / raw) To: bitbake-devel; +Cc: Zhenfeng.Zhao 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 12b4543ac9f54c0db0ee4a7ee546a71946a051f8: classes/rootfs_*: fix splitting package dependency strings (2012-05-13 20:09:56 +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 | 3 ++- bitbake/lib/bb/shell.py | 6 +++--- bitbake/lib/bb/ui/crumbs/builddetailspage.py | 3 ++- bitbake/lib/bb/ui/crumbs/hig.py | 2 +- bitbake/lib/bb/ui/crumbs/imagedetailspage.py | 3 ++- bitbake/lib/bb/ui/ncurses.py | 4 ++-- 7 files changed, 17 insertions(+), 13 deletions(-) ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-14 8:07 [PATCH 0/2] replace os.system and os.popen with subbprocess module Robert Yang @ 2012-05-14 8:07 ` Robert Yang 2012-05-14 13:59 ` Chris Larson 0 siblings, 1 reply; 19+ messages in thread From: Robert Yang @ 2012-05-14 8:07 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 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(-) 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) -- 1.7.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-14 8:07 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang @ 2012-05-14 13:59 ` Chris Larson 2012-05-15 6:49 ` Robert Yang 0 siblings, 1 reply; 19+ messages in thread From: Chris Larson @ 2012-05-14 13:59 UTC (permalink / raw) To: Robert Yang; +Cc: bitbake-devel, Zhenfeng.Zhao 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. -- Christopher Larson ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/2] replace os.popen with subprocess.Popen 2012-05-14 13:59 ` Chris Larson @ 2012-05-15 6:49 ` Robert Yang 0 siblings, 0 replies; 19+ messages in thread From: Robert Yang @ 2012-05-15 6:49 UTC (permalink / raw) To: Chris Larson; +Cc: bitbake-devel, Zhenfeng.Zhao 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2012-05-30 1:21 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-05-20 12:36 [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module Robert Yang 2012-05-20 12:36 ` [PATCH 1/2] replace os.system with subprocess.call Robert Yang 2012-05-20 12:36 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang 2012-05-29 14:58 ` Wang, Shane 2012-05-29 15:17 ` Wang, Shane 2012-05-30 1:02 ` Robert Yang 2012-05-30 1:10 ` Wang, Shane 2012-05-20 18:16 ` [PATCH 0/2] V4 replace os.system and os.popen with subbprocess module Chris Larson 2012-05-23 10:30 ` Richard Purdie -- strict thread matches above, loose matches on Subject: below -- 2012-05-16 5:55 [PATCH 0/2] V3 " 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 2012-05-15 9:53 [PATCH 0/2] V2 replace os.system and os.popen with subbprocess module Robert Yang 2012-05-15 9:53 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang 2012-05-15 14:43 ` Chris Larson 2012-05-16 1:29 ` Robert Yang 2012-05-14 8:07 [PATCH 0/2] replace os.system and os.popen with subbprocess module Robert Yang 2012-05-14 8:07 ` [PATCH 2/2] replace os.popen with subprocess.Popen Robert Yang 2012-05-14 13:59 ` Chris Larson 2012-05-15 6:49 ` 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.