Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1910333002:
Add MB configs for ClangToTLinux* bots on FYI (Closed)
DescriptionAdd MB configs for ClangToTLinux* bots on FYI
BUG=481692
R=dpranke@chromium.org
Patch Set 1 #Patch Set 2 : Rebase Update #
Total comments: 4
Messages
Total messages: 22 (8 generated)
PTAL. This is the set of Linux AND Clang bots.
lgtm
The CQ bit was checked by aneeshm@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1910333002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1910333002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...) mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by aneeshm@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org Link to the patchset: https://codereview.chromium.org/1910333002/#ps20001 (title: "Rebase Update")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1910333002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1910333002/20001
Okay, I double-checked the bots and it looks like there might be one mistake. I'll see if I can catch this CL before it lands and patch it; if not, we'll need to fix it in a follow-up CL. This change won't have an effect until we enable MB on the bot, so it shouldn't break anything if it lands as-is. https://codereview.chromium.org/1910333002/diff/20001/tools/mb/mb_config.pyl File tools/mb/mb_config.pyl (right): https://codereview.chromium.org/1910333002/diff/20001/tools/mb/mb_config.pyl#... tools/mb/mb_config.pyl:1537: 'static', 'release', Hm. Looks like this bot specifies "release_extra_cflags=-fno-sanitize-recover=undefined" as part of its GYP_DEFINES, which isn't part of this set (or something supported by GN, for that matter). Maybe fix that in a follow-up CL?
The CQ bit was unchecked by dpranke@chromium.org
thakis@chromium.org changed reviewers: + thakis@chromium.org
Looks roughly right, thanks! Probably easiest to turn on mb one-by-one and compare flags before and after the switch for each bot as its switched. mb keeps setting GYP_DEFINES while hooks run, right? I think update.py looks at some of these defines. https://codereview.chromium.org/1910333002/diff/20001/tools/mb/mb_config.pyl File tools/mb/mb_config.pyl (right): https://codereview.chromium.org/1910333002/diff/20001/tools/mb/mb_config.pyl#... tools/mb/mb_config.pyl:1549: 'swarming_gyp_clang_no_chrome_plugins_linux_dump_symbols_shared_release': [ can you add the comment # Enable debug info, as on official builders, to catch issues with # optimized debug info. from the recipes file here? I was wondering why this was here
krasin@google.com changed reviewers: + krasin@google.com
https://codereview.chromium.org/1910333002/diff/20001/tools/mb/mb_config.pyl File tools/mb/mb_config.pyl (right): https://codereview.chromium.org/1910333002/diff/20001/tools/mb/mb_config.pyl#... tools/mb/mb_config.pyl:1537: 'static', 'release', Good catch! This was introduced due to an LLVM bug: https://code.google.com/p/chromium/codesearch#chromium/build/scripts/slave/re... which is still not fixed: https://llvm.org/bugs/show_bug.cgi?id=25569
On 2016/04/22 00:53:23, Nico wrote: > Looks roughly right, thanks! Probably easiest to turn on mb one-by-one and > compare flags before and after the switch for each bot as its switched. > That tends to be painfully slow. I think it's easy enough to enable multiple bots at once and check them all (which is what we/I usually do). > mb keeps setting GYP_DEFINES while hooks run, right? I think update.py looks at > some of these defines. Sort of. The GYP_DEFINES that are set during runhooks come from the bot configs, not from MB, so they have to still be kept in sync. But, yes, they are still set, but we set also set GYP_CHROMIUM_NO_ACTION so that GYP itself is skipped.
On 2016/04/22 00:57:30, Dirk Pranke wrote: > On 2016/04/22 00:53:23, Nico wrote: > > Looks roughly right, thanks! Probably easiest to turn on mb one-by-one and > > compare flags before and after the switch for each bot as its switched. > > > > That tends to be painfully slow. I think it's easy enough to enable multiple > bots > at once and check them all (which is what we/I usually do). That's ok, I can do that (it's just adding a single 'mb' line to a recipe, right?). I'd prefer to check that nothing gets accidentally lost.
https://codereview.chromium.org/1910333002/diff/20001/tools/mb/mb_config.pyl File tools/mb/mb_config.pyl (right): https://codereview.chromium.org/1910333002/diff/20001/tools/mb/mb_config.pyl#... tools/mb/mb_config.pyl:1549: 'swarming_gyp_clang_no_chrome_plugins_linux_dump_symbols_shared_release': [ On 2016/04/22 00:53:23, Nico wrote: > can you add the comment > > # Enable debug info, as on official builders, to catch issues with > # optimized debug info. > > from the recipes file here? I was wondering why this was here Yes, I can add that comment.
On 2016/04/22 01:05:06, Nico wrote: > On 2016/04/22 00:57:30, Dirk Pranke wrote: > > On 2016/04/22 00:53:23, Nico wrote: > > > Looks roughly right, thanks! Probably easiest to turn on mb one-by-one and > > > compare flags before and after the switch for each bot as its switched. > > > > > > > That tends to be painfully slow. I think it's easy enough to enable multiple > > bots > > at once and check them all (which is what we/I usually do). > > That's ok, I can do that (it's just adding a single 'mb' line to a recipe, > right?). I'd prefer to check that nothing gets accidentally lost. If you like. Are you going to do that for all of the other Clang bots as well? I think aneeshm@ will hopefully have a CL for the rest of them to review tomorrow.
Description was changed from ========== Add MB configs for ClangToTLinux* bots on FYI BUG=481692 R=dpranke@chromium.org ========== to ========== Add MB configs for ClangToTLinux* bots on FYI BUG=481692 R=dpranke@chromium.org ==========
Message was sent while issue was closed.
Closing in favor of https://codereview.chromium.org/1908293002/
Message was sent while issue was closed.
yeah, i can switch all of them once the configs are in place and check that everything stays the same. thanks for helping with this :-) |
