Closed
Bug 813911
Opened 12 years ago
Closed 11 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)
1.81 KB,
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•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d70010fceafa
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Updated•11 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
•