[Mac/GN] Let //chrome/browser:browser be a static library.

[Mac/GN] Let //chrome/browser:browser be a static library.

This requires //chrome:chrome_framework to link with -ObjC to force categories
to load from static archives.

BUG=618797
R=brettw@chromium.org

Committed: https://crrev.com/d4ee99258720a95788c8465aef4562c479872f03
Cr-Commit-Position: refs/heads/master@{#402836}

[brettw, 2016-06-21 22:51:46]

LGTM, but is this a more general thing that should apply more widely to all
targets? How do we know when to use this or not?

[Robert Sesek, 2016-06-21 23:19:49]

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

[Robert Sesek, 2016-06-21 23:19:50]

On 2016/06/21 22:51:46, brettw wrote:
> LGTM, but is this a more general thing that should apply more widely to all
> targets? How do we know when to use this or not?

Hm yes it looks like we'll need to apply this to other targets (c.f. trybot
failures). The -ObjC flag needs to be applied to any targets linking a static
archive that contain Obj-C categories.

I guess we could add -ObjC globally to executable targets. thakis: WDYT?

[Nico, 2016-06-22 14:08:35]

On 2016/06/21 23:19:50, Robert Sesek wrote:
> On 2016/06/21 22:51:46, brettw wrote:
> > LGTM, but is this a more general thing that should apply more widely to all
> > targets? How do we know when to use this or not?
> 
> Hm yes it looks like we'll need to apply this to other targets (c.f. trybot
> failures). The -ObjC flag needs to be applied to any targets linking a static
> archive that contain Obj-C categories.
> 
> I guess we could add -ObjC globally to executable targets. thakis: WDYT?

I think that's fine.

In gyp, we added it ad-hoc when it was needed, it's currently in 11 files
(https://cs.chromium.org/search/?q=wl,-objc+file:gyp&sq=package:chromium&type=cs).
I don't think there's a disadvantage in adding this globally -- at least in
static library builds, it shouldn't make a difference.

[brettw, 2016-06-22 18:12:39]

There are two configs:

//build/config/mac:mac_dynamic_flags which are applied to executables, shared
libraries, and loadable modules.

and

//build/config/mac:mac_executable_flags which are applied to only executables.

It sounds like you're talking about the former.

[Nico, 2016-06-22 18:20:48]

Yes.

On Wed, Jun 22, 2016 at 2:12 PM, <brettw@chromium.org> wrote:

> There are two configs:
>
> //build/config/mac:mac_dynamic_flags which are applied to executables,
> shared
> libraries, and loadable modules.
>
> and
>
> //build/config/mac:mac_executable_flags which are applied to only
> executables.
>
> It sounds like you're talking about the former.
>
> https://codereview.chromium.org/2083153002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Robert Sesek, 2016-06-22 18:22:06]

Another option would be to define a new config like "contains_objc_category"
that could be set as a public_config on targets that declare ObjC categories.
That config would just specify the -ObjC ldlfag.

[brettw, 2016-06-22 18:23:34]

Are there any downsides to specifying this flag when it's not necessary?

[Robert Sesek, 2016-06-22 22:32:01]

On 2016/06/22 18:23:34, brettw wrote:
> Are there any downsides to specifying this flag when it's not necessary?

I don't think so. It'll cause the linker to do a tiny amount more work when
specified, but that probably won't have much impact. Switched to adding the flag
in mac_dynamic_flags.

[Robert Sesek, 2016-06-29 15:36:55]

The CQ bit was checked by rsesek@chromium.org

[Robert Sesek, 2016-06-29 15:36:56]

The patchset sent to the CQ was uploaded after l-g-t-m from brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/2083153002/#ps20001
(title: "global -ObjC")

[commit-bot: I haz the power, 2016-06-29 15:37:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 16:44:54]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-29 16:45:41]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-06-29 16:49:00]

Description was changed from

==========
[Mac/GN] Let //chrome/browser:browser be a static library.

This requires //chrome:chrome_framework to link with -ObjC to force categories
to load from static archives.

BUG=618797
R=brettw@chromium.org
==========

to

==========
[Mac/GN] Let //chrome/browser:browser be a static library.

This requires //chrome:chrome_framework to link with -ObjC to force categories
to load from static archives.

BUG=618797
R=brettw@chromium.org

Committed: https://crrev.com/d4ee99258720a95788c8465aef4562c479872f03
Cr-Commit-Position: refs/heads/master@{#402836}
==========

[commit-bot: I haz the power, 2016-06-29 16:49:02]

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