Improve ninja's Xcode emulation

Improve ninja's Xcode emulation

When cloning the target dictionaries for "iphoneos", correctly filter
the ARCHS value to only include value that apply to the SDK (iphoneos
or iphonesimulator).

Take into consideration the VALID_ARCHS filter if defined and support
the $(ARCHS_STANDARD) and $(ARCHS_STANDARD_INCLUDING_64_BIT) defaults
from Xcode 5.0.

If ARCHS is not set (which is the default for most of the projects)
then result will be identical, but it can now be set to the macro
$(ARCHS_STANDARD_INCLUDING_64_BIT) to build correctly for arm64 and
armv7 with both ninja and Xcode (including xcodebuild).

Update test/ios/gyptest-archs.py to test build for both simulator
and device, with different configuration (no ARCHS and filter that
only keep 32-bit platforms, explicit ARCHS with filters for 32-bit
or 64-bit platforms, and explicit ARCHS with no filter). This breaks
the test with the "make" generator as its support for Xcode emulation
is inexistant.

BUG=http://crbug.com/339477
R=justincohen@chromium.org, mark@chromium.org, thakist@chromium.org

Committed: https://code.google.com/p/gyp/source/detail?r=1850

[sdefresne, 2014-02-05 21:48:02]

PTAL

[Mark Mentovai, 2014-02-05 21:51:46]

+thakis too

https://codereview.chromium.org/138533006/diff/1/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/138533006/diff/1/pylib/gyp/xcode_emulation.py...
pylib/gyp/xcode_emulation.py:1423: return sdkroot == 'iphoneos'
Shouldn’t this test be a little more comprehensive? Isn’t “iphoneos7.0” also a
device SDK? Can’t this also be a path to an SDK?

[sdefresne, 2014-02-06 09:53:19]

Also fixed the tests on the bots since older version of Xcode does not support
$(ARCHS_STANDARD) no $(ARCHS_STANDARD_INCLUDING_64_BIT).

https://codereview.chromium.org/138533006/diff/1/pylib/gyp/xcode_emulation.py
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/138533006/diff/1/pylib/gyp/xcode_emulation.py...
pylib/gyp/xcode_emulation.py:1423: return sdkroot == 'iphoneos'
On 2014/02/05 21:51:46, Mark Mentovai wrote:
> Shouldn’t this test be a little more comprehensive? Isn’t “iphoneos7.0” also a
> device SDK? Can’t this also be a path to an SDK?

Done.

[Nico, 2014-02-07 18:57:56]

Thanks for the patch! Justin wrote this code, he should review.

As a general point: Try to do one thing per CL, it looks like you unrelatedly
did a bunch of style changes. Keep things like this in separate changes.

https://codereview.chromium.org/138533006/diff/190001/pylib/gyp/xcode_emulati...
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/138533006/diff/190001/pylib/gyp/xcode_emulati...
pylib/gyp/xcode_emulation.py:274: @classmethod
Why this change?

https://codereview.chromium.org/138533006/diff/190001/pylib/gyp/xcode_emulati...
pylib/gyp/xcode_emulation.py:1458: for arch in (xcode_settings.get('ARCHS', [])
or ['$(ARCHS_STANDARD)']):
No need for the default [] if you do `or default` anyhow. Pass the thing after
the or as default param instead?

https://codereview.chromium.org/138533006/diff/190001/pylib/gyp/xcode_emulati...
pylib/gyp/xcode_emulation.py:1476: iphoneos_config_dict =
copy.deepcopy(config_dict)
Why this variable name change?

[sdefresne, 2014-02-07 20:19:33]

Thank you for the review. I usually try to break my CLs in individual parts but
here, there are no changes that are purely cosmetic (except maybe renaming the
variable |new_config_dict| to |iphoneos_config_dict|). Were you referring to the
|@classmethod| annotation? I did add a comment to explain why I did the change
in that CL.

https://codereview.chromium.org/138533006/diff/190001/pylib/gyp/xcode_emulati...
File pylib/gyp/xcode_emulation.py (right):

https://codereview.chromium.org/138533006/diff/190001/pylib/gyp/xcode_emulati...
pylib/gyp/xcode_emulation.py:274: @classmethod
On 2014/02/07 18:57:56, Nico wrote:
> Why this change?

I want to call this method from the module function _IOSDefaultArchForSDKRoot.
As this method does not use self, it is easier to turn it into a classmethod and
to invoke it as a classmethod in _IOSDefaultArchForSDKRoot than creating a fake
object there just to invoke the method (the other option would have been to
change the method to be a module function, but this would have made my CL even
more intrusive).

I can add a TODO to turn the method into a function, or I can do it in a
separate CL and rebase this current CL one the other one.

Note: I had to turn the other method into classmethod for the same reason (they
are called by this method).

https://codereview.chromium.org/138533006/diff/190001/pylib/gyp/xcode_emulati...
pylib/gyp/xcode_emulation.py:1458: for arch in (xcode_settings.get('ARCHS', [])
or ['$(ARCHS_STANDARD)']):
On 2014/02/07 18:57:56, Nico wrote:
> No need for the default [] if you do `or default` anyhow. Pass the thing after
> the or as default param instead?

There are some xcode_settings objects that don't define the ARCHS key, and some
other that do define it to the empty list. I need to use the get method to
prevent an exception in the first case, and the or to avoid handling the second
case, ie. I want that this |xcode_settings.get('ARCHS', []) or
['$(ARCHS_STANDARD)']| to return |['$(ARCHS_STANDARD)']| if |xcode_settings| is
either |{}| or |{'ARCHS': []}|.

https://codereview.chromium.org/138533006/diff/190001/pylib/gyp/xcode_emulati...
pylib/gyp/xcode_emulation.py:1476: iphoneos_config_dict =
copy.deepcopy(config_dict)
On 2014/02/07 18:57:56, Nico wrote:
> Why this variable name change?

I found the old name confusing when updating the code. I can change it back.

[justincohen, 2014-02-10 21:41:32]

The ARCHS changes LGTM.  I don't know about the @classmethod changes, are you
fine with that, thakis@?

Thanks!

https://codereview.chromium.org/138533006/diff/190001/test/ios/gyptest-archs.py
File test/ios/gyptest-archs.py (right):

https://codereview.chromium.org/138533006/diff/190001/test/ios/gyptest-archs....
test/ios/gyptest-archs.py:99: product = name.replace(' ', '')
Why not just rename the products to not have spaces, to simplify this?

[sdefresne, 2014-02-11 16:30:45]

Thank you all for the reviews.

I've rebased my code, so my CL no longer contains any superfluous
stylistic changes. Can you have a last look at it?

https://codereview.chromium.org/138533006/diff/190001/test/ios/gyptest-archs.py
File test/ios/gyptest-archs.py (right):

https://codereview.chromium.org/138533006/diff/190001/test/ios/gyptest-archs....
test/ios/gyptest-archs.py:99: product = name.replace(' ', '')
On 2014/02/10 21:41:33, justincohen wrote:
> Why not just rename the products to not have spaces, to simplify this?

Done.

[Mark Mentovai, 2014-02-11 17:20:53]

LGTM

[justincohen, 2014-02-11 17:37:36]

LGTM.  I wonder when the bots will move to Xcode 5?

[sdefresne, 2014-02-12 08:15:17]

Committed patchset #4 manually as r1850 (presubmit successful).