[GN]: Support for loadable modules

[GN]: support for loadable modules

+ Adds a new target type loadable_module and tool type solink_module. loadable_module will trigger the solink_module tool type in the toolchain.
+ Updates reference.md and editor configs.

BUG=369774
Committed: https://crrev.com/027840da082368b8d51f06b9c310807540e50eea
Cr-Commit-Position: refs/heads/master@{#354077}

[Bons, 2015-10-04 23:49:42]

andybons@chromium.org changed reviewers:
+ brettw@chromium.org, mark@chromium.org, rsesek@chromium.org

[Bons, 2015-10-05 19:15:40]

After speaking with Mark, using darwin_bundle for both a loadable module and as
an indicator to create an application bundle is not valid since you can create a
bundle (.framework) with a bundle (loadable module .so file). Confusing, I know.
So now there are two vars:

loadable_module for creating an .so from a shared_library
darwin_bundle for creating a directory with a standardized hierarchical
structure to produce either an .app or .framework for executable and
shared_library targets respectively.

[Bons, 2015-10-05 21:36:21]

Including gn-dev for any input.

[Bons, 2015-10-06 20:29:19]

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

https://codereview.chromium.org/1386783003/diff/60001/build/toolchain/mac/BUI...
build/toolchain/mac/BUILD.gn:171: #    tool("solink") {
This will be called solink with the above changed to dylink.

Or if we want to keep with gyp conventions, this would be named solink_module

[brettw, 2015-10-08 04:34:24]

I haven't forgotten about this. My other project is taking all of my time right
now. Hopefully I can get to this soonish.

[Bons, 2015-10-11 19:56:16]

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

[Bons, 2015-10-11 19:57:59]

OK. Unit tests have been added. Please take another look.

Example output can be seen within the ninja_binary_target_writer_unittest.cc
additions.

[Bons, 2015-10-11 20:16:27]

On 2015/10/11 19:57:59, Bons wrote:
> OK. Unit tests have been added. Please take another look.
> 
> Example output can be seen within the ninja_binary_target_writer_unittest.cc
> additions.

I will revert the changes to the BUILD and BUILDCONFIG files after the LGTM
since they show how this change will be integrated into the toolchain. I’ll plan
to land those after the next roll.

[brettw, 2015-10-13 22:59:07]

I don't understand the addition of the darwin bundle flag. If we're not using it
now, why not wait to add it until we need it? Since I was hoping to implement
bundles as templates anyway, we shouldn't even need a builtin variable if this
is successful.

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/binary_target...
File tools/gn/binary_target_generator.cc (right):

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/binary_target...
tools/gn/binary_target_generator.cc:141: if (target_->output_type() ==
Target::STATIC_LIBRARY)
This code should also be run for source sets. I think it would be better to
filter in the conditions we want to read this variable for rather than exclude
the other ones (i.e. check for executable, etc).

[Bons, 2015-10-13 23:12:47]

Removed darwin_bundle.

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/binary_target...
File tools/gn/binary_target_generator.cc (right):

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/binary_target...
tools/gn/binary_target_generator.cc:141: if (target_->output_type() ==
Target::STATIC_LIBRARY)
On 2015/10/13 22:59:07, brettw wrote:
> This code should also be run for source sets. I think it would be better to
> filter in the conditions we want to read this variable for rather than exclude
> the other ones (i.e. check for executable, etc).

removed so non-issue.

[brettw, 2015-10-13 23:13:33]

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/docs/language.md
File tools/gn/docs/language.md (right):

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/docs/language...
tools/gn/docs/language.md:379: * `loadable_module`: An .so loadable only at
runtime.
Can you write ".dll or .so"

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/functions_tar...
File tools/gn/functions_target.cc (right):

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/functions_tar...
tools/gn/functions_target.cc:366: "\n"
Can you add a paragraph at the top here about how a loadable module is a dynamic
library loaded explicitly at runtime rather than automatically linked. What you
have below feels like it goes after a general intro.

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/functions_tar...
tools/gn/functions_target.cc:399: "  depend on the shared library via
\"data_deps\" or, on Darwin\n"
I'd remove "on Darwin platforms" since this should work everywhere.

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/functions_tar...
tools/gn/functions_target.cc:535: *err = Err(function, "Expected two arguments.
Try \"gn help target\".");
I think the old one with two string args is better. It should give:

ERROR: Expected two arguments.
target("asdasd") {
^-----
The other info here.

I think this structure makes sense. The initial error on the error line is a
very mechanical description of the error only.

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/ninja_toolcha...
File tools/gn/ninja_toolchain_writer.cc (right):

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/ninja_toolcha...
tools/gn/ninja_toolchain_writer.cc:113: || type == Toolchain::TYPE_SOLINK_MODULE
|| goes at end-of-line

[Bons, 2015-10-13 23:30:52]

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/docs/language.md
File tools/gn/docs/language.md (right):

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/docs/language...
tools/gn/docs/language.md:379: * `loadable_module`: An .so loadable only at
runtime.
On 2015/10/13 23:13:32, brettw wrote:
> Can you write ".dll or .so"

Done.

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/functions_tar...
File tools/gn/functions_target.cc (right):

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/functions_tar...
tools/gn/functions_target.cc:366: "\n"
On 2015/10/13 23:13:32, brettw wrote:
> Can you add a paragraph at the top here about how a loadable module is a
dynamic
> library loaded explicitly at runtime rather than automatically linked. What
you
> have below feels like it goes after a general intro.

Done.

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/functions_tar...
tools/gn/functions_target.cc:535: *err = Err(function, "Expected two arguments.
Try \"gn help target\".");
On 2015/10/13 23:13:33, brettw wrote:
> I think the old one with two string args is better. It should give:
> 
> ERROR: Expected two arguments.
> target("asdasd") {
> ^-----
> The other info here.
> 
> I think this structure makes sense. The initial error on the error line is a
> very mechanical description of the error only.

Ah that was a mistake. I was just trying to remove the "dude" portion.

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/ninja_toolcha...
File tools/gn/ninja_toolchain_writer.cc (right):

https://codereview.chromium.org/1386783003/diff/140001/tools/gn/ninja_toolcha...
tools/gn/ninja_toolchain_writer.cc:113: || type == Toolchain::TYPE_SOLINK_MODULE
On 2015/10/13 23:13:33, brettw wrote:
> || goes at end-of-line

Done.

[brettw, 2015-10-14 17:45:57]

This looks very nice. LGTM

https://codereview.chromium.org/1386783003/diff/180001/tools/gn/docs/referenc...
File tools/gn/docs/reference.md (right):

https://codereview.chromium.org/1386783003/diff/180001/tools/gn/docs/referenc...
tools/gn/docs/reference.md:381: forward_dependent_configs_from
I think this is a merge mistake and that Thiago removed this stuff. Maybe
regenerate the file after you merge (same for the other forward* stuff below).

https://codereview.chromium.org/1386783003/diff/180001/tools/gn/ninja_target_...
File tools/gn/ninja_target_writer.cc (left):

https://codereview.chromium.org/1386783003/diff/180001/tools/gn/ninja_target_...
tools/gn/ninja_target_writer.cc:77: CHECK(0);
Add << "foo" and move back to check

[Bons, 2015-10-14 18:03:42]

https://codereview.chromium.org/1386783003/diff/180001/tools/gn/docs/referenc...
File tools/gn/docs/reference.md (right):

https://codereview.chromium.org/1386783003/diff/180001/tools/gn/docs/referenc...
tools/gn/docs/reference.md:381: forward_dependent_configs_from
On 2015/10/14 17:45:56, brettw wrote:
> I think this is a merge mistake and that Thiago removed this stuff. Maybe
> regenerate the file after you merge (same for the other forward* stuff below).

Done.

https://codereview.chromium.org/1386783003/diff/180001/tools/gn/ninja_target_...
File tools/gn/ninja_target_writer.cc (left):

https://codereview.chromium.org/1386783003/diff/180001/tools/gn/ninja_target_...
tools/gn/ninja_target_writer.cc:77: CHECK(0);
On 2015/10/14 17:45:57, brettw wrote:
> Add << "foo" and move back to check

Done.

[Bons, 2015-10-14 18:03:50]

The CQ bit was checked by andybons@chromium.org

[Bons, 2015-10-14 18:03:50]

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/1386783003/#ps200001
(title: " ")

[commit-bot: I haz the power, 2015-10-14 18:05:10]

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

[commit-bot: I haz the power, 2015-10-14 18:12:32]

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

[commit-bot: I haz the power, 2015-10-14 18:12:33]

Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Bons, 2015-10-14 18:18:07]

The CQ bit was checked by andybons@chromium.org

[Bons, 2015-10-14 18:18:07]

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/1386783003/#ps220001
(title: " ")

[commit-bot: I haz the power, 2015-10-14 18:18:35]

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

[commit-bot: I haz the power, 2015-10-14 18:49:52]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2015-10-14 18:50:34]

Patchset 12 (id:??) landed as
https://crrev.com/027840da082368b8d51f06b9c310807540e50eea
Cr-Commit-Position: refs/heads/master@{#354077}

[Nico, 2016-02-11 00:32:52]

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

[Nico, 2016-02-11 00:32:52]

https://codereview.chromium.org/1386783003/diff/220001/tools/gn/function_tool...
File tools/gn/function_toolchain.cc (right):

https://codereview.chromium.org/1386783003/diff/220001/tools/gn/function_tool...
tools/gn/function_toolchain.cc:402: "      \"solink\": Linker for shared
libraries\n"
this didn't add solink_module to the documentation. can you fix that?

[Bons, 2016-02-12 16:09:22]

https://codereview.chromium.org/1386783003/diff/220001/tools/gn/function_tool...
File tools/gn/function_toolchain.cc (right):

https://codereview.chromium.org/1386783003/diff/220001/tools/gn/function_tool...
tools/gn/function_toolchain.cc:402: "      \"solink\": Linker for shared
libraries\n"
On 2016/02/11 00:32:52, Nico wrote:
> this didn't add solink_module to the documentation. can you fix that?

since you’re in the this code already why not add it?

[Nico, 2016-02-12 16:20:01]

https://codereview.chromium.org/1386783003/diff/220001/tools/gn/function_tool...
File tools/gn/function_toolchain.cc (right):

https://codereview.chromium.org/1386783003/diff/220001/tools/gn/function_tool...
tools/gn/function_toolchain.cc:402: "      \"solink\": Linker for shared
libraries\n"
On 2016/02/12 16:09:22, Bons wrote:
> On 2016/02/11 00:32:52, Nico wrote:
> > this didn't add solink_module to the documentation. can you fix that?
> 
> since you’re in the this code already why not add it?

I don't know what to write :-) I looked at the docs to find out what this does
and then noticed that it doesn't say. I gather it's similar to the same thing in
gyp; but when I last talked with brettw about this over a year ago, I think the
plan was to make loadable_module a bit on shared_library. Since things turned
out differently, I'm not sure what to write, so I need some help at least. Can
you give me a short summary of what to write?

I guess solink_module is used for loadable_module() targets and is generally
very similar to solink. Is solink_module() ever used on non-mac? Does
link_output vs depend_output make sense for it? (Neither's set on Windows for
solink_module in Chromium's case.)

[Bons, 2016-02-12 16:21:17]

On 2016/02/12 16:20:01, Nico wrote:
>
https://codereview.chromium.org/1386783003/diff/220001/tools/gn/function_tool...
> File tools/gn/function_toolchain.cc (right):
> 
>
https://codereview.chromium.org/1386783003/diff/220001/tools/gn/function_tool...
> tools/gn/function_toolchain.cc:402: "      \"solink\": Linker for shared
> libraries\n"
> On 2016/02/12 16:09:22, Bons wrote:
> > On 2016/02/11 00:32:52, Nico wrote:
> > > this didn't add solink_module to the documentation. can you fix that?
> > 
> > since you’re in the this code already why not add it?
> 
> I don't know what to write :-) I looked at the docs to find out what this does
> and then noticed that it doesn't say. I gather it's similar to the same thing
in
> gyp; but when I last talked with brettw about this over a year ago, I think
the
> plan was to make loadable_module a bit on shared_library. Since things turned
> out differently, I'm not sure what to write, so I need some help at least. Can
> you give me a short summary of what to write?
> 
> I guess solink_module is used for loadable_module() targets and is generally
> very similar to solink. Is solink_module() ever used on non-mac? Does
> link_output vs depend_output make sense for it? (Neither's set on Windows for
> solink_module in Chromium's case.)

Ah got it. I’ll take a look and send something over once I clear those cobwebs
out later today.