Fixed bug in dependency recursion in isolate_driver.py.

Fixed bug in dependency recursion in isolate_driver.py.

When run in a component build, isolate_driver recursively descends the
dependency graph to discover implicit shared library dependencies. A
bug in the recursion caused dependencies at certain depths in the
graph to be missed.

Also changed the recursion to use a set to record newly added
dependencies to avoid adding them to the isolate multiple times.
Expanded the ignored set of suffixes slightly.

BUG=378021
TEST=GLES2 conformance tests run via isolate in component build on Mac OS
R=maruel@chromium.org,vadimsh@chromium.org,csharp@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=274023

[Ken Russell (switch to Gerrit), 2014-05-30 21:55:26]

Please review. Thanks.

[Vadim Sh., 2014-05-30 22:15:25]

lgtm with suggestion

https://codereview.chromium.org/304993006/diff/1/tools/isolate_driver.py
File tools/isolate_driver.py (right):

https://codereview.chromium.org/304993006/diff/1/tools/isolate_driver.py#newc...
tools/isolate_driver.py:142: def recurse(target, build_steps, rules_seen,
dependencies_added):
Please make this function _not_ to return anything, and explicitly document that
it mutates |dependencies_added| in place. Also it makes sense to rename to
'collect_deps' or something like that, to indicate it gathers deps in passed
set.

Functions that return results and have side effects on arguments and
confusing...

https://codereview.chromium.org/304993006/diff/1/tools/isolate_driver.py#newc...
tools/isolate_driver.py:219: binary_deps = post_process_deps(build_dir, recurse(
and then:
binary_deps = set()
collect_deps(target, build_steps, None, binary_deps)
binary_deps = post_process_deps(build_dir, binary_deps)

[Ken Russell (switch to Gerrit), 2014-05-30 22:43:49]

https://codereview.chromium.org/304993006/diff/1/tools/isolate_driver.py
File tools/isolate_driver.py (right):

https://codereview.chromium.org/304993006/diff/1/tools/isolate_driver.py#newc...
tools/isolate_driver.py:142: def recurse(target, build_steps, rules_seen,
dependencies_added):
On 2014/05/30 22:15:25, Vadim Sh. wrote:
> Please make this function _not_ to return anything, and explicitly document
that
> it mutates |dependencies_added| in place. Also it makes sense to rename to
> 'collect_deps' or something like that, to indicate it gathers deps in passed
> set.
> 
> Functions that return results and have side effects on arguments and
> confusing... 

Thanks for the good suggestion. Done.

https://codereview.chromium.org/304993006/diff/1/tools/isolate_driver.py#newc...
tools/isolate_driver.py:219: binary_deps = post_process_deps(build_dir, recurse(
On 2014/05/30 22:15:25, Vadim Sh. wrote:
> and then:
> binary_deps = set()
> collect_deps(target, build_steps, None, binary_deps)
> binary_deps = post_process_deps(build_dir, binary_deps)

Done. I switched the order of the dependencies_added and rules_seen arguments so
None could continue to be the last argument.

[Ken Russell (switch to Gerrit), 2014-05-30 22:43:53]

The CQ bit was checked by kbr@chromium.org

[commit-bot: I haz the power, 2014-05-30 22:50:36]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kbr@chromium.org/304993006/10001

[commit-bot: I haz the power, 2014-05-31 04:40:36]

Change committed as 274023