Use the lowest MacOS SDK available on a system

Use the lowest MacOS SDK available on a system

[kustermann, 2013-03-12 16:51:40]

In order to build on my MacBook I still need to select the correct compiler in
~/.gyp/include.gypi:

  1 {
  2   'variables': {
  3     'xcode_gcc_version': 'com.apple.compilers.llvmgcc42',
  4   }
  5 }

Otherwise I can't build.

https://codereview.chromium.org/12672003/diff/1/compiler/dartium.gyp
File compiler/dartium.gyp (left):

https://codereview.chromium.org/12672003/diff/1/compiler/dartium.gyp#oldcode10
compiler/dartium.gyp:10: '../tools/gyp/xcode.gypi',
In case we're building dartium, we want to use whatever chrome uses for
building, right? So we should not include that file!

(otherwise my solution doesn't work because we don't have DART_ROOT in
find_mac_sdk.py)

https://codereview.chromium.org/12672003/diff/1/tools/gyp/find_sdk.py
File tools/gyp/find_sdk.py (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/find_sdk.py#newcode2
tools/gyp/find_sdk.py:2: # Copyright (c) 2012 The Chromium Authors. All rights
reserved.
We can change that if necessary.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/xcode.gypi
File tools/gyp/xcode.gypi (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/xcode.gypi#newcode21
tools/gyp/xcode.gypi:21: 'mac_sdk%': '<!(python
<(DART_ROOT)/tools/gyp/find_sdk.py 10.6)',
I'm not too familiar with GYP. But it seems like we need 'DART_ROOT' here. If I
use '<(DEPTH)' it doesn't work.

[Ivan Posva, 2013-03-13 09:44:40]

We can fix the setting of the compiler default in a separate CL.

-Ivan

https://codereview.chromium.org/12672003/diff/1/tools/generate_projects.py
File tools/generate_projects.py (right):

https://codereview.chromium.org/12672003/diff/1/tools/generate_projects.py#ne...
tools/generate_projects.py:61: args += ['-D', 'DART_ROOT=' + dart_root]
I would prefer if we did not have to pass this value.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/all.gypi
File tools/gyp/all.gypi (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/all.gypi#newcode35
tools/gyp/all.gypi:35: 'msvs.gypi',
In that case msvs.gypi should also move under the OS=="win" condition.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/common.gypi
File tools/gyp/common.gypi (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/common.gypi#newcode38
tools/gyp/common.gypi:38: 'msvs.gypi',
ditto.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/find_sdk.py
File tools/gyp/find_sdk.py (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/find_sdk.py#newcode81
tools/gyp/find_sdk.py:81: raise Exception("This script only runs on Mac")
Instead of raising an exception we could just exit if not on Mac OS X and then
we do not have to have conditional inclusion of the different gypis. I am fine
either way though.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/xcode.gypi
File tools/gyp/xcode.gypi (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/xcode.gypi#newcode21
tools/gyp/xcode.gypi:21: 'mac_sdk%': '<!(python
<(DART_ROOT)/tools/gyp/find_sdk.py 10.6)',
On 2013/03/12 16:51:40, kustermann wrote:
> I'm not too familiar with GYP. But it seems like we need 'DART_ROOT' here. If
I
> use '<(DEPTH)' it doesn't work.

That DEPTH doesn't work is strange. What errors were you getting?

https://codereview.chromium.org/12672003/diff/1/tools/gyp/xcode.gypi#newcode47
tools/gyp/xcode.gypi:47: 'SYMROOT': '<(DEPTH)/xcodebuild',
We use DEPTH successfully here. What is different?

https://codereview.chromium.org/12672003/diff/1/tools/gyp_dart.py
File tools/gyp_dart.py (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp_dart.py#newcode24
tools/gyp_dart.py:24: 'DART_ROOT=' + dart_root, '-Idart/tools/gyp/all.gypi',
ditto.

[ricow1, 2013-03-13 10:10:42]

https://codereview.chromium.org/12672003/diff/1/tools/gyp/find_sdk.py
File tools/gyp/find_sdk.py (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/find_sdk.py#newcode2
tools/gyp/find_sdk.py:2: # Copyright (c) 2012 The Chromium Authors. All rights
reserved.
On 2013/03/12 16:51:40, kustermann wrote:
> We can change that if necessary.

Check runtime/bin/net/nss.gyp for the structure on the copyright notice

[kustermann, 2013-03-13 15:30:29]

https://codereview.chromium.org/12672003/diff/1/tools/generate_projects.py
File tools/generate_projects.py (right):

https://codereview.chromium.org/12672003/diff/1/tools/generate_projects.py#ne...
tools/generate_projects.py:61: args += ['-D', 'DART_ROOT=' + dart_root]
On 2013/03/13 09:44:40, Ivan Posva wrote:
> I would prefer if we did not have to pass this value.

As discussed offline, we don't need this anymore after we removed the two
unnecessary runhooks.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/all.gypi
File tools/gyp/all.gypi (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/all.gypi#newcode35
tools/gyp/all.gypi:35: 'msvs.gypi',
On 2013/03/13 09:44:40, Ivan Posva wrote:
> In that case msvs.gypi should also move under the OS=="win" condition.

I know, but I like to keep my CLs as small and orthogonal as possible. I changed
it now.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/common.gypi
File tools/gyp/common.gypi (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/common.gypi#newcode38
tools/gyp/common.gypi:38: 'msvs.gypi',
On 2013/03/13 09:44:40, Ivan Posva wrote:
> ditto.

Done.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/find_sdk.py
File tools/gyp/find_sdk.py (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/find_sdk.py#newcode2
tools/gyp/find_sdk.py:2: # Copyright (c) 2012 The Chromium Authors. All rights
reserved.
On 2013/03/13 10:10:42, ricow1 wrote:
> On 2013/03/12 16:51:40, kustermann wrote:
> > We can change that if necessary.
> 
> Check runtime/bin/net/nss.gyp for the structure on the copyright notice

Done.

https://codereview.chromium.org/12672003/diff/1/tools/gyp/xcode.gypi
File tools/gyp/xcode.gypi (right):

https://codereview.chromium.org/12672003/diff/1/tools/gyp/xcode.gypi#newcode21
tools/gyp/xcode.gypi:21: 'mac_sdk%': '<!(python
<(DART_ROOT)/tools/gyp/find_sdk.py 10.6)',
On 2013/03/13 09:44:40, Ivan Posva wrote:
> On 2013/03/12 16:51:40, kustermann wrote:
> > I'm not too familiar with GYP. But it seems like we need 'DART_ROOT' here.
If
> I
> > use '<(DEPTH)' it doesn't work.
> 
> That DEPTH doesn't work is strange. What errors were you getting?

After removing the two runhooks we can use DEPTH now.

[ricow1, 2013-03-15 08:36:37]

LGTM (cc'ing Peter)

[Ivan Posva, 2013-03-18 04:33:57]

Martin,

Can we please also have a similar solution for automatically setting the
xcode_gcc_version value?

Thanks!
-Ivan

P.S. It looks like this change was committed, but this review seems to be
lacking the reference to the revision.

[kustermann, 2013-03-18 10:33:27]

> Can we please also have a similar solution for automatically setting the
> xcode_gcc_version value?

Sure. I'll do another CL for that.

On 2013/03/18 04:33:57, Ivan Posva wrote:
> Martin,
> 
> Can we please also have a similar solution for automatically setting the
> xcode_gcc_version value?
> 
> Thanks!
> -Ivan
> 
> P.S. It looks like this change was committed, but this review seems to be
> lacking the reference to the revision.

[ahe, 2013-03-20 10:50:03]

LGTM, except for the find_mac_sdk.py file.  Couldn't we import it through
third_party?

https://codereview.chromium.org/12672003/diff/16001/tools/gyp/find_mac_sdk.py
File tools/gyp/find_mac_sdk.py (right):

https://codereview.chromium.org/12672003/diff/16001/tools/gyp/find_mac_sdk.py...
tools/gyp/find_mac_sdk.py:2: # Copyright (c) 2012 The Chromium Authors. All
rights reserved.
Something fishy with the copyrights in this file.

[kustermann, 2013-03-21 08:43:00]

I'm not sure if we want to make another third_party dependency  just because of
one file. IMHO this file is self contained and doesn't need to be in sync with
the one in chromium.

https://codereview.chromium.org/12672003/diff/16001/tools/gyp/find_mac_sdk.py
File tools/gyp/find_mac_sdk.py (right):

https://codereview.chromium.org/12672003/diff/16001/tools/gyp/find_mac_sdk.py...
tools/gyp/find_mac_sdk.py:2: # Copyright (c) 2012 The Chromium Authors. All
rights reserved.
On 2013/03/20 10:50:03, ahe wrote:
> Something fishy with the copyrights in this file.

Well, as you can see on line 10 this file was copied from the chromium
repository. Ricow told me I should look into 'runtime/bin/net/nss.gyp' (because
that was copied as well) -- so I just followed that style of having two
copyright notices.