Move //build/module_args/v8.gni to //build_overrides.

Move //build/module_args/v8.gni to //build_overrides.

The original intent of the build/module_args directory was to
provide a place where different repos could customize settings
as needed depending on their dependencies and needs.

However, given that you can't really yet embed GN by just
deps-ing in //build/config and //build/toolchain, you end up
needing to deps-in all of build, and so there was no good way to
have different settings per-repo in //build/module_args.

This CL changes the approach such that we will have an additional
top-level directory called //build_overrides, which is therefore
separate from //build and can be properly customized.

It is unfortunate that we need to use two top-level directories
for GN, but we don't have a good alternative at this time. Once
we can remove the GYP build and further clean up the structure
and dependencies of //build we will hopefully be able to do better.

R=brettw@chromium.org
BUG=541791
Committed: https://crrev.com/d4da5ab4987c2c22d96dcf8c4b3ac2449aa16c0f
Cr-Commit-Position: refs/heads/master@{#353690}

[Dirk Pranke, 2015-10-09 23:22:20]

The CQ bit was checked by dpranke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-10-09 23:24:10]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1397493004/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1397493004/1

[brettw, 2015-10-09 23:44:08]

lgtm

[commit-bot: I haz the power, 2015-10-10 00:28:06]

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

[commit-bot: I haz the power, 2015-10-10 00:28:06]

Dry run: This issue passed the CQ dry run.

[tfarina, 2015-10-10 18:25:28]

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

[tfarina, 2015-10-10 18:25:28]

Dirk, do you mind putting a README file explaining what should goes in in
build_overrides?

[Dirk Pranke, 2015-10-10 18:32:21]

On 2015/10/10 18:25:28, tfarina wrote:
> Dirk, do you mind putting a README file explaining what should goes in in
> build_overrides?

Sure, I can do that.

[Dirk Pranke, 2015-10-13 04:15:36]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2015-10-13 04:15:43]

The CQ bit was unchecked by dpranke@chromium.org

[Dirk Pranke, 2015-10-13 04:27:59]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2015-10-13 04:28:00]

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/1397493004/#ps40001
(title: "gn formatting")

[commit-bot: I haz the power, 2015-10-13 04:29:35]

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

[tfarina, 2015-10-13 04:40:46]

I looked at everything. I have a few questions that could help me understand
what is going on here, but I guess this lgtm.

https://codereview.chromium.org/1397493004/diff/40001/build/module_args/v8.gni
File build/module_args/v8.gni (right):

https://codereview.chromium.org/1397493004/diff/40001/build/module_args/v8.gn...
build/module_args/v8.gni:6: # to import //build_overrides/v8.gni instead.
Will v8 DEPS build_overrides? Looks like this might answer my other question.
And things start to make sense :)

https://codereview.chromium.org/1397493004/diff/40001/build_overrides/README.md
File build_overrides/README.md (right):

https://codereview.chromium.org/1397493004/diff/40001/build_overrides/README....
build_overrides/README.md:9: have its own copy of a file, where each file
defines a set of variables
by 'a file' you mean that which product like v8 or webrtc will have to have a
v8.gni, webrtc.gni in their trees? Can the name be anything they want or does it
need to match exactly with the one build_overrides?

[Dirk Pranke, 2015-10-13 04:44:16]

The CQ bit was unchecked by dpranke@chromium.org

[Dirk Pranke, 2015-10-13 04:45:35]

On 2015/10/13 04:40:46, tfarina wrote:
> I looked at everything. I have a few questions that could help me understand
> what is going on here, but I guess this lgtm.
> 
> https://codereview.chromium.org/1397493004/diff/40001/build/module_args/v8.gni
> File build/module_args/v8.gni (right):
> 
>
https://codereview.chromium.org/1397493004/diff/40001/build/module_args/v8.gn...
> build/module_args/v8.gni:6: # to import //build_overrides/v8.gni instead.
> Will v8 DEPS build_overrides? Looks like this might answer my other question.
> And things start to make sense :)

No, they have their own copy with values that might be different from the ones
in the chromium tree.

> 
>
https://codereview.chromium.org/1397493004/diff/40001/build_overrides/README.md
> File build_overrides/README.md (right):
> 
>
https://codereview.chromium.org/1397493004/diff/40001/build_overrides/README....
> build_overrides/README.md:9: have its own copy of a file, where each file
> defines a set of variables
> by 'a file' you mean that which product like v8 or webrtc will have to have a
> v8.gni, webrtc.gni in their trees? Can the name be anything they want or does
it
> need to match exactly with the one build_overrides?

Yes, they would have a v8.gni. Other files in //v8 then
import("//build_overrides/v8.gni") and get the
values that are customized to the checkout. 

I will try to reword things in the README to be clearer.

[Dirk Pranke, 2015-10-13 04:53:57]

Okay, I tried again.

tfarina, what do you think?

[tfarina, 2015-10-13 05:11:49]

SLGTM. Thanks for adding this README file, it really helped me understand how
this is going to work and how it will have to be pushed to the third_party
projects.

https://codereview.chromium.org/1397493004/diff/60001/build_overrides/README.md
File build_overrides/README.md (right):

https://codereview.chromium.org/1397493004/diff/60001/build_overrides/README....
build_overrides/README.md:10: in a standalone configuration, v8 can create a
file, build_overrides/v8.gni,
Right, I think with this example I can see how this looks like.

v8 in their tree will have to create a build_overrides/ directory and create a
v8.gni.

Then here in our tree (chromium) we will also have a build_overrides that does
what? 'overrides' their variables.

Very clever! :)

If they are fine with having to create this build_overrides/ directory, I don't
know, but who am I, right? I think Henrik was happy at least ;)

[Dirk Pranke, 2015-10-13 05:34:18]

The CQ bit was checked by dpranke@chromium.org

[Dirk Pranke, 2015-10-13 05:34:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
tfarina@chromium.org, brettw@chromium.org
Link to the patchset: https://codereview.chromium.org/1397493004/#ps60001
(title: "rework README to hopefully be clearer")

[commit-bot: I haz the power, 2015-10-13 05:34:54]

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

[commit-bot: I haz the power, 2015-10-13 06:20:45]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-13 06:21:36]

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

[kjellander_chromium, 2015-10-13 07:31:34]

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

[kjellander_chromium, 2015-10-13 07:31:35]

Thanks for doing this! I got a question below.

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

https://codereview.chromium.org/1397493004/diff/60001/build/config/BUILD.gn#n...
build/config/BUILD.gn:11: import("//build_overrides/v8.gni")
Having this being included in the //build tree will enforce us to create a dummy
v8.gni to put into our own //build_overrides directory of WebRTC, which feels
wrong.
It's not a big deal, but it's likely to break for us every time a new file is
introduced (other projects etc).

Without it, I get a failure about that file being missing when I tried to roll
past this CL. See https://codereview.webrtc.org/1403453003/#ps1 for context
(I'll add a build_overrides dir as a workaround in PS#2).

https://codereview.chromium.org/1397493004/diff/60001/build_overrides/README.md
File build_overrides/README.md (right):

https://codereview.chromium.org/1397493004/diff/60001/build_overrides/README....
build_overrides/README.md:10: in a standalone configuration, v8 can create a
file, build_overrides/v8.gni,
On 2015/10/13 05:11:49, tfarina wrote:
> Right, I think with this example I can see how this looks like.
> 
> v8 in their tree will have to create a build_overrides/ directory and create a
> v8.gni.
> 
> Then here in our tree (chromium) we will also have a build_overrides that does
> what? 'overrides' their variables.
> 
> Very clever! :)
> 
> If they are fine with having to create this build_overrides/ directory, I
don't
> know, but who am I, right? I think Henrik was happy at least ;)
> 

Yeah, I'm happy for the WebRTC use case, since we face similar problems as V8,
being both standalone and a part of Chromium.

[Dirk Pranke, 2015-10-13 17:11:41]

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

https://codereview.chromium.org/1397493004/diff/60001/build/config/BUILD.gn#n...
build/config/BUILD.gn:11: import("//build_overrides/v8.gni")
On 2015/10/13 07:31:35, kjellander (chromium) wrote:
> Having this being included in the //build tree will enforce us to create a
dummy
> v8.gni to put into our own //build_overrides directory of WebRTC, which feels
> wrong.

It should feel wrong, because it probably is wrong :). I think, ideally, //build
should not import or depend on anything outside of //build.

However, we're not there yet, and we're going to have to land a bunch of
CLs to clean things up.

In the meantime, yeah, you'll have to create a dummy v8.gni. Hopefully we can
avoid needing any others and get rid of this one.