Issue metadata
Sign in to add a comment
|
Investigate WebRTC Windows compile coverage. |
||||||||||||||||||||||
Issue descriptionWe're lacking proper coverage on our Windows compilation trybots, which has been demonstrated by bug 756840 (GPU bots building with MSVC, catching errors we couldn't) and https://bugs.chromium.org/p/webrtc/issues/detail?id=8108 (size_t to int warning didn't fail our build). I suspect it's caused by our bots building with different flags than Chromium and/or change default for Clang for Windows.
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/8b844b16eb8d71313d6b5a06e9469f968fcb0446 commit 8b844b16eb8d71313d6b5a06e9469f968fcb0446 Author: kjellander <kjellander@webrtc.org> Date: Mon Aug 21 04:37:21 2017 Only disable -Wthread-safety-attributes warning for Clang. Clang is enabled by default and we currently lack coverage for building on Windows with MSVC (see crbug.com/757293 ). This should unblock rolling WebRTC into Chromium DEPS. We need to improve our trybot coverage for standalone WebRTC to prevent things like this in the future though ( crbug.com/756840 ). BUG= webrtc:8119 , chromium:756840 , chromium:757293 TBR=hbos@webrtc.org NOTRY=True Review-Url: https://codereview.webrtc.org/3003473002 Cr-Commit-Position: refs/heads/master@{#19420} [modify] https://crrev.com/8b844b16eb8d71313d6b5a06e9469f968fcb0446/webrtc/modules/video_coding/BUILD.gn
,
Aug 21 2017
I read some more in 82385 and it appears Windows was flipped to Clang at Jul 28 in https://chromium.googlesource.com/chromium/src/+/d2c91228a51bdf37ae3b2e501fb53c0528f1629c but then recently flipped back 2 days ago in https://chromium.googlesource.com/chromium/src.git/+/7cd2f6bd73c6a9c6200704e995b18ebb3168ead6 Our bots are using these same configurations, so our coverage should be fine, and we should probably keep our Win Clang bots until this flip sticks. What we should probably do is to add att least one bot that explicitly sets is_clang=false so we can catch errors like bug 756840 next time Clang is flipped on Windows. I'll set that up.
,
Aug 21 2017
In the two flip-CLs mentioned above, Chromium flips their Clang-specific bots to build with MSVC instead, so they don't lose coverage. As we didn't do the same (we didn't notice), we were bitten by a compile regression on MSVC during their first flip-attempt. I'll CC some of us to bug 82385 so we can stay informed when the next attempt is going to be made.
,
Aug 21 2017
+charujain as trooper, since the above explains some of the errors blocking the autoroller
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/c2d4d577552c1cac19155a2a36796cd7435554e7 commit c2d4d577552c1cac19155a2a36796cd7435554e7 Author: kjellander <kjellander@webrtc.org> Date: Mon Aug 21 09:57:53 2017 MB: Add configs for Windows bots enforcing MSVC compilation. BUG= chromium:757293 NOTRY=True Review-Url: https://codereview.webrtc.org/2996343002 Cr-Commit-Position: refs/heads/master@{#19422} [modify] https://crrev.com/c2d4d577552c1cac19155a2a36796cd7435554e7/tools_webrtc/mb/mb_config.pyl
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/0534141056df48e999bde31c88605ff66147aa7d commit 0534141056df48e999bde31c88605ff66147aa7d Author: Henrik Kjellander <kjellander@chromium.org> Date: Mon Aug 21 10:02:15 2017 WebRTC: Add Windows bots building explicitly with MSVC enabled. This should protect us from breakages during the transition to using Clang as the default compiler on Windows. See http://crbug.com/82385 for details. Required MB changes are done in https://codereview.webrtc.org/2996343002/ BUG= 757293 TBR=ehmaldonado@chromium.org Change-Id: Iad73a157121e72ac087b16237ac931d32b8e4850 Reviewed-on: https://chromium-review.googlesource.com/622791 Reviewed-by: Henrik Kjellander <kjellander@chromium.org> Commit-Queue: Henrik Kjellander <kjellander@chromium.org> [modify] https://crrev.com/0534141056df48e999bde31c88605ff66147aa7d/masters/master.tryserver.webrtc/slaves.cfg [add] https://crrev.com/0534141056df48e999bde31c88605ff66147aa7d/scripts/slave/recipes/webrtc/standalone.expected/tryserver_webrtc_win_msvc_rel.json [modify] https://crrev.com/0534141056df48e999bde31c88605ff66147aa7d/masters/master.client.webrtc/master_win_cfg.py [modify] https://crrev.com/0534141056df48e999bde31c88605ff66147aa7d/masters/master.tryserver.webrtc/master.cfg [add] https://crrev.com/0534141056df48e999bde31c88605ff66147aa7d/scripts/slave/recipes/webrtc/standalone.expected/client_webrtc_win32_release__msvc_.json [modify] https://crrev.com/0534141056df48e999bde31c88605ff66147aa7d/scripts/slave/recipe_modules/webrtc/builders.py [modify] https://crrev.com/0534141056df48e999bde31c88605ff66147aa7d/masters/master.client.webrtc/slaves.cfg
,
Aug 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/external/webrtc.git/+/7542bf80e2ec4cce6ccdcd70843521e59a62101c commit 7542bf80e2ec4cce6ccdcd70843521e59a62101c Author: kjellander <kjellander@webrtc.org> Date: Mon Aug 21 16:51:47 2017 CQ: Add win_msvc_rel to default trybot set. BUG= chromium:757293 TBR=mbonadei@webrtc.org NOTRY=True Review-Url: https://codereview.webrtc.org/3002063002 Cr-Commit-Position: refs/heads/master@{#19434} [modify] https://crrev.com/7542bf80e2ec4cce6ccdcd70843521e59a62101c/infra/config/cq.cfg
,
Aug 22 2017
Remaining task for this bug is to investigate why we couldn't catch the error that was fixed in https://chromium-review.googlesource.com/c/external/webrtc/+/623949 on our bots. I will do some tests once we've unblocked rolling deps (tracked in https://bugs.chromium.org/p/webrtc/issues/detail?id=8109).
,
Aug 29 2017
I think we're good here now as rolling is unblocked. |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by kjellander@chromium.org
, Aug 21 2017