Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2838253002:
mb: Don't use minial_symbols for Clang ToT debug builders (Closed)
Descriptionmb: Don't use minimal_symbols for Clang ToT debug builders
The last Clang roll slipped through the net of ToT and CQ builders though it
would immediately assert on Mac debug builds using the regular symbol level.
Make sure the debug ToT bots don't build with minimal_symbols so we can catch
this in the future.
BUG=714769
Review-Url: https://codereview.chromium.org/2838253002
Cr-Commit-Position: refs/heads/master@{#467156}
Committed: https://chromium.googlesource.com/chromium/src/+/7fa45855fb213b42550245183ee37ce33d5e0902
Patch Set 1 #
Messages
Total messages: 16 (5 generated)
hans@chromium.org changed reviewers: + thakis@chromium.org
Please take a look. Do you remember if there's a reason why these used minimal_symbol? I couldn't see any recent changes to this.
On 2017/04/25 20:37:15, hans wrote: > Please take a look. > > Do you remember if there's a reason why these used minimal_symbol? I couldn't > see any recent changes to this. Typo "minial_symbols". Have you looked through git history? ...I just did this for you, and it looks like the setting comes from here: https://codereview.chromium.org/1155993004 It sounds like neither author nor reviewer remember. Maybe the builder/tester setup means we push a lot more data with this disabled? Do we want to keep minimal_symbols for some of the bots, for more test coverage?
On 2017/04/25 20:52:45, Nico wrote: > On 2017/04/25 20:37:15, hans wrote: > > Please take a look. > > > > Do you remember if there's a reason why these used minimal_symbol? I couldn't > > see any recent changes to this. > > Typo "minial_symbols". > > Have you looked through git history? I was lazy and didn't look past https://codereview.chromium.org/2357483002 > ...I just did this for you, and it looks > like the setting comes from here: https://codereview.chromium.org/1155993004 > > It sounds like neither author nor reviewer remember. Maybe the builder/tester > setup means we push a lot more data with this disabled? That could be it, I suppose. I guess we'll find out. > Do we want to keep minimal_symbols for some of the bots, for more test coverage? I don't know. Shouldn't it mostly be a subset of symbol_level=2 anyway?
Level=2 means -g while =1 means line tables only. I could imagine them having different failure modes. But as-is is fine too if you prefer. On Apr 25, 2017 5:21 PM, <hans@chromium.org> wrote: > On 2017/04/25 20:52:45, Nico wrote: > > On 2017/04/25 20:37:15, hans wrote: > > > Please take a look. > > > > > > Do you remember if there's a reason why these used minimal_symbol? I > couldn't > > > see any recent changes to this. > > > > Typo "minial_symbols". > > > > Have you looked through git history? > > I was lazy and didn't look past https://codereview.chromium.org/2357483002 > > > ...I just did this for you, and it looks > > like the setting comes from here: https://codereview.chromium. > org/1155993004 > > > > It sounds like neither author nor reviewer remember. Maybe the > builder/tester > > setup means we push a lot more data with this disabled? > > That could be it, I suppose. I guess we'll find out. > > > Do we want to keep minimal_symbols for some of the bots, for more test > coverage? > > I don't know. Shouldn't it mostly be a subset of symbol_level=2 anyway? > > https://codereview.chromium.org/2838253002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2017/04/25 21:28:26, Nico wrote: > Level=2 means -g while =1 means line tables only. I could imagine them > having different failure modes. But as-is is fine too if you prefer. I was thinking if we manage -g, just doing line tables should be covered by that. That's not necessarily true of course. But I think our release builders and asan and such also do =1 builds, so there is some coverage even if those are not is_debug=true builds.
Lgtm, please watch cycle times a bit though. Also, typo in cl description still present.
Description was changed from ========== mb: Don't use minial_symbols for Clang ToT debug builders The last Clang roll slipped through the net of ToT and CQ builders though it would immediately assert on Mac debug builds using the regular symbol level. Make sure the debug ToT bots don't build with minimal_symbols so we can catch this in the future. BUG=714769 ========== to ========== mb: Don't use minimal_symbols for Clang ToT debug builders The last Clang roll slipped through the net of ToT and CQ builders though it would immediately assert on Mac debug builds using the regular symbol level. Make sure the debug ToT bots don't build with minimal_symbols so we can catch this in the future. BUG=714769 ==========
On 2017/04/25 21:39:04, Nico wrote: > Also, typo in cl description still present. Done.
The CQ bit was checked by hans@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1493156448240060, "parent_rev":
"c2f72fc86077822222610a0ed35ef068a010733a", "commit_rev":
"7fa45855fb213b42550245183ee37ce33d5e0902"}
Message was sent while issue was closed.
Description was changed from ========== mb: Don't use minimal_symbols for Clang ToT debug builders The last Clang roll slipped through the net of ToT and CQ builders though it would immediately assert on Mac debug builds using the regular symbol level. Make sure the debug ToT bots don't build with minimal_symbols so we can catch this in the future. BUG=714769 ========== to ========== mb: Don't use minimal_symbols for Clang ToT debug builders The last Clang roll slipped through the net of ToT and CQ builders though it would immediately assert on Mac debug builds using the regular symbol level. Make sure the debug ToT bots don't build with minimal_symbols so we can catch this in the future. BUG=714769 Review-Url: https://codereview.chromium.org/2838253002 Cr-Commit-Position: refs/heads/master@{#467156} Committed: https://chromium.googlesource.com/chromium/src/+/7fa45855fb213b42550245183ee3... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/7fa45855fb213b42550245183ee3...
Message was sent while issue was closed.
On 2017/04/25 21:39:04, Nico wrote: > please watch cycle times a bit though. Cycle times seem to be up (there's no scale on the graph, but I assume the bump to the right is due to this): https://build.chromium.org/p/chromium.fyi/stats/ClangToTWin64(dbg) https://build.chromium.org/p/chromium.fyi/stats/ClangToTWin(dbg) https://build.chromium.org/p/chromium.fyi/stats/ClangToTMac%20(dbg) It looks like builds are 40-ish minutes slower, which is a lot in absolute terms, but these bots are already running for several hours. What do you think?
Message was sent while issue was closed.
I think we should have some of the bots use the faster config, as said upthread. I don't think it's very important either way though :-) On Wed, Apr 26, 2017 at 12:40 PM, <hans@chromium.org> wrote: > On 2017/04/25 21:39:04, Nico wrote: > > please watch cycle times a bit though. > > Cycle times seem to be up (there's no scale on the graph, but I assume the > bump > to the right is due to this): > > https://build.chromium.org/p/chromium.fyi/stats/ClangToTWin64(dbg) > https://build.chromium.org/p/chromium.fyi/stats/ClangToTWin(dbg) > https://build.chromium.org/p/chromium.fyi/stats/ClangToTMac%20(dbg) > > It looks like builds are 40-ish minutes slower, which is a lot in absolute > terms, but these bots are already running for several hours. > > What do you think? > > https://codereview.chromium.org/2838253002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
