Openembedded Bitbake Development
 help / color / mirror / Atom feed
From: Robert Yang <liezhi.yang@windriver.com>
To: "Wang, Shane" <shane.wang@intel.com>
Cc: Kang Kai <Kai.Kang@windriver.com>,
	"bitbake-devel@lists.openembedded.org"
	<bitbake-devel@lists.openembedded.org>,
	"Zhenfeng.Zhao@windriver.com" <Zhenfeng.Zhao@windriver.com>
Subject: Re: [PATCH 2/2] replace os.popen with subprocess.Popen
Date: Wed, 30 May 2012 09:02:44 +0800	[thread overview]
Message-ID: <4FC571B4.5090106@windriver.com> (raw)
In-Reply-To: <3AB6CE7F274E534CAFD089D127A8A1FC23BB3A1C@SHSMSX102.ccr.corp.intel.com>



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)
>
>
>
>



  parent reply	other threads:[~2012-05-30  1:13 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FC571B4.5090106@windriver.com \
    --to=liezhi.yang@windriver.com \
    --cc=Kai.Kang@windriver.com \
    --cc=Zhenfeng.Zhao@windriver.com \
    --cc=bitbake-devel@lists.openembedded.org \
    --cc=shane.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox