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

Issue 637317 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 0
Type: Bug



Sign in to add a comment

Code search stopped updating around August 8th

Project Member Reported by jamescook@chromium.org, Aug 12 2016

Issue description

This CL is landed:
https://codereview.chromium.org/2237903003/

With this change:
https://codereview.chromium.org/2215993002/diff/40001/chrome/browser/metrics/metrics_reporting_state.h

But the changes are not visible:
https://cs.chromium.org/chromium/src/chrome/browser/metrics/metrics_reporting_state.h

This is significantly impacting the productivity of my team and several others.

Starting with dsansome, not sure how infra triage works these days.

 

Comment 1 by d...@chromium.org, Aug 12 2016

Cc: emso@chromium.org
+emso@, who was recommended as a maintainer of code search.

Unfortunately everyone involved so far is not PST, so this may take some timezone shifts to fully resolve.
Labels: Infra-Troopers
Status: Available (was: Assigned)
Adding to the trooper queue so this can get looked at sooner.

See also https://bugs.chromium.org/p/chromium/issues/detail?id=634202

Comment 3 by d...@chromium.org, Aug 12 2016

AFAICT everyone who knows anything about this is already in the bug. As trooper today, I honestly have no idea how to actually make this situation better.

Comment 4 by abw@chromium.org, Aug 12 2016

Cc: dsansome@chromium.org
Labels: -Pri-1 Pri-0
Owner: seanmccullough@chromium.org
codesearch not updating for multiple days is a wide enough problem that I'm pushing it to P0. Sean, rope in as many CITizens as might help.
The builder that generates the index pack has been failing "compile translation_unit clang tool" since the 7th:
https://build.chromium.org/p/chromium.infra.cron/builders/Chromium%20Linux%20Codesearch/builds/2750



Was there a change in third_party/llvm recently?

/b/build/slave/Chromium_Linux_Codesearch/build/src/third_party/llvm/tools/lld/lib/ReaderWriter/FileArchive.cpp:53:13: error: ‘const class llvm::object::Archive::Child’ has no member named ‘getError’
     if (ci->getError())

Comment 7 by d...@chromium.org, Aug 12 2016

Should we be pinning the LLVM version that we use?
Cc: mar...@chromium.org
maruel points out that it's using GCC 4.8.2, which may be too out of date (4.8.2 is from 2013):
Updating Clang to 277962-1...
Downloading pre-built GCC 4.8.2...



Comment 9 by szager@chromium.org, Aug 12 2016

Cc: thakis@chromium.org
FYI, last successful build on that bot:

https://build.chromium.org/p/chromium.infra.cron/builders/Chromium%20Linux%20Codesearch/builds/2749

... also used GCC 4.8.2.  That seems like a red herring.

Adding thakis@, who knows a thing or two about clang.
re #7: that's certainly possible. How would we test that? The script looks like it tries to blow away at least one of the llvm builds it touches at some point in the process:
https://cs.chromium.org/chromium/build/scripts/slave/recipes/chromium_codesearch.py?q=codesearch&sq=package:chromium&l=134

re #8: maruel also points out gcc version may also be a red herring.

Here's the line where I think it's downloading the pre-built 4.8.2:
https://cs.chromium.org/chromium/src/tools/clang/scripts/update.py?l=309

re #9: it might be a red herring, but it also could be that a 4.8.2-incompatible change was introduced to the llvm code on the next build attempt. Previous builds succeeded despite the older gcc.
I think you should be looking at the clang code diffs between successful and unsuccessful builds, it's much more likely that there's a bad commit in clang, rather than gcc incompatibility.
For reference, got_clang_revision 274369-1 succeeded and the next build at 277962-1 failed. 
Owner: dsansome@chromium.org
Looks like this was caused by https://chromium.googlesource.com/chromium/src/+/78f40fbff7fe6ea54b0de1492268f4f5f76f6eb6 that rolled clang from 274369 to 277962.

That was done a week ago so I'm a bit hesitant to revert it, instead I'll try to fix the translation_unit tool.
LLVM archaeology time:

thakis happened to pick clang r277962 which was after a breaking change between Object/Archive.h and the lld tool.

https://github.com/llvm-mirror/llvm/commit/aacf2fbfe4c8c0a030a36a602814c0d472a8a447 - this changed Archive::child_iterator from yielding ErrorOr<Child> (which had a getError() method) to Child (which doesn't).
https://github.com/llvm-mirror/lld/commit/f83e27b899e421947f494721fe44bcd5c40f68d5 - this fixed LLD to not call getError() on the iterator.

But wait!  The LLD tool isn't actually checked out any more - it was reverted in https://codereview.chromium.org/2217113003.  If LLD was at r277962 too it would compile fine.

So is there a stale tools/lld checkout sitting on slave14-c1 that's being included in the build just because the directory is present?

I'll SSH to slave14-c1 and rm -rf third_party/llvm/tools/lld.
It gets further now, but now there's actually a break in translation_tool itself:

/usr/local/google/home/dsansome/cr/c/src/tools/clang/translation_unit/TranslationUnitGenerator.cpp: In member function ‘virtual void {anonymous}::IncludeFinderPPCallbacks::InclusionDirective(clang::SourceLocation, const clang::Token&, llvm::StringRef, bool, clang::CharSourceRange, const clang::FileEntry*, llvm::StringRef, llvm::StringRef, const clang::Module*)’:
/usr/local/google/home/dsansome/cr/c/src/tools/clang/translation_unit/TranslationUnitGenerator.cpp:166:20: error: ‘llvm::sys::path’ has not been declared
         llvm::sys::path::parent_path(current_files_.top().c_str()).str();

Project Member

Comment 16 by bugdroid1@chromium.org, Aug 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f44c5e7a7d5f7a592ea8bd1750789d51c09cd344

commit f44c5e7a7d5f7a592ea8bd1750789d51c09cd344
Author: David Sansome <dsansome@chromium.org>
Date: Mon Aug 15 04:18:03 2016

Add a missing include to translation_unit - required after the roll to clang r277962.

BUG= 637317 
TBR=thakis

Review URL: https://codereview.chromium.org/2248583002 .

Cr-Commit-Position: refs/heads/master@{#411938}

[modify] https://crrev.com/f44c5e7a7d5f7a592ea8bd1750789d51c09cd344/tools/clang/translation_unit/TranslationUnitGenerator.cpp

The source archive is being created and uploaded to GCS now, but the cron job that copies it internally doesn't seem to be working.  I'm debugging it now.
It was failing because it's expecting the chromeos source to be uploaded at the same revision as chromium source, which wasn't the case because I only manually ran the chromeos build.

I've fixed it by deleting the (stale) chromeos src and rerunning the cron job.
I'm going home now but the frontends should pick up the new source code soon.  I'll check up on them tomorrow.

I've created  issue 637807  to add alerts for these builders.

Comment 20 by abw@google.com, Aug 15 2016

Owner: seanmccullough@chromium.org
Thanks, Dave ! I'll hand this over to the current trooper, to validate things are working. (And by freak coincidence, today's trooper is ... Sean ! Welcome back to the bug :)
Status: Fixed (was: Available)
Confirmed this is working now!
Cc: h...@chromium.org krasin@chromium.org
Thanks for fixing this! I was on vacation and didn't see this until just now.

Your analysis is correct. krasin made bots check out lld for a bit, but we had to revert that. I tried to make sure the lld checkout is gone in https://codereview.chromium.org/2220113002/ but made a mistake, so that CL got reverted -- and most bots had cycled with it anyhow, and krasin had manually removed the checkout on most bots. We forgot about the codesearch bot.

Sign in to add a comment