Fix official build in GN

Fix official build in GN

Changes process_version template. In GYP this is used in two ways and the GN version only supported one of these. This change also deletes an unnecessary fork of the template file, and fixes the dependencies for the extra files (previously they were just passed with "-f" and not added to the inputs of the target.

Adds a cdm_adapter template which basically matches the GYP version. Uses this for the media clearkey adapter and the widevine one in the official build.

Fixes for the crypto targets when compiling with the official build's checked-in sysroots.

Committed: https://crrev.com/70d2f6ea98ae618b24a7c517570b74d20b92a7c7
Cr-Commit-Position: refs/heads/master@{#318083}

[brettw, 2015-02-25 00:14:43]

brettw@chromium.org changed reviewers:
+ ddorwin@chromium.org, dpranke@chromium.org

[brettw, 2015-02-25 00:15:01]

ddorwin: owners for widevine
dpranke: everything else

[Dirk Pranke, 2015-02-25 00:23:20]

lgtm w/ a few nits

https://codereview.chromium.org/949233003/diff/20001/chrome/chrome_watcher/BU...
File chrome/chrome_watcher/BUILD.gn (right):

https://codereview.chromium.org/949233003/diff/20001/chrome/chrome_watcher/BU...
chrome/chrome_watcher/BUILD.gn:18: sources = [
Is this formatted correctly? I'd expect this to be on one line.

https://codereview.chromium.org/949233003/diff/20001/chrome/version.gni
File chrome/version.gni (right):

https://codereview.chromium.org/949233003/diff/20001/chrome/version.gni#newco...
chrome/version.gni:41: #     sources = [ ""myfile.h.in"
Nit: you have two leading quotes and a missing end brace.

https://codereview.chromium.org/949233003/diff/20001/chrome/version.gni#newco...
chrome/version.gni:48: "sources or template_file must be defined for
$target_name")
It's not actually clear in the docs above that you must specify either sources
or template_file. Is that correct? If so, you should fix the docs, and I'd also
change this to "Either sources or template_file ..." to be a bit more
grammatical.

https://codereview.chromium.org/949233003/diff/20001/chrome/version.gni#newco...
chrome/version.gni:74: template_path
nit: should there be a trailing comma?

https://codereview.chromium.org/949233003/diff/20001/chrome_elf/BUILD.gn
File chrome_elf/BUILD.gn (right):

https://codereview.chromium.org/949233003/diff/20001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:11: ]
nit: is this formatted properly?

https://codereview.chromium.org/949233003/diff/20001/media/cdm/ppapi/BUILD.gn
File media/cdm/ppapi/BUILD.gn (right):

https://codereview.chromium.org/949233003/diff/20001/media/cdm/ppapi/BUILD.gn...
media/cdm/ppapi/BUILD.gn:65: ]
Nit: formatted correctly?

[brettw, 2015-02-25 00:51:32]

Should have fixed the rest.

https://codereview.chromium.org/949233003/diff/20001/chrome/chrome_watcher/BU...
File chrome/chrome_watcher/BUILD.gn (right):

https://codereview.chromium.org/949233003/diff/20001/chrome/chrome_watcher/BU...
chrome/chrome_watcher/BUILD.gn:18: sources = [
On 2015/02/25 00:23:19, Dirk Pranke wrote:
> Is this formatted correctly? I'd expect this to be on one line.

sources and deps are always multi-line.

[brettw, 2015-02-25 00:52:45]

The CQ bit was checked by brettw@chromium.org

[brettw, 2015-02-25 00:52:46]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/949233003/#ps40001 (title:
"android fix, review comments, gn format")

[commit-bot: I haz the power, 2015-02-25 00:53:46]

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

[ddorwin, 2015-02-25 01:18:32]

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/BUILD.gn
File media/cdm/ppapi/BUILD.gn (right):

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/BUILD.gn...
media/cdm/ppapi/BUILD.gn:54: process_version("clearkeycdmadapter_resources") {
Ditto with below but less important since this is close to "adapter_resources"
in the WV file.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/BUILD.gn...
media/cdm/ppapi/BUILD.gn:59: output = clearkeycdmadapter_rc_file
Should this be [clearkey_]adapter_resource_file like WV?

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.gni (right):

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:10: # TODO(GYP) On Mac/Linux this should be a
loadable_module.
I assume "GYP" TODOs will be resolved before switching the build system.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:43: # Note GYP sets rpath but this is set by
default on shared libraries.
Are you saying there is a difference between GYP and GN or that the GYP setting
is unnecessary?

When I implemented this, I found that rpath was set to the executable's current
directory. The problem is that the adapter and CDM may not be in the same
directory as the executable. Thus, the GYP code sets the rpath to *this*
binary's current directory so that implicit loading will work correctly.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:46: configs += [
"//build/config/compiler:no_size_t_to_int_warning" ]
Reference crbug.com/167187?

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:48: if (defined(invoker.all_dependent_configs))
{
Is this needed anywhere else? Is there a better solution planned?

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
File third_party/widevine/cdm/BUILD.gn (right):

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
third_party/widevine/cdm/BUILD.gn:6: import("//chrome/version.gni")
Same TODO?

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
third_party/widevine/cdm/BUILD.gn:82: visibility = [ ":*" ]
This is not in the clearkey BUILD.gn. Why do we need it here?

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
third_party/widevine/cdm/BUILD.gn:100: libs = [ "rt" ]
The GYP file only did this on Linux. Is this a nop on other platforms?

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
third_party/widevine/cdm/BUILD.gn:115: group("adapter") {
I assume this is the equivalent of 'type': 'none' in GYP.

[brettw, 2015-02-25 01:19:14]

The CQ bit was unchecked by brettw@chromium.org

[Dirk Pranke, 2015-02-25 01:19:19]

On 2015/02/25 00:51:32, brettw wrote:
> On 2015/02/25 00:23:19, Dirk Pranke wrote:
> > Is this formatted correctly? I'd expect this to be on one line.
> 
> sources and deps are always multi-line.

Hmm ... I'm not sure how I feel about that. I can certainly see the argument for
why they should be multi-line, but it also seems a bit inconsistent w/ all the
other ones not being multi-line.

[brettw, 2015-02-25 01:32:42]

New snap up

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/BUILD.gn
File media/cdm/ppapi/BUILD.gn (right):

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/BUILD.gn...
media/cdm/ppapi/BUILD.gn:59: output = clearkeycdmadapter_rc_file
On 2015/02/25 01:18:32, ddorwin wrote:
> Should this be [clearkey_]adapter_resource_file like WV?

These are just local variables so I'm not stressing about globally consistent
naming across disparate files. I just named them how I felt like at the time.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.gni (right):

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:10: # TODO(GYP) On Mac/Linux this should be a
loadable_module.
On 2015/02/25 01:18:32, ddorwin wrote:
> I assume "GYP" TODOs will be resolved before switching the build system.

Yes.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:43: # Note GYP sets rpath but this is set by
default on shared libraries.
THis is in the GN build, not GYP. I updated the comment.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:46: configs += [
"//build/config/compiler:no_size_t_to_int_warning" ]
On 2015/02/25 01:18:32, ddorwin wrote:
> Reference crbug.com/167187?

Done.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:48: if (defined(invoker.all_dependent_configs))
{
Most of these are unused for the cdm adapter targets but I made this template
support all the regular target variables to avoid surprising behavior later.

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
File third_party/widevine/cdm/BUILD.gn (right):

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
third_party/widevine/cdm/BUILD.gn:82: visibility = [ ":*" ]
On 2015/02/25 01:18:32, ddorwin wrote:
> This is not in the clearkey BUILD.gn. Why do we need it here?

Just nice hygiene. I added it in the other place.

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
third_party/widevine/cdm/BUILD.gn:100: libs = [ "rt" ]
On 2015/02/25 01:18:32, ddorwin wrote:
> The GYP file only did this on Linux. Is this a nop on other platforms?

Good catch

https://codereview.chromium.org/949233003/diff/40001/third_party/widevine/cdm...
third_party/widevine/cdm/BUILD.gn:115: group("adapter") {
On 2015/02/25 01:18:32, ddorwin wrote:
> I assume this is the equivalent of 'type': 'none' in GYP.

Yes

[ddorwin, 2015-02-25 02:07:56]

LGTM.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.gni (right):

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:43: # Note GYP sets rpath but this is set by
default on shared libraries.
On 2015/02/25 01:32:42, brettw wrote:
> THis is in the GN build, not GYP. I updated the comment.

Thanks.

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:48: if (defined(invoker.all_dependent_configs))
{
On 2015/02/25 01:32:42, brettw wrote:
> Most of these are unused for the cdm adapter targets but I made this template
> support all the regular target variables to avoid surprising behavior later.

Thanks. I was just curious if there should be some sort of macro or other
mechanism that passes all these things and can serve as a central place to make
updates if new members are added.

[ddorwin, 2015-02-25 02:08:44]

On 2015/02/25 02:07:56, ddorwin wrote:
> LGTM.
^ For media/ and third_party/widevine/

[brettw, 2015-02-25 02:19:54]

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
File media/cdm/ppapi/cdm_adapter.gni (right):

https://codereview.chromium.org/949233003/diff/40001/media/cdm/ppapi/cdm_adap...
media/cdm/ppapi/cdm_adapter.gni:48: if (defined(invoker.all_dependent_configs))
{
I've considered a better way to do this, but as in this case, every use
typically has one or two exceptions that make it difficult to come up with an
easy-to-understand rule. I've felt it's easier and clearer just to be annoyingly
verbose.

[brettw, 2015-02-25 02:20:01]

The CQ bit was checked by brettw@chromium.org

[brettw, 2015-02-25 02:20:01]

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

[commit-bot: I haz the power, 2015-02-25 02:21:18]

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

[commit-bot: I haz the power, 2015-02-25 04:01:38]

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

[commit-bot: I haz the power, 2015-02-25 04:01:40]

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

[brettw, 2015-02-25 17:51:11]

The CQ bit was checked by brettw@chromium.org

[brettw, 2015-02-25 17:51:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, ddorwin@chromium.org
Link to the patchset: https://codereview.chromium.org/949233003/#ps80001 (title:
"remove ppapi dep")

[commit-bot: I haz the power, 2015-02-25 17:52:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/949233003/80001

[commit-bot: I haz the power, 2015-02-25 18:47:08]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-02-25 18:48:34]

Patchset 5 (id:??) landed as
https://crrev.com/70d2f6ea98ae618b24a7c517570b74d20b92a7c7
Cr-Commit-Position: refs/heads/master@{#318083}