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)

defect

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)
Rank: 11
Priority: -- → P1
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.
Flags: needinfo?(jwwang)
Anyway, this is breaking nearly all the builds at the moment and actively blocking me running useful simulations :(
Severity: critical → blocker
Checking...
Attachment #8938250 - Flags: review?(gsquelart)
Assignee: nobody → jwwang
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.
Flags: needinfo?(jwwang)
Pushed by jwwang@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/06a19fbe2581 fix build error: unused variable 'rv'. r=gerald
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Flags: needinfo?(amarchesini)
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 → ---
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 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
Status: REOPENED → RESOLVED
Closed: 8 years ago7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Flags: needinfo?(jwwang)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: