GN Don't enable linker version script for components

GN Don't enable linker version script for components

It breaks native JNI exporting. Change aligns with GYP behaviour.

BUG=532905
Committed: https://crrev.com/3f53232116444520bec48390d8812f18aa08606f
Cr-Commit-Position: refs/heads/master@{#367470}

[agrieve, 2015-12-22 16:37:26]

agrieve@chromium.org changed reviewers:
+ brettw@chromium.org

[agrieve, 2015-12-22 16:37:27]

🎅

[agrieve, 2015-12-23 03:28:14]

On 2015/12/22 16:37:27, agrieve wrote:
> 🎅

Looks like I'll need to update this to take into account
https://codereview.chromium.org/1545493002 (changing base -> shared_library
rather than component)

[agrieve, 2015-12-23 15:54:43]

On 2015/12/23 03:28:14, agrieve wrote:
> On 2015/12/22 16:37:27, agrieve wrote:
> > 🎅
> 
> Looks like I'll need to update this to take into account
> https://codereview.chromium.org/1545493002 (changing base -> shared_library
> rather than component)

Updated. ptal

[brettw, 2016-01-04 22:24:53]

https://codereview.chromium.org/1548473003/diff/20001/build/config/BUILDCONFI...
File build/config/BUILDCONFIG.gn (right):

https://codereview.chromium.org/1548473003/diff/20001/build/config/BUILDCONFI...
build/config/BUILDCONFIG.gn:582: # Some targets (e.g. //base) should user
static_library rather than source_set
...rather than source set in non-component builds...

Also, can you document this above the component template instead? It should say
the arguments are the same as a normal library with this addition: <current
thing you have>

https://codereview.chromium.org/1548473003/diff/20001/build/config/BUILDCONFI...
build/config/BUILDCONFIG.gn:585: defined(invoker.never_use_source_set) &&
invoker.never_use_source_set
I'd feel better if this parameter was called
"invoker.component_never_use_source_set" which will make the caller in base more
clear.

https://codereview.chromium.org/1548473003/diff/20001/build/config/BUILDCONFI...
build/config/BUILDCONFIG.gn:589: } else if (_never_use_source_set) {
Why not just write:
  } else if (defined(invoker.component_never_use_source_set) &&
invoker.component_never_use_source_set) {

and avoid both the temporary and the weird assert thing above? I think mine is
clear enough.

[brettw, 2016-01-04 22:25:00]

LGTM with above changes

[agrieve, 2016-01-05 02:46:47]

https://codereview.chromium.org/1548473003/diff/20001/build/config/BUILDCONFI...
File build/config/BUILDCONFIG.gn (right):

https://codereview.chromium.org/1548473003/diff/20001/build/config/BUILDCONFI...
build/config/BUILDCONFIG.gn:582: # Some targets (e.g. //base) should user
static_library rather than source_set
On 2016/01/04 22:24:53, brettw wrote:
> ...rather than source set in non-component builds...
> 
> Also, can you document this above the component template instead? It should
say
> the arguments are the same as a normal library with this addition: <current
> thing you have>

Done.

https://codereview.chromium.org/1548473003/diff/20001/build/config/BUILDCONFI...
build/config/BUILDCONFIG.gn:585: defined(invoker.never_use_source_set) &&
invoker.never_use_source_set
On 2016/01/04 22:24:53, brettw wrote:
> I'd feel better if this parameter was called
> "invoker.component_never_use_source_set" which will make the caller in base
more
> clear.

Done.

https://codereview.chromium.org/1548473003/diff/20001/build/config/BUILDCONFI...
build/config/BUILDCONFIG.gn:589: } else if (_never_use_source_set) {
On 2016/01/04 22:24:53, brettw wrote:
> Why not just write:
>   } else if (defined(invoker.component_never_use_source_set) &&
> invoker.component_never_use_source_set) {
> 
> and avoid both the temporary and the weird assert thing above? I think mine is
> clear enough.

I tried this at first, but it complains of an unused variable when
is_component_build == true (presumably since the else-if never needs to be
evaluated).

[agrieve, 2016-01-05 02:47:34]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-01-05 02:47:35]

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/1548473003/#ps40001
(title: "review comments")

[commit-bot: I haz the power, 2016-01-05 02:48:27]

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

[commit-bot: I haz the power, 2016-01-05 03:23:22]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-05 03:24:05]

Description was changed from

==========
GN Don't enable linker version script for components

It breaks native JNI exporting. Change aligns with GYP behaviour.

BUG=532905
==========

to

==========
GN Don't enable linker version script for components

It breaks native JNI exporting. Change aligns with GYP behaviour.

BUG=532905

Committed: https://crrev.com/3f53232116444520bec48390d8812f18aa08606f
Cr-Commit-Position: refs/heads/master@{#367470}
==========

[commit-bot: I haz the power, 2016-01-05 03:24:06]

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