Replace V8 build_overrides with GN default_args |
||
Issue descriptionGN has a new default_args features for configuring repository-specific flags. We should switch the V8 flags to use this.
,
Jan 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7b20d2e9e9113b3ad2ee1db858e8d16d2b947058 commit 7b20d2e9e9113b3ad2ee1db858e8d16d2b947058 Author: brettw <brettw@chromium.org> Date: Tue Jan 24 01:43:19 2017 Remove unnecessary uses of build_overrides/v8.gni. I think this file used to have more stuff in it like v8_use_external_startup_data. This flag has now moved to v8/gni/v8.gni. BUG= 684096 Review-Url: https://codereview.chromium.org/2652853002 Cr-Commit-Position: refs/heads/master@{#445584} [modify] https://crrev.com/7b20d2e9e9113b3ad2ee1db858e8d16d2b947058/BUILD.gn [modify] https://crrev.com/7b20d2e9e9113b3ad2ee1db858e8d16d2b947058/chrome/BUILD.gn [modify] https://crrev.com/7b20d2e9e9113b3ad2ee1db858e8d16d2b947058/chrome/android/BUILD.gn [modify] https://crrev.com/7b20d2e9e9113b3ad2ee1db858e8d16d2b947058/content/shell/android/BUILD.gn
,
Jan 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d6e10d81b19b35794e834f6bd602d892b2bf2f9 commit 3d6e10d81b19b35794e834f6bd602d892b2bf2f9 Author: pdfium-deps-roller <pdfium-deps-roller@chromium.org> Date: Tue Jan 24 02:16:40 2017 Roll src/third_party/pdfium/ 4022f87eb..7a82a6294 (1 commit). https://pdfium.googlesource.com/pdfium.git/+log/4022f87eb871..7a82a6294a34 $ git log 4022f87eb..7a82a6294 --date=short --no-merges --format='%ad %ae %s' 2017-01-23 brettw Remove obsolete include of build_overrides/v8.gni BUG= 684096 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md If the roll is causing failures, see: http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls TBR=dsinclair@chromium.org Review-Url: https://codereview.chromium.org/2649923004 Cr-Commit-Position: refs/heads/master@{#445602} [modify] https://crrev.com/3d6e10d81b19b35794e834f6bd602d892b2bf2f9/DEPS
,
Jan 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8a08c1ded65a1e9271afc3872f88180edae0c5ce commit 8a08c1ded65a1e9271afc3872f88180edae0c5ce Author: brettw <brettw@chromium.org> Date: Tue Jan 24 18:04:37 2017 Explicitly set some V8 build flags These are currently implicitly set if undefined in v8/BUILD.gn. As part of moving to the new default_args system, this will need to be explicit to roll the new V8 in. This should be a build no-op. BUG= 684096 Review-Url: https://codereview.chromium.org/2648383003 Cr-Commit-Position: refs/heads/master@{#445757} [modify] https://crrev.com/8a08c1ded65a1e9271afc3872f88180edae0c5ce/build_overrides/v8.gni
,
Jan 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cc77cea884ab97d4eab6abc8c7f809377941a382 commit cc77cea884ab97d4eab6abc8c7f809377941a382 Author: brettw <brettw@chromium.org> Date: Tue Jan 24 22:16:47 2017 Explicitly set v8_enable_gdbjit_default for v8 overrides. This is a fix for https://codereview.chromium.org/2648383003 where v8_enable_gdbjit should actually be v8_enable_gdbjit_default. This was pointed out in the review for https://codereview.chromium.org/2654663003/ BUG= 684096 R=machenbach@chromium.org Review-Url: https://codereview.chromium.org/2652083002 Cr-Commit-Position: refs/heads/master@{#445835} [modify] https://crrev.com/cc77cea884ab97d4eab6abc8c7f809377941a382/build_overrides/v8.gni
,
Jan 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/04a0e487ece54c7e1568f53e3dc8b247dcf6ca74 commit 04a0e487ece54c7e1568f53e3dc8b247dcf6ca74 Author: brettw <brettw@chromium.org> Date: Wed Jan 25 21:29:59 2017 Use default_args for configuring V8. Removes the values in build_overrides/v8.gni and uses the new default_args in the "//.gn" file to set these arguments. Once all places are updated, we can remove build_overrides/v8.gni. BUG= 684096 Review-Url: https://codereview.chromium.org/2648233006 Cr-Commit-Position: refs/heads/master@{#446124} [modify] https://crrev.com/04a0e487ece54c7e1568f53e3dc8b247dcf6ca74/.gn [modify] https://crrev.com/04a0e487ece54c7e1568f53e3dc8b247dcf6ca74/build_overrides/v8.gni
,
Jan 26 2017
This feature causes error when running "gn gen" with "target_os" set to "ios".
$ gn gen --args='target_os="ios"' out/xxx
ERROR at //.gn:24:28: Build argument has no effect.
v8_extra_library_files = [
^
The variable "v8_extra_library_files" was set as a build argument
but never appeared in a declare_args() block in any buildfile.
To view all possible args, run "gn args --list <builddir>"
Done. Made 2833 targets from 577 files in 1191ms
Would it be possible to fix this in gn (as iOS does not want to depends on v8 just to silence the warning)? Maybe do not warn about unused args if they are declared in default_args.
,
Jan 26 2017
sdefresne: This makes sense.
,
Jan 27 2017
Is there any plan to address this issue? Should we revert the CL?
,
Jan 27 2017
I'll do a mitigation today. It can't be easily reverted (requires a V8 roll).
,
Jan 27 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08d54e1ba0047c6b42710d99a3687caadbdc3df8 commit 08d54e1ba0047c6b42710d99a3687caadbdc3df8 Author: brettw <brettw@chromium.org> Date: Fri Jan 27 21:28:42 2017 Add dummy V8 build args to prevent for the iOS build. This is to prevent unused build overrides on iOS where V8 is not used but the default_arg overrides in //.gn are still present. Longer-term we should probably not warn for unused args in the dotfile and then these can be removed. BUG= 684096 TBR=sdefresne Review-Url: https://codereview.chromium.org/2656903006 Cr-Commit-Position: refs/heads/master@{#446779} [modify] https://crrev.com/08d54e1ba0047c6b42710d99a3687caadbdc3df8/ios/BUILD.gn
,
Feb 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e97ef561379aadc367fcde3ebc5b37fa4001796f commit e97ef561379aadc367fcde3ebc5b37fa4001796f Author: brettw <brettw@chromium.org> Date: Tue Feb 07 22:36:58 2017 Remove build_overrides/v8.gni. This is no longer used by V8 and has been replaced by the default_args variable in the //.gn file. BUG= 684096 Review-Url: https://codereview.chromium.org/2660523002 Cr-Commit-Position: refs/heads/master@{#448749} [delete] https://crrev.com/46dad8ad40bd4d9e579974ee96568dd9b5cf6551/build_overrides/v8.gni
,
Feb 7 2017
,
Feb 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3273e13c90a9aca77955c9a386bc66f57dded38f commit 3273e13c90a9aca77955c9a386bc66f57dded38f Author: mthiesse <mthiesse@chromium.org> Date: Wed Feb 08 00:15:33 2017 Revert of Remove build_overrides/v8.gni. (patchset #2 id:20001 of https://codereview.chromium.org/2660523002/ ) Reason for revert: build_overrides/v8.gni is still used by clank/java/BUILD.gn Original issue's description: > Remove build_overrides/v8.gni. > > This is no longer used by V8 and has been replaced by the default_args variable > in the //.gn file. > > BUG= 684096 > > Review-Url: https://codereview.chromium.org/2660523002 > Cr-Commit-Position: refs/heads/master@{#448749} > Committed: https://chromium.googlesource.com/chromium/src/+/e97ef561379aadc367fcde3ebc5b37fa4001796f TBR=machenbach@chromium.org,brettw@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 684096 Review-Url: https://codereview.chromium.org/2684833002 Cr-Commit-Position: refs/heads/master@{#448813} [add] https://crrev.com/3273e13c90a9aca77955c9a386bc66f57dded38f/build_overrides/v8.gni
,
Feb 9 2017
The following revision refers to this bug: https://chrome-internal.googlesource.com/clank/internal/apps/+/77f15355248c15d8b4c6822a23732eda6e0b9142 commit 77f15355248c15d8b4c6822a23732eda6e0b9142 Author: Brett Wilson <brettw@chromium.org> Date: Thu Feb 09 21:38:30 2017
,
Feb 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b54d63099501ccd240c5ab1d4e8f2ddbb0015b2d commit b54d63099501ccd240c5ab1d4e8f2ddbb0015b2d Author: brettw <brettw@chromium.org> Date: Thu Feb 09 23:12:37 2017 Remove build_overrides/v8.gni. This is no longer used by V8 and has been replaced by the default_args variable in the //.gn file. BUG= 684096 Review-Url: https://codereview.chromium.org/2660523002 Cr-Commit-Position: refs/heads/master@{#449452} [delete] https://crrev.com/24835036681c326052b092b9697333ad9f4c2a9b/build_overrides/v8.gni
,
Feb 10 2017
FYI: I filed https://bugs.chromium.org/p/webrtc/issues/detail?id=7141 for similar work in WebRTC.
,
Feb 20 2017
Hi brettw@, I am not sure if I missed something but default_args seems to break building chromium for chromeos. I followed simple chrome instruction to build chromium for chromeos. http://www.chromium.org/chromium-os/how-tos-and-troubleshooting/building-chromium-browser But I failed at gn gen step after entering sdk: (sdk kevin R58-9282.0.0) cychiang@cychiang-z840 ~/work/chromium/src $ gn gen out_$SDK_BOARD/Release --args="$GN_ARGS" ERROR at //.gn:30:16: Assignment had no effect. default_args = { ^ You set the variable "default_args" here and it was unused before it went out of scope. Could you please take a look? Thanks!
,
Feb 20 2017
That error probably means your GN binary is out-of-date.
,
Feb 22 2017
Hi brettw@, I found that I was on the wrong branch in src/v8 so it did not use default_args. After running gclient sync again the problem was gone. Thanks again!
,
Apr 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7ce21e2df98546eb664d6cbcd11a592d1599dd48 commit 7ce21e2df98546eb664d6cbcd11a592d1599dd48 Author: sdefresne <sdefresne@chromium.org> Date: Thu Apr 06 15:15:23 2017 [ios] Fix warning when running "gn gen". Fix the following warning (incorrectly labelled as an error): ERROR at //.gn:44:25: Build argument has no effect. v8_enable_inspector = true ^--- The variable "v8_enable_inspector" was set as a build argument but never appeared in a declare_args() block in any buildfile. BUG= 684096 Review-Url: https://codereview.chromium.org/2799203003 Cr-Commit-Position: refs/heads/master@{#462476} [modify] https://crrev.com/7ce21e2df98546eb664d6cbcd11a592d1599dd48/ios/BUILD.gn
,
Apr 6 2017
I'm seeing the same error that comment #21 on a regular Windows checkout/build.
,
Jun 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/994145519d481aa8f691a06299486fb533128832 commit 994145519d481aa8f691a06299486fb533128832 Author: Sylvain Defresne <sdefresne@chromium.org> Date: Wed Jun 07 08:35:02 2017 Fix a warning when running "gn gen" with target_os="ios". Add declaration of gn argument v8_check_microtasks_scopes_consistency to src/ios/BUILD.gn to silence the warning when running "gn gen". Add a comment to .gn to remind people to keep in sync src/ios/BUILD.gn. BUG= 684096 Change-Id: If9a083a0474366ec1eb488fcb8fee6f739d96bea Reviewed-on: https://chromium-review.googlesource.com/525574 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#477582} [modify] https://crrev.com/994145519d481aa8f691a06299486fb533128832/.gn [modify] https://crrev.com/994145519d481aa8f691a06299486fb533128832/ios/BUILD.gn
,
Dec 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a37803c95fe4014438ae1687a8ff2301c40e3ca9 commit a37803c95fe4014438ae1687a8ff2301c40e3ca9 Author: Petr Hosek <phosek@chromium.org> Date: Mon Dec 11 23:43:45 2017 Don't print warning for unused default args We will still produce an error if --fail-on-unused-args is used. Bug: 684096 Change-Id: I4a0640ad83072750d555c8de9e6391f76942ae38 Reviewed-on: https://chromium-review.googlesource.com/792606 Commit-Queue: Petr Hosek <phosek@chromium.org> Reviewed-by: Scott Graham <scottmg@chromium.org> Reviewed-by: Brett Wilson <brettw@chromium.org> Cr-Commit-Position: refs/heads/master@{#523254} [modify] https://crrev.com/a37803c95fe4014438ae1687a8ff2301c40e3ca9/tools/gn/args.cc [modify] https://crrev.com/a37803c95fe4014438ae1687a8ff2301c40e3ca9/tools/gn/args.h [modify] https://crrev.com/a37803c95fe4014438ae1687a8ff2301c40e3ca9/tools/gn/setup.cc |
||
►
Sign in to add a comment |
||
Comment 1 by brettw@chromium.org
, Jan 24 2017