[Mac/GN] Set up the component build.

[Mac/GN] Set up the component build.

This adds the required framework to libs[] wherever they are required (most
of the changes in this CL). It also enables the component build optimization
of creating a non-bundled dylib to roll up all the sources and dependencies.
The framework then links that, which allows the build to not copy the bundled
library if any sources change. This is based on the technique implemented
https://codereview.chromium.org/11420019/.

BUG=431177
R=thakis@chromium.org,brettw@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/02aa51cf444626f4781824c17178376c0459ad83
Cr-Commit-Position: refs/heads/master@{#392823}

[Robert Sesek, 2016-05-06 16:25:41]

Description was changed from

==========
[Mac/GN] Set up the component build.

This adds the required framework to libs[] wherever they are required (most
of the changes in this CL). It also enables the component build optimization
of creating a non-bundled dylib to roll up all the sources and dependencies.
The framework then links that, which allows the build to not copy the bundled
library if any sources change. This is based on the technique implemented
https://codereview.chromium.org/11420019/.

BUG=431177
R=thakis@chromium.org,brettw@chromium.org
==========

to

==========
[Mac/GN] Set up the component build.

This adds the required framework to libs[] wherever they are required (most
of the changes in this CL). It also enables the component build optimization
of creating a non-bundled dylib to roll up all the sources and dependencies.
The framework then links that, which allows the build to not copy the bundled
library if any sources change. This is based on the technique implemented
https://codereview.chromium.org/11420019/.

BUG=431177
R=thakis@chromium.org,brettw@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Robert Sesek, 2016-05-06 18:59:47]

Patchset #1 (id:1) has been deleted

[Robert Sesek, 2016-05-06 19:00:50]

Patchset #1 (id:20001) has been deleted

[Robert Sesek, 2016-05-06 19:17:36]

thakis: Please review.
brettw/dpranke: I called out the hack I mentioned in the meeting.

https://codereview.chromium.org/1961473003/diff/40001/build/toolchain/mac/BUI...
File build/toolchain/mac/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/build/toolchain/mac/BUI...
build/toolchain/mac/BUILD.gn:141: link_command = "$ld -shared "
This is the hack I mentioned in the standup:

GN doesn't have any concept of install name vs output name, nor can you
filter/check set ldflags. This is an issue with bundled dylib targets, where the
install name differs from the output name, and the install name has to be set by
the target itself. The solution here places the generic
is_component_build-generated install name flag ahead of ldflags, so that if the
target specifies the install name in ldflags, the latter one on the command line
takes precedence.

[Dirk Pranke, 2016-05-06 22:10:06]

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

[Dirk Pranke, 2016-05-06 22:10:07]

this mostly lgtm , though I don't yet understand the rpath and install_name
magic fully.

I assume all of the additional libs are needed because in static builds the
targets were source_sets that were getting combined with other source_sets that
provided the needed libraries?

https://codereview.chromium.org/1961473003/diff/40001/build/secondary/third_p...
File build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/build/secondary/third_p...
build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn:53:
"@loader_path/../../../../../..",
uh ... where do the six parent directories come from here?

https://codereview.chromium.org/1961473003/diff/40001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/chrome/BUILD.gn#newcode552
chrome/BUILD.gn:552: "@loader_path/../../../../../../..",
same question ...

https://codereview.chromium.org/1961473003/diff/40001/media/base/mac/BUILD.gn
File media/base/mac/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/media/base/mac/BUILD.gn...
media/base/mac/BUILD.gn:9: # classes declared in the files below will cause
warnings at run-time.
You can/should use the `visibility` variable to enforce this, then.

https://codereview.chromium.org/1961473003/diff/40001/tools/perf/chrome_telem...
File tools/perf/chrome_telemetry_build/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/tools/perf/chrome_telem...
tools/perf/chrome_telemetry_build/BUILD.gn:44: }
I think this bug/workaround has been fixed and we should be able to just change
this to //chrome across the board, but we should investigate and test that in a
separate CL. We might need to change //chrome to declare :chrome_initial or
:reorder_imports as data_deps as well as public_deps, I'd have to look back at
the GN fix to remember exactly the details.

[Robert Sesek, 2016-05-06 22:20:49]

On 2016/05/06 22:10:07, Dirk Pranke wrote:
> this mostly lgtm , though I don't yet understand the rpath and install_name
> magic fully.
> 
> I assume all of the additional libs are needed because in static builds the
> targets were source_sets that were getting combined with other source_sets
that
> provided the needed libraries?

install_name is embedded into shared libraries at link time. When another image
links against that library, it records the install name as the path to find the
library when loading. If the install_name has an @rpath in it, the loader will
use the list of rpaths in the image to substitute/search @rpath for.

https://codereview.chromium.org/1961473003/diff/40001/build/secondary/third_p...
File build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/build/secondary/third_p...
build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn:53:
"@loader_path/../../../../../..",
On 2016/05/06 22:10:07, Dirk Pranke wrote:
> uh ... where do the six parent directories come from here?

Left a comment.

https://codereview.chromium.org/1961473003/diff/40001/chrome/BUILD.gn
File chrome/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/chrome/BUILD.gn#newcode552
chrome/BUILD.gn:552: "@loader_path/../../../../../../..",
On 2016/05/06 22:10:07, Dirk Pranke wrote:
> same question ...

Commented.

https://codereview.chromium.org/1961473003/diff/40001/media/base/mac/BUILD.gn
File media/base/mac/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/media/base/mac/BUILD.gn...
media/base/mac/BUILD.gn:9: # classes declared in the files below will cause
warnings at run-time.
On 2016/05/06 22:10:07, Dirk Pranke wrote:
> You can/should use the `visibility` variable to enforce this, then.

Done.

https://codereview.chromium.org/1961473003/diff/40001/tools/perf/chrome_telem...
File tools/perf/chrome_telemetry_build/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/40001/tools/perf/chrome_telem...
tools/perf/chrome_telemetry_build/BUILD.gn:44: }
On 2016/05/06 22:10:07, Dirk Pranke wrote:
> I think this bug/workaround has been fixed and we should be able to just
change
> this to //chrome across the board, but we should investigate and test that in
a
> separate CL. We might need to change //chrome to declare :chrome_initial or
> :reorder_imports as data_deps as well as public_deps, I'd have to look back at
> the GN fix to remember exactly the details.

Yeah, I figured this would be better to handle in another CL, as I don't
understand why it was done initially.

[Dirk Pranke, 2016-05-06 22:30:18]

lgtm.

https://codereview.chromium.org/1961473003/diff/60001/build/secondary/third_p...
File build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/60001/build/secondary/third_p...
build/secondary/third_party/crashpad/crashpad/handler/BUILD.gn:51: # The handler
is in Chromium.app/Contents/Versions/X/Chromium Framework.framework/Helpers/
I wonder if there's some automated way to get this via rebase_path() or some
other cleverness ...

[Robert Sesek, 2016-05-09 19:38:31]

thakis: PTAL

[Robert Sesek, 2016-05-09 20:58:19]

Patchset #3 (id:80001) has been deleted

[Nico, 2016-05-10 20:24:06]

lgtm

Feel free to throw a shoe (or similar) when I drag my feet -- sorry :-(

https://codereview.chromium.org/1961473003/diff/100001/content/shell/BUILD.gn
File content/shell/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/100001/content/shell/BUILD.gn...
content/shell/BUILD.gn:650: ldflags = [
want to add the comment here too?

https://codereview.chromium.org/1961473003/diff/100001/media/BUILD.gn
File media/BUILD.gn (left):

https://codereview.chromium.org/1961473003/diff/100001/media/BUILD.gn#oldcode674
media/BUILD.gn:674: deps += [ "//media/base/mac" ]
this is no longer needed?

...nvm, the next file explains

[Robert Sesek, 2016-05-10 21:32:44]

On 2016/05/10 20:24:06, Nico wrote:
> lgtm
> 
> Feel free to throw a shoe (or similar) when I drag my feet -- sorry :-(

I was going to offer to bribe you instead. But okay! :)

https://codereview.chromium.org/1961473003/diff/100001/content/shell/BUILD.gn
File content/shell/BUILD.gn (right):

https://codereview.chromium.org/1961473003/diff/100001/content/shell/BUILD.gn...
content/shell/BUILD.gn:650: ldflags = [
On 2016/05/10 20:24:06, Nico wrote:
> want to add the comment here too?

Done.

[Robert Sesek, 2016-05-10 21:33:15]

brettw: looking for an OWNERS stamp

[brettw, 2016-05-10 21:39:29]

lgtm

[Robert Sesek, 2016-05-10 21:53:25]

The CQ bit was checked by rsesek@chromium.org

[Robert Sesek, 2016-05-10 21:53:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, thakis@chromium.org
Link to the patchset: https://codereview.chromium.org/1961473003/#ps120001
(title: "Comment and rebase")

[commit-bot: I haz the power, 2016-05-10 21:53:50]

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

[commit-bot: I haz the power, 2016-05-11 00:48:27]

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

[commit-bot: I haz the power, 2016-05-11 00:48:29]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Dirk Pranke, 2016-05-11 00:51:11]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2016-05-11 00:51:54]

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

[commit-bot: I haz the power, 2016-05-11 02:14:09]

Committed patchset #4 (id:120001)

[commit-bot: I haz the power, 2016-05-11 02:15:06]

Description was changed from

==========
[Mac/GN] Set up the component build.

This adds the required framework to libs[] wherever they are required (most
of the changes in this CL). It also enables the component build optimization
of creating a non-bundled dylib to roll up all the sources and dependencies.
The framework then links that, which allows the build to not copy the bundled
library if any sources change. This is based on the technique implemented
https://codereview.chromium.org/11420019/.

BUG=431177
R=thakis@chromium.org,brettw@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
[Mac/GN] Set up the component build.

This adds the required framework to libs[] wherever they are required (most
of the changes in this CL). It also enables the component build optimization
of creating a non-bundled dylib to roll up all the sources and dependencies.
The framework then links that, which allows the build to not copy the bundled
library if any sources change. This is based on the technique implemented
https://codereview.chromium.org/11420019/.

BUG=431177
R=thakis@chromium.org,brettw@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/02aa51cf444626f4781824c17178376c0459ad83
Cr-Commit-Position: refs/heads/master@{#392823}
==========

[commit-bot: I haz the power, 2016-05-11 02:15:08]

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