Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2116183002:
Land chromium-side work to clean up handling of v8_target_cpu in the GN build. (Closed)
|
Created:
4 years, 5 months ago by Dirk Pranke Modified:
4 years, 5 months ago CC:
chromium-reviews, Nico Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionLand chromium-side work to clean up handling of v8_target_cpu in the GN build.
Currently v8_target_cpu can only be set to one particular architecture,
and that won't work for monochrome/webview builds where we need
to be able to build two different snapshots for two different architectures.
The way things are set are also confusing for when you need to do builds
for a target_cpu that is different from the host_cpu and the value of the
v8_target_cpu might get out of sync between target and host.
This change changes all that by making the cpu that v8 targets
a function of the current toolchain (thus declaring a v8_current_cpu
and using that instead).
R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org
BUG=625353
Committed: https://crrev.com/8a2de90db035b90a891f0b980ab6162fd3995499
Cr-Commit-Position: refs/heads/master@{#405551}
Patch Set 1 #Patch Set 2 : for review #
Total comments: 14
Patch Set 3 : rework to use custom_toolchain, not buildconfig #Patch Set 4 : work around two-sided-patch limitation #Patch Set 5 : fix missing import #
Messages
Total messages: 46 (22 generated)
Description was changed from ========== chromium-side work in progress to handle v8_target_cpu properly. BUG=621581 ========== to ========== chromium-side work to clean up handling of v8_target_cpu. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=621581 ==========
dpranke@chromium.org changed reviewers: + brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org
Okay, I think this should all work correctly now (in conjunction with the v8-side change in https://codereview.chromium.org/2116913002/). Please take a look? https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... File build/config/BUILDCONFIG.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:162: is_msan = false I don't like this being here, but the only alternative I see is to make everyone who wants to do an MSan build to also have to set v8_target_cpu explicitly in their args file, and I don't know if that's acceptable. If we do want to make that change, I would prefer to land this as-is for now to unblock progress on monochrome and the lkgr builders, and then deal with that in a follow-up CL.
Description was changed from ========== chromium-side work to clean up handling of v8_target_cpu. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=621581 ========== to ========== chromium-side work to clean up handling of v8_target_cpu. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=621581 ==========
machenbach@chromium.org changed reviewers: + machenbach@chromium.org
non-owner lgtm and a few questions and suggestions. https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... File build/config/BUILDCONFIG.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:153: # mips_arch_variant) to be set to their defaults either if the current_cpu nit: For consistency, maybe write either "current_cpu or v8_current_cpu" (like it's implemented) or "target_cpu or v8_target_cpu" (might be easier to understand). https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:162: is_msan = false On 2016/07/07 02:42:00, Dirk Pranke wrote: > I don't like this being here, but the only alternative I see is to make everyone > who wants to do an MSan build to also have to set v8_target_cpu explicitly in > their args file, and I don't know if that's acceptable. > > If we do want to make that change, I would prefer to land this as-is for now to > unblock progress on monochrome and the lkgr builders, and then deal with that in > a follow-up CL. I'd find it ok if developers need to specify v8_target_cpu explicitly. In the infrastructure we talk about max 5 bots, where we'd need to add this config. Furthermore: It is not incorrect to run x64 v8 with msan. Using the simulator just finds a few more bugs in v8, but is also slower. I think this condition below is pure convenience. https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:178: declare_args() { Curiosity: Is there a semantic difference in having a second declare_args block? Is this purely for depending on v8_target_cpu from the other block? https://codereview.chromium.org/2116183002/diff/20001/build/toolchain/linux/B... File build/toolchain/linux/BUILD.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/toolchain/linux/B... build/toolchain/linux/BUILD.gn:41: clang_toolchain("clang_x86_v8_arm") { Should we add here the other combinations we use in v8, too? E.g. x86_v8_mipsel, etc. Or can we make our own toolchain definitions in the v8 standalone configs?
lgtm
https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... File build/config/BUILDCONFIG.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:153: # mips_arch_variant) to be set to their defaults either if the current_cpu On 2016/07/07 07:24:22, Michael Achenbach wrote: > nit: For consistency, maybe write either "current_cpu or v8_current_cpu" (like > it's implemented) or "target_cpu or v8_target_cpu" (might be easier to > understand). Will do (using target_cpu). https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:162: is_msan = false On 2016/07/07 07:24:22, Michael Achenbach wrote: > On 2016/07/07 02:42:00, Dirk Pranke wrote: > > I don't like this being here, but the only alternative I see is to make > everyone > > who wants to do an MSan build to also have to set v8_target_cpu explicitly in > > their args file, and I don't know if that's acceptable. > > > > If we do want to make that change, I would prefer to land this as-is for now > to > > unblock progress on monochrome and the lkgr builders, and then deal with that > in > > a follow-up CL. > > I'd find it ok if developers need to specify v8_target_cpu explicitly. In the > infrastructure we talk about max 5 bots, where we'd need to add this config. > > Furthermore: It is not incorrect to run x64 v8 with msan. Using the simulator > just finds a few more bugs in v8, but is also slower. I think this condition > below is pure convenience. Ah, I thought that x64 v8 msan didn't work at all. If it does work, then I agree that we should probably just get rid of this and update the bots (and devs). I'd still like to do that in a follow-on CL, to make things easier, though. Let's see what brettw@ says. https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:178: declare_args() { On 2016/07/07 07:24:22, Michael Achenbach wrote: > Curiosity: Is there a semantic difference in having a second declare_args block? Generically speaking, yes. See crbug.com/542846. This particular usage is a little different than what you see in that bug, though. > Is this purely for depending on v8_target_cpu from the other block? This is done this way so that we can both get the right default value for v8_current_cpu if v8_target_cpu isn't set by the user, and also get the right value if it is set. https://codereview.chromium.org/2116183002/diff/20001/build/toolchain/linux/B... File build/toolchain/linux/BUILD.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/toolchain/linux/B... build/toolchain/linux/BUILD.gn:41: clang_toolchain("clang_x86_v8_arm") { On 2016/07/07 07:24:23, Michael Achenbach wrote: > Should we add here the other combinations we use in v8, too? E.g. x86_v8_mipsel, > etc. Or can we make our own toolchain definitions in the v8 standalone configs? You can do either as you like. It might be easier to do them here for now.
Description was changed from ========== chromium-side work to clean up handling of v8_target_cpu. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=621581 ========== to ========== Land chromium-side work to clean up handling of v8_target_cpu in the GN build. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=621581 ==========
https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... File build/config/BUILDCONFIG.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:162: is_msan = false I have a negative reaction to this since I spent a lot of effort trying to get this exact variable (with the other *san ones) out of this file. It this is just to support a specific config with a few bots, I think we should just specify the toolchain we want on those bots. https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:167: if (v8_target_cpu == "") { I really think we should avoid having V8 stuff in this file. WHat other options are there?
Thanks, LGTM with 2 comments, let me know once the patch is ready, I will patch it locally to verify with my monochrome patches https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... File build/config/BUILDCONFIG.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:180: # toolchains. It should never be explicitly set by the user. The v8_current_cpu will show up in the result of "gn args <out_dir> --list", right? beside check the comment, is there other way to tell that this variable actually shouldn't be set as args. https://codereview.chromium.org/2116183002/diff/20001/build/toolchain/linux/B... File build/toolchain/linux/BUILD.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/toolchain/linux/B... build/toolchain/linux/BUILD.gn:41: clang_toolchain("clang_x86_v8_arm") { On 2016/07/07 17:28:12, Dirk Pranke wrote: > On 2016/07/07 07:24:23, Michael Achenbach wrote: > > Should we add here the other combinations we use in v8, too? E.g. > x86_v8_mipsel, > > etc. Or can we make our own toolchain definitions in the v8 standalone > configs? > > You can do either as you like. It might be easier to do them here for now. Yes, We need define x86_v8_mipsel and x64_v8_mips64el now since you already use them in v8 side.
From IM which may get lost: If you're passing args to toolchains, that implies this is all to support non-default toolchains. In this case, any setting of default_toolchain in BUILDCONFIG is a no-op. You already know what toolchain you're running under at that point. The default_toolchain setup in BUILDCONFIG is only to bootstrap the default one when there is no context. So I'm not actually sure what the V8 stuff in BUILDCONFIG is doing at all.
Description was changed from ========== Land chromium-side work to clean up handling of v8_target_cpu in the GN build. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=621581 ========== to ========== Land chromium-side work to clean up handling of v8_target_cpu in the GN build. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=625353 ==========
https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... File build/config/BUILDCONFIG.gn (right): https://codereview.chromium.org/2116183002/diff/20001/build/config/BUILDCONFI... build/config/BUILDCONFIG.gn:162: is_msan = false On 2016/07/07 17:56:21, brettw (plz ping after 24h) wrote: > I have a negative reaction to this since I spent a lot of effort trying to get > this exact variable (with the other *san ones) out of this file. > > It this is just to support a specific config with a few bots, I think we should > just specify the toolchain we want on those bots. > I'm not totally sure if x64 msan would work well with x64 v8. Maybe double check with kcc and eugenis. But even if it wouldn't, we could still require the extra setting of v8_target_cpu as gn arg and maybe assert the correct configuration deeper in the build configs (e.g. in v8).
Updates: Brett and I talked about this patch for a while yesterday and sketeched out another potential approach that doesn't require stuff to get hard-coded into BUILDCONFIG.gn. I'll work on that and see if it actually works, and if so, update this patch and the related CLs to see if everyone agrees that it will also be acceptable. @michaelbai - you should be able to test things with the existing patches, and if you hit issues, it would be good to know about them sooner rather than later.
On 2016/07/08 17:24:46, Dirk Pranke wrote: > Updates: Brett and I talked about this patch for a while yesterday and sketeched > out another potential approach > that doesn't require stuff to get hard-coded into BUILDCONFIG.gn. > > I'll work on that and see if it actually works, and if so, update this patch and > the related CLs to see if > everyone agrees that it will also be acceptable. > > @michaelbai - you should be able to test things with the existing patches, and > if you hit issues, it > would be good to know about them sooner rather than later. I tried, this patch worked for Monochrome
Patchset #3 (id:40001) has been deleted
Okay, as discussed w/ brettw@ offline, I've produced a version of this patch that backs out all of the changes to BUILDCONFIG.gn and changes the logic in v8_target_cpu.gni somewhat. Now, you can set which CPU to target by either setting `v8_target_cpu="arm64"` as before, or by setting `custom_toolchain="//build/toolchain/linux:clang_x64_v8_arm64"`. If you do the former, the default toolchain will be ":clang_x64", but it'll actually act as if it was ":clang_x64_v8_arm64", which is potentially a bit confusing. So, on things like builders, you probably want to use custom_toolchain instead. Regardless, this keeps the v8 and sanitizer logic out of BUILDCONFIG, which is a good thing, so I think overall this patch is an improvement. Please take another look?
lgtm Tried this patch, it still works :)
lgtm
lgtm
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jochen@chromium.org Link to the patchset: https://codereview.chromium.org/2116183002/#ps60001 (title: "rework to use custom_toolchain, not buildconfig")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...) cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by dpranke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
The CQ bit was checked by dpranke@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by dpranke@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from michaelbai@chromium.org, brettw@chromium.org, jochen@chromium.org, machenbach@chromium.org Link to the patchset: https://codereview.chromium.org/2116183002/#ps100001 (title: "fix missing import")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Land chromium-side work to clean up handling of v8_target_cpu in the GN build. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=625353 ========== to ========== Land chromium-side work to clean up handling of v8_target_cpu in the GN build. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=625353 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:100001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Land chromium-side work to clean up handling of v8_target_cpu in the GN build. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=625353 ========== to ========== Land chromium-side work to clean up handling of v8_target_cpu in the GN build. Currently v8_target_cpu can only be set to one particular architecture, and that won't work for monochrome/webview builds where we need to be able to build two different snapshots for two different architectures. The way things are set are also confusing for when you need to do builds for a target_cpu that is different from the host_cpu and the value of the v8_target_cpu might get out of sync between target and host. This change changes all that by making the cpu that v8 targets a function of the current toolchain (thus declaring a v8_current_cpu and using that instead). R=brettw@chromium.org, jochen@chromium.org, michaelbai@chromium.org BUG=625353 Committed: https://crrev.com/8a2de90db035b90a891f0b980ab6162fd3995499 Cr-Commit-Position: refs/heads/master@{#405551} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/8a2de90db035b90a891f0b980ab6162fd3995499 Cr-Commit-Position: refs/heads/master@{#405551}
Message was sent while issue was closed.
On 2016/07/14 20:11:51, commit-bot: I haz the power wrote: > Patchset 5 (id:??) landed as > https://crrev.com/8a2de90db035b90a891f0b980ab6162fd3995499 > Cr-Commit-Position: refs/heads/master@{#405551} Since v8-side change (https://codereview.chromium.org/2116913002) GN gen for Chromium Linux mipsel is failing due to missing toolchain definitions for mipsel: ERROR Unresolved dependencies. //chrome/test:browser_tests_js_webui_action(//build/toolchain/linux:clang_mipsel) needs //v8:v8_shell(//build/toolchain/linux:clang_x86_v8_mipsel) //:chromium_builder_asan(//build/toolchain/linux:clang_mipsel) needs //v8:v8_shell(//build/toolchain/linux:clang_x86_v8_mipsel) //v8:run_mksnapshot(//build/toolchain/linux:clang_mipsel) needs //v8:mksnapshot(//build/toolchain/linux:clang_x86_v8_mipsel) //chrome/test:unit_tests_js_action(//build/toolchain/linux:clang_mipsel) needs //v8:v8_shell(//build/toolchain/linux:clang_x86_v8_mipsel) This can be seen at buildbot: http://www.rt-rk.com/mips-buildbot/builders/build_and_test_chromium_trunk_for... I noticed this comment in review above: " > Should we add here the other combinations we use in v8, too? E.g. x86_v8_mipsel, > etc. Or can we make our own toolchain definitions in the v8 standalone configs? You can do either as you like. It might be easier to do them here for now. " Is someone working on adding missing mipsel and mips64el toolchain definitions? If not, I can add these definitions. Same is happening for Chromium on Android for mipsel: https://build.chromium.org/p/chromium.android/builders/Android%20MIPS%20Build... http://www.rt-rk.com/mips-buildbot/builders/Release_build/builds/1938/steps/B...
Message was sent while issue was closed.
On 2016/07/18 12:33:45, lmilko wrote: > On 2016/07/14 20:11:51, commit-bot: I haz the power wrote: > > Patchset 5 (id:??) landed as > > https://crrev.com/8a2de90db035b90a891f0b980ab6162fd3995499 > > Cr-Commit-Position: refs/heads/master@{#405551} > > Since v8-side change (https://codereview.chromium.org/2116913002) GN gen for > Chromium Linux mipsel is failing due to missing toolchain definitions for > mipsel: > > ERROR Unresolved dependencies. > //chrome/test:browser_tests_js_webui_action(//build/toolchain/linux:clang_mipsel) > needs //v8:v8_shell(//build/toolchain/linux:clang_x86_v8_mipsel) > //:chromium_builder_asan(//build/toolchain/linux:clang_mipsel) > needs //v8:v8_shell(//build/toolchain/linux:clang_x86_v8_mipsel) > //v8:run_mksnapshot(//build/toolchain/linux:clang_mipsel) > needs //v8:mksnapshot(//build/toolchain/linux:clang_x86_v8_mipsel) > //chrome/test:unit_tests_js_action(//build/toolchain/linux:clang_mipsel) > needs //v8:v8_shell(//build/toolchain/linux:clang_x86_v8_mipsel) > > This can be seen at buildbot: > http://www.rt-rk.com/mips-buildbot/builders/build_and_test_chromium_trunk_for... > > I noticed this comment in review above: > " > > Should we add here the other combinations we use in v8, too? E.g. > x86_v8_mipsel, > > etc. Or can we make our own toolchain definitions in the v8 standalone > configs? > > You can do either as you like. It might be easier to do them here for now. > " > > Is someone working on adding missing mipsel and mips64el toolchain definitions? > If not, I can add these definitions. > > Same is happening for Chromium on Android for mipsel: > https://build.chromium.org/p/chromium.android/builders/Android%20MIPS%20Build... > http://www.rt-rk.com/mips-buildbot/builders/Release_build/builds/1938/steps/B... I'll see if I can add them today.
Message was sent while issue was closed.
I've posted an attempt to define the needed toolchains in https://codereview.chromium.org/2159003002/ .
Message was sent while issue was closed.
This has another small glitch, which I filed in https://bugs.chromium.org/p/chromium/issues/detail?id=629825 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
