On Thu, Jul 31, 2014 at 02:54:43AM +0900, Masahiro YAMADA wrote: > Hi Tom, > > > 2014-07-30 23:24 GMT+09:00 Tom Rini <tr...@ti.com>: > > > > The function subprocess.check_output() only exists in Python 2.7 and > > later (and there are long term supported distributions that ship with > > 2.6.x). Replace this with a call to subprocess.Popen() and then checking > > output via communicate() > > Unfortunately.. :-( > Anyway, thanks for catching this issue. > > > > Signed-off-by: Tom Rini <tr...@ti.com> > > --- > > tools/genboardscfg.py | 9 ++++++--- > > 1 files changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/tools/genboardscfg.py b/tools/genboardscfg.py > > index 805e4eb..6588392 100755 > > --- a/tools/genboardscfg.py > > +++ b/tools/genboardscfg.py > > @@ -83,7 +83,8 @@ def check_top_directory(): > > def get_make_cmd(): > > """Get the command name of GNU Make.""" > > try: > > - make_cmd = subprocess.check_output([SHOW_GNU_MAKE]) > > + process = subprocess.Popen([SHOW_GNU_MAKE], stdout=subprocess.PIPE) > > + make_cmd, unused = process.communicate() > > except subprocess.CalledProcessError: > > print >> sys.stderr, 'GNU Make not found' > > sys.exit(1) > > > > No good. > > Unlike check_output(), the communicate() method never throws > CalledProcessError exception, > which means the lines in except are never run. > > When scripts/show-gnu-make fails, the function will not error out, but > just return a null string. > To know if the subprocess succeeded or not, 'returncode' should be checked. > > > The correct code should be something like this: > > > def get_make_cmd(): > """Get the command name of GNU Make.""" > process = subprocess.Popen([SHOW_GNU_MAKE], stdout=subprocess.PIPE) > ret = process.communicate() > if process.returncode: > print >> sys.stderr, 'GNU Make not found' > sys.exit(1) > return ret[0].strip() > > > > > > > @@ -493,8 +494,10 @@ def main(): > > sys.exit(1) > > else: > > try: > > - jobs = int(subprocess.check_output(['getconf', > > - '_NPROCESSORS_ONLN'])) > > + process = subprocess.Popen(['getconf', '_NPROCESSORS_ONLN'], > > + stdout=subprocess.PIPE) > > + jobstr, unused = process.communicate() > > + jobs = int(jobstr) > > except subprocess.CalledProcessError: > > print 'info: failed to get the number of CPUs. Set jobs to 1' > > jobs = 1 > > -- > > Ditto. > 'except subprocess.CalledProcessError:' is meaningless and never > catches an exception. > > In this case, 'getconf' may not exist on some systems. > > If the 'getconf' command is not found, Popen() will throw OSError exception. > If the command is found but fails by some reason, int() will throw ValueError. > We cannot handle the other exceptions. > > So, we can write the code something like this: > > ... > else: > try: > jobs = int(subprocess.Popen(['getconf', '_NPROCESSORS_ONLN'], > stdout=subprocess.PIPE).communicate()[0]) > except (OSError, ValueError): > # getconf command not found or fails > print 'info: failed to get the number of CPUs. Set jobs to 1' > jobs = 1 > gen_boards_cfg(jobs)
Arg. Do you want me to fold / rework things like that or do you want to post a v9 of just this patch, adapted as you've shown? -- Tom
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot