Closed
Bug 813911
Opened 12 years ago
Closed 12 years ago
nrappkit code doesn't yet build on Android
Categories
(Core :: WebRTC, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: gcp, Assigned: gcp)
References
Details
(Whiteboard: [WebRTC] [blocking-webrtc-] [qa-])
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
gcp
:
review+
|
Details | Diff | Splinter Review |
Fix nrappkit so it compiles on android.
Assignee | ||
Updated•12 years ago
|
Attachment #683942 -
Attachment is patch: true
Updated•12 years ago
|
Priority: -- → P2
Whiteboard: [WebRTC] [blocking-webrtc-]
Assignee | ||
Updated•12 years ago
|
Attachment #683942 -
Flags: review?(dmose)
Attachment #683942 -
Flags: feedback?(ted)
Comment 1•12 years ago
|
||
Comment on attachment 683942 [details] [diff] [review]
Patch 1. Fix nrappkit build.
Review of attachment 683942 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/mtransport/third_party/nrappkit/nrappkit.gyp
@@ +194,5 @@
> './src/port/win32/include/csi_platform.h',
> ],
> }],
> + ## Android (Linux)
> + [ 'OS == "android"', {
Can't you just combine this into the Linux block above, by changing it to
[ 'OS == "linux" or OS == "android"'. {
@@ +200,5 @@
> + '-Wall',
> + '-Wno-parentheses',
> + '-Wno-strict-prototypes',
> + '-Wmissing-prototypes',
> + ],
We don't actually use the cflags here, do we? We only use cflags_mozilla, right?
Attachment #683942 -
Flags: feedback?(ted) → feedback-
Comment 2•12 years ago
|
||
Comment on attachment 683942 [details] [diff] [review]
Patch 1. Fix nrappkit build.
Review of attachment 683942 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/mtransport/third_party/nrappkit/nrappkit.gyp
@@ +200,5 @@
> + '-Wall',
> + '-Wno-parentheses',
> + '-Wno-strict-prototypes',
> + '-Wmissing-prototypes',
> + ],
Ted's correct: cflags is ignored; it needs to be cflags_mozilla
@@ +218,5 @@
>
> + 'include_dirs': [
> + 'src/port/linux/include'
> + ],
> +
Spurious spaces
Assignee | ||
Comment 3•12 years ago
|
||
Incorporate review comments.
Attachment #683942 -
Attachment is obsolete: true
Attachment #683942 -
Flags: review?(dmose)
Attachment #690429 -
Flags: review?(dmose)
Comment 4•12 years ago
|
||
Comment on attachment 690429 [details] [diff] [review]
Patch 1.v2 Fix nrappkit build.
Review of attachment 690429 [details] [diff] [review]:
-----------------------------------------------------------------
Separately from the android case thing, this blows up on r5c because it can't find <rpc/rpc.h>. I'll upload a hack fix in a minute that simply disables that on both Android and Linux.
::: media/mtransport/third_party/nrappkit/nrappkit.gyp
@@ +197,2 @@
> ## Linux
> + [ '(OS == "linux") or (OS == "Android")', {
android needs to be lower-case, which makes this iteration of the patch not work at all. In order to catch issues like this, I think you need to run "make -f client.mk configure" before building.
Attachment #690429 -
Flags: review?(dmose) → review-
Comment 5•12 years ago
|
||
This fixes the android casing problem, and it also fixes the lack of <rpc/rpc.h> by simply setting NO_REG_RPC for both Android and Linux. Presumably, the right thing to do is to split those cases apart, but maybe turning it off for Linux is ok.
Assignee | ||
Comment 6•12 years ago
|
||
Comment on attachment 691087 [details] [diff] [review]
Patch 1.v3, Fix nrappkit build
r=dmose on IRC. According to ekr the RPC code is a leftover from days past.
Attachment #691087 -
Flags: review+
Updated•12 years ago
|
Attachment #690429 -
Attachment is obsolete: true
Updated•12 years ago
|
Attachment #691087 -
Attachment description: Patch 1.v3, WIP Fix nrappkit build → Patch 1.v3, Fix nrappkit build
Comment 7•12 years ago
|
||
Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d70010fceafa
Comment 8•12 years ago
|
||
Setting in-testsuite-; since this is a build fix; the compiler acts as an automated test.
Flags: in-testsuite-
Comment 9•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Updated•12 years ago
|
Whiteboard: [WebRTC] [blocking-webrtc-] → [WebRTC] [blocking-webrtc-] [qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•