Closed
Bug 816822
Opened 13 years ago
Closed 13 years ago
Android build patches for WebRTC unit tests
Categories
(Core :: WebRTC, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: dmosedale, Assigned: gcp)
References
Details
(Whiteboard: [WebRTC] [blocking-webrtc-] [qa-])
Attachments
(1 file, 3 obsolete files)
15.48 KB,
patch
|
dmosedale
:
review+
dmosedale
:
feedback+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•13 years ago
|
||
I didn't get as much time to poke at this today as I was hoping.
This patch is a hack that makes the signaling tests build; my suspicion is that the right fix there will not touch gtest-port.h but will do the appropriate override somewhere at a higher level.
Still need to fix the mtransport tests.
Assignee | ||
Comment 2•13 years ago
|
||
I think we can just do -DGTEST_HAS_TR1_TUPLE=0 on Android.
Assignee | ||
Comment 3•13 years ago
|
||
Cleaner fix for dmose's issue. Also fixes all remaining issues.
IIRC in one of the earlier patches I made Android set Userspace_os_Linux and added ifdef ANDROID's, this code is doing the opposite. Should probably clean this up to make all patches uniform.
Attachment #686909 -
Attachment is obsolete: true
Reporter | ||
Comment 4•13 years ago
|
||
Something on mozilla-inbound caused this patch to bit-rot. Here's an updated version that applies.
Attachment #687116 -
Attachment is obsolete: true
Flags: needinfo?
Assignee | ||
Updated•13 years ago
|
Attachment #687931 -
Flags: review?(dmose)
Attachment #687931 -
Flags: feedback?(ted)
Updated•13 years ago
|
Priority: -- → P2
Whiteboard: [WebRTC] [blocking-webrtc-]
Comment 5•13 years ago
|
||
Comment on attachment 687931 [details] [diff] [review]
Patch 1, v3, tests to make C++ unit tests build on Android
Review of attachment 687931 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/mtransport/test/sctp_unittest.cpp
@@ +97,5 @@
> EXPECT_GE(r, 0);
>
> memset(&local_addr_, 0, sizeof(local_addr_));
> local_addr_.sconn_family = AF_CONN;
> +#if !defined(__Userspace_os_Linux) && !defined(__Userspace_os_Windows) && !defined(__Userspace_os_Android)
As per before, maybe this wants to just be defined(__Userspace_os_Darwin) ?
Attachment #687931 -
Flags: feedback?(ted) → feedback+
Flags: needinfo?
Reporter | ||
Comment 6•13 years ago
|
||
The old patch was bitrotted by some recent checkin. Here's an unbitrotted version, with feedback+ and review? carried forward.
Attachment #687931 -
Attachment is obsolete: true
Attachment #687931 -
Flags: review?(dmose)
Attachment #691094 -
Flags: review?(dmose)
Attachment #691094 -
Flags: feedback+
Reporter | ||
Comment 7•13 years ago
|
||
Comment on attachment 691094 [details] [diff] [review]
Patch 1, v4, fixes to make C++ unit tests build on Android
Review of attachment 691094 [details] [diff] [review]:
-----------------------------------------------------------------
This patch looks good, my only question is about the license on the nrappkit files. The license doesn't actually state what license it is, which makes it harder to analyse.
If these are just copies of other files in the tree, and licensing@mozilla.org has already OKed this license, r=dmose. If not, can you either elaborate on why we don't need approval, or else mail them for approval?
r=dmose conditional on the licensing stuff being sorted out
Attachment #691094 -
Flags: review?(dmose) → review+
Reporter | ||
Comment 8•13 years ago
|
||
Oh, this also still needs the tweak the Ted requested in comment 5 before it lands.
Assignee | ||
Comment 9•13 years ago
|
||
>If these are just copies of other files in the tree
They are copies of the already-present linux/ files, with small fixes for Android.
Reporter | ||
Updated•13 years ago
|
Assignee: nobody → gpascutto
Reporter | ||
Updated•13 years ago
|
OS: Mac OS X → Android
Hardware: x86 → ARM
Reporter | ||
Comment 10•13 years ago
|
||
Landed on mozilla-inbound:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5bbd7e9882b3
Setting in-testsuite-, because this is a build fix, so the compiler itself acts as an automated test.
Flags: in-testsuite-
Comment 11•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Updated•13 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
•