Allow specificying that only target_os should be used in a gclient file

Allow specificying that only target_os should be used in a gclient file

This allows specificying that a target OS list should override the base
list instead of adding to it. This is useful for iOS builds, where the
desired behavior is to pull only the iOS deps, not the union of the Mac
and iOS deps.


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

[stuartmorgan, 2012-11-01 15:41:17]

This is basically a follow-up to http://codereview.chromium.org/10127004 to make
it useful for iOS. Currently all iOS builds must *always* be synced with "--deps
ios" or you get a bunch of stuff that shouldn't be there (e.g., a full WebKit
checkout), which is error-prone and annoying.

I went back and forth between this, and a totally different key that would
behave like target_os except overriding, but the addition of a second flag
argument seemed like it would be less confusing then two similarly-named keys
with the same form that behaved differently. Let me know what you think.

[M-A Ruel, 2012-11-01 18:35:49]

I'm fine with the idea, just have one question

https://codereview.chromium.org/11363036/diff/1/gclient.py
File gclient.py (right):

https://codereview.chromium.org/11363036/diff/1/gclient.py#newcode896
gclient.py:896: if config_dict.get('target_os_only', False):
Should it work if 'target_os' is not present?

[stuartmorgan, 2012-11-02 05:50:02]

https://codereview.chromium.org/11363036/diff/1/gclient.py
File gclient.py (right):

https://codereview.chromium.org/11363036/diff/1/gclient.py#newcode896
gclient.py:896: if config_dict.get('target_os_only', False):
On 2012/11/01 18:35:49, Marc-Antoine Ruel wrote:
> Should it work if 'target_os' is not present?

I figured that would fall under "just don't do that", but there's no reason not
to make it ignored in that case, so done.

[M-A Ruel, 2012-11-02 12:07:21]

lgtm

https://codereview.chromium.org/11363036/diff/3001/gclient.py
File gclient.py (right):

https://codereview.chromium.org/11363036/diff/3001/gclient.py#newcode897
gclient.py:897: if len(target_os) > 0 and config_dict.get('target_os_only',
False):
if target_os and ...
will work just fine.

in fact, I'd prefer it to raise an exception:
if not target_os and config_dict.get('target_os_only', False):
  raise gclient_utils.Error('Can\'t use target_os_only if target_os is not
specified')

Do not assume users won't do stupid things. :)

[stuartmorgan, 2012-11-02 12:36:12]

https://codereview.chromium.org/11363036/diff/3001/gclient.py
File gclient.py (right):

https://codereview.chromium.org/11363036/diff/3001/gclient.py#newcode897
gclient.py:897: if len(target_os) > 0 and config_dict.get('target_os_only',
False):
On 2012/11/02 12:07:21, Marc-Antoine Ruel wrote:
> in fact, I'd prefer it to raise an exception:

Good call; done (and test updated to check that it does).

[stuartmorgan, 2012-11-02 13:13:18]

Would you mind landing this for me? Submitting to depot_tools for the first time
seems to be a yak-shaving expedition; every time I try the precommit checks run
for 10 minutes then bail and tell me I have to install some new thing (first
AppEngine, now Mercurial and rietveld, who knows what after that).

[commit-bot: I haz the power, 2012-11-02 13:34:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stuartmorgan@chromium.org/11363036/8001

[commit-bot: I haz the power, 2012-11-02 13:37:25]

Change committed as 165671

[M-A Ruel, 2012-11-02 13:37:54]

On 2012/11/02 13:37:25, I haz the power (commit-bot) wrote:
> Change committed as 165671

Was easy, eh?

[stuartmorgan, 2012-11-02 13:39:07]

Much obliged :)

[stuartmorgan, 2012-11-02 13:39:34]

(Sorry, didn't think CQ worked on sub-repos)