grit should differentiate host and target os

[GoogleCodeExporter]

grit should follow the same convention as gyp,
which is that iOS and Mac are different.

https://code.google.com/searchframe#OAMlx_jo-ck/src/tools/grit/grit/node/base.py
&exact_package=chromium&q=is_macosx&l=467

The specific case is for messages that are iOS only.
Current workaround is to test for pp_ifdef('ios').

Original discussion:
https://codereview.chromium.org/11966004/

Original issue reported on code.google.com by bul...@chromium.org on 18 Jan 2013 at 1:31

[GoogleCodeExporter]

Specifically, the problem is that sys.platform is reading the host, and iOS 
builds don't have the host == target property that the desktop builds do.

Original comment by stuartmorgan@chromium.org on 18 Jan 2013 at 2:01

[GoogleCodeExporter]

I have no objection to this behavior being changed, assuming it affects only 
builds targeting iOS.  I'm happy to look at patches that fix this bug.

For completeness of information in the bug, where should the target OS property 
come from?  A command-line option, an environment variable, or what?

Original comment by joi@chromium.org on 21 Jan 2013 at 10:37

[GoogleCodeExporter]

We already pass "-D ios" to grit, so using that seems like the easiest option.

Original comment by stuartmorgan@chromium.org on 21 Jan 2013 at 10:57

[GoogleCodeExporter]

I'm not sure I think it's a good idea to use "-D ios" as the basis of a generic 
solution.  It seems that we either:

a) Do something for iOS only, in which case there doesn't seem to be a need to 
make any change (you are already passing "-D ios" and checking that in your 
<if> conditions); or

b) Do something generic for all platforms, in which case we should have, in 
addition to the current is_linux, is_win etc. variables available in the <if> 
statement, something like target_is_linux, target_is_win, etc. and have some 
generic flag (not a -D flag, and not one flag per platform, but rather 
something like --target=xyz) that sets the target platform for GRIT.

WDYT?

Original comment by joi@chromium.org on 21 Jan 2013 at 1:35

[GoogleCodeExporter]

Sure, I'm fine with an optional --target flag that overrides the 
auto-detection. I'm not clear on why we'd need is_foo and target_is_foo though; 
in a grd file wouldn't you only ever care about the target? I'd think the logic 
would be to set the existing variables based on the flag if present, and the 
host if not.

Original comment by stuartmorgan@chromium.org on 21 Jan 2013 at 2:16

[GoogleCodeExporter]

Good point, agreed the flag should simply change the behavior of the existing 
is_xyz variables.

I need to fix another bug in GRIT today, and since this looks simple I may fix 
it while I'm in there.

Original comment by joi@chromium.org on 21 Jan 2013 at 2:36

• Changed state: Accepted

[GoogleCodeExporter]

There's a related bug for Android at 
https://code.google.com/p/chromium/issues/detail?id=239434 and I have a CL to 
fix it. I didn't see a need to split the host and target, as stuartmorgan 
suggests in #5, so I just added an is_android condition and have chromium 
passing -t android instead of -D android.

iOS could do the same thing. I don't particularly want to add this to my CL 
right now because I need this fixed fairly urgently to unbreak the WebView and 
I don't have the environment to test building iOS, but feel free to copy the 
approach once that lands :)

Original comment by torne@chromium.org on 14 May 2013 at 11:40

[GoogleCodeExporter]

OK, I've landed the android changes for this. I suggest that someone from the 
iOS side does exactly the same thing as the changes I made for 
https://code.google.com/p/chromium/issues/detail?id=239434 :)

Original comment by torne@chromium.org on 21 May 2013 at 3:43

[GoogleCodeExporter]

is_ios was landed at r123. We can now use it instead of pp_ifdef('ios').

Original comment by tfar...@chromium.org on 21 Jun 2013 at 2:18

[GoogleCodeExporter]

I believe this is fixed, right? Marking fixed, torne@/stuartmorgan@/bulach@ 
please reopen if I'm wrong.

Original comment by joi@chromium.org on 28 Jun 2013 at 10:33

• Changed state: Fixed

[GoogleCodeExporter]

Yep, I just need to change the existing uses now.

Original comment by stuartmorgan@chromium.org on 28 Jun 2013 at 10:38