compile_ib_files.py: Wait for subprocess.Popen() to finish.

compile_ib_files.py: Handle non-zero ibtool return code.

Without check_output() the Popen() will always return None as returncode, this
prevents error handling during the build process.

With check_output() it will raise an exception in case of non-zero return code.

BUG=None

Review-Url: https://codereview.chromium.org/2947393002
Cr-Commit-Position: refs/heads/master@{#481948}
Committed: https://chromium.googlesource.com/chromium/src/+/edda34d840b837f1bd20d9398845d1bc38140461

[themblsha2, 2017-06-22 11:40:36]

Description was changed from

==========
compile_ib_files.py: Wait for subprocess.Popen() to finish.

Without wait() the Popen() will always return None as returncode, this
prevents error handling during the build process.

BUG=None
==========

to

==========
compile_ib_files.py: Wait for subprocess.Popen() to finish.

Without wait() the Popen() will always return None as returncode, this
prevents error handling during the build process.

BUG=None
==========

[themblsha2, 2017-06-22 11:40:37]

mblsha.yandex.team@gmail.com changed reviewers:
+ rsesek@chromium.org, sdefresne@chromium.org

[themblsha2, 2017-06-22 11:42:14]

Similar change needs to be done to at least tools/gyp/pylib/gyp/mac_tool.py. I
plan on updating that when this change is approved.

[Robert Sesek, 2017-06-22 15:13:04]

https://codereview.chromium.org/2947393002/diff/1/build/config/mac/compile_ib...
File build/config/mac/compile_ib_files.py (right):

https://codereview.chromium.org/2947393002/diff/1/build/config/mac/compile_ib...
build/config/mac/compile_ib_files.py:43: ibtoolout =
subprocess.Popen(ibtool_args, stdout=subprocess.PIPE)
Any reason to not use subprocess.check_output?

[themblsha2, 2017-06-23 13:31:49]

Description was changed from

==========
compile_ib_files.py: Wait for subprocess.Popen() to finish.

Without wait() the Popen() will always return None as returncode, this
prevents error handling during the build process.

BUG=None
==========

to

==========
compile_ib_files.py: Handle non-zero ibtool return code.

Without check_output() the Popen() will always return None as returncode, this
prevents error handling during the build process.

With check_output() it will raise an exception in case of non-zero return code.

BUG=None
==========

[themblsha2, 2017-06-23 13:32:43]

On 2017/06/22 15:13:04, Robert Sesek wrote:
> Any reason to not use subprocess.check_output?

Your original patch didn't and I tried to do a minimal change :-) Reimplemented.

[Robert Sesek, 2017-06-23 15:24:33]

LGTM

On 2017/06/23 13:32:43, themblsha2 wrote:
> On 2017/06/22 15:13:04, Robert Sesek wrote:
> > Any reason to not use subprocess.check_output?
> 
> Your original patch didn't and I tried to do a minimal change :-)
Reimplemented.

That's fair :). I just noticed that the rest of the build/config/mac scripts use
check_output.

[sdefresne, 2017-06-23 16:48:58]

lgtm

https://codereview.chromium.org/2947393002/diff/40001/build/config/mac/compil...
File build/config/mac/compile_ib_files.py (right):

https://codereview.chromium.org/2947393002/diff/40001/build/config/mac/compil...
build/config/mac/compile_ib_files.py:54: print current_section_header
nit: can you either use "print(ccurent_section_header)" or
"sys.stdout.write(current_section_header + '\n")" [or something equivalent]? In
python 3, print is a function and require parentheses (I know we do not support
python 3, but why add work for the conversion).

[themblsha2, 2017-06-23 17:21:46]

On 2017/06/23 16:48:58, sdefresne wrote:
> nit: can you either use "print(ccurent_section_header)" or
> "sys.stdout.write(current_section_header + '\n")" [or something equivalent]?
In
> python 3, print is a function and require parentheses (I know we do not
support
> python 3, but why add work for the conversion).

I was under the impression that you're stuck in Python2 forever :-) Good that
it's not out of the question, updated the patch.

[themblsha2, 2017-06-23 17:21:51]

The CQ bit was checked by mblsha.yandex.team@gmail.com

[themblsha2, 2017-06-23 17:21:52]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org,
sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2947393002/#ps60001
(title: "Update print to Python3-compatible syntax.")

[commit-bot: I haz the power, 2017-06-23 17:22:08]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-23 17:42:34]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498238511797780,
"parent_rev": "12ef88559c5d269f36884f46e06f44334ea446f7", "commit_rev":
"edda34d840b837f1bd20d9398845d1bc38140461"}

[commit-bot: I haz the power, 2017-06-23 17:42:50]

Description was changed from

==========
compile_ib_files.py: Handle non-zero ibtool return code.

Without check_output() the Popen() will always return None as returncode, this
prevents error handling during the build process.

With check_output() it will raise an exception in case of non-zero return code.

BUG=None
==========

to

==========
compile_ib_files.py: Handle non-zero ibtool return code.

Without check_output() the Popen() will always return None as returncode, this
prevents error handling during the build process.

With check_output() it will raise an exception in case of non-zero return code.

BUG=None

Review-Url: https://codereview.chromium.org/2947393002
Cr-Commit-Position: refs/heads/master@{#481948}
Committed:
https://chromium.googlesource.com/chromium/src/+/edda34d840b837f1bd20d9398845...
==========

[commit-bot: I haz the power, 2017-06-23 17:42:53]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/edda34d840b837f1bd20d9398845...