New issue
Advanced search Search tips

Issue 608380 link

Starred by 3 users
Status: Fixed
Owner:
thakis@chromium.org
Closed: Jun 2016
Cc:
justincohen@chromium.org
sdefresne@chromium.org
ios-bugs@chromium.org
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

Remove generator *= [ninja|xcode] and re_run_targets from gyp

Project Member Reported by justincohen@chromium.org, May 2 2016

Comment 1 by justincohen@chromium.org, May 2 2016

Labels: M-52

Comment 2 by thakis@chromium.org, May 2 2016 

Does this bug have to be private?

Comment 3 by justincohen@chromium.org, May 2 2016

Labels: -Restrict-View-Google

Comment 4 by justincohen@chromium.org, May 9 2016

Labels: -M-52 M-53

Comment 5 by thakis@chromium.org, Jun 13 2016

Cc: -thakis@chromium.org justincohen@chromium.org
Owner: thakis@chromium.org
Status: Started (was: Assigned)
https://codereview.chromium.org/2066533002/

Comment 6 by thakis@chromium.org, Jun 13 2016 

Also, I spent some time explaining why target_arch doesn't work for iOS to someone today -- now that we're on ninja everywhere, it _could_ work for iOS, right?

Comment 8 by thakis@chromium.org, Jun 13 2016

Status: Fixed (was: Started)

Comment 9 by justincohen@chromium.org, Jun 13 2016

Cc: sdefresne@chromium.org
I think that's correct, but sdefresne@ can speak to it more authoritatively. Are you interested in making these changes on gyp even with the GN changes coming?

Comment 10 by sdefresne@chromium.org, Jun 13 2016 

Regarding comment #6: even with ninja it would be pretty hard to support target_arch on iOS (as ninja gyp generator still abuses "Configurations"). I'd say that we probably won't fix it before migration to gn is complete (and that already supports target_cpu correctly on iOS).

Comment 11 by thakis@chromium.org, Jun 13 2016 

What do you mean by "abuses 'Configurations'"?

I agree that it doesn't make sense to invest too much time given that gn is coming, but every day with surprising stuff in the tree costs people who try to understand that stuff time. (For example, someone updated libpng today and had to spend some time reading about iOS peculiarities.) So if it would take 1h to make iOS look like the rest of the world and this saves 3 people 1h not having to learn about it, it'd be kind of worth it. Sounds like it might not be easy to change though.

Comment 12 by sdefresne@chromium.org, Jun 13 2016 

It does this after parsing, variable substitutions and condition evaluations have been performed:

def _AddIOSDeviceConfigurations(targets):
  """Clone all targets and append -iphoneos to the name. Configure these targets
  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)
      configs[config_name + '-iphoneos'] = iphoneos_config_dict
      configs[config_name + '-iphonesimulator'] = config_dict
      if toolset == 'target':
        iphoneos_config_dict['xcode_settings']['SDKROOT'] = 'iphoneos'
  return targets

This is to be able to generate a single Xcode project that build for device and simulator, but it also means that we cannot select the file list based on the target_cpu (in particular skia wants to use this to select either assembly language source file with optimisation for either arm, arm64, x86 or x64).

Comment 13 by sdefresne@chromium.org, Jun 13 2016 

I'd say that fixing this would takes more than a few hours (it would probably takes weeks if we want to keep the developer workflow unchanged).

Comment 14 by thakis@chromium.org, Jun 13 2016 

Thanks for explaining! Doesn't look like it's worth it then :-)
►

Sign in to add a comment