Closed Bug 1473423 Opened 2 years ago Closed 2 years ago

cppunittest symbols are excluded from


(Firefox Build System :: General, enhancement)

Not set


(firefox63 fixed)

Tracking Status
firefox63 --- fixed


(Reporter: aklotz, Assigned: aklotz)




(1 file)

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

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:

It's probably a PITA to put a conditional there, but that just invokes this Python script:

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.
(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:
Flags: needinfo?(ted)
Assignee: nobody → aklotz
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/
@@ +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?
Attachment #8993462 - Flags: review?(core-build-config-reviews) → review+
I forgot to say: this looks really nice!
Bug 1473423: Allow test symbols to be included in full symbol archive in try builds; r=ted
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.