Closed Bug 1304042 Opened 8 years ago Closed 7 years ago

buildsymbols should fail if running dsymutil fails

Categories

(Toolkit :: Crash Reporting, defect)

Product:
Toolkit
Component:
Crash Reporting
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla56
Tracking Flags:
Tracking Status
firefox52 --- wontfix
firefox56 --- fixed

People

(Reporter: ted, Assigned: chmanchester)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1304042 - Fail the build if running dsymutil fails.
59 bytes, text/x-review-board-request
mshal
: review+
Details 
I'm not sure why, but currently the buildsymbols step doesn't fail on mac if running dsymutil fails. This means that bug 1301751 slipped in and we didn't notice because the builds didn't break. :-(
It's fairly likely that this try push will have broken builds because it's exposing existing brokenness. :-(
...or it could fail because I typoed the name of the "success" variable. Also apparently concurrent.futures doesn't handle sending a `CalledProcessException` back from a job, so it still failed.
As usual, it's never as easy as it seems. I fixed the things in comment 6, but I also had to fix the invocation of symbolstore.py in Makefile.in, because it was being invoked in a shell pipeline, so its exit status was being ignored. :-(

I tested locally with a dsymutil binary in my path that fails and `mach buildsymbols` correctly exits with an error in that case.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #7)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=491d798446c8

The opt build here is correctly red. llvm-dsymutil crashed and buildsymbols failed because of that.
Comment on attachment 8792901 [details]
bug 1304042 - buildsymbols should fail if running dsymutil fails.

https://reviewboard.mozilla.org/r/79766/#review79204

::: Makefile.in:316
(Diff revision 3)
>  	$(PYTHON) $(topsrcdir)/toolkit/crashreporter/tools/symbolstore.py \
>  	  $(MAKE_SYM_STORE_ARGS)                                          \
>  	  $(foreach dir,$(SYM_STORE_SOURCE_DIRS),-s $(dir))               \
>  	  $(DUMP_SYMS_BIN)                                                \
>  	  $(DIST)/crashreporter-symbols                                   \
> -	  $(MAKE_SYM_STORE_PATH) | grep -iv test >                        \
> +	  $(MAKE_SYM_STORE_PATH) >                        		  \

Nit: trailing whitespace
Attachment #8792901 - Flags: review?(gps) → review+
No longer blocks: 1301751
Depends on: 1301751
So this got backed out for exposing two (existing) failures:
1) The tier-2 taskcluster OS X opt builds are failing to run llvm-dsymutil on the gtest XUL library. This is unfortunate and I don't know of a way to make this work, but hopefully bug 1304815 will fix it when it lands.
2) There were some odd failures in Windows builds, like: https://treeherder.mozilla.org/logviewer.html#?job_id=36549310&repo=mozilla-inbound#L19896
 12:26:41 INFO - Unexpected error: [Error 2] The system cannot find the file specified: 'dist\\crashreporter-symbols\\shlibsign.pdb\\5BCF415274384AF4A8FBD848F550CC3A1\\shlibsign.pdb' 
I don't know quite what's happening there, but I'll have to sort that out before relanding this.
Depends on: 1304815
Flags: needinfo?(ted)
Mass wontfix for bugs affecting firefox 52.
status-firefox52: affected → wontfix
See Also: → 1371017
I'll rebase the patch here.
Assignee: ted → cmanchester
(In reply to Chris Manchester (:chmanchester) from comment #16)
> I'll rebase the patch here.

This is failing where it should and nowhere it shouldn't on try.
Comment on attachment 8886330 [details]
Bug 1304042 - Fail the build if running dsymutil fails.

https://reviewboard.mozilla.org/r/157070/#review162216

LGTM - I assume we can't land this until after bug 1380381 is fixed?
Attachment #8886330 - Flags: review?(mshal) → review+
(In reply to Michael Shal [:mshal] from comment #19)
> Comment on attachment 8886330 [details]
> Bug 1304042 - Fail the build if running dsymutil fails.
> 
> https://reviewboard.mozilla.org/r/157070/#review162216
> 
> LGTM - I assume we can't land this until after bug 1380381 is fixed?

Right. I guess the trees are closed for it anyway :/
Depends on: 1380381
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4c8ee44afffb
Fail the build if running dsymutil fails. r=mshal
https://hg.mozilla.org/mozilla-central/rev/4c8ee44afffb
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Attachment #8792901 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: