GN: forward_variables_from interacts badly with file-scope variables |
||
Issue description
third_party/Source/wtf/BUILD.gn declares visibility at the top and then defines a "wtf" component:
visibility = [ "//third_party/WebKit/*" ]
...
component("wtf") {
...
}
This visibility has no effect because component() does:
forward_variables_from(invoker, "*")
and forwarding all variables doesn't look in nested scopes or it would duplicate all global variables every time it was called. This means visibility is not picked up in this case.
This is a general problem with forwarding all variables. Since there is a lot of stuff in the global scope, I'm reluctant to forward everything from nested scopes.
One compromise might be if we knew what various scopes meant and never forwarded stuff from the global BUILDCONFIG scope, it would match people's mental model of what's happening in most cases. But this is more difficult to implement and will still be surprising in some cases.
For now, I'm planning on special-casing visibility since it's the main thing we do at the file scope.
,
Mar 14 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/58c2b3a90b634d71ccc8de68417789f21a59a6eb commit 58c2b3a90b634d71ccc8de68417789f21a59a6eb Author: brettw <brettw@chromium.org> Date: Mon Mar 14 19:35:56 2016 Forward visibility explicitly in GN component. As described in the added comment, this picks up file-scope visibility in components, where before it did not apply. Fixes //ui/ozone:ozone's visibility which was not being applied because of this. I believe the intent of the ozone target is to be public and the rest should be file-local. BUG= 594610 Review URL: https://codereview.chromium.org/1800703002 Cr-Commit-Position: refs/heads/master@{#381043} [modify] https://crrev.com/58c2b3a90b634d71ccc8de68417789f21a59a6eb/build/config/BUILDCONFIG.gn [modify] https://crrev.com/58c2b3a90b634d71ccc8de68417789f21a59a6eb/ui/ozone/BUILD.gn
,
Mar 14 2016
I'm going to call this fixed because it's not clear how much more we can or should do.
,
Feb 5 2018
I just ran into (what I think might be) this issue with a V8 checkout:
$ ninja -C out/x64.debug d8 -j 240 -l 12
ninja: Entering directory `out/x64.debug'
[1/1] Regenerating ninja files
FAILED: build.ninja
../../../chromium/src/buildtools/mac/gn --root=../.. -q gen .
ERROR at //build/config/compiler/BUILD.gn:201:12: Assignment had no effect.
inputs = []
^
You set the variable "inputs" here and it was unused before it went
out of scope.
See //build/config/BUILDCONFIG.gn:518:3: which caused the file to be included.
"//build/config/compiler:afdo",
^-----------------------------
ninja: error: rebuilding 'build.ninja': subcommand failed
Error! - V8 compilation finished with errors.
Is this the same issue (but with a different variable) or is something else going on?
,
Feb 5 2018
mathiasb: For inputs to work on configs you need a GN >= 528762. To fix that, V8 should roll to the HEAD of the buildtools repo. |
||
►
Sign in to add a comment |
||
Comment 1 by brettw@chromium.org
, Mar 14 2016