Closed
Bug 1425996
Opened 6 years ago
Closed 6 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•6 years ago
|
Rank: 11
Priority: -- → P1
The early/first merge will be Jan. 11th. Tracking so we can make sure to check back in early January.
Reporter | ||
Comment 2•6 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•6 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•6 years ago
|
||
Checking...
Comment hidden (mozreview-request) |
Assignee | ||
Updated•6 years ago
|
Attachment #8938250 -
Flags: review?(gsquelart)
Assignee: nobody → jwwang
Comment 6•6 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•6 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•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/06a19fbe2581
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Updated•6 years ago
|
Flags: needinfo?(amarchesini)
Reporter | ||
Comment 10•6 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•6 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•6 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•6 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•6 years ago
|
||
Looks like those are indeed the only spots still needing patching. Going to land it with Gerald's previous r+.
Comment 15•6 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•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bc335a5f19e8
Status: REOPENED → RESOLVED
Closed: 6 years ago → 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Assignee | ||
Updated•6 years ago
|
Flags: needinfo?(jwwang)
Reporter | ||
Updated•6 years ago
|
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•