New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 746968 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

LLVM CL breaking paladins?

Project Member Reported by jorgelo@chromium.org, Jul 20 2017

Issue description

https://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?
 
Labels: -Pri-1 Pri-0
This is hitting PreCQ, upgrading to P0.
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.
Cc: jorgelo@chromium.org
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.
Cc: akes...@chromium.org grundler@chromium.org
Cc: elijahtaylor@chromium.org achuith@chromium.org wuchengli@chromium.org charliemooney@chromium.org
 Issue 747005  has been merged into this issue.
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).
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.

Mike says a straight revert should work. Feel free to chump https://chromium-review.googlesource.com/c/579578/. At a 30-min meeting now.
Sorry, feel free to chump https://chromium-review.googlesource.com/c/579807/.
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.

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.

Project Member

Comment 16 by bugdroid1@chromium.org, 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

Thanks Richard.
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).
Labels: -Pri-0 Pri-1
Status: Started (was: Untriaged)
Status: Fixed (was: Started)
The BuildPackages stage has passed for all affected CQ bots.
Project Member

Comment 21 by bugdroid1@chromium.org, 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

Comment 22 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment