Lots of misc bug fixes to make the GN build of ios work again.

Lots of misc bug fixes to make the GN build of ios work again.

or at least the 'gn_all' target.

R=sdefresnes@chromium.org, thakis@chromium.org
BUG=548286
Committed: https://crrev.com/a8d2fb9a68349fb265656f3b3771ce3ac61e0f37
Cr-Commit-Position: refs/heads/master@{#356670}

[Dirk Pranke, 2015-10-28 01:02:47]

Description was changed from

==========
Lots of misc bug fixes to make the GN build of ios work again.

or at least the 'gn_all' target.

R=sdefresnes@chromium.org, thakis@chromium.org
BUG=548286
==========

to

==========
Lots of misc bug fixes to make the GN build of ios work again.

or at least the 'gn_all' target.

R=sdefresnes@chromium.org, thakis@chromium.org
BUG=548286
==========

[Dirk Pranke, 2015-10-28 01:02:47]

dpranke@chromium.org changed reviewers:
+ rsleevi@chromium.org

[Dirk Pranke, 2015-10-28 01:03:38]

This should at least get 'gn_all' to compile; 'all' still fails w/ lots of
errors due to various components getting pulled in.

Nico, can you double-check the //build/config stuff? 

Ryan, can you OWNERS-stamp the //net change?

[Ryan Sleevi, 2015-10-28 01:07:35]

//net LGTM

[Nico, 2015-10-28 01:10:09]

build/ lgtm, but don't you need -Wno-deprecated-declarations too?

sdefresne told me that he got warnings such as

../../base/mac/authorization_util.mm:114:21: error:
'AuthorizationExecuteWithPrivileges' is deprecated: first deprecated in OS X
10.7 [-Werror,-Wdeprecated-declarations]
  OSStatus status = AuthorizationExecuteWithPrivileges(authorization,
                    ^
/Applications/Xcode7.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/System/Library/Frameworks/Security.framework/Headers/Authorization.h:432:10:
note: 'AuthorizationExecuteWithPrivileges' has been explicitly marked deprecated
here
OSStatus AuthorizationExecuteWithPrivileges(AuthorizationRef authorization,


if just bumping the deployment target. That's why I added that to
https://codereview.chromium.org/1426433002

[Nico, 2015-10-28 01:18:18]

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

[Nico, 2015-10-28 01:18:32]

(fixed sdefresne's email)

[Dirk Pranke, 2015-10-28 01:40:09]

On 2015/10/28 01:10:09, Nico wrote:
> build/ lgtm, but don't you need -Wno-deprecated-declarations too?
> 
> sdefresne told me that he got warnings such as
> 
> ../../base/mac/authorization_util.mm:114:21: error:
> 'AuthorizationExecuteWithPrivileges' is deprecated: first deprecated in OS X
> 10.7 [-Werror,-Wdeprecated-declarations]
>   OSStatus status = AuthorizationExecuteWithPrivileges(authorization,
>                     ^
>
/Applications/Xcode7.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/System/Library/Frameworks/Security.framework/Headers/Authorization.h:432:10:
> note: 'AuthorizationExecuteWithPrivileges' has been explicitly marked
deprecated
> here
> OSStatus AuthorizationExecuteWithPrivileges(AuthorizationRef authorization,
> 
> 
> if just bumping the deployment target. That's why I added that to
> https://codereview.chromium.org/1426433002

I think what's happening is that GN is actually smart enough to re-evaluate
mac_deployment_target in host context
and use the 10.6 setting there. At least, that's what I would expect to happen,
but it's also possible that 'gn_all' doesn't actually
build any $host_toolchain targets yet that would trigger the warning.

[Nico, 2015-10-28 01:43:38]

On 2015/10/28 01:40:09, Dirk Pranke wrote:
> On 2015/10/28 01:10:09, Nico wrote:
> > build/ lgtm, but don't you need -Wno-deprecated-declarations too?
> > 
> > sdefresne told me that he got warnings such as
> > 
> > ../../base/mac/authorization_util.mm:114:21: error:
> > 'AuthorizationExecuteWithPrivileges' is deprecated: first deprecated in OS X
> > 10.7 [-Werror,-Wdeprecated-declarations]
> >   OSStatus status = AuthorizationExecuteWithPrivileges(authorization,
> >                     ^
> >
>
/Applications/Xcode7.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/System/Library/Frameworks/Security.framework/Headers/Authorization.h:432:10:
> > note: 'AuthorizationExecuteWithPrivileges' has been explicitly marked
> deprecated
> > here
> > OSStatus AuthorizationExecuteWithPrivileges(AuthorizationRef authorization,
> > 
> > 
> > if just bumping the deployment target. That's why I added that to
> > https://codereview.chromium.org/1426433002
> 
> I think what's happening is that GN is actually smart enough to re-evaluate
> mac_deployment_target in host context
> and use the 10.6 setting there. At least, that's what I would expect to
happen,
> but it's also possible that 'gn_all' doesn't actually
> build any $host_toolchain targets yet that would trigger the warning.

Hm, the problem was that Xcode's clang warns on using the 10.6 SDK with libc++,
and the iOS bots use Xcode's clang for production builds. But I can't see any
mentions of "clang_xcode" in any .gn files, so I guess that's just not ported
yet. For host binaries, it would make sense to use our clang instead of Xcode's
clang anyways, so once we add clang_xcode to gn for ios builds we need to make
sure that it only changes the target toolchain.

[Dirk Pranke, 2015-10-28 01:49:52]

On 2015/10/28 01:43:38, Nico wrote:
> On 2015/10/28 01:40:09, Dirk Pranke wrote:
> > On 2015/10/28 01:10:09, Nico wrote:
> > > build/ lgtm, but don't you need -Wno-deprecated-declarations too?
> > > 
> > > sdefresne told me that he got warnings such as
> > > 
> > > ../../base/mac/authorization_util.mm:114:21: error:
> > > 'AuthorizationExecuteWithPrivileges' is deprecated: first deprecated in OS
X
> > > 10.7 [-Werror,-Wdeprecated-declarations]
> > >   OSStatus status = AuthorizationExecuteWithPrivileges(authorization,
> > >                     ^
> > >
> >
>
/Applications/Xcode7.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/System/Library/Frameworks/Security.framework/Headers/Authorization.h:432:10:
> > > note: 'AuthorizationExecuteWithPrivileges' has been explicitly marked
> > deprecated
> > > here
> > > OSStatus AuthorizationExecuteWithPrivileges(AuthorizationRef
authorization,
> > > 
> > > 
> > > if just bumping the deployment target. That's why I added that to
> > > https://codereview.chromium.org/1426433002
> > 
> > I think what's happening is that GN is actually smart enough to re-evaluate
> > mac_deployment_target in host context
> > and use the 10.6 setting there. At least, that's what I would expect to
> happen,
> > but it's also possible that 'gn_all' doesn't actually
> > build any $host_toolchain targets yet that would trigger the warning.
> 
> Hm, the problem was that Xcode's clang warns on using the 10.6 SDK with
libc++,
> and the iOS bots use Xcode's clang for production builds. But I can't see any
> mentions of "clang_xcode" in any .gn files, so I guess that's just not ported
> yet. For host binaries, it would make sense to use our clang instead of
Xcode's
> clang anyways, so once we add clang_xcode to gn for ios builds we need to make
> sure that it only changes the target toolchain.

Right, using the Xcode clang is still a TODO:

https://code.google.com/p/chromium/codesearch?q=pdfium.py#chromium/src/build/...

We already set the host toolchain and the target toolchain to two different
values, so I don't expect that to be an issue
(i.e., likely to break).

[sdefresne, 2015-10-28 09:10:58]

On 2015/10/28 at 01:49:52, dpranke wrote:
> On 2015/10/28 01:43:38, Nico wrote:
> > On 2015/10/28 01:40:09, Dirk Pranke wrote:
> > > On 2015/10/28 01:10:09, Nico wrote:
> > > > build/ lgtm, but don't you need -Wno-deprecated-declarations too?
> > > > 
> > > > sdefresne told me that he got warnings such as
> > > > 
> > > > ../../base/mac/authorization_util.mm:114:21: error:
> > > > 'AuthorizationExecuteWithPrivileges' is deprecated: first deprecated in
OS X
> > > > 10.7 [-Werror,-Wdeprecated-declarations]
> > > >   OSStatus status = AuthorizationExecuteWithPrivileges(authorization,
> > > >                     ^
> > > >
> > >
> >
/Applications/Xcode7.0.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.11.sdk/System/Library/Frameworks/Security.framework/Headers/Authorization.h:432:10:
> > > > note: 'AuthorizationExecuteWithPrivileges' has been explicitly marked
> > > deprecated
> > > > here
> > > > OSStatus AuthorizationExecuteWithPrivileges(AuthorizationRef
authorization,
> > > > 
> > > > 
> > > > if just bumping the deployment target. That's why I added that to
> > > > https://codereview.chromium.org/1426433002
> > > 
> > > I think what's happening is that GN is actually smart enough to
re-evaluate
> > > mac_deployment_target in host context
> > > and use the 10.6 setting there. At least, that's what I would expect to
> > happen,
> > > but it's also possible that 'gn_all' doesn't actually
> > > build any $host_toolchain targets yet that would trigger the warning.
> > 
> > Hm, the problem was that Xcode's clang warns on using the 10.6 SDK with
libc++,
> > and the iOS bots use Xcode's clang for production builds. But I can't see
any
> > mentions of "clang_xcode" in any .gn files, so I guess that's just not
ported
> > yet. For host binaries, it would make sense to use our clang instead of
Xcode's
> > clang anyways, so once we add clang_xcode to gn for ios builds we need to
make
> > sure that it only changes the target toolchain.
> 
> Right, using the Xcode clang is still a TODO:
> 
>
https://code.google.com/p/chromium/codesearch?q=pdfium.py#chromium/src/build/...
> 
> We already set the host toolchain and the target toolchain to two different
values, so I don't expect that to be an issue
> (i.e., likely to break).

AFAIK we still do not build any host target on iOS with gn. The deprecation
warning are not related to Xcode clang they are due to using 10.7 SDK. The
choice of the 10.7 SDK was due to the Xcode clang complaining that libc++ and
10.6 SDK are incompatible.

[sdefresne, 2015-10-28 09:13:40]

Thank you for the cleanup. lgtm.

https://codereview.chromium.org/1417743003/diff/1/build/config/mac/mac_sdk.gni
File build/config/mac/mac_sdk.gni (right):

https://codereview.chromium.org/1417743003/diff/1/build/config/mac/mac_sdk.gn...
build/config/mac/mac_sdk.gni:12: if (is_ios) {
This will only be required when gn supports using Xcode's version of clang.
Since this is only required when building "target" targets on iOS, maybe we can
convince gn to use ToT clang for "host" targets and Xcode clang for "target"
targets and avoid having the override to mac_deployment_target. WDYT?

[Dirk Pranke, 2015-10-28 17:10:24]

https://codereview.chromium.org/1417743003/diff/1/build/config/mac/mac_sdk.gni
File build/config/mac/mac_sdk.gni (right):

https://codereview.chromium.org/1417743003/diff/1/build/config/mac/mac_sdk.gn...
build/config/mac/mac_sdk.gni:12: if (is_ios) {
On 2015/10/28 09:13:40, sdefresne wrote:
> This will only be required when gn supports using Xcode's version of clang.
> Since this is only required when building "target" targets on iOS, maybe we
can
> convince gn to use ToT clang for "host" targets and Xcode clang for "target"
> targets and avoid having the override to mac_deployment_target. WDYT?

That sounds fine.

[Nico, 2015-10-28 17:12:46]

(still lgtm either way)

https://codereview.chromium.org/1417743003/diff/1/build/config/mac/mac_sdk.gni
File build/config/mac/mac_sdk.gni (right):

https://codereview.chromium.org/1417743003/diff/1/build/config/mac/mac_sdk.gn...
build/config/mac/mac_sdk.gni:12: if (is_ios) {
On 2015/10/28 17:10:24, Dirk Pranke wrote:
> On 2015/10/28 09:13:40, sdefresne wrote:
> > This will only be required when gn supports using Xcode's version of clang.
> > Since this is only required when building "target" targets on iOS, maybe we
> can
> > convince gn to use ToT clang for "host" targets and Xcode clang for "target"
> > targets and avoid having the override to mac_deployment_target. WDYT?
> 
> That sounds fine.

Cool, if the changes in this file aren't needed in the gn build, that's even
nicer.

[Dirk Pranke, 2015-10-28 21:49:58]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2015-10-28 21:49:58]

The patchset sent to the CQ was uploaded after l-g-t-m from thakis@chromium.org,
sdefresne@chromium.org, rsleevi@chromium.org
Link to the patchset: https://codereview.chromium.org/1417743003/#ps20001
(title: "revert mac_deployment_target change")

[commit-bot: I haz the power, 2015-10-28 21:51:44]

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

[commit-bot: I haz the power, 2015-10-28 22:25:32]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-28 22:26:10]

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