Get the alder branch to compile on windows

RESOLVED FIXED

Status

()

Core
WebRTC
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: fabrice, Assigned: ted)

Tracking

Other Branch
All
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa-])

Attachments

(2 attachments, 5 obsolete attachments)

(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
Blocks: 688178
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.
Component: Video/Audio → WebRTC
QA Contact: video.audio → webrtc-general

Updated

6 years ago
Depends on: 643692

Comment 2

6 years ago
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/"
(Assignee)

Comment 3

6 years ago
Bleh, I guess I didn't test that because I never build with gmake anymore. I'll have to look into that.

Updated

5 years ago
Depends on: 734856

Updated

5 years ago
Depends on: 734859
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
Created attachment 612405 [details] [diff] [review]
Disable building signaling
Created attachment 612408 [details] [diff] [review]
Disable building signaling
Attachment #612405 - Attachment is obsolete: true
(Assignee)

Comment 7

5 years ago
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.
Assignee: nobody → ted.mielczarek
(Assignee)

Updated

5 years ago
Assignee: ted.mielczarek → rjesup

Updated

5 years ago
Depends on: 753895

Updated

5 years ago
Depends on: 768450

Updated

5 years ago
Depends on: 768325

Updated

5 years ago
QA Contact: jsmith
(Assignee)

Updated

5 years ago
Assignee: rjesup → ted.mielczarek
(Assignee)

Updated

5 years ago
No longer depends on: 768450
(Assignee)

Updated

5 years ago
Depends on: 783631
(Assignee)

Comment 8

5 years ago
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
(Assignee)

Updated

5 years ago
No longer depends on: 768325
Duplicate of this bug: 768325
(Assignee)

Updated

5 years ago
Attachment #612404 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Attachment #612408 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Blocks: 729541
(Assignee)

Updated

5 years ago
Depends on: 783703
(Assignee)

Comment 10

5 years ago
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.
(Assignee)

Comment 11

5 years ago
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.
(Assignee)

Comment 12

5 years ago
Created attachment 654703 [details] [diff] [review]
random hacks

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

Updated

5 years ago
Attachment #654696 - Flags: review+
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?

Updated

5 years ago
Attachment #654696 - Flags: review+
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.

Updated

5 years ago
Attachment #654700 - Flags: review+
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
Attachment #654703 - Flags: review+
(Assignee)

Comment 16

5 years ago
(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.
(Assignee)

Comment 17

5 years ago
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.
(Assignee)

Updated

5 years ago
Attachment #654700 - Attachment is obsolete: true
(Assignee)

Comment 18

5 years ago
Comment on attachment 654703 [details] [diff] [review]
random hacks

This patch got folded into the other one.
Attachment #654703 - Attachment is obsolete: true
(Assignee)

Comment 19

5 years ago
https://hg.mozilla.org/projects/alder/rev/e2cf1d2d6905
https://hg.mozilla.org/projects/alder/rev/cd0c792da3dd
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 20

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

Updated

5 years ago
Whiteboard: ]qa-]

Updated

5 years ago
Whiteboard: ]qa-] → [qa-]
You need to log in before you can comment on or make changes to this bug.