Closed
Bug 1425996
Opened 8 years ago
Closed 7 years ago
Various builds will be busted when Gecko 59 merges to Beta on 2018-01-11
Categories
(Core :: WebRTC: Signaling, defect, P1)
Core
WebRTC: Signaling
Tracking
()
VERIFIED
FIXED
mozilla59
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox57 | --- | unaffected |
firefox58 | --- | unaffected |
firefox59 | + | verified |
People
(Reporter: ryanvm, Assigned: jwwang)
References
Details
Attachments
(1 file)
[Tracking Requested - why for this release]: Broken builds on the next merge day.
I believe MOZ_DIAGNOSTIC_ASSERT usage needs to be wrapped in MOZ_DIAGNOSTIC_ASSERT_ENABLED ifdef guards?
https://treeherder.mozilla.org/logviewer.html#?job_id=150903173&repo=try
/builds/worker/workspace/build/src/media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:211:14: error: unused variable 'rv' [-Werror,-Wunused-variable]
Flags: needinfo?(amarchesini)
Updated•8 years ago
|
Rank: 11
Priority: -- → P1
Comment 1•8 years ago
|
||
The early/first merge will be Jan. 11th.
Tracking so we can make sure to check back in early January.
Reporter | ||
Comment 2•8 years ago
|
||
JW, maybe you can help? DebugOnly isn't an option because MOZ_DIAGNOSTIC_ASSERT is used on opt builds as well. But I'm not sure exactly how to MOZ_DIAGNOSTIC_ASSERT_ENABLED ifdef my way to victory here either given how rv is being assigned in the first place.
Flags: needinfo?(jwwang)
Reporter | ||
Comment 3•8 years ago
|
||
Anyway, this is breaking nearly all the builds at the moment and actively blocking me running useful simulations :(
Severity: critical → blocker
Assignee | ||
Comment 4•8 years ago
|
||
Checking...
Comment hidden (mozreview-request) |
Assignee | ||
Updated•8 years ago
|
Attachment #8938250 -
Flags: review?(gsquelart)
Assignee: nobody → jwwang
Comment 6•8 years ago
|
||
mozreview-review |
Comment on attachment 8938250 [details]
Bug 1425996 - fix build error: unused variable 'rv'.
https://reviewboard.mozilla.org/r/209002/#review214690
::: dom/media/MediaDecoder.cpp:1232
(Diff revision 1)
> nsresult rv = mReader->OwnerThread()->Dispatch(
> NewRunnableMethod("MediaFormatReader::NotifyDataArrived",
> mReader.get(),
> &MediaFormatReader::NotifyDataArrived));
> MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));
> + Unused << rv;
LGTM.
But could you please check that non-unified build still works (for Chris' bug 1388605).
Attachment #8938250 -
Flags: review?(gsquelart) → review+
Assignee | ||
Comment 7•8 years ago
|
||
It is sad that disabling unified-build for the entire dom/media folder causes build errors even for files not changed at all. I will land the patch and fix build errors for servo build if any.
Flags: needinfo?(jwwang)
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/06a19fbe2581
fix build error: unused variable 'rv'. r=gerald
Comment 9•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•7 years ago
|
Flags: needinfo?(amarchesini)
Reporter | ||
Comment 10•7 years ago
|
||
Still busted :(
https://treeherder.mozilla.org/logviewer.html#?job_id=153041955&repo=try is off current m-c tip (rev f844e99f26b1)
Status: RESOLVED → REOPENED
Flags: needinfo?(jwwang)
Resolution: FIXED → ---
Target Milestone: mozilla59 → ---
Reporter | ||
Comment 11•7 years ago
|
||
Oh, this is in the other spot in MediaPipeline.cpp. I'll try giving it the same treatment that the first location got.
Reporter | ||
Comment 12•7 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #11)
> Oh, this is in the other spot in MediaPipeline.cpp. I'll try giving it the
> same treatment that the first location got.
Which just moves the bustage to ChannelMediaDecoder.cpp :(
Comment 13•7 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #12)
> Which just moves the bustage to ChannelMediaDecoder.cpp :(
At least that is the last file in dom/media which doesn't have the Unused << after the MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv)) calls :-)
Reporter | ||
Comment 14•7 years ago
|
||
Looks like those are indeed the only spots still needing patching. Going to land it with Gerald's previous r+.
Comment 15•7 years ago
|
||
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc335a5f19e8
Add in a few more instances of Unused that got missed in the first patch. r=gerald
Comment 16•7 years ago
|
||
bugherder |
Status: REOPENED → RESOLVED
Closed: 8 years ago → 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(jwwang)
Reporter | ||
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•