1262337 - Symbols for the test plugins aren't being included in the crashreporter symbols package

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Ryan VanderMeulen [:RyanVM]]

As shown in the logs below, we're missing symbols for the test plugins, which could give us some useful insight into what's going on in these failures. Can we add them?

https://treeherder.mozilla.org/logviewer.html#?repo=fx-team&job_id=8410798
https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-central&job_id=3614859

Comment 1

[(not currently active) Ted Mielczarek]

We're explicitly excluding *test* files from the symbol archive:
https://dxr.mozilla.org/mozilla-central/rev/17a0ded9bb99c05c25729c306b91771483109067/Makefile.in#307

I believe that was in the interest of saving disk space on the symbol server at one point. We could probably relax that restriction now. Additionally, however, on non-Windows we only dump symbols for things in dist/bin, and the test plugins don't get installed there, so we'd have to fix that too:
https://dxr.mozilla.org/mozilla-central/rev/17a0ded9bb99c05c25729c306b91771483109067/Makefile.in#278

Comment 2

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #1)
> We're explicitly excluding *test* files from the symbol archive:
> https://dxr.mozilla.org/mozilla-central/rev/
> 17a0ded9bb99c05c25729c306b91771483109067/Makefile.in#307

I assume removing this line would suffice if we decided to change that?

> I believe that was in the interest of saving disk space on the symbol server
> at one point. We could probably relax that restriction now. Additionally,
> however, on non-Windows we only dump symbols for things in dist/bin, and the
> test plugins don't get installed there, so we'd have to fix that too:
> https://dxr.mozilla.org/mozilla-central/rev/
> 17a0ded9bb99c05c25729c306b91771483109067/Makefile.in#278

If someone wanted to work on this, do you have any other pointers for what work needs to be done?

Comment 3

[(not currently active) Ted Mielczarek]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #2)
> I assume removing this line would suffice if we decided to change that?

There are a number of lines there that attempt to exclude test files, so you'd want to remove all of them.

> If someone wanted to work on this, do you have any other pointers for what
> work needs to be done?

What I would do if I were going to fix this would be to just stop passing in the paths to the script, and instead have the script read the binaries.json file that is generated nowadays:
https://dxr.mozilla.org/mozilla-central/source/obj-x86_64-pc-linux-gnu/binaries.json

That's generated by the build backend from data in moz.build files, so it contains every binary the build is going to generate. You'd probably just fix Dumper.Process to read that data and iterate over the files it contains:
https://dxr.mozilla.org/mozilla-central/rev/29d5a4175c8b74f45482276a53985cf2568b4be2/toolkit/crashreporter/tools/symbolstore.py#558

You might need to do something to let unit tests override the data. You might also need to special-case Mac universal builds, since we dump symbols from the stuff in dist/universal for those:
https://dxr.mozilla.org/mozilla-central/rev/29d5a4175c8b74f45482276a53985cf2568b4be2/Makefile.in#273

I don't know exactly what the right thing to do there is, although binaries.json does list where the files should get installed to, so you should be able to put together a path relative to dist/universal.

Comment 5

[(not currently active) Ted Mielczarek]

Attachment: bug 1262337 - add symbols for test plugins to symbols zip on Windows.

Review commit: https://reviewboard.mozilla.org/r/73282/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73282/

Comment 6

[(not currently active) Ted Mielczarek]

I downloaded the symbol package from the Win32 build:
https://archive.mozilla.org/pub/firefox/try-builds/[e-mail]-6734e259714fccad81755af69d81240a12367ee2/try-win32/firefox-51.0a1.en-US.win32.crashreporter-symbols.zip

and it contains symbols for the test plugins:
```
$ unzip -l firefox-51.0a1.en-US.win32.crashreporter-symbols.zip | grep np.*test
   698736  2016-08-22 09:16   npctrltest.pdb/A9FB6AD926B9498CB8A48895F902C3921/npctrltest.sym
   698725  2016-08-22 09:16   npsecondtest.pdb/42BA9E2FAEDB40379DF629C48E9DA8C11/npsecondtest.sym
   698571  2016-08-22 09:16   npswftest.pdb/32EB8C9AA901420580B397BD7D50346F1/npswftest.sym
   698690  2016-08-22 09:16   nptest.pdb/E72499AF7C954EF09133EBB8305A609F1/nptest.sym
   698570  2016-08-22 09:16   nptestjava.pdb/A9A3726584EE454B8D11F0913C4643D31/nptestjava.sym
   698720  2016-08-22 09:16   npthirdtest.pdb/E95D5F54902C497BB329D83A20BDC3AB1/npthirdtest.sym
```

This won't fix non-Windows platforms, but it should help with the frequent test failures that seem to be on Windows.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8783487 [details]
bug 1262337 - add symbols for test plugins to symbols zip on Windows.

https://reviewboard.mozilla.org/r/73282/#review71182

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7cf11eac32ba
add symbols for test plugins to symbols zip on Windows. r=gps

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7cf11eac32ba

Comment 10

[(not currently active) Ted Mielczarek]

I filed bug 1297387 as a follow-up to fix this for other platforms.

Comment 11

[Sylvestre Ledru [:Sylvestre]]

Do we want to uplift that to aurora? Thanks

Comment 12

[Benjamin Smedberg]

It's probably not necessary: we already got the short-term randomorange problem probably figured out!