Reland: Make clusterfuzz_archive include llvm-symbolizer and sancov.

Reland: Make clusterfuzz_archive include llvm-symbolizer and sancov.

BUG=636168
Reland of https://codereview.chromium.org/2393293002/

Committed: https://chromium.googlesource.com/chromium/tools/build/+/da6fd0010ab28c51ce38c95a2752a6c99738c08e

[Oliver Chang, 2016-10-07 17:32:52]

Description was changed from

==========
Reland: Make clusterfuzz_archive include llvm-symbolizer and sancov.

BUG=636168
==========

to

==========
Reland: Make clusterfuzz_archive include llvm-symbolizer and sancov.

BUG=636168
==========

[Oliver Chang, 2016-10-07 17:32:52]

ochang@chromium.org changed reviewers:
+ inferno@chromium.org, thakis@chromium.org

[Oliver Chang, 2016-10-07 17:32:57]

ptal

[inferno, 2016-10-07 17:37:59]

lgtm

[Oliver Chang, 2016-10-07 17:47:40]

Description was changed from

==========
Reland: Make clusterfuzz_archive include llvm-symbolizer and sancov.

BUG=636168
==========

to

==========
Reland: Make clusterfuzz_archive include llvm-symbolizer and sancov.

BUG=636168

Reland of https://codereview.chromium.org/2393293002/
==========

[Nico, 2016-10-07 19:29:32]

https://codereview.chromium.org/2394163006/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2394163006/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:282: if self.m.path.exists(tool_src)
and not self.m.path.exists(tool_dst):
This relies on all bots that this runs on being clobber bots -- else,
eventually, when a new clang with new binaries is released, this won't overwrite
the old ones. Isn't there a "overwrite always" flag for file.copy?

[Oliver Chang, 2016-10-07 19:53:33]

https://codereview.chromium.org/2394163006/diff/40001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2394163006/diff/40001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:282: if self.m.path.exists(tool_src)
and not self.m.path.exists(tool_dst):
On 2016/10/07 19:29:32, Nico wrote:
> This relies on all bots that this runs on being clobber bots -- else,
> eventually, when a new clang with new binaries is released, this won't
overwrite
> the old ones. Isn't there a "overwrite always" flag for file.copy?

Good point. What I needed was an ignore_errors flag, which shutil.copy accepts,
but the file recipe_module does not forward. 

I've removed the second check and made it catch an exception instead.

[Nico, 2016-10-07 20:04:31]

https://codereview.chromium.org/2394163006/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2394163006/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:290: # exception. Either way, this
shouldn't cause the whole build to fail.
Doesn't this still have the "only works on clobber bots" problem?

[Oliver Chang, 2016-10-07 20:34:58]

https://codereview.chromium.org/2394163006/diff/60001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/archive/api.py (right):

https://codereview.chromium.org/2394163006/diff/60001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/archive/api.py:290: # exception. Either way, this
shouldn't cause the whole build to fail.
On 2016/10/07 20:04:31, Nico wrote:
> Doesn't this still have the "only works on clobber bots" problem?

file.copy should always replace the destination. This guards against a weird
case we noticed on the lkgr bots that causes an exception. Since the destination
is already a link of the source, it doesn't matter if it fails.

[Nico, 2016-10-07 20:44:41]

On 2016/10/07 20:34:58, Oliver Chang wrote:
>
https://codereview.chromium.org/2394163006/diff/60001/scripts/slave/recipe_mo...
> File scripts/slave/recipe_modules/archive/api.py (right):
> 
>
https://codereview.chromium.org/2394163006/diff/60001/scripts/slave/recipe_mo...
> scripts/slave/recipe_modules/archive/api.py:290: # exception. Either way, this
> shouldn't cause the whole build to fail.
> On 2016/10/07 20:04:31, Nico wrote:
> > Doesn't this still have the "only works on clobber bots" problem?
> 
> file.copy should always replace the destination. This guards against a weird
> case we noticed on the lkgr bots that causes an exception. Since the
destination
> is already a link of the source, it doesn't matter if it fails.

Is it understood why that hard link is there? Is it because
https://cs.chromium.org/chromium/src/build/sanitizers/BUILD.gn?q=llvm-symboli...
by gn is compiled to

build llvm-symbolizer: copy
../../third_party/llvm-build/Release+Asserts/bin/llvm-symbolizer

rule copy
  command = ln -f ${in} ${out} 2>/dev/null || (rm -rf ${out} && cp -af ${in}
${out})


...yes, I think that explains this, and this is a good fix. lgtm, thanks!

[Oliver Chang, 2016-10-10 15:28:37]

The CQ bit was checked by ochang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-10 15:28:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 15:32:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-10 15:32:19]

Dry run: This issue passed the CQ dry run.

[Oliver Chang, 2016-10-10 16:03:49]

The CQ bit was checked by ochang@chromium.org

[Oliver Chang, 2016-10-10 16:03:49]

The patchset sent to the CQ was uploaded after l-g-t-m from inferno@chromium.org
Link to the patchset: https://codereview.chromium.org/2394163006/#ps60001
(title: "fix")

[commit-bot: I haz the power, 2016-10-10 16:03:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-10 16:10:15]

Description was changed from

==========
Reland: Make clusterfuzz_archive include llvm-symbolizer and sancov.

BUG=636168

Reland of https://codereview.chromium.org/2393293002/
==========

to

==========
Reland: Make clusterfuzz_archive include llvm-symbolizer and sancov.

BUG=636168

Reland of https://codereview.chromium.org/2393293002/

Committed:
https://chromium.googlesource.com/chromium/tools/build/+/da6fd0010ab28c51ce38...
==========

[commit-bot: I haz the power, 2016-10-10 16:10:16]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/tools/build/+/da6fd0010ab28c51ce38...