Allow GN configs to have sub-configs.

Allow GN configs to have sub-configs.

Some of the configs are getting unwieldy (the "compiler" one) and some are used to just forward to other variants like default_symbols and default_optimization.

This adds the ability for a config to have configs to add flags by reference. This allows large configs to be split apart into more logical units, and allows forwarding configs like default_optimization to be written in the obvious way.

An example of what this enables is https://codereview.chromium.org/1341373002/

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

[brettw, 2015-09-15 20:13:35]

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

[brettw, 2015-09-15 20:14:12]

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

[commit-bot: I haz the power, 2015-09-15 20:14:25]

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

[commit-bot: I haz the power, 2015-09-15 20:56:41]

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

[commit-bot: I haz the power, 2015-09-15 20:56:42]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2015-09-15 21:31:31]

this mostly looks okay, but I'd like to be consistent on the terminology. 

"nested configs" makes it sound like we're allowing configs to be assigned
lexically inside other configs, which is not what this is.

"sub-configs" doesn't have a clear meaning to me at all.

Can we use something like "dependent configs"? That seems like the right
concept, more-or-less.

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config.cc
File tools/gn/config.cc (right):

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config.cc#newc...
tools/gn/config.cc:32: // Nested configs, flatten.
As noted elsewhere, I don't like the use of "nested" here.

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config.h
File tools/gn/config.h (right):

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config.h#newco...
tools/gn/config.h:17: // A config can contain other configs. We track both the
data assigned directly
"contain" seems like a confusing way to describe this. Does "depend on" work?

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config.h#newco...
tools/gn/config.h:21: // flags.
I don't think I understand this sentence; aren't you essentially doing the
recursion inline?

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config.h#newco...
tools/gn/config.h:32: // The values set directly on this config. This will not
contain datra from
typo: "data"

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config_unittes...
File tools/gn/config_unittest.cc (right):

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config_unittes...
tools/gn/config_unittest.cc:61: TEST(Config, DoublyNested) {
same comment.

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config_values.cc
File tools/gn/config_values.cc (right):

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config_values....
tools/gn/config_values.cc:36: #undef APPEND_VALUES
I'm not sure the macro enhances readability much here.

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config_values.h
File tools/gn/config_values.h (left):

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/config_values....
tools/gn/config_values.h:74: DISALLOW_COPY_AND_ASSIGN(ConfigValues);
why did this get deleted?

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/variables.cc
File tools/gn/variables.cc (right):

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/variables.cc#n...
tools/gn/variables.cc:547: "     not normally relevant and complicates the
implementation.)\n"
How hard would it be to assert that a config is either only used as part of a
composite, or only used not as part of a composite? It seems like you really
want things to be exclusive-or here in order to avoid weirdness.

I'm guessing that might be a fair amount of bookkeeping?

[brettw, 2015-09-16 19:26:33]

I standardized on "subconfig" to avoid confusion with "all dependent configs".

New snap up.

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/variables.cc
File tools/gn/variables.cc (right):

https://codereview.chromium.org/1342183003/diff/20001/tools/gn/variables.cc#n...
tools/gn/variables.cc:547: "     not normally relevant and complicates the
implementation.)\n"
Yes, that's basically the same amount of bookkeeping as doing the full thing. I
actually started writing it without this limitation but decided it wasn't worth
the effort. However, I added a description of how I think it should work in
config.cc since I've spent a fair bit of time thinking about it.

[brettw, 2015-09-16 19:26:37]

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

[commit-bot: I haz the power, 2015-09-16 19:27:28]

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

[commit-bot: I haz the power, 2015-09-16 20:43:00]

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

[commit-bot: I haz the power, 2015-09-16 20:43:00]

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

[brettw, 2015-09-16 20:44:51]

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

[commit-bot: I haz the power, 2015-09-16 20:46:52]

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

[Dirk Pranke, 2015-09-16 21:03:20]

lgtm, thanks

https://codereview.chromium.org/1342183003/diff/40001/tools/gn/config.cc
File tools/gn/config.cc (right):

https://codereview.chromium.org/1342183003/diff/40001/tools/gn/config.cc#newc...
tools/gn/config.cc:41: // list. To the same for Target.configs_ when a target is
resolved. This
typo: "do the same"

[brettw, 2015-09-16 21:33:33]

The CQ bit was checked by brettw@chromium.org

[brettw, 2015-09-16 21:33:33]

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/1342183003/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-09-16 21:34:08]

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

[commit-bot: I haz the power, 2015-09-16 22:31:39]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-09-16 22:32:10]

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