GN: Overwrite declared arg via command line |
|||||||
Issue descriptionCan v8_use_external_startup_data be overridden in a chromium build? On the one hand, there is the default, declared as a gn arg, which is true. On the other hand, there is “v8_use_external_startup_data = !is_ios” as a build override in chromium. There is no logic to not override if the user changes the gn arg. The same would hold for v8_optimized_debug. This would mean that the declared arg cannot be overwritten via command line.
,
May 31 2016
I tried out setting "v8_use_external_startup_data = true" as gen arg and got the following error:
gn args out/exp
Generating files...
ERROR at //BUILD.gn:14:1: Value collision.
import("//build_overrides/v8.gni")
^--------------------------------
This import contains "v8_use_external_startup_data"
See //build_overrides/v8.gni:17:32: defined here.
v8_use_external_startup_data = !is_ios
^------
Which would clobber the one in your current scope
See build arg file (use "gn args <out_dir>" to edit):9:32: defined here.
v8_use_external_startup_data = false
^----
Executing import should not conflict with anything in the current
scope unless the values are identical.
---------------------------------------------------------
Could we not just move the default from //build_overrides/v8.gni into v8? Something like:
declare_args() {
v8_use_external_startup_data = !is_ios
}
,
May 31 2016
,
Jun 2 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/c0774148d4f05eb8cf0267302ceecc7815346d18 commit c0774148d4f05eb8cf0267302ceecc7815346d18 Author: machenbach <machenbach@chromium.org> Date: Thu Jun 02 07:17:02 2016 [gn] Turn on external_startup_data by default except on ios This sets the default for the feature, as chromium expects it: It is turned on for all platforms except ios. Chromium's build_override can be removed after this. This will also allow to override the value as a gn arg. BUG= chromium:474921 , chromium:616034 NOTRY=true Review-Url: https://codereview.chromium.org/2025803003 Cr-Commit-Position: refs/heads/master@{#36666} [modify] https://crrev.com/c0774148d4f05eb8cf0267302ceecc7815346d18/BUILD.gn
,
Jun 21 2016
I believe this is fixed ...
,
Jun 21 2016
No - it's still pending on this one: https://codereview.chromium.org/2058033002/ Didn't have the time to clean it up.
,
Jul 1 2016
I don't think this is blocking the GN migration at this point, but please correct me if I've misremembered.
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0fffeb2adaa3c284b760922c1aecce1516b998ce commit 0fffeb2adaa3c284b760922c1aecce1516b998ce Author: machenbach <machenbach@chromium.org> Date: Mon Jul 18 19:42:08 2016 gn: Remove unnecessary v8 defaults Remove chromium defaults for v8_optimized_debug and v8_use_external_startup_data. This is not needed after v8 provides these defaults: https://codereview.chromium.org/2025803003/ https://codereview.chromium.org/2024833002/ It also interferes if somebody tries to override the gn args with a different value. BUG= chromium:616034 TBR=alokp@chromium.org, brettw@chromium.org Review-Url: https://codereview.chromium.org/2058033002 Cr-Commit-Position: refs/heads/master@{#406067} [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/BUILD.gn [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/build_overrides/v8.gni [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/chrome/BUILD.gn [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/chrome/installer/mini_installer/BUILD.gn [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/chrome/test/BUILD.gn [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/chrome/test/base/js2gtest.gni [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/chromecast/BUILD.gn [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/content/shell/BUILD.gn [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/content/test/BUILD.gn [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/gin/BUILD.gn [modify] https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce/net/BUILD.gn
,
Jul 18 2016
,
Jul 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac commit 3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac Author: grt <grt@chromium.org> Date: Mon Jul 18 19:59:19 2016 Revert of gn: Remove unnecessary v8 defaults (patchset #7 id:120001 of https://codereview.chromium.org/2058033002/ ) Reason for revert: Seems to have broken Google Chrome Mac builder: /b/c/b/Google_Chrome_Mac/src/buildtools/mac/gn gen //out/Release --check -> returned 1 ERROR at //chrome/BUILD.gn:514:7: Replacing nonempty list. remove_configs = [ "//build/config/mac:strip_all" ] ^------------- This overwrites a previously-defined nonempty list (length 2). See //v8/gni/v8.gni:55:3: for previous definition remove_configs += [ "//build/config/compiler:default_optimization" ] ^------------------------------------------------------------------ with another one (length 1). Did you mean "+=" to append instead? If you really want to do this, do remove_configs = [] before reassigning. See //BUILD.gn:202:7: which caused the file to be included. "//chrome", ^--------- GN gen failed: 1 Original issue's description: > gn: Remove unnecessary v8 defaults > > Remove chromium defaults for v8_optimized_debug and > v8_use_external_startup_data. > > This is not needed after v8 provides these defaults: > https://codereview.chromium.org/2025803003/ > https://codereview.chromium.org/2024833002/ > > It also interferes if somebody tries to override the gn args > with a different value. > > BUG= chromium:616034 > TBR=alokp@chromium.org, brettw@chromium.org > > Committed: https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce > Cr-Commit-Position: refs/heads/master@{#406067} TBR=jochen@chromium.org,dpranke@chromium.org,vogelheim@chromium.org,alokp@chromium.org,brettw@chromium.org,machenbach@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= chromium:616034 Review-Url: https://codereview.chromium.org/2160663003 Cr-Commit-Position: refs/heads/master@{#406072} [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/BUILD.gn [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/build_overrides/v8.gni [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/chrome/BUILD.gn [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/chrome/installer/mini_installer/BUILD.gn [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/chrome/test/BUILD.gn [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/chrome/test/base/js2gtest.gni [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/chromecast/BUILD.gn [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/content/shell/BUILD.gn [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/content/test/BUILD.gn [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/gin/BUILD.gn [modify] https://crrev.com/3c8546cbb39fecf4fcb0d0d29c9bca787ea1a7ac/net/BUILD.gn
,
Jul 20 2016
,
Jul 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/d02290ceb9c466e703b5386d2d28933c220ae4f7 commit d02290ceb9c466e703b5386d2d28933c220ae4f7 Author: machenbach <machenbach@chromium.org> Date: Wed Jul 20 09:16:27 2016 [gn] Properly qualify variable names that escape v8 context This prepares for relanding: https://codereview.chromium.org/2058033002/ All toplevel variables are visible in files including v8.gni. This works around potential name clashes. BUG= chromium:616034 Review-Url: https://codereview.chromium.org/2169443002 Cr-Commit-Position: refs/heads/master@{#37896} [modify] https://crrev.com/d02290ceb9c466e703b5386d2d28933c220ae4f7/gni/v8.gni
,
Jul 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d commit d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d Author: machenbach <machenbach@chromium.org> Date: Fri Jul 22 09:05:23 2016 gn: Remove unnecessary v8 defaults Remove chromium defaults for v8_optimized_debug and v8_use_external_startup_data. This is not needed after v8 provides these defaults: https://codereview.chromium.org/2025803003/ https://codereview.chromium.org/2024833002/ It also interferes if somebody tries to override the gn args with a different value. BUG= chromium:616034 TBR=alokp@chromium.org, brettw@chromium.org Committed: https://crrev.com/0fffeb2adaa3c284b760922c1aecce1516b998ce Review-Url: https://codereview.chromium.org/2058033002 Cr-Original-Commit-Position: refs/heads/master@{#406067} Cr-Commit-Position: refs/heads/master@{#407106} [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/BUILD.gn [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/build_overrides/v8.gni [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/chrome/BUILD.gn [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/chrome/installer/mini_installer/BUILD.gn [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/chrome/test/BUILD.gn [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/chrome/test/base/js2gtest.gni [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/chromecast/BUILD.gn [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/content/shell/BUILD.gn [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/content/test/BUILD.gn [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/gin/BUILD.gn [modify] https://crrev.com/d65ec5c9df401b4a60c6a57b2c9d5a539fe9191d/net/BUILD.gn
,
Jul 22 2016
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by machenb...@chromium.org
, May 31 2016