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

RESOLVED FIXED in Firefox 63

Status

enhancement
RESOLVED FIXED
Last year
11 months ago

People

(Reporter: aklotz, Assigned: aklotz)

Tracking

Trunk
mozilla63
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

Last year
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.
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.
Assignee

Comment 2

Last year
(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?
Flags: needinfo?(ted)
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
Flags: needinfo?(ted)
Assignee

Updated

11 months ago
Assignee: nobody → aklotz
Status: NEW → ASSIGNED
Assignee

Comment 4

11 months ago
Attachment #8993462 - Flags: review?(core-build-config-reviews)
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
Attachment #8993462 - Flags: review?(core-build-config-reviews) → review+
I forgot to say: this looks really nice!
Assignee

Comment 7

11 months ago
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

11 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3268a54a7fab
Status: ASSIGNED → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.