Allow DEPS file to specify 'target_os'.

Allow DEPS file to specify 'target_os'.


BUG=155792
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=163955

[Siva Chandra, 2012-10-23 18:45:38]

There is a presubmit warning about accessing a protected member of a client
object. However, I am sending this out to see if the overall logic is correct.

[M-A Ruel, 2012-10-23 18:47:45]

Why not set it in .gclient?

[cmp, 2012-10-23 18:52:42]

For our use case, we always want to set this for our gclient solutions, so it
doesn't make sense to set it in .gclient when we can set it and control it in
our DEPS file.  See the linked bug.

[cmp, 2012-10-23 18:54:55]

lgtm with nit

https://codereview.chromium.org/11246004/diff/1/tests/gclient_test.py
File tests/gclient_test.py (right):

https://codereview.chromium.org/11246004/diff/1/tests/gclient_test.py#newcode339
tests/gclient_test.py:339: options.deps_os = "unix"
nit: single quotes here instead of double quotes

[Siva Chandra, 2012-10-23 23:09:58]

I added "# pylint: disable W0212" for one of the lines which accesses a
protected member of a client class.

[Isaac (away), 2012-10-23 23:39:31]

M-A, I think you said it would be OK to move target_os into a local property
from global.  This is so that we can using android specific deps files and
ensure developers always sync the target_os=['android'] deps.

However this current syntax is not sufficient -- it needs to be a local_scoped
variable or else parsing a sub-dep has side effects on the parents.  I talked to
siva and he's going to emulate the recursion_limit property (which is a tree
lookup) & recursion_overvide hook.

[Siva Chandra, 2012-10-23 23:43:04]

I am going to wait for cmp's and maruel's comments before I make further
changes.

[M-A Ruel, 2012-10-23 23:53:30]

https://codereview.chromium.org/11246004/diff/5002/gclient.py
File gclient.py (right):

https://codereview.chromium.org/11246004/diff/5002/gclient.py#newcode458
gclient.py:458: # Check if 'target_os' is present in the DEPS file. If yes, then
add OSes
It's a layering violation. It should only affect its subtree.

So you need to add a member and a property to DependencySettings, similar to how
recursion_limit is done.

[Siva Chandra, 2012-10-24 23:31:04]

PTAL patch set 5.

[M-A Ruel, 2012-10-24 23:33:36]

lgtm

[cmp, 2012-10-24 23:37:30]

lgtm

[commit-bot: I haz the power, 2012-10-24 23:40:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sivachandra@chromium.org/11246004/15001

[commit-bot: I haz the power, 2012-10-24 23:42:51]

Change committed as 163955