GN: special treatment of "configs" make it hard to use in templates |
|||
Issue description
The "configs" variable of "executable", "shared_library", ... targets is treated specially and initialised with default global values.
This means that when setting the variable in a target user has to use "configs += [ ... ]" and also means that it is difficult to write a template wrapping one of those targets while still allowing the user to expand "configs".
Let's use the following simple shared_library and try to write a template that will wrap it (an example of such a template is mac_framework in //build/config/mac/rules.gni):
config("config") {
# Some configuration required for foo.
}
shared_library("foo") {
sources = foo_sources
configs += [ ":config" ]
}
The obvious way to write such a template as either user get an error during generation or the configs used by the target lose the defaults.
template("shared_library_tmpl_1") {
shared_library(target_name) {
forward_variables_from(invoker, "*")
}
}
shared_library_tmpl_1("foo_tmpl_1") {
sources = foo_sources
configs += [ ":config" ]
}
shared_library_tmpl_1("foo_tmpl_2") {
sources = foo_sources
configs = [ ":config" ]
}
The "foo_tmpl_1" causes the following error while running "gn gen":
$ gn gen out/gn
ERROR at //foo/BUILD.gn:29:3: Undefined variable for +=.
configs += [ ":config" ]
^------
I don't have something with this name in scope now.
while "foo_tmpl_2" overrides the default configs and causes foo and foo_tmpl_2 resulting in probable compilation failure.
$ gn desc out/gn //foo:foo configs
//build/config:feature_flags
//build/config/compiler:compiler
//build/config/compiler:clang_stackrealign
//build/config/compiler:compiler_arm_fpu
//build/config/compiler:chromium_code
//build/config/compiler:default_include_dirs
//build/config/compiler:default_optimization
//build/config/compiler:default_symbols
//build/config/compiler:no_rtti
//build/config/compiler:runtime_library
//build/config/sanitizers:default_sanitizer_flags
//build/config/sanitizers:default_sanitizer_coverage_flags
//build/config/gcc:no_exceptions
//build/config/gcc:symbol_visibility_hidden
//build/config/clang:find_bad_constructs
//build/config/clang:extra_warnings
//build/config:debug
//build/config:default_libs
//build/config:shared_library_config
//ios/web:config
$ gn desc out/gn //foo:foo_tmpl_2 configs
//ios/web:config
The template needs to be written this way to work:
template("shared_library_tmpl_2") {
shared_library(target_name) {
forward_variables_from(invoker, "*", ["configs"])
if (defined(invoker.configs)) {
configs += invoker.configs
}
}
}
shared_library_tmpl_2("foo_tmpl_3") {
sources = foo_sources
configs = [ ":config" ]
}
Then we have the correct list of configs for foo_tmpl_3. However, I argue that this is difficult to write this template correctly due to how configs is treated.
,
Apr 11 2016
It also makes it much harder to remove a config in a wrapped "executable", "shared_library", ... target. See for example build/config/mac/rules.gni framework_bundle target in issue 1879493002. We need to compile the target without "//build/config/gcc:symbol_visibility_hidden", but we cannot just do "configs -= ..." and instead have to define an extra parameter to the template "removed_configs".
,
Apr 11 2016
See https://codereview.chromium.org/1879493002 (the issue id was not autolinked).
,
Apr 11 2016
I think GN's behavior actually captures what's going on fairly well. I'm not sure that any other way of having a default list of configs that you can modify per target is really much clearer. So, I think perhaps the thing to fix is the foo_tmpl_1 error; if the template was smart enough to defer processing of whether the variable was used or not until the template was expanded, things would work right, and the syntax would be consistent. I agree that tmpl_3 is confusing.
,
Apr 11 2016
It would be best to also fix https://bugs.chromium.org/p/chromium/issues/detail?id=602217 at the same time. I guess that if we are able to defer the +=/-= after the template is run somehow both bug should be fixed, but won't this introduce other issues?
,
Apr 11 2016
The behaviour makes much more sense when you realize that there's a set_defaults() call in BUILDCONFIG.gn https://code.google.com/p/chromium/codesearch#chromium/src/build/config/BUILDCONFIG.gn&q=buildconf&sq=package:chromium&l=571 The test() template deals with this by having a set_defaults() for test as well, then in the template using: configs = [] configs = invoker.configs
,
Apr 11 2016
If this is a common template type, you can use set_defaults in BUILDCONFIG.gn to initialize the default list of configs to be the one you want for your your target type. This is how the test and component templates work, where they behave exactly like some other target. The invoker can then do configs +=/-= like normal.
The default list of configs is local to BUILDCONFIG.gn so the set_defaults call needs to be in there to use it. set_defaults("test") is there for that reason, even thought the test template is in some .gni file. Since we only had one of these cases, this seemed less bad than introducing a bunch more global variables for this use. If we start doing this for more than just a couple of target types, we should probably export the default_executable_configs & friends from BUILDCONFIG.gn
,
Apr 11 2016
Well, I guess I'll need it for the following targets: ios_framework_bundle mac_framework_bundle probably also for: ios_app_bundle mac_app_bundle If you're fine with me adding other such uses of set_defaults, then I'll do just that.
,
Apr 11 2016
Yes, I'm fine with that. If we get too many we can fix the variable scoping.
,
Apr 12 2016
OK, this is working as intended I guess. Marking as WontFix and will use "set_defaults" for my templates (sending the CL to brettw@ for review). |
|||
►
Sign in to add a comment |
|||
Comment 1 Deleted