[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

Reduce running time of "gn gen" from 2.5 to 2.2s on MacBook Pro
laptop.

BUG=609541
Committed: https://crrev.com/268142df3c289fcd2605cd68f41e04ebade1067b
Cr-Commit-Position: refs/heads/master@{#394505}

[sdefresne, 2016-05-18 09:41:26]

Description was changed from

==========
[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

BUG=None
==========

to

==========
[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

BUG=609541
==========

[sdefresne, 2016-05-18 09:41:26]

sdefresne@chromium.org changed reviewers:
+ thakis@chromium.org

[sdefresne, 2016-05-18 09:42:39]

Please take a look.

https://codereview.chromium.org/1993653002/diff/1/build/config/mac/sdk_info.py
File build/config/mac/sdk_info.py (right):

https://codereview.chromium.org/1993653002/diff/1/build/config/mac/sdk_info.p...
build/config/mac/sdk_info.py:25: lines = subprocess.check_output(['xcodebuild',
'-version']).splitlines()
I have not found an "xcrun" substitute for those so I've kept this invocation of
"xcodebuild". It appears to be much faster than the other, so let's see if this
does better.

[Nico, 2016-05-18 11:21:06]

Thanks! I'd mention "reduces time for running `gn gen` on mac from 2.5s to 2.2s"
in the cl description, since that's the interesting effect of this change.

https://codereview.chromium.org/1993653002/diff/1/build/config/mac/sdk_info.py
File build/config/mac/sdk_info.py (right):

https://codereview.chromium.org/1993653002/diff/1/build/config/mac/sdk_info.p...
build/config/mac/sdk_info.py:43: 'xcrun', '-sdk', platform,
'--show-sdk-build-version']).strip()
Hm, my xcrun doesn't know --show-sdk-build-version yet. I'm on Xcode 7.0. Some
of the bots are on 5.0 still I think.

Since this computes xcode_version above, maybe this could check if the
xcode_version is new enough and if so call xcrun and else call xcodebuild to get
it?

Why is this field needed at all? gyp never queries for ProductBuildVersion as
far as I can tell.

It does query --show-sdk-build-version, but only:

      if xcode >= '0720':
        cache['DTSDKBuild'] = self._GetSdkVersionInfoItem(
            sdk_root, '--show-sdk-build-version')
      elif xcode >= '0430':
        cache['DTSDKBuild'] = sdk_version
      else:
        cache['DTSDKBuild'] = cache['BuildMachineOSBuild']

(where sdk_version is the result from --show-sdk-version)

Can we do the same here (we can assume 4.3), with a TODO to unconditionally use
--show-sdk-build-version once all bots are on Xcode 7.2?

[sdefresne, 2016-05-18 14:45:02]

https://codereview.chromium.org/1993653002/diff/1/build/config/mac/sdk_info.py
File build/config/mac/sdk_info.py (right):

https://codereview.chromium.org/1993653002/diff/1/build/config/mac/sdk_info.p...
build/config/mac/sdk_info.py:43: 'xcrun', '-sdk', platform,
'--show-sdk-build-version']).strip()
On 2016/05/18 11:21:06, Nico (vacation May 19 to 22) wrote:
> Hm, my xcrun doesn't know --show-sdk-build-version yet. I'm on Xcode 7.0. Some
> of the bots are on 5.0 still I think.
> 
> Since this computes xcode_version above, maybe this could check if the
> xcode_version is new enough and if so call xcrun and else call xcodebuild to
get
> it?
> 
> Why is this field needed at all? gyp never queries for ProductBuildVersion as
> far as I can tell.
> 
> It does query --show-sdk-build-version, but only:
> 
>       if xcode >= '0720':
>         cache['DTSDKBuild'] = self._GetSdkVersionInfoItem(
>             sdk_root, '--show-sdk-build-version')
>       elif xcode >= '0430':
>         cache['DTSDKBuild'] = sdk_version
>       else:
>         cache['DTSDKBuild'] = cache['BuildMachineOSBuild']
> 
> (where sdk_version is the result from --show-sdk-version)
> 
> Can we do the same here (we can assume 4.3), with a TODO to unconditionally
use
> --show-sdk-build-version once all bots are on Xcode 7.2?

I don't know why gyp does not use it, but when I build downstream, the value
that ends up in my Info.plist is the value returned by "xcrun -sdk iphoneos
--show-sdk-build-version". I also get the same value when I build with
xcodebuild so it is possible that it comes from one environment variable set by
Xcode when it invokes ninja in hybrid build mode.

I can remove it but then I have a diff when I compare the output of
gyp/xcodebuild and gn builds.

[Nico, 2016-05-18 14:47:50]

I think we're talking past each other. I'm not saying remove it, I'm saying:

1. That xcrun flag doesn't work in xcrun in Xcodes older than Xcode 7.2
2. gyp checks if Xcode is >= 7.2 and if so calls xcrun --show-sdk-build-version,
else it puts xcrun --show-sdk-version into DTSDKBuild
3. Can we do the same here?

[sdefresne, 2016-05-18 18:39:41]

Description was changed from

==========
[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

BUG=609541
==========

to

==========
[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

Reduce running time of "gn gen" from 2.5 to 2.2s on MacBook Pro
laptop.

BUG=609541
==========

[sdefresne, 2016-05-18 18:40:24]

On 2016/05/18 14:47:50, Nico (vacation May 19 to 22) wrote:
> I think we're talking past each other. I'm not saying remove it, I'm saying:
> 
> 1. That xcrun flag doesn't work in xcrun in Xcodes older than Xcode 7.2
> 2. gyp checks if Xcode is >= 7.2 and if so calls xcrun
--show-sdk-build-version,
> else it puts xcrun --show-sdk-version into DTSDKBuild
> 3. Can we do the same here?

Oh sorry, yes I think I totally misread your original comments.

I've updated the code to use the same logic as gyp, PTAL.

[Nico, 2016-05-18 18:41:42]

lgtm!

https://codereview.chromium.org/1993653002/diff/40001/build/config/mac/sdk_in...
File build/config/mac/sdk_info.py (right):

https://codereview.chromium.org/1993653002/diff/40001/build/config/mac/sdk_in...
build/config/mac/sdk_info.py:42: if xcode_version >= '0720':
Maybe add an "TODO: unconditionally do this once all bots are on 7.2"

[sdefresne, 2016-05-18 18:50:38]

On 2016/05/18 18:41:42, Nico (vacation May 19 to 22) wrote:
> lgtm!
> 
>
https://codereview.chromium.org/1993653002/diff/40001/build/config/mac/sdk_in...
> File build/config/mac/sdk_info.py (right):
> 
>
https://codereview.chromium.org/1993653002/diff/40001/build/config/mac/sdk_in...
> build/config/mac/sdk_info.py:42: if xcode_version >= '0720':
> Maybe add an "TODO: unconditionally do this once all bots are on 7.2"

I don't think we requires using Xcode 7.2+ to develop on Mac yet, so added a
slightly different TODO.

[sdefresne, 2016-05-18 18:50:51]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-05-18 18:50:51]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1993653002/#ps60001
(title: "Add a TODO to remove check for Xcode version later")

[commit-bot: I haz the power, 2016-05-18 18:51:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1993653002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1993653002/60001

[commit-bot: I haz the power, 2016-05-18 19:33:49]

Description was changed from

==========
[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

Reduce running time of "gn gen" from 2.5 to 2.2s on MacBook Pro
laptop.

BUG=609541
==========

to

==========
[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

Reduce running time of "gn gen" from 2.5 to 2.2s on MacBook Pro
laptop.

BUG=609541
==========

[commit-bot: I haz the power, 2016-05-18 19:33:50]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-18 19:36:14]

Description was changed from

==========
[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

Reduce running time of "gn gen" from 2.5 to 2.2s on MacBook Pro
laptop.

BUG=609541
==========

to

==========
[GN] Use "xcrun" instead of "xcodebuild" to get information on SDK.

Use "xcrun" instead of "xcodebuild" as it is much faster and available
on all version of OS X supported for building Chrome.

Reduce running time of "gn gen" from 2.5 to 2.2s on MacBook Pro
laptop.

BUG=609541

Committed: https://crrev.com/268142df3c289fcd2605cd68f41e04ebade1067b
Cr-Commit-Position: refs/heads/master@{#394505}
==========

[commit-bot: I haz the power, 2016-05-18 19:36:15]

Patchset 4 (id:??) landed as
https://crrev.com/268142df3c289fcd2605cd68f41e04ebade1067b
Cr-Commit-Position: refs/heads/master@{#394505}