Fix webrtc tests building on OpenBSD (missing -lsndio)

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gaston, Assigned: gaston)

Tracking

Trunk
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox39 unaffected, firefox40 fixed, firefox41 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch Add -lsndio on OpenBSD (obsolete) — Splinter Review
Similar to bug 1127510, currently if enabled webrtc tests fail to build on OpenBSD with missing symbols to sndio (which is needed by audio_device and cubeb)
See http://buildbot.rhaalovely.net/builders/mozilla-central-amd64/builds/1478/steps/build/logs/stdio for the full log.

The attached patch fixes the build for me.
Blocks: 1101651
Attachment #8600584 - Flags: review?(rjesup)
Attachment #8600584 - Flags: review?(rbarker)
Attachment #8600584 - Flags: review?(rjesup) → review+
Comment on attachment 8600584 [details] [diff] [review]
Add -lsndio on OpenBSD

This is a buildfile change, you officially should get a build peer (though it's trivial and for OpenBSD and for a cpp unit test)
Attachment #8600584 - Attachment is patch: true
Attachment #8600584 - Flags: review?(mh+mozilla)
Attachment #8600584 - Flags: review?(rbarker) → review+
Comment on attachment 8600584 [details] [diff] [review]
Add -lsndio on OpenBSD

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

::: media/webrtc/signaling/test/common.build
@@ +140,5 @@
>  if CONFIG['OS_TARGET'] == 'Darwin':
>      OS_LIBS += CONFIG['TK_LIBS']
>  
> +if CONFIG['OS_TARGET'] == 'OpenBSD':
> +    OS_LIBS += ['sndio']

Shouldn't this only be needed in media/webrtc/signaling/test/standalone/moz.build?
Attachment #8600584 - Flags: review?(mh+mozilla) → feedback+
That also works, i was more concerned about having it in a more "central" place so it wouldnt break if more tests were enabled/tweaked in other directories.

Can you mark it checkin-needed if you r+ it ? i wont be really available to land it (on a long trip)
Assignee: nobody → landry
Attachment #8602690 - Flags: review?(mh+mozilla)
Comment on attachment 8602690 [details] [diff] [review]
Add -lsndio on OpenBSD, only in standalone dir this time

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

::: media/webrtc/signaling/test/standalone/moz.build
@@ +57,5 @@
>  
> +if CONFIG['OS_TARGET'] == 'OpenBSD':
> +    OS_LIBS += ['sndio']
> +
> +

There's one too many carriage return.
Attachment #8602690 - Flags: review?(mh+mozilla) → review+
Attachment #8600584 - Attachment is obsolete: true
Sorry i messed up the bug # in the patch..
Will have to look at the details, but my aurora build from today failed, so it might also apply there.. depending on the merges.
Comment on attachment 8602690 [details] [diff] [review]
Add -lsndio on OpenBSD, only in standalone dir this time

Approval Request Comment
[Feature/regressing bug #]: 	1101651 
[User impact if declined]: failure to build on OpenBSD
[Describe test coverage new/current, TreeHerder]: in m-i and m-c, NPOTB
[Risks and why]: only affects a single tier3 platform

Can a sherrif fix the bug number when commiting ?
Attachment #8602690 - Flags: approval-mozilla-aurora?
Comment on attachment 8602690 [details] [diff] [review]
Add -lsndio on OpenBSD, only in standalone dir this time

Approved, seems like a test-only build.
Attachment #8602690 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Dunno the status of the merges, but it might have to go to -beta - 39 is still affected
Keywords: checkin-needed
Fx39 is on beta.
Status: NEW → RESOLVED
Closed: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Comment on attachment 8602690 [details] [diff] [review]
Add -lsndio on OpenBSD, only in standalone dir this time

Approval Request Comment

[Feature/regressing bug #]: 	1101651 
[User impact if declined]: failure to build on OpenBSD
[Describe test coverage new/current, TreeHerder]: in m-i and m-c and m-a now, NPOTB
[Risks and why]: only affects a single tier3 platform

Then i guess i'll have to ask for beta approval.. sigh
Attachment #8602690 - Flags: approval-mozilla-beta?
Comment on attachment 8602690 [details] [diff] [review]
Add -lsndio on OpenBSD, only in standalone dir this time

Approved for uplift to beta (39)
Attachment #8602690 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Bug 1101651 never landed on Fx39 (which is what mozilla-beta is).
Attachment #8602690 - Flags: approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.