Last Comment Bug 699646 - Get the alder branch to compile on windows
: Get the alder branch to compile on windows
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: WebRTC (show other bugs)
: Other Branch
: All Windows 7
: -- normal (vote)
: ---
Assigned To: Ted Mielczarek [:ted.mielczarek]
: Jason Smith [:jsmith]
Mentors:
: 768325 (view as bug list)
Depends on: 643692 734856 734859 753895 783631 783703
Blocks: 688178 729541
  Show dependency treegraph
 
Reported: 2011-11-03 17:06 PDT by [:fabrice] Fabrice Desré
Modified: 2012-09-16 10:17 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
disable some methods for accessing Firefox profiles and AppData directories to help Windows builds (15.54 KB, patch)
2012-04-04 17:04 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Review
Disable building signaling (2.10 KB, patch)
2012-04-04 17:05 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Review
Disable building signaling (2.48 KB, patch)
2012-04-04 17:08 PDT, Randell Jesup [:jesup]
no flags Details | Diff | Review
fixes to make signaling build on windows (8.02 KB, patch)
2012-08-23 11:07 PDT, Ted Mielczarek [:ted.mielczarek]
rjesup: review+
ekr: review+
Details | Diff | Review
link webrtc into libxul instead of gkmedias (8.01 KB, patch)
2012-08-23 11:10 PDT, Ted Mielczarek [:ted.mielczarek]
rjesup: review+
Details | Diff | Review
random hacks (19.19 KB, patch)
2012-08-23 11:11 PDT, Ted Mielczarek [:ted.mielczarek]
rjesup: review+
Details | Diff | Review
link webrtc into libxul instead of gkmedias. (24.70 KB, patch)
2012-08-27 13:40 PDT, Ted Mielczarek [:ted.mielczarek]
no flags Details | Diff | Review

Description [:fabrice] Fabrice Desré 2011-11-03 17:06:55 PDT
We need some help from the build masters to figure out how to make the alder branch (http://hg.mozilla.org/projects/alder/) to compile on windows.
Comment 1 Randell Jesup [:jesup] 2012-03-01 21:40:33 PST
Builds on Win32 currently for private builds

For building on the Win32 build/try servers, the remaining issues are DirectShow base classes, reimplementing the capture filters using the base classes, lack of json python module, maybe others.
Comment 2 Ethan Hugg [:ehugg] 2012-03-05 14:47:39 PST
This appears to work if you use pymake (python -OO build/pymake/make.py -f client.mk), but not if you use gmake (make -f client.mk).  In both cases the variables at the top of the webrtc Makefiles start with "c:/" when gmake requires that they start with "/c/"
Comment 3 Ted Mielczarek [:ted.mielczarek] 2012-03-08 11:36:33 PST
Bleh, I guess I didn't test that because I never build with gmake anymore. I'll have to look into that.
Comment 4 Randell Jesup [:jesup] 2012-04-04 17:04:34 PDT
Created attachment 612404 [details] [diff] [review]
disable some methods for accessing Firefox profiles and AppData directories to help Windows builds

These probably aren't even needed for us, and most of them aren't used in the source tree
Comment 5 Randell Jesup [:jesup] 2012-04-04 17:05:11 PDT
Created attachment 612405 [details] [diff] [review]
Disable building signaling
Comment 6 Randell Jesup [:jesup] 2012-04-04 17:08:04 PDT
Created attachment 612408 [details] [diff] [review]
Disable building signaling
Comment 7 Ted Mielczarek [:ted.mielczarek] 2012-04-05 09:30:21 PDT
The signalling code issue is the last remaining problem (AFAIK). Things were building on Windows prior to that landing, and they're green on Tinderbox now with that code disabled. I'll see if I can fix that.
Comment 8 Ted Mielczarek [:ted.mielczarek] 2012-08-18 09:24:45 PDT
A couple of changes from bug 729511 were landed as well to fix Windows issues:
https://hg.mozilla.org/projects/alder/rev/e4cbd59b7230
https://hg.mozilla.org/projects/alder/rev/ec60a3e99a70

As well as this change which fixed an error with "make -j1" that we were only hitting on the Windows builders:
https://hg.mozilla.org/projects/alder/rev/9a1734a6910d
Comment 9 Ted Mielczarek [:ted.mielczarek] 2012-08-23 10:53:07 PDT
*** Bug 768325 has been marked as a duplicate of this bug. ***
Comment 10 Ted Mielczarek [:ted.mielczarek] 2012-08-23 11:07:32 PDT
Created attachment 654696 [details] [diff] [review]
fixes to make signaling build on windows

A whole bunch of fixes to get signaling code compiling on Windows.
Comment 11 Ted Mielczarek [:ted.mielczarek] 2012-08-23 11:10:44 PDT
Created attachment 654700 [details] [diff] [review]
link webrtc into libxul instead of gkmedias

This changes the linkage so that the WebRTC libs go into libxul instead of gkmedias on Windows. I added a make variable so we can flip this off as a fallback position if we need to. That configuration isn't likely to build on Windows, but it should mostly just be a matter of fixing up the symbol exports from gkmedias.
Comment 12 Ted Mielczarek [:ted.mielczarek] 2012-08-23 11:11:57 PDT
Created attachment 654703 [details] [diff] [review]
random hacks

These are a bunch of other fixes that became necessary once we linked everything on Windows.
Comment 13 Eric Rescorla (:ekr) 2012-08-23 12:24:17 PDT
Comment on attachment 654696 [details] [diff] [review]
fixes to make signaling build on windows

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

This looks pretty sound. Do you think we should fix the ERROR thing first?
Comment 14 Eric Rescorla (:ekr) 2012-08-23 12:25:54 PDT
Comment on attachment 654703 [details] [diff] [review]
random hacks

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

::: media/webrtc/signaling/test/Makefile.in
@@ +90,5 @@
> +#   signaling_unittests.cpp \
> +#   mediapipeline_unittest.cpp \
> +#   mediaconduit_unittests.cpp \
> +#   webrtc_standalone_test.cpp \
> +#   $(NULL)

We should probably resurrect these before we push.
Comment 15 Randell Jesup [:jesup] 2012-08-23 12:43:53 PDT
Comment on attachment 654703 [details] [diff] [review]
random hacks

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

I note that this turns off the unit tests, so this will need to be followed up on and fixed or enabled for all other platforms (for now).  See comments.  r+ if the tests are at least enabled for other platforms.

::: media/mtransport/test/Makefile.in
@@ +101,5 @@
> +#   sockettransportservice_unittest.cpp \
> +#   transport_unittests.cpp \
> +#   runnable_utils_unittest.cpp \
> +#   sctp_unittest.cpp \
> +#   $(NULL)

We should resurrect these before pushing, or make them ifndef windows

::: media/webrtc/signaling/src/sipcc/cpr/win32/cpr_win_string.c
@@ +65,5 @@
>  strdup (const char *input_str)
>  {
>      return (_strdup(input_str));
>  }
> +*/

#if 0 instead of /*... */, maybe?  or #if !MOZILLA (whatever sym is appropriate).  Or just remove it.

::: media/webrtc/signaling/src/sipcc/cpr/win32/cpr_win_string.h
@@ +51,5 @@
>  #define cpr_strtok(a,b,c) strtok_s(a,b,c)
>  
>  int strncasecmp (const char *s1, const char *s2, size_t length);
>  int strcasecmp (const char *s1, const char *s2);
> +//char *strdup (const char *input_str);

Ditto comments from .c file

::: media/webrtc/signaling/test/Makefile.in
@@ +90,5 @@
> +#   signaling_unittests.cpp \
> +#   mediapipeline_unittest.cpp \
> +#   mediaconduit_unittests.cpp \
> +#   webrtc_standalone_test.cpp \
> +#   $(NULL)

Or at least make them ifndef windows
Comment 16 Ted Mielczarek [:ted.mielczarek] 2012-08-24 08:48:37 PDT
(In reply to Eric Rescorla from comment #13)
> This looks pretty sound. Do you think we should fix the ERROR thing first?

That's already filed as bug 783654, I don't think it matters. The workaround here is pretty small, if you want to clean that up afterwards I understand, but I think we should just land this as-is.

Sorry about leaving the commented-out bits, I was just hacking and slashing. The unittests I'll probably ifdef Windows for now, I have to tweak something to get them building on Linux again.
Comment 17 Ted Mielczarek [:ted.mielczarek] 2012-08-27 13:40:42 PDT
Created attachment 655744 [details] [diff] [review]
link webrtc into libxul instead of gkmedias.

This is what I plan on landing. I've flipped things around so that we only build this "webrtc in libxul" config on Windows, which is the only platform where we split gkmedias off from libxul anyway, so it's really just bookkeeping on other platforms. With webrtc-in-libxul, the signaling unittests can't link to the webrtc bits properly, so this is simpler for right now. This builds fine for me on Windows and Linux.
Comment 18 Ted Mielczarek [:ted.mielczarek] 2012-08-27 13:52:00 PDT
Comment on attachment 654703 [details] [diff] [review]
random hacks

This patch got folded into the other one.
Comment 20 Ted Mielczarek [:ted.mielczarek] 2012-08-28 06:10:43 PDT
Followup to fix a few things that crept in in the interim:
http://hg.mozilla.org/projects/alder/rev/0bd0222c660e

Note You need to log in before you can comment on or make changes to this bug.