Add support for fat build to "ios_framework_bundle" target.

Add support for fat build to "ios_framework_bundle" target.

For a fat build, the "ios_framework_bundle" and "framework_bundle"
are a bit different for the non-default toolchain. The main target
is a shared library (instead of a bundle) and the "+link" target
depends on the "+link" target of the default toolchain (because it
is a fat binary framework so it make sense to link against that one).

The fat shared library is created using "lipo" to concatenate all
the thin shared libraries together.

BUG=603180
Committed: https://crrev.com/31139a2a7ffb3b502b0a963b45c391759b167154
Cr-Commit-Position: refs/heads/master@{#405969}

[sdefresne, 2016-07-11 15:49:05]

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

[sdefresne, 2016-07-11 15:49:05]

Followup to https://codereview.chromium.org/2123253004/ for
ios_framework_bundle. Could you take a look?

[Robert Sesek, 2016-07-11 17:44:20]

There are failures on the Mac bots that look legitimate.

[sdefresne, 2016-07-12 09:21:17]

Patchset #2 (id:20001) has been deleted

[sdefresne, 2016-07-12 09:21:30]

On 2016/07/11 17:44:20, Robert Sesek wrote:
> There are failures on the Mac bots that look legitimate.

Should be fixed, please take another look.

[sdefresne, 2016-07-12 09:28:55]

Patchset #2 (id:40001) has been deleted

[Robert Sesek, 2016-07-12 23:57:16]

Sorry for the lag on this. Rietveld made this hard to review diff-wise.

https://codereview.chromium.org/2135323002/diff/60001/build/config/mac/base_r...
File build/config/mac/base_rules.gni (right):

https://codereview.chromium.org/2135323002/diff/60001/build/config/mac/base_r...
build/config/mac/base_rules.gni:251: visibility += [ ":*($default_toolchain)" ]
Why :* here? Can't this be limited to the _lipo_shared_library_target? (And
can't you ignore any invoker visibility?)

https://codereview.chromium.org/2135323002/diff/60001/build/config/mac/base_r...
build/config/mac/base_rules.gni:291: _link_shared_library_visibility = []
Remove

https://codereview.chromium.org/2135323002/diff/60001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2135323002/diff/60001/chrome/BUILD.gn#newcode892
chrome/BUILD.gn:892: ":chrome_framework_link_shared_library",
Why this name change? I think it may be a little confusing given that "+link"
exists and changes behavior. Similarly, if this comes up for iOS, "_lipo" will
be in the name, when "_shared_library" is probably cleaner there, too. Maybe
just the thin shared_library targets should have a "_arch" in the name instead,
and leave the pre-bundled shared_library output named as _shared_library.

[sdefresne, 2016-07-13 10:57:36]

The CQ bit was checked by sdefresne@chromium.org to run a CQ dry run

[sdefresne, 2016-07-13 10:57:50]

Please take another look.

https://codereview.chromium.org/2135323002/diff/60001/build/config/mac/base_r...
File build/config/mac/base_rules.gni (right):

https://codereview.chromium.org/2135323002/diff/60001/build/config/mac/base_r...
build/config/mac/base_rules.gni:251: visibility += [ ":*($default_toolchain)" ]
On 2016/07/12 23:57:16, Robert Sesek wrote:
> Why :* here? Can't this be limited to the _lipo_shared_library_target? (And
> can't you ignore any invoker visibility?)

I cannot ignore the invoker visibility because this break non-default toolchain
when doing fat builds. Changed to allow just _lipo_shared_library_target.

https://codereview.chromium.org/2135323002/diff/60001/build/config/mac/base_r...
build/config/mac/base_rules.gni:291: _link_shared_library_visibility = []
On 2016/07/12 23:57:16, Robert Sesek wrote:
> Remove

I can't as "gn" complain with the following error:

ERROR at //build/config/mac/base_rules.gni:292:7: Replacing nonempty list.
      _link_shared_library_visibility = [ ":$_lipo_shared_library_target" ]
      ^------------------------------
This overwrites a previously-defined nonempty list (length 1).
See //build/config/mac/base_rules.gni:285:39: for previous definition
    _link_shared_library_visibility = [ ":$_shared_library_bundle_data" ]
                                      ^---------------------------------
with another one (length 1). Did you mean "+=" to append instead? If you
really want to do this, do
  _link_shared_library_visibility = []
before reassigning.

Since my goal is to replace the list, I have to do the empty list assignment
first.

https://codereview.chromium.org/2135323002/diff/60001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/2135323002/diff/60001/chrome/BUILD.gn#newcode892
chrome/BUILD.gn:892: ":chrome_framework_link_shared_library",
On 2016/07/12 23:57:16, Robert Sesek wrote:
> Why this name change? I think it may be a little confusing given that "+link"
> exists and changes behavior. Similarly, if this comes up for iOS, "_lipo" will
> be in the name, when "_shared_library" is probably cleaner there, too. Maybe
> just the thin shared_library targets should have a "_arch" in the name
instead,
> and leave the pre-bundled shared_library output named as _shared_library.

Done.

[commit-bot: I haz the power, 2016-07-13 10:57:53]

Dry run: 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-07-13 12:42:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-13 12:42:39]

Dry run: This issue passed the CQ dry run.

[Robert Sesek, 2016-07-13 20:27:54]

LGTM

https://codereview.chromium.org/2135323002/diff/60001/build/config/mac/base_r...
File build/config/mac/base_rules.gni (right):

https://codereview.chromium.org/2135323002/diff/60001/build/config/mac/base_r...
build/config/mac/base_rules.gni:291: _link_shared_library_visibility = []
On 2016/07/13 10:57:50, sdefresne wrote:
> On 2016/07/12 23:57:16, Robert Sesek wrote:
> > Remove
> 
> I can't as "gn" complain with the following error:
> 
> ERROR at //build/config/mac/base_rules.gni:292:7: Replacing nonempty list.
>       _link_shared_library_visibility = [ ":$_lipo_shared_library_target" ]
>       ^------------------------------
> This overwrites a previously-defined nonempty list (length 1).
> See //build/config/mac/base_rules.gni:285:39: for previous definition
>     _link_shared_library_visibility = [ ":$_shared_library_bundle_data" ]
>                                       ^---------------------------------
> with another one (length 1). Did you mean "+=" to append instead? If you
> really want to do this, do
>   _link_shared_library_visibility = []
> before reassigning.
> 
> Since my goal is to replace the list, I have to do the empty list assignment
> first.

Acknowledged.

[sdefresne, 2016-07-18 08:51:40]

The CQ bit was checked by sdefresne@chromium.org

[commit-bot: I haz the power, 2016-07-18 08:51:54]

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-07-18 08:53:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-18 08:53:28]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[sdefresne, 2016-07-18 09:03:19]

The CQ bit was checked by sdefresne@chromium.org

[sdefresne, 2016-07-18 09:03:20]

The patchset sent to the CQ was uploaded after l-g-t-m from rsesek@chromium.org
Link to the patchset: https://codereview.chromium.org/2135323002/#ps100001
(title: "Rebase")

[commit-bot: I haz the power, 2016-07-18 09:03:26]

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-07-18 09:50:15]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-07-18 09:50:20]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-18 09:51:27]

Description was changed from

==========
Add support for fat build to "ios_framework_bundle" target.

For a fat build, the "ios_framework_bundle" and "framework_bundle"
are a bit different for the non-default toolchain. The main target
is a shared library (instead of a bundle) and the "+link" target
depends on the "+link" target of the default toolchain (because it
is a fat binary framework so it make sense to link against that one).

The fat shared library is created using "lipo" to concatenate all
the thin shared libraries together.

BUG=603180
==========

to

==========
Add support for fat build to "ios_framework_bundle" target.

For a fat build, the "ios_framework_bundle" and "framework_bundle"
are a bit different for the non-default toolchain. The main target
is a shared library (instead of a bundle) and the "+link" target
depends on the "+link" target of the default toolchain (because it
is a fat binary framework so it make sense to link against that one).

The fat shared library is created using "lipo" to concatenate all
the thin shared libraries together.

BUG=603180

Committed: https://crrev.com/31139a2a7ffb3b502b0a963b45c391759b167154
Cr-Commit-Position: refs/heads/master@{#405969}
==========

[commit-bot: I haz the power, 2016-07-18 09:51:28]

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