LLVM CL breaking paladins? |
||||||||
Issue descriptionhttps://uberchromegw.corp.google.com/i/chromeos/builders/edgar-paladin/builds/168 https://uberchromegw.corp.google.com/i/chromeos/builders/quawks-paladin/builds/168 https://uberchromegw.corp.google.com/i/chromeos/builders/reef-paladin/builds/3034 mesa-17.1.0-r12: ./configure: line 22372: test: : integer expression expected mesa-17.1.0-r12: ./configure: line 22378: test: : integer expression expected mesa-17.1.0-r12: configure: error: LLVM 3.3.0 or newer is required for gallium Can https://chromium-review.googlesource.com/c/572768/ be related?
,
Jul 20 2017
This is hitting PreCQ, upgrading to P0.
,
Jul 20 2017
It's definitely related. Interesting ... we can't seem to find llvm-config-host, for example:
/build/quawks/usr/bin/llvm-config-host: line 2: /build/amd64-generic/usr/lib/llvm/bin/llvm-config: No such file or directory
Not sure why {SYSROOT} for quawks is defined as /build/amd64-generic/, not /build/quawks. However feel free to revert while I investigate.
,
Jul 20 2017
,
Jul 20 2017
Well, reverting is hard because these are ebuild changes so they need to be undone while also uprev'ing the CL. You have a bit more time before MTV wakes up, If there's no progress I'll revert in about 45 mins.
,
Jul 20 2017
https://chromium-review.googlesource.com/c/579807/ attempt to revert.
,
Jul 20 2017
,
Jul 20 2017
Issue 747005 has been merged into this issue.
,
Jul 20 2017
https://chromium-review.googlesource.com/c/579578/ fixes this as well.
,
Jul 20 2017
So a couple of things failed in the attempt to upgrade LLVM.
1) inadequate testing: the pre-cq tests were likely using the previous version of llvm package instead of the new version. Toolchain changes need to be "widely used" before being committed. Can this sort of "cross build" toolchain use testing be automated? Wasn't there a set of experimental builders for testing toolchain changes? Can pre-cq be hardwired to run on those
2) the master paladin build failed for "other reasons"
https://luci-milo.appspot.com/buildbot/chromeos/master-paladin/15449
00:40:18: ERROR: Could not submit henryhsu:571623:7c8250e4
00:40:22: INFO: Running cidb query on pid 486, repr(query) starts with <sqlalchemy.sql.expression.Insert object at 0x7f2e597e1c90>
00:40:22: ERROR: Could not submit henryhsu:571624:014dcfaf
00:40:25: INFO: Running cidb query on pid 486, repr(query) starts with <sqlalchemy.sql.expression.Insert object at 0x7f2e58e25990>
A failure like this could be masking issues with one of the commits - though in this case I don't think any arc-camera changes would break unrelated builds (e.g. samus).
,
Jul 20 2017
This bug is also breaking the reef pre-cq builder, which means it's holding up a large majority of all CLs. Ordinarily, I'd say revert first, fix second. But I'm concerned about the impact of deleting the 4.0.1 version of the package in favor of a 3.8.1-r4 version; I don't know if that will work. My thinking: We need to chump something ASAP. If it doesn't work, then we'll have to chump something else. If the revert doesn't work, then we'd have to chump another ebuild change, which could make a bad situation worse. So, my vote is to chump the putative forward fix first, and then see what happens.
,
Jul 20 2017
Mike says a straight revert should work. Feel free to chump https://chromium-review.googlesource.com/c/579578/. At a 30-min meeting now.
,
Jul 20 2017
Sorry, feel free to chump https://chromium-review.googlesource.com/c/579807/.
,
Jul 20 2017
Mike hit -1 on the forward fix. If he says the revert will work, that's good enough for me. The revert change is working its way through the pre-cq now, and is building mesa at this very moment. We can give that process another 5-10 minutes to prove out the concept, after which we can chump.
,
Jul 20 2017
From the pre-cq build logs: Pending 34/724, Building 1/1, [Time 09:52:44 | Elapsed 37m59.3s | Load 9.99 29.53 46.7] Completed media-libs/mesa-17.1.0-r12 (in 6m3.9s) Time to be a chump.
,
Jul 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/c8c805b81abaf04be54a751f8c80953a51c46766 commit c8c805b81abaf04be54a751f8c80953a51c46766 Author: Jorge Lucangeli Obes <jorgelo@chromium.org> Date: Thu Jul 20 16:56:30 2017 Revert "sys-devel/llvm: uprev llvm to 4.0.1" This reverts commit 7013f1beedd00409369ef8babce83cc4e411cce1. Reason: Breaking CQ, PreCQ, PFQ. mesa-17.1.0-r12: ./configure: line 22372: test: : integer expression expected mesa-17.1.0-r12: ./configure: line 22378: test: : integer expression expected mesa-17.1.0-r12: configure: error: LLVM 3.3.0 or newer is required for gallium BUG= chromium:746968 TEST=emerge-kevin mesa Change-Id: I9b11d4e0e8984d6ac3694234aa7c8576c9ab922f Reviewed-on: https://chromium-review.googlesource.com/579807 Trybot-Ready: Jorge Lucangeli Obes <jorgelo@chromium.org> Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org> Tested-by: Richard Barnette <jrbarnette@google.com> [modify] https://crrev.com/c8c805b81abaf04be54a751f8c80953a51c46766/sys-devel/llvm/Manifest [modify] https://crrev.com/c8c805b81abaf04be54a751f8c80953a51c46766/profiles/targets/chromeos/package.keywords [add] https://crrev.com/c8c805b81abaf04be54a751f8c80953a51c46766/sys-devel/llvm/llvm-3.8.1-r4.ebuild [delete] https://crrev.com/a7eb920642a56748b5765e13368e6fceced0ad5c/sys-devel/llvm/llvm-4.0.1.ebuild [delete] https://crrev.com/a7eb920642a56748b5765e13368e6fceced0ad5c/sys-devel/llvm/files/9999/0007-llvm-config-Clean-up-exported-values-update-for-shar.patch
,
Jul 20 2017
Thanks Richard.
,
Jul 20 2017
to clarify things, there's two versions of llvm in the tree. there's the one the toolchain team maintains and goes through the multi-level testing process that Grant describes. that version typically has a svn snapshot in its version like: llvm-5.0_pre300080_p20170707-r2 but that's not this version. the other one is used for target boards only for its runtime libs (largely for mesa/graphics). that one is a standard board package and the preCQ/CQ fully vet it. it uses release versions like: llvm-4.0.1 that's the one here. from some CLs i've seen go by, the underlying breakage isn't specific to llvm per-se ... we've made that mistake in other packages in the past. looks like it's expanding/hardcoding SYSROOT from one build and then the binpkg is getting re-used for other boards where the SYSROOT is invalid (since we share binpkgs between boards when possible).
,
Jul 20 2017
,
Jul 20 2017
The BuildPackages stage has passed for all affected CQ bots.
,
Jul 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/681d6e8f8dbcc403d20ed07fc55d8a1b0c228fbb commit 681d6e8f8dbcc403d20ed07fc55d8a1b0c228fbb Author: Gurchetan Singh <gurchetansingh@chromium.org> Date: Fri Jul 21 20:21:25 2017 sys-devel/llvm: uprev llvm to 4.0.1 (reland) We were expanding the ${SYSROOT} variable at build time instead of at runtime, leading to CQ failures. When creating the llvm-config wrapper, some boards pointed to the wrong sysroot. For example, the llvm-config wrapper for reef pointed to: /build/amd64-generic/usr/lib/llvm/bin/llvm-config instead of /build/reef/usr/lib/llvm/bin/llvm-config Let's escape the ${SYSROOT} variable so it points to the correct place. BUG= chromium:746968 TEST=emerge-reef llvm mesa Change-Id: Iba3336a353e0c41e5ad0f8e187937a37dc3aaad8 Reviewed-on: https://chromium-review.googlesource.com/580488 Commit-Ready: Gurchetan Singh <gurchetansingh@chromium.org> Tested-by: Gurchetan Singh <gurchetansingh@chromium.org> Reviewed-by: Stéphane Marchesin <marcheu@chromium.org> Reviewed-by: Mike Frysinger <vapier@chromium.org> [add] https://crrev.com/681d6e8f8dbcc403d20ed07fc55d8a1b0c228fbb/sys-devel/llvm/llvm-4.0.1-r1.ebuild [modify] https://crrev.com/681d6e8f8dbcc403d20ed07fc55d8a1b0c228fbb/sys-devel/llvm/Manifest [modify] https://crrev.com/681d6e8f8dbcc403d20ed07fc55d8a1b0c228fbb/profiles/targets/chromeos/package.keywords [delete] https://crrev.com/a987eb898f05c3910a4d17932c286862586bf6da/sys-devel/llvm/llvm-3.8.1-r4.ebuild [add] https://crrev.com/681d6e8f8dbcc403d20ed07fc55d8a1b0c228fbb/sys-devel/llvm/files/9999/0007-llvm-config-Clean-up-exported-values-update-for-shar.patch
,
Jan 22 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by jorgelo@chromium.org
, Jul 20 2017