GN: disallow imports from BUILDCONFIG.gn |
|
Issue descriptionCurrently it seems you can import .gni files from the BUILDCONFIG.gn file. Chrome currently doesn't do this, but this operation should be explicitly disallowed by GN. The problem is that .gni files are executed independently of each other and BUILD.gn files, and then merged in to where they're needed. This independent execution context is only the global state set up by BUILDCONFIG.gn. But when you import something from BUILDCONFIG, what is the context for executing the .gni file? I didn't check the code but it's either getting no context or the partially constructed one from when the first import statement was generated. Both of these are bad because the .gni file will expect this stuff to be set up. Forthermore, the result of executing the .gni file in this context will be cached and applied to further imports. If that file is used elsewhere it will give unexpected results.
,
Apr 3 2018
If it's only imported from BUILDCONFIG.gn, it's not a problem, but then nothing guarantees that. An alternative would be to guarantee such a file is never imported anywhere else. But I think allowing it is pretty weird, since you have to keep in mind when editing the .gni file that the global state will be only partially (or not at all) set up.
,
Apr 3 2018
For reference, we (Vivaldi) also imports the chromium dotgn file into our dotgn file. Also, our BUILDCONFIG file, which is prepended (using our multi-file extension) before the chromium BUILDCONFIG file (since import of the file does not work, due to what is described in Bug 828214 ), also imports a couple of other files, needed to configure the variables before the chromium build config is loaded. The reason for the imports is to separate their content into function specific units, rather than one file for everything.
,
Apr 5 2018
Did a bit of testing: Converting import to an inline operation for build config is a 4 line patch |
|
►
Sign in to add a comment |
|
Comment 1 by dpranke@chromium.org
, Apr 3 2018