media/webrtc/signaling/test/signaling_unittests.cpp fails to build with musl libc (no <sys/queue.h>)

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: felix.janda, Assigned: felix.janda)

Tracking

Trunk
mozilla41
Other
Linux
Points:
---

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Posted patch Proposed patch (obsolete) — Splinter Review
Build of media/webrtc/signaling/test/signaling_unittests.cpp fails on linux systems with musl libc (http://musl-libc.org) because signaling_unittests.cpp includes media/mtransport/test/stunserver.cpp, which includes media/mtransport/third_party/nrappkit/src/share/nr_api.h, which tries to include sys/queue.h, which is missing on musl.

There is a copy of sys/queue.h in media/mtransport/third_party/nrappkit/src/port/generic/include/sys/queue.h, which is used for the compilation of mtransport since media/mtransport/third_party/nrappkit/src/port/generic/include is in LOCAL_INCLUDES of media/mtransport/build/moz.build.

The attached patch puts media/mtransport/third_party/nrappkit/src/port/generic/include also into LOCAL_INCLUDES of media/webrtc/signaling/test/moz.build to fix the compilation.
Attachment #8589479 - Flags: review?(ekr)
Comment on attachment 8589479 [details] [diff] [review]
Proposed patch

Review of attachment 8589479 [details] [diff] [review]:
-----------------------------------------------------------------

This patch looks fine, but please provide a try push demonstrating it does not cause problems and then ask for re-review.
Attachment #8589479 - Flags: review?(ekr)
This needs rebasing.
Assignee: nobody → felix.janda
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Posted patch Rebased patch (obsolete) — Splinter Review
Attachment #8589479 - Attachment is obsolete: true
Does now also the same to media/mtransport/test/moz.build thereby making the include consistent between mtransport/test/moz.build, mtransport/common.build and webrtc/signaling/test/common.build.
Attachment #8592416 - Attachment is obsolete: true
Comment on attachment 8593600 [details] [diff] [review]
Expanded patch

Review of attachment 8593600 [details] [diff] [review]:
-----------------------------------------------------------------

Please provide a try push demonstrating that this doesn't break the build for other platforms.
(In reply to Eric Rescorla (:ekr) from comment #5)
> Please provide a try push demonstrating that this doesn't break the build
> for other platforms.

Sorry, I don't have access to the try server. Could you do a try push for me?
Attachment #8593600 - Flags: review?(ekr)
Attachment #8593600 - Flags: review?(ekr) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a5f9103a3f66
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.