Improve ninja's Xcode emulation

pylib/gyp/xcode_emulation.py

Index: pylib/gyp/xcode_emulation.py
diff --git a/pylib/gyp/xcode_emulation.py b/pylib/gyp/xcode_emulation.py
index 30f27d5832d99bd02ede773af5e59ed7c513b127..4ee00217eec06abfb6a58c74f32fd34344b243a8 100644
--- a/pylib/gyp/xcode_emulation.py
+++ b/pylib/gyp/xcode_emulation.py
@@ -271,7 +271,8 @@ class XcodeSettings(object):
     # CURRENT_ARCH / NATIVE_ARCH env vars?
     return self.xcode_settings[configname].get('ARCHS', [self._DefaultArch()])
 
-  def _GetStdout(self, cmdlist):
+  @classmethod

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

Why this change?

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

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).

+  def _GetStdout(cls, cmdlist):
     job = subprocess.Popen(cmdlist, stdout=subprocess.PIPE)
     out = job.communicate()[0]
     if job.returncode != 0:
@@ -877,7 +878,8 @@ class XcodeSettings(object):
     return self._GetStdout(['sw_vers', '-buildVersion'])
 
   # This method ported from the logic in Homebrew's CLT version check
-  def _CLTVersion(self):
+  @classmethod
+  def _CLTVersion(cls):
     # pkgutil output looks like
     #   package-id: com.apple.pkg.CLTools_Executables
     #   version: 5.0.1.0.1.1382131676
@@ -892,12 +894,13 @@ class XcodeSettings(object):
     regex = re.compile('version: (?P<version>.+)')
     for key in [MAVERICKS_PKG_ID, STANDALONE_PKG_ID, FROM_XCODE_PKG_ID]:
       try:
-        output = self._GetStdout(['/usr/sbin/pkgutil', '--pkg-info', key])
+        output = cls._GetStdout(['/usr/sbin/pkgutil', '--pkg-info', key])
         return re.search(regex, output).groupdict()['version']
       except:
         continue
 
-  def _XcodeVersion(self):
+  @classmethod
+  def _XcodeVersion(cls):
     # `xcodebuild -version` output looks like
     #    Xcode 4.6.3
     #    Build version 4H1503
@@ -906,9 +909,9 @@ class XcodeSettings(object):
     #    Component versions: DevToolsCore-1809.0; DevToolsSupport-1806.0
     #    BuildVersion: 10M2518
     # Convert that to '0463', '4H1503'.
-    if len(XcodeSettings._xcode_version_cache) == 0:
+    if not cls._xcode_version_cache:
       try:
-        version_list = self._GetStdout(['xcodebuild', '-version']).splitlines()
+        version_list = cls._GetStdout(['xcodebuild', '-version']).splitlines()
         # In some circumstances xcodebuild exits 0 but doesn't return
         # the right results; for example, a user on 10.7 or 10.8 with
         # a bogus path set via xcode-select
@@ -917,7 +920,7 @@ class XcodeSettings(object):
         if len(version_list) < 2:
           raise GypError, "xcodebuild returned unexpected results"
       except:
-        version = self._CLTVersion()
+        version = cls._CLTVersion()
         if version:
           version = re.match('(\d\.\d\.?\d*)', version).groups()[0]
         else:
@@ -931,8 +934,8 @@ class XcodeSettings(object):
       version = (version + '0' * (3 - len(version))).zfill(4)
       if build:
         build = build.split()[-1]
-      XcodeSettings._xcode_version_cache = (version, build)
-    return XcodeSettings._xcode_version_cache
+      cls._xcode_version_cache = (version, build)
+    return cls._xcode_version_cache
 
   def _XcodeIOSDeviceFamily(self, configname):
     family = self.xcode_settings[configname].get('TARGETED_DEVICE_FAMILY', '1')
@@ -982,7 +985,8 @@ class XcodeSettings(object):
     project, then the environment variable was empty. Starting with this
     version, Xcode uses the name of the newest SDK installed.
     """
-    if self._XcodeVersion() < '0500':
+    xcode, xcode_build = self._XcodeVersion()
+    if xcode < '0500':
       return ''
     default_sdk_path = self._XcodeSdkPath('')
     default_sdk_root = XcodeSettings._sdk_root_cache.get(default_sdk_path)
@@ -1418,18 +1422,63 @@ def _HasIOSTarget(targets):
   return False
 
 
+def _IOSIsDeviceSDKROOT(sdkroot):
+  """Tests if |sdkroot| is a SDK for building for device."""
+  return 'iphoneos' in sdkroot.lower()
+
+
+def _IOSDefaultArchForSDKRoot(sdkroot):
+  xcode, xcode_build = XcodeSettings._XcodeVersion()
+  if xcode < '0500':
+    if _IOSIsDeviceSDKROOT(sdkroot):
+      return {'$(ARCHS_STANDARD)': ['armv7']}
+    else:
+      return {'$(ARCHS_STANDARD)': ['i386']}
+  else:
+    if _IOSIsDeviceSDKROOT(sdkroot):
+      return {
+          '$(ARCHS_STANDARD)': ['armv7', 'armv7s'],
+          '$(ARCHS_STANDARD_INCLUDING_64_BIT)': ['armv7', 'armv7s', 'arm64'],
+      }
+    else:
+      return {
+          '$(ARCHS_STANDARD)': ['i386'],
+          '$(ARCHS_STANDARD_INCLUDING_64_BIT)': ['i386', 'x86_64'],
+      }
+
+
+def _FilterIOSArchitectureForSDKROOT(xcode_settings):
+  """Filter the ARCHS value from the |xcode_settings| dictionary to only
+  contains architectures valid for the sdk configured in SDKROOT value."""
+  defaults_archs = _IOSDefaultArchForSDKRoot(xcode_settings.get('SDKROOT', ''))
+  allowed_archs = set()
+  for archs in defaults_archs.itervalues():
+    allowed_archs.update(archs)
+  selected_archs = set()
+  for arch in (xcode_settings.get('ARCHS', []) or ['$(ARCHS_STANDARD)']):

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

No need for the default [] if you do `or default` anyhow. Pass the thing after
the or as default param instead?

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

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': []}|.

+    if arch in defaults_archs:
+      selected_archs.update(defaults_archs[arch])
+    elif arch in allowed_archs:
+      selected_archs.add(arch)
+  valid_archs = set(xcode_settings.get('VALID_ARCHS', []))
+  if valid_archs:
+    selected_archs = selected_archs & valid_archs
+  xcode_settings['ARCHS'] = list(selected_archs)
+
+
 def _AddIOSDeviceConfigurations(targets):
   """Clone all targets and append -iphoneos to the name. Configure these targets
-  to build for iOS devices."""
-  for target_dict in targets.values():
-    for config_name in target_dict['configurations'].keys():
-      config = target_dict['configurations'][config_name]
-      new_config_name = config_name + '-iphoneos'
-      new_config_dict = copy.deepcopy(config)
-      if target_dict['toolset'] == 'target':
-        new_config_dict['xcode_settings']['ARCHS'] = ['armv7']
-        new_config_dict['xcode_settings']['SDKROOT'] = 'iphoneos'
-      target_dict['configurations'][new_config_name] = new_config_dict
+  to build for iOS devices and use correct architectures for those builds."""
+  for target_dict in targets.itervalues():
+    toolset = target_dict['toolset']
+    configs = target_dict['configurations']
+    for config_name, config_dict in dict(configs).iteritems():
+      iphoneos_config_dict = copy.deepcopy(config_dict)

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

Why this variable name change?

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

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

+      configs[config_name + '-iphoneos'] = iphoneos_config_dict
+      if toolset == 'target':
+        iphoneos_config_dict['xcode_settings']['SDKROOT'] = 'iphoneos'
+      _FilterIOSArchitectureForSDKROOT(iphoneos_config_dict['xcode_settings'])
+      _FilterIOSArchitectureForSDKROOT(config_dict['xcode_settings'])
   return targets
 
 def CloneConfigurationForDeviceAndEmulator(target_dicts):