1473423 - cppunittest symbols are excluded from crashreporter-symbols-full.zip

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[(No longer employed by Mozilla) Aaron Klotz]

I've got a crashing cppunittest that only fails when built using our CI; I can't reproduce this with a local build.

Unfortunately (on try at least) it looks like we aren't saving the PDBs for cppunittests anywhere, so this is a lot harder for me to debug than it otherwise would be.

We *do* save the breakpad symbols for the cppunittests into crashreporter-symbols.zip.

Can we either fix this omission, or else add the cppunittest PDBs to a separate artifact? I don't really care too much as long as I can access them somehow.

Comment 1

[(not currently active) Ted Mielczarek]

I think we did this because they bloated the symbol archive and they didn't seem useful since we don't ship those to users. Now that we can upload symbols from try builds things are a bit different. I'd be fine with always including them on try builds since try build artifacts aren't stored for very long anyway.

Currently it's just these --exclude bits that control that behavior:
https://dxr.mozilla.org/mozilla-central/rev/6c0fa9a675c91390ca27664ffb626c56e8afea4d/Makefile.in#268

It's probably a PITA to put a conditional there, but that just invokes this Python script:
https://dxr.mozilla.org/mozilla-central/rev/6c0fa9a675c91390ca27664ffb626c56e8afea4d/python/mozbuild/mozbuild/action/symbols_archive.py

I would propose that we change the arguments to that script to get rid of --exclude/--include/--compress and simply have a `--full-archive` option or something like that, so that the `symbolsfullarchive` target could pass that to include everything, and the `symbolsarchive` target would not pass it and would only include .sym files. We could then have the script internally check whether it's a try build and if so skip the filtering.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Ted Mielczarek [:ted] [:ted.mielczarek] from comment #1)
> We could then have the script internally
> check whether it's a try build and if so skip the filtering.

I have a patch written that does everything except for this: What's the best way for me to check that it's a try build? MH_BRANCH from the environment? Something else?

Comment 3

[(not currently active) Ted Mielczarek]

Awesome! Yeah, that's probably the most sensible way. You'll want to use `os.environ.get` so that it has graceful fallback for non-automation builds, like:
https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/mobile/android/mach_commands.py#419

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Allow test symbols to be included in symbol archives on try

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8993462 [details] [diff] [review]
Allow test symbols to be included in symbol archives on try

Review of attachment 8993462 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/action/symbols_archive.py
@@ +43,5 @@
> +    if args.full_archive:
> +        # We allow symbols for tests to be included when building on try
> +        if os.environ.get('MH_BRANCH', 'unknown') != 'try':
> +            excludes = ['*test*', '*Test*']
> +        compress = ['**/*.sym']

Considering that compress defaults to this and you've now removed the option, maybe just remove the compress argument to `make_archive` and hardcode it to this value?
https://dxr.mozilla.org/mozilla-central/rev/085cdfb90903d4985f0de1dc7786522d9fb45596/python/mozbuild/mozbuild/action/symbols_archive.py#19

Comment 6

[(not currently active) Ted Mielczarek]

I forgot to say: this looks really nice!

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3268a54a7fab9e9565038d589229e9f2bf1cdff9
Bug 1473423: Allow test symbols to be included in full symbol archive in try builds; r=ted

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/3268a54a7fab