Closed Bug 1425996 Opened 4 years ago Closed 4 years ago
Various builds will be busted when Gecko 59 merges to Beta on 2018-01-11
59 bytes, text/x-review-board-request
[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]
The early/first merge will be Jan. 11th. Tracking so we can make sure to check back in early January.
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.
Anyway, this is breaking nearly all the builds at the moment and actively blocking me running useful simulations :(
Severity: critical → blocker
Attachment #8938250 - Flags: review?(gsquelart)
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+
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.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/06a19fbe2581 fix build error: unused variable 'rv'. r=gerald
Still busted :( https://treeherder.mozilla.org/logviewer.html#?job_id=153041955&repo=try is off current m-c tip (rev f844e99f26b1)
Oh, this is in the other spot in MediaPipeline.cpp. I'll try giving it the same treatment that the first location got.
(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 :(
(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 :-)
Looks like those are indeed the only spots still needing patching. Going to land it with Gerald's previous r+.
Pushed by email@example.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
You need to log in before you can comment on or make changes to this bug.