Add Archive step for iOS builds.

Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile and
stripping after linking makes graybox testing from xctest impossible.

Instead, add a new 'Archive' configuration -- the same as what Xcode does --
where visibility is set to hidden and stripping is done after linking, but skip
those steps on the remaining configurations.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of
Release-iphoneos.

BUG=
TBR=thakis@chromium.org

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

[justincohen, 2016-04-21 21:07:26]

Description was changed from

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile makes
graybox testing from xctest impossible.  Instead, add a new 'Archive'
configuration as in Xcode with visibility set to hidden, and change the
remaining iOS configurations to not change visibility.

BUG=
==========

to

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile makes
graybox testing from xctest impossible.  Instead, add a new 'Archive'
configuration as in Xcode with visibility set to hidden, and change the
remaining iOS configurations to not change visibility.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of 
Release-iphoneos.


BUG=
==========

[justincohen, 2016-04-21 21:12:51]

justincohen@chromium.org changed reviewers:
+ baxley@chromium.org, rohitrao@chromium.org, sdefresne@chromium.org,
smut@google.com

[justincohen, 2016-04-21 21:12:52]

It seems like the easiest, cleanest way to get xctests working with Chromium
when
Chromium is the test_host is to remove -fvisibility=hidden.  However, removing
this
can increase the binary size by over 10%, so it's no good to ship a product like
this.

Xcode has a concept of an 'Archive' step when submitting binaries.  I propose we
add
an ios/officially gated configuration named Archive that is the same as Release,
except
for the -fvisibility change.


Adding an Archive step to our build process pros and cons:

cons:
 - Will add 10-15 minutes to our official builder cycle time
 - Means tests are run on a build with 1 change to compile flags
(-fvisibility=hidden)

pros:
 - Means xctests can link with Chromium much more easilly
 - Removes need for dummy test_host target for xctests.
 - Removes the conversation about using exported_symbols_list, which is hard to
maintain
 - Removes the conversation about a 'white list' of files due to the polluted
nature of TEST_HOST after running xctests.

I'll need to add mark@chromium.org and thakis@chromium.org to this CL, but I'd
like to get your feedback first.

What do you think?

[justincohen, 2016-04-21 21:50:43]

Description was changed from

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile makes
graybox testing from xctest impossible.  Instead, add a new 'Archive'
configuration as in Xcode with visibility set to hidden, and change the
remaining iOS configurations to not change visibility.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of 
Release-iphoneos.


BUG=
==========

to

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile and 
stripping after linking makes graybox testing from xctest impossible.

Instead, add a new 'Archive' configuration -- the same as what Xcode does --
where visibility is set to hidden and stripping is done after linking, but skip
those steps on the remaining configurations.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of 
Release-iphoneos.

BUG=
==========

[sdefresne, 2016-04-22 07:31:02]

On 2016/04/21 at 21:12:52, justincohen wrote:
> It seems like the easiest, cleanest way to get xctests working with Chromium
when
> Chromium is the test_host is to remove -fvisibility=hidden.  However, removing
this
> can increase the binary size by over 10%, so it's no good to ship a product
like this.
> 
> Xcode has a concept of an 'Archive' step when submitting binaries.  I propose
we add
> an ios/officially gated configuration named Archive that is the same as
Release, except
> for the -fvisibility change.
> 
> 
> Adding an Archive step to our build process pros and cons:
> 
> cons:
>  - Will add 10-15 minutes to our official builder cycle time
>  - Means tests are run on a build with 1 change to compile flags
(-fvisibility=hidden)
> 
> pros:
>  - Means xctests can link with Chromium much more easilly
>  - Removes need for dummy test_host target for xctests.
>  - Removes the conversation about using exported_symbols_list, which is hard
to maintain
>  - Removes the conversation about a 'white list' of files due to the polluted
nature of TEST_HOST after running xctests.
> 
> I'll need to add mark@chromium.org and thakis@chromium.org to this CL, but I'd
> like to get your feedback first.
> 
> What do you think?

lg

[sdefresne, 2016-04-22 08:18:38]

Arggggg again, I clicked send and it did not send my comments...

https://codereview.chromium.org/1907053003/diff/40001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1907053003/diff/40001/build/common.gypi#newco...
build/common.gypi:3578: [ 'OS=="ios" and buildtype=="Official"', {
I'd like not to gate this on "buildtype". It will make easier for developers to
experiment with Archive build if they do not have to change "buildtype".

[baxley, 2016-04-22 13:14:49]

On 2016/04/21 21:12:52, justincohen wrote:
> It seems like the easiest, cleanest way to get xctests working with Chromium
> when
> Chromium is the test_host is to remove -fvisibility=hidden.  However, removing
> this
> can increase the binary size by over 10%, so it's no good to ship a product
like
> this.
> 
> Xcode has a concept of an 'Archive' step when submitting binaries.  I propose
we
> add
> an ios/officially gated configuration named Archive that is the same as
Release,
> except
> for the -fvisibility change.
> 
> 
> Adding an Archive step to our build process pros and cons:
> 
> cons:
>  - Will add 10-15 minutes to our official builder cycle time
>  - Means tests are run on a build with 1 change to compile flags
> (-fvisibility=hidden)
> 
> pros:
>  - Means xctests can link with Chromium much more easilly
>  - Removes need for dummy test_host target for xctests.
>  - Removes the conversation about using exported_symbols_list, which is hard
to
> maintain
>  - Removes the conversation about a 'white list' of files due to the polluted
> nature of TEST_HOST after running xctests.
> 
> I'll need to add mailto:mark@chromium.org and mailto:thakis@chromium.org to
this CL, but I'd
> like to get your feedback first.
> 
> What do you think?

I think it's our best option. UI Testing and KIF also rely on XCTest, so
alternative frameworks would have the same issues (need to work with XCTest).

I don't think it's terrible that there's one change in the build for the tests.
The biggest issue is probably the cycle time, but if it's just for official we
could probably deal with it?

[rohitrao (ping after 24h), 2016-04-22 14:06:44]

LGTM

This seems like a reasonable compromise.  We should make sure Sana is ok with
building the extra configuration, and I'm curious to hear what Mark and Nico
have to say.

In general, if the only difference is symbols, I'm not concerned about testing
different bits than we ship.

[justincohen, 2016-04-22 14:09:23]

https://codereview.chromium.org/1907053003/diff/40001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1907053003/diff/40001/build/common.gypi#newco...
build/common.gypi:3578: [ 'OS=="ios" and buildtype=="Official"', {
On 2016/04/22 08:18:37, sdefresne wrote:
> I'd like not to gate this on "buildtype". It will make easier for developers
to
> experiment with Archive build if they do not have to change "buildtype".

I added this to try to bring down gyp generation time for most developers, but I
guess we'll be on GN soon so it won't matter.  Removing.

[justincohen, 2016-04-22 14:12:55]

justincohen@chromium.org changed reviewers:
+ mark@chromium.org, thakis@chromium.org

[justincohen, 2016-04-22 14:12:56]

Hey mark@, thakis@

This is a followup to a short conversation on IRC we had about running xctests
with 'greybox' testing (linking into Chromium) with builds compiled with
-fvisibility=hidden and post build stripping.

Xcode has the concept of an 'Archive' build step used for creating a release
build to be sent to Apple.   This CL removes -fviz and stripping for Release,
and moves them to an Archive configuration.  This means what we submit and what
we test are slightly different in only these two ways.

PTAL!

[Nico, 2016-04-22 14:33:50]

I don't recall any irc discussion about this, so maybe you talked to Mark. (Then
again, I don't recall most events in my life.)

Do we really need a full new config for this? Could this just be some build
toggle? (GYP_DEFINES=ios_archive=1 or what have you)?

Why do we need this? The CL description says that Xcode has this step, but why
do we think having this step is a good idea?

[justincohen, 2016-04-22 14:42:39]

xctests link against their TEST_HOST with the -bundle_loader flags that points
to Chromium.  If we want an xctest that verifies or changes any internal state
within Chromium, it'll need to see more symbols than what we allow after
stripping and -fviz.

We could use a gyp flag instead of a new configuration, but this seemed cleaner
and less prone to error.  The official bot scripts will be updated to add an
extra 'chrome' compile step in Archive-iphoneos, and the package scripts will
only look for prodcuts inside out/Archive-iphoneos.

If we add a gyp flag we could build Release 'chrome' with the new flag, package
up 'chrome', blow away the Release directory, rerun hooks, and compile/test.

The latter seemed more complicated and prone to error in the official bots?

wdyt?

[Nico, 2016-04-22 15:07:50]

On 2016/04/22 14:42:39, justincohen wrote:
> xctests link against their TEST_HOST with the -bundle_loader flags that points
> to Chromium.  If we want an xctest that verifies or changes any internal state
> within Chromium, it'll need to see more symbols than what we allow after
> stripping and -fviz.
> 
> We could use a gyp flag instead of a new configuration, but this seemed
cleaner
> and less prone to error.  The official bot scripts will be updated to add an
> extra 'chrome' compile step in Archive-iphoneos, and the package scripts will
> only look for prodcuts inside out/Archive-iphoneos.
> 
> If we add a gyp flag we could build Release 'chrome' with the new flag,
package
> up 'chrome', blow away the Release directory, rerun hooks, and compile/test.
> 
> The latter seemed more complicated and prone to error in the official bots?
> 
> wdyt?

Makes sense, but come gn you need to do the multi-step hopping anyhoo.

The way we do this in non-xctests is to link in everything to each test binary,
instead of having test binaries load symbols dynamically from the framework.
Could this setup work with xctest? then you wouldn't have to build everything
twice.

[sdefresne, 2016-04-22 15:11:15]

On 2016/04/22 at 15:07:50, thakis wrote:
> On 2016/04/22 14:42:39, justincohen wrote:
> > xctests link against their TEST_HOST with the -bundle_loader flags that
points
> > to Chromium.  If we want an xctest that verifies or changes any internal
state
> > within Chromium, it'll need to see more symbols than what we allow after
> > stripping and -fviz.
> > 
> > We could use a gyp flag instead of a new configuration, but this seemed
cleaner
> > and less prone to error.  The official bot scripts will be updated to add an
> > extra 'chrome' compile step in Archive-iphoneos, and the package scripts
will
> > only look for prodcuts inside out/Archive-iphoneos.
> > 
> > If we add a gyp flag we could build Release 'chrome' with the new flag,
package
> > up 'chrome', blow away the Release directory, rerun hooks, and compile/test.
> > 
> > The latter seemed more complicated and prone to error in the official bots?
> > 
> > wdyt?
> 
> Makes sense, but come gn you need to do the multi-step hopping anyhoo.

With gn we'll use different output directories per configuration. We can have an
output directory for building the version used by the tests and one for building
the final version that is going to be shipped.

> The way we do this in non-xctests is to link in everything to each test
binary, instead of having test binaries load symbols dynamically from the
framework. Could this setup work with xctest? then you wouldn't have to build
everything twice.

[Mark Mentovai, 2016-04-22 15:15:04]

The IRC discussion was mostly “Xcode SUCKS!”

[justincohen, 2016-04-22 15:16:12]

For unit tests that would work, and TEST_HOST would be an empty shell.  However,
for integration tests I think we need to load the whole app, and I don't think
we can have a shell TEST_HOST and still load up all of Chromium.app.

Or we could make Chromium.app itself an empty shell, and move all of Chromium
code into a shared framework.  But that seems like overkill, and I'm not sure if
it would even work.

sdefresne@, who is making leaps and strides on iOS -> GN migration.  Do you have
a strong preference over a new configuration vs a flag?

[Mark Mentovai, 2016-04-22 15:17:17]

So Archive is the new basis for shipping configurations? OK, LGTM.

That matches how Xcode behaves, but our tools might have Release baked in to
lots of places. If that becomes a problem, you may want to leave the current
behavior as Release, and have the -fvisibility=default version called “Release
(busted-up)” or something else.

[sdefresne, 2016-04-22 15:18:43]

On 2016/04/22 at 15:16:12, justincohen wrote:
> For unit tests that would work, and TEST_HOST would be an empty shell. 
However, for integration tests I think we need to load the whole app, and I
don't think we can have a shell TEST_HOST and still load up all of Chromium.app.
> 
> Or we could make Chromium.app itself an empty shell, and move all of Chromium
code into a shared framework.  But that seems like overkill, and I'm not sure if
it would even work.
> 
> sdefresne@, who is making leaps and strides on iOS -> GN migration.  Do you
have a strong preference over a new configuration vs a flag?

No preferences. With GN I'll implement this with a flag, and the "configuration"
is just a bunch of flag you have to set at the same time (like is_debug,
target_cpu, ...).

[Nico, 2016-04-22 15:20:48]

I think none of Chrome's non-iOS integration tests load chrome.exe itself, they
all link in all the code instead. If possible, keeping iOS like the other
chromes sounds generally good to me. But in the end I defer to y'all.

[smut, 2016-04-23 01:01:32]

On 2016/04/22 15:20:48, Nico wrote:
> I think none of Chrome's non-iOS integration tests load chrome.exe itself,
they
> all link in all the code instead. If possible, keeping iOS like the other
> chromes sounds generally good to me. But in the end I defer to y'all.

lgtm, infra-side change is ready:
https://chromereviews.googleplex.com/410067013.

[justincohen, 2016-04-24 15:32:29]

The CQ bit was checked by justincohen@chromium.org

[justincohen, 2016-04-24 15:32:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
rohitrao@chromium.org
Link to the patchset: https://codereview.chromium.org/1907053003/#ps60001
(title: "Removing offical buildtype gate")

[commit-bot: I haz the power, 2016-04-24 15:32:38]

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

[commit-bot: I haz the power, 2016-04-24 15:38:09]

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

[commit-bot: I haz the power, 2016-04-24 15:38:12]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[justincohen, 2016-04-24 15:39:38]

Description was changed from

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile and 
stripping after linking makes graybox testing from xctest impossible.

Instead, add a new 'Archive' configuration -- the same as what Xcode does --
where visibility is set to hidden and stripping is done after linking, but skip
those steps on the remaining configurations.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of 
Release-iphoneos.

BUG=
==========

to

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile and 
stripping after linking makes graybox testing from xctest impossible.

Instead, add a new 'Archive' configuration -- the same as what Xcode does --
where visibility is set to hidden and stripping is done after linking, but skip
those steps on the remaining configurations.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of 
Release-iphoneos.

BUG=
TBR=thakis@chromium.org
==========

[justincohen, 2016-04-24 15:39:46]

The CQ bit was checked by justincohen@chromium.org

[commit-bot: I haz the power, 2016-04-24 15:39:58]

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

[commit-bot: I haz the power, 2016-04-24 16:44:28]

Description was changed from

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile and 
stripping after linking makes graybox testing from xctest impossible.

Instead, add a new 'Archive' configuration -- the same as what Xcode does --
where visibility is set to hidden and stripping is done after linking, but skip
those steps on the remaining configurations.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of 
Release-iphoneos.

BUG=
TBR=thakis@chromium.org
==========

to

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile and 
stripping after linking makes graybox testing from xctest impossible.

Instead, add a new 'Archive' configuration -- the same as what Xcode does --
where visibility is set to hidden and stripping is done after linking, but skip
those steps on the remaining configurations.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of 
Release-iphoneos.

BUG=
TBR=thakis@chromium.org
==========

[commit-bot: I haz the power, 2016-04-24 16:44:29]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-24 16:45:48]

Description was changed from

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile and 
stripping after linking makes graybox testing from xctest impossible.

Instead, add a new 'Archive' configuration -- the same as what Xcode does --
where visibility is set to hidden and stripping is done after linking, but skip
those steps on the remaining configurations.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of 
Release-iphoneos.

BUG=
TBR=thakis@chromium.org
==========

to

==========
Add Archive step for iOS builds.

Future xctest support will set the test's bundle_loader to Chromium and look
for Chromium internals.  Setting `-fvisibility=hidden` during compile and
stripping after linking makes graybox testing from xctest impossible.

Instead, add a new 'Archive' configuration -- the same as what Xcode does --
where visibility is set to hidden and stripping is done after linking, but skip
those steps on the remaining configurations.

This CL should land concurrently with an official bot script change to compile
`ninja -C out/Release-iphoneos ; ninja -C out/Archive-iphoneos chrome`
and to pull official binaries from out/Archive-iphoneos instead of
Release-iphoneos.

BUG=
TBR=thakis@chromium.org

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

[commit-bot: I haz the power, 2016-04-24 16:45:50]

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

[justincohen, 2016-04-25 15:04:03]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1919903002/ by justincohen@chromium.org.

The reason for reverting is: Downstream iOS CLs aren't ready for this yet. 
Reverting for now..