mac, GN: Pass 10.6 as deployment target.

mac, GN: Pass 10.6 as deployment target.

Previously, GN was using the sdk version as the deployment target,
which meant gn-built binaries couldn't run on OS X versions
older than 10.10 after updating the SDK to 10.10.
With this fix, mac GN matches the behavior of Mac GYP.

BUG=463170
Committed: https://crrev.com/e194adc1f7bdeeb500dcd357283d512651b6bf1b
Cr-Commit-Position: refs/heads/master@{#342908}

[erikchen, 2015-08-11 20:29:59]

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

[erikchen, 2015-08-11 20:30:00]

thakis: Please review.

[Nico, 2015-08-11 20:37:48]

not lgtm

We have to fix these, not suppress them.

[Nico, 2015-08-11 20:38:14]

Unless you also up the deployment target, there shouldn't be more warnings
either, no?

[Nico, 2015-08-11 20:39:04]

See also the similar discussion in
https://codereview.chromium.org/1180583002/

On Tue, Aug 11, 2015 at 1:38 PM, <thakis@chromium.org> wrote:

> Unless you also up the deployment target, there shouldn't be more warnings
> either, no?
>
> https://codereview.chromium.org/1285493004/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Nico, 2015-08-11 20:41:40]

(complained on https://codereview.chromium.org/1250913002/#msg45)

https://codereview.chromium.org/1285493004/diff/1/base/BUILD.gn
File base/BUILD.gn (right):

https://codereview.chromium.org/1285493004/diff/1/base/BUILD.gn#newcode15
base/BUILD.gn:15: if (is_ios || is_mac) {
…why is this block even here? It was removed from
https://codereview.chromium.org/1180583002 …

[Nico, 2015-08-11 20:51:05]

Maybe we don't pass the right -mmacosx-version-min in the gn build?

[erikchen, 2015-08-11 20:51:40]

On 2015/08/11 20:38:14, Nico (hiding) wrote:
> Unless you also up the deployment target, there shouldn't be more warnings
> either, no?

Right now GN does not have the same behavior as GYP, which appears to ignore
these warnings (although I don't know why yet). Right now, GN bots are red on
the waterfall because they can't build base_unittests.

In the short term, we can:
1) Revert the 10.10 SDK change. (The change triggered the warnings, but isn't
the root cause of the problem)
2) Leave the bots red.
3) Stop compiling base_unittests and/or take the bots offline
4) Submit this CL.

The long term solution is to stop using deprecated functions. I've filed a bug:
https://code.google.com/p/chromium/issues/detail?id=519661

[Nico, 2015-08-11 20:53:19]

On 2015/08/11 20:51:40, erikchen wrote:
> On 2015/08/11 20:38:14, Nico (hiding) wrote:
> > Unless you also up the deployment target, there shouldn't be more warnings
> > either, no?
> 
> Right now GN does not have the same behavior as GYP, which appears to ignore
> these warnings (although I don't know why yet).

I'm guessing it doesn't get them in the first place. I'm guessing gn doesn't
pass the deployment flag correctly.

> Right now, GN bots are red on
> the waterfall because they can't build base_unittests.
> 
> In the short term, we can:
> 1) Revert the 10.10 SDK change. (The change triggered the warnings, but isn't
> the root cause of the problem)
> 2) Leave the bots red.
> 3) Stop compiling base_unittests and/or take the bots offline
> 4) Submit this CL.

Leave the bot red while investigating, for a while, and if fixing takes more
than end-of-today, revert.

[erikchen, 2015-08-11 21:03:07]

On 2015/08/11 20:53:19, Nico (hiding) wrote:
> On 2015/08/11 20:51:40, erikchen wrote:
> > On 2015/08/11 20:38:14, Nico (hiding) wrote:
> > > Unless you also up the deployment target, there shouldn't be more warnings
> > > either, no?
> > 
> > Right now GN does not have the same behavior as GYP, which appears to ignore
> > these warnings (although I don't know why yet).
> 
> I'm guessing it doesn't get them in the first place. I'm guessing gn doesn't
> pass the deployment flag correctly.
> 
> > Right now, GN bots are red on
> > the waterfall because they can't build base_unittests.
> > 
> > In the short term, we can:
> > 1) Revert the 10.10 SDK change. (The change triggered the warnings, but
isn't
> > the root cause of the problem)
> > 2) Leave the bots red.
> > 3) Stop compiling base_unittests and/or take the bots offline
> > 4) Submit this CL.
> 
> Leave the bot red while investigating, for a while, and if fixing takes more
> than end-of-today, revert.

Yup, it looks like GN is setting -mmacosx-version-min=10.10. Investigating.

[erikchen, 2015-08-11 21:12:48]

thakis: PTAL

[Nico, 2015-08-11 21:15:19]

lgtm, thanks!. I tweaked the CL description a bit.

[erikchen, 2015-08-11 21:15:47]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2015-08-11 21:16:40]

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

[Nico, 2015-08-11 21:16:40]

https://codereview.chromium.org/1285493004/diff/40001/build/config/mac/mac_sd...
File build/config/mac/mac_sdk.gni (right):

https://codereview.chromium.org/1285493004/diff/40001/build/config/mac/mac_sd...
build/config/mac/mac_sdk.gni:14: # Path to a specific version of the Mac SDKJ,
not including a backslash at
While here, maybe you can s/SDKJ/SDK/ if you haven't hit cq already.

[Nico, 2015-08-11 21:16:49]

On 2015/08/11 21:16:40, Nico (hiding) wrote:
>
https://codereview.chromium.org/1285493004/diff/40001/build/config/mac/mac_sd...
> File build/config/mac/mac_sdk.gni (right):
> 
>
https://codereview.chromium.org/1285493004/diff/40001/build/config/mac/mac_sd...
> build/config/mac/mac_sdk.gni:14: # Path to a specific version of the Mac SDKJ,
> not including a backslash at
> While here, maybe you can s/SDKJ/SDK/ if you haven't hit cq already.

…too late. Never mind :-)

[erikchen, 2015-08-11 21:18:26]

https://codereview.chromium.org/1285493004/diff/40001/build/config/mac/mac_sd...
File build/config/mac/mac_sdk.gni (right):

https://codereview.chromium.org/1285493004/diff/40001/build/config/mac/mac_sd...
build/config/mac/mac_sdk.gni:14: # Path to a specific version of the Mac SDKJ,
not including a backslash at
On 2015/08/11 21:16:40, Nico (hiding) wrote:
> While here, maybe you can s/SDKJ/SDK/ if you haven't hit cq already.

Done.

[erikchen, 2015-08-11 21:18:31]

The CQ bit was checked by erikchen@chromium.org

[erikchen, 2015-08-11 21:18:31]

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/1285493004/#ps60001
(title: "Fix comment typo.")

[commit-bot: I haz the power, 2015-08-11 21:19:50]

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

[commit-bot: I haz the power, 2015-08-11 21:53:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-08-11 21:53:41]

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