699646 - Get the alder branch to compile on windows

Status: RESOLVED FIXED

(Core :: WebRTC, defect)

Description

[[:fabrice] Fabrice Desré]

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] (needinfo me)]

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]]

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

[(not currently active) Ted Mielczarek]

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] (needinfo me)]

Attachment: 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] (needinfo me)]

Attachment: Disable building signaling

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Attachment: Disable building signaling

Comment 7

[(not currently active) Ted Mielczarek]

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

[(not currently active) Ted Mielczarek]

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 10

[(not currently active) Ted Mielczarek]

Attachment: fixes to make signaling build on windows

A whole bunch of fixes to get signaling code compiling on Windows.

Comment 11

[(not currently active) Ted Mielczarek]

Attachment: 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

[(not currently active) Ted Mielczarek]

Attachment: random hacks

These are a bunch of other fixes that became necessary once we linked everything on Windows.

Comment 13

[Eric Rescorla (:ekr)]

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)]

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] (needinfo me)]

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

[(not currently active) Ted Mielczarek]

(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

[(not currently active) Ted Mielczarek]

Attachment: 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

[(not currently active) Ted Mielczarek]

Comment on attachment 654703 [details] [diff] [review]
random hacks

This patch got folded into the other one.

Comment 19

[(not currently active) Ted Mielczarek]

https://hg.mozilla.org/projects/alder/rev/e2cf1d2d6905
https://hg.mozilla.org/projects/alder/rev/cd0c792da3dd

Comment 20

[(not currently active) Ted Mielczarek]

Followup to fix a few things that crept in in the interim:
http://hg.mozilla.org/projects/alder/rev/0bd0222c660e