Add GN files for chrome_elf

Add GN files for chrome_elf

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

[Slava Chigrin, 2015-01-15 21:28:19]

vchigrin@yandex-team.ru changed reviewers:
+ caitkp@chromium.org, robertshield@chromium.org

[Slava Chigrin, 2015-01-15 21:34:13]

https://codereview.chromium.org/832583007/diff/1/chrome_elf/BUILD.gn
File chrome_elf/BUILD.gn (right):

https://codereview.chromium.org/832583007/diff/1/chrome_elf/BUILD.gn#newcode193
chrome_elf/BUILD.gn:193: shared_library("blacklist_test_dll_1") {
Not sure, whether this is correct equivalent for GYP loadable_module. Could
anybody help, what difference between loadable_module and shared_library in GYP?
I search GYP documentation but did not found mentions of loadable_module....

Thank you in advance!

[Slava Chigrin, 2015-01-18 20:59:09]

vchigrin@yandex-team.ru changed reviewers:
+ brettw@chromium.org

[Slava Chigrin, 2015-01-18 21:27:07]

Since there is a lot of non-trivial work in GN, decided to add brettw@.

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

https://codereview.chromium.org/832583007/diff/20001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:193: shared_library("blacklist_test_dll_1") {
Not sure, whether this is correct equivalent for GYP loadable_module. Could
anybody help, what difference between loadable_module and shared_library in GYP?
I search GYP documentation but did not found mentions of loadable_module....

Thank you in advance!

[robertshield, 2015-01-19 01:24:17]

I defer to Brett for the structure and syntax of the .gn files.

Chrome_elf relies on chrome.exe's imports being ordered just so during build.

Please ensure that the test here still passes with this CL:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome_elf/elf_imp...

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

https://codereview.chromium.org/832583007/diff/20001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:193: shared_library("blacklist_test_dll_1") {
On 2015/01/18 21:27:06, Slava Chigrin wrote:
> Not sure, whether this is correct equivalent for GYP loadable_module. Could
> anybody help, what difference between loadable_module and shared_library in
GYP?
> I search GYP documentation but did not found mentions of loadable_module....
> 
> Thank you in advance!

Ack, there was a difference between shared_library and loadable_module,
hopefully this is expressable in gn. I *believe* that a loadable_module target
means generate a dll without an import lib and also modifies gyp's dependency
calculation (my memory here is fuzzy).

This was important to some of the blacklist tests iirc.

[Slava Chigrin, 2015-01-19 12:41:23]

Thank you for review.
These targets can't be build on vanilla master, since they require chrome.exe,
that currently can not be build on Windows with GN. Although locally I managed
to compile them (I have a bunch of fixes in windows GN, that I currently
upstreaming to chromium). On latest patch all tests from chrome_elf_unittests
pass. Although I had to modify compiler options (see changes in
build/config/compiler/BUILD.gn), since without them chrome_elf.dll has some
parasitic imports, that cause ELFImportsTest.ChromeElfSanityCheck test to fail.
Seems, GYP build has the same options, that I've added here.

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

https://codereview.chromium.org/832583007/diff/20001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:193: shared_library("blacklist_test_dll_1") {
On 2015/01/19 01:24:17, robertshield wrote:
> On 2015/01/18 21:27:06, Slava Chigrin wrote:
> > Not sure, whether this is correct equivalent for GYP loadable_module. Could
> > anybody help, what difference between loadable_module and shared_library in
> GYP?
> > I search GYP documentation but did not found mentions of loadable_module....
> > 
> > Thank you in advance!
> 
> Ack, there was a difference between shared_library and loadable_module,
> hopefully this is expressable in gn. I *believe* that a loadable_module target
> means generate a dll without an import lib and also modifies gyp's dependency
> calculation (my memory here is fuzzy).
> 
> This was important to some of the blacklist tests iirc.

Ah, understand, hope, placing blacklist_test_dll_1 in data_deps rather then in
deps solves this problem. At least all unit tests of chrome_elf passes.

https://codereview.chromium.org/832583007/diff/40001/build/config/compiler/BU...
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/832583007/diff/40001/build/config/compiler/BU...
build/config/compiler/BUILD.gn:859: "/Gw",
Like that in GYP
https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&...

https://codereview.chromium.org/832583007/diff/40001/build/config/compiler/BU...
build/config/compiler/BUILD.gn:862: common_optimize_on_ldflags = [ "/OPT:REF" ]
Like that
https://code.google.com/p/chromium/codesearch#chromium/src/build/internal/rel...

[Slava Chigrin, 2015-01-21 22:18:59]

Rebased and added test.gni import (it become necessary after recent changes in
repository).

https://codereview.chromium.org/832583007/diff/60001/build/config/compiler/BU...
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/832583007/diff/60001/build/config/compiler/BU...
build/config/compiler/BUILD.gn:860: "/Gw",
Like GYP
https://code.google.com/p/chromium/codesearch#chromium/src/build/common.gypi&...

https://codereview.chromium.org/832583007/diff/60001/build/config/compiler/BU...
build/config/compiler/BUILD.gn:863: common_optimize_on_ldflags = [ "/OPT:REF" ]
Like GYP

https://code.google.com/p/chromium/codesearch#chromium/src/build/internal/rel...

[brettw, 2015-01-22 20:12:10]

Thanks for looking at this!

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn
File chrome_elf/BUILD.gn (right):

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:29: configs += [ "//build/config/win:windowed" ]
I think you'll also need to do
  configs -= [ "//build/config/win:console" ]

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:40: source_set("chrome_elf_lib") {
I would normally not duplicate the directory name in the target name. So this
would just be "lib" and below we would have "constants", "common", etc. Source
sets can have non-unique names.

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:169: group("chrome_elf_unittests") {
GN can do this better which will let us clean up this file. Delete this target
and name the test file "chrome_elf_unittests". Then in the unittest target add:
  data_deps = [
    ":chrome_elf",
    "//chrome",
  ]
This will cause these things to get built but not linked with the target.

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:191: [ "/DEF:" +
rebase_path("blacklist/test/blacklist_test_main_dll.def") ]
I usually add the second parameter to rebase path:
  rebase_path("blacklist/test/blacklist_test_main_dll.def", root_build_dir)
which will make the path relative, shorten the command lines a bit and make them
look a bit better. Not required.

[Slava Chigrin, 2015-01-22 22:06:00]

Thank you for review, uploaded new version.

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn
File chrome_elf/BUILD.gn (right):

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:29: configs += [ "//build/config/win:windowed" ]
On 2015/01/22 20:12:10, brettw wrote:
> I think you'll also need to do
>   configs -= [ "//build/config/win:console" ]

Done.

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:40: source_set("chrome_elf_lib") {
On 2015/01/22 20:12:10, brettw wrote:
> I would normally not duplicate the directory name in the target name. So this
> would just be "lib" and below we would have "constants", "common", etc. Source
> sets can have non-unique names.

Done.

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:169: group("chrome_elf_unittests") {
On 2015/01/22 20:12:10, brettw wrote:
> GN can do this better which will let us clean up this file. Delete this target
> and name the test file "chrome_elf_unittests". Then in the unittest target
add:
>   data_deps = [
>     ":chrome_elf",
>     "//chrome",
>   ]
> This will cause these things to get built but not linked with the target.

Done.

https://codereview.chromium.org/832583007/diff/60001/chrome_elf/BUILD.gn#newc...
chrome_elf/BUILD.gn:191: [ "/DEF:" +
rebase_path("blacklist/test/blacklist_test_main_dll.def") ]
On 2015/01/22 20:12:10, brettw wrote:
> I usually add the second parameter to rebase path:
>   rebase_path("blacklist/test/blacklist_test_main_dll.def", root_build_dir)
> which will make the path relative, shorten the command lines a bit and make
them
> look a bit better. Not required.

Done.

[brettw, 2015-01-22 22:17:48]

lgtm

https://codereview.chromium.org/832583007/diff/100001/chrome_elf/BUILD.gn
File chrome_elf/BUILD.gn (right):

https://codereview.chromium.org/832583007/diff/100001/chrome_elf/BUILD.gn#new...
chrome_elf/BUILD.gn:136: "//base:base_static",
But this above "//sandbox" for sorting purposes.

https://codereview.chromium.org/832583007/diff/100001/chrome_elf/BUILD.gn#new...
chrome_elf/BUILD.gn:160: ":blacklist",
Move these two :blacklist ones to the top.

https://codereview.chromium.org/832583007/diff/100001/chrome_elf/BUILD.gn#new...
chrome_elf/BUILD.gn:167: "//chrome",
At bottom.

[Slava Chigrin, 2015-01-23 09:59:46]

Done, Thank you for review!

https://codereview.chromium.org/832583007/diff/100001/chrome_elf/BUILD.gn
File chrome_elf/BUILD.gn (right):

https://codereview.chromium.org/832583007/diff/100001/chrome_elf/BUILD.gn#new...
chrome_elf/BUILD.gn:136: "//base:base_static",
On 2015/01/22 22:17:48, brettw wrote:
> But this above "//sandbox" for sorting purposes.

Done.

https://codereview.chromium.org/832583007/diff/100001/chrome_elf/BUILD.gn#new...
chrome_elf/BUILD.gn:160: ":blacklist",
On 2015/01/22 22:17:48, brettw wrote:
> Move these two :blacklist ones to the top.

Done.

https://codereview.chromium.org/832583007/diff/100001/chrome_elf/BUILD.gn#new...
chrome_elf/BUILD.gn:167: "//chrome",
On 2015/01/22 22:17:48, brettw wrote:
> At bottom.

Done.

[Slava Chigrin, 2015-01-23 10:24:40]

The CQ bit was checked by vchigrin@yandex-team.ru

[commit-bot: I haz the power, 2015-01-23 10:25:30]

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

[commit-bot: I haz the power, 2015-01-23 11:12:49]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2015-01-23 11:13:54]

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