Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for building Mumble and Murmur using the MinGW toolchain. #2907

Merged
merged 42 commits into from Mar 17, 2017

Conversation

@davidebeatrici
Copy link
Member

commented Mar 5, 2017

This pull request implements support for building Mumble and Murmur with the MinGW toolchain.

Both x86 and x86_64 targets are supported, and most features work.

For now, we use, and test against MXE (http://www.mxe.cc).

The PR also implements a Linux-based cross-build of Mumble and Murmur on Travis CI. This environment uses Wine as a test runner.

@davidebeatrici davidebeatrici added the build label Mar 5, 2017

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mingw-v2 branch 2 times, most recently from 58f80ad to e7d4cca Mar 5, 2017

@@ -6,6 +6,9 @@
#include "murmur_pch.h"

#ifdef Q_OS_WIN
# ifdef _INC_QOS2

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

Why? What error did you get?

qos2.h is already included via murmur_pch.h.... Was this a redefinition error... Or something else?

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member
../Connection.cpp: In destructor 'virtual Connection::~Connection()':
../Connection.cpp:58:51: error: 'QOSRemoveSocketFromFlow' was not declared in this scope
   if (! QOSRemoveSocketFromFlow(hQoS, 0, dwFlow, 0))
                                                   ^
../Connection.cpp: In member function 'void Connection::setToS()':
../Connection.cpp:70:70: error: 'QOSTrafficTypeAudioVideo' was not declared in this scope
 if (! QOSAddSocketToFlow(hQoS, qtsSocket->socketDescriptor(), NULL, QOSTrafficT
                                                                     ^
../Connection.cpp:70:96: error: 'QOS_NON_ADAPTIVE_FLOW' was not declared in this scope
 QoS, qtsSocket->socketDescriptor(), NULL, QOSTrafficTypeAudioVideo, QOS_NON_ADA
                                                                     ^
../Connection.cpp:70:126: error: 'QOSAddSocketToFlow' was not declared in this scope
 tDescriptor(), NULL, QOSTrafficTypeAudioVideo, QOS_NON_ADAPTIVE_FLOW, &dwFlow))
                                                                              ^

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

But it still didn't solve the error... there's another check in the header:
#if (_WIN32_WINNT >= 0x0600)

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

You should probably define _WIN32_WINNT before including windows.h or other windows headers then. In mumble_pch.hpp.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

I currently did it in Connection.cpp, to see if it works.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

Still not working. It is now defined in mumble_pch.hpp, let's see if it works.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mingw-v2 branch from f76e657 to c506533 Mar 5, 2017

@@ -81,6 +81,9 @@
#define interface struct
#endif

#ifndef _WIN32_WINNT

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

Please only define for MinGW. MSVC has a default somwhere, so that works already. Let's not explicitly set it to Vista for MSVC.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

Done.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

Perfect, it's working.

mumble_pch.hpp:85:0: warning: "_WIN32_WINNT" redefined
 # define _WIN32_WINNT 0x0600 // We need to do this for MinGW on Linux
 ^
In file included from /usr/lib/mxe/usr/i686-w64-mingw32.static/include/crtdefs.h:10:0,
                 from /usr/lib/mxe/usr/i686-w64-mingw32.static/include/stddef.h:7,
                 from /usr/lib/mxe/usr/lib/gcc/i686-w64-mingw32.static/5.4.0/include/stddef.h:1,
                 from /usr/lib/mxe/usr/lib/gcc/i686-w64-mingw32.static/5.4.0/include/c++/cstddef:45,
                 from /usr/lib/mxe/usr/i686-w64-mingw32.static/qt5/include/QtCore/qglobal.h:46,
                 from /usr/lib/mxe/usr/i686-w64-mingw32.static/qt5/include/QtCore/QtCore:4,
                 from mumble_pch.hpp:28:
/usr/lib/mxe/usr/i686-w64-mingw32.static/include/_mingw.h:225:0: note: this is the location of the previous definition
 #define _WIN32_WINNT 0x502
 ^

According to MSDN, 0x502 is Windows Server 2003 with SP1 or Windows XP with SP2...

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

Probably need to move it up earlier then... Qt also includes windows.h

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

The problem is:

/usr/lib/mxe/usr/i686-w64-mingw32.static/include/qos2.h:87:3: error: 'PQOS_FLOWID' has not been declared
   PQOS_FLOWID FlowId
   ^
/usr/lib/mxe/usr/i686-w64-mingw32.static/include/qos2.h:112:3: error: 'QOS_FLOWID' has not been declared
   QOS_FLOWID FlowId,
   ^
/usr/lib/mxe/usr/i686-w64-mingw32.static/include/qos2.h:122:3: error: 'QOS_FLOWID' has not been declared
   QOS_FLOWID FlowId,
   ^
/usr/lib/mxe/usr/i686-w64-mingw32.static/include/qos2.h:133:3: error: 'QOS_FLOWID' has not been declared
   QOS_FLOWID FlowId,
   ^

typedef UINT32 QOS_FLOWID is only in qos2.h, but at line 141, after the things that require it...

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

Shouldn't it throw a warning?

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

We don't want warnings in our codebase.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

In all internal builds and CI builds we use warnings-as-errors.
We strive to keep the build warning free.

User packages are not shipped with that option on, because we can't predict the future.
But we try our best to ensure that we don't ship anything to end-users that will trigger warnings.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

Of course, it's not a permanent fix.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

I thought we were trying to get this merged? :)

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mingw-v2 branch 2 times, most recently from 94037a5 to bddbdd8 Mar 5, 2017

@@ -82,12 +82,13 @@
#endif

// We need to do this for MinGW on Linux
#ifdef __MINGW32__ && if _WIN32_WINNT < 0x0600
#if defined(__MINGW32__) && if (_WIN32_WINNT < 0x0600)

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

Unrelated changes in this commit

# undef _WIN32_WINNT
# define _WIN32_WINNT 0x0600
#endif

#include <winsock2.h>
typedef UINT32 QOS_FLOWID, *PQOS_FLOWID;

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

Please gate this for MinGW.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

Done.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mingw-v2 branch 2 times, most recently from 8b18344 to 85c9e33 Mar 5, 2017

@@ -3,6 +3,11 @@
// that can be found in the LICENSE file at the root of the
// Mumble source tree or at <https://www.mumble.info/LICENSE>.

// We need to do this for MinGW on Linux

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

No, we need it for MinGW. Not just on Linux.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

Done.

@@ -3,6 +3,11 @@
// that can be found in the LICENSE file at the root of the
// Mumble source tree or at <https://www.mumble.info/LICENSE>.

// We need to do this for MinGW on Linux

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 5, 2017

Member

Also, please move this down just before #include <QtCore/QtCore>.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Mar 5, 2017

Author Member

Done.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mingw-v2 branch 16 times, most recently from 4ea24c4 to 84314e0 Mar 5, 2017

davidebeatrici added 26 commits Mar 6, 2017
compiler.pri: Define "MINGW_HAS_SECURE_API" to enable secure functions
MXE's MinGW has the secure functions disabled by default, because their headers check if "MINGW_HAS_SECURE_API" is defined.
opus-build: Rename "Win32" folder to "win32"
This fixes the following error with MinGW on Linux:
../opus-src/celt/bands.c:31:20: fatal error: config.h: No such file or directory
 #include "config.h"
                    ^
src: Fix QoS build with MinGW
For some reason QOS_FLOWID is defined in "qos2.h" after the lines that require it and PQOS_FLOWID is not defined at all.

MinGW's qos2.h header doesn't define QOS_NON_ADAPTIVE_FLOW, so we define it oursevles for MinGW to use.
mumble_pch.hpp: Set up Windows macros _WIN32_WINNT and NTDDI_VERSION to
target Windows 7 on MinGW.

"qos2.h" checks if the Windows version is equal or greater than 0x0600 (Windows Vista/Windows Server 2008).

The other headers we are using check if NTDDI_VERSION is equal or greater than Windows 7 before defining the features we need.
mumble_pch.hpp: Include <ws2tcpip.h> to fix MinGW build
Fixes:
error: 'socklen_t' does not name a type
DirectSound.cpp: Replace "LPDIRECTSOUNDNOTIFY8" with "LPDIRECTSOUNDNO…
…TIFY"

MSDN says that "IDirectSoundNotify8 is a define for IDirectSoundNotify. The two interface names are interchangeable."
https://msdn.microsoft.com/en-us/library/windows/desktop/ee418244(v=vs.85).aspx
Move "getenvQString()" function to a dedicated header and rename it
We do this in order to have a single function we can include in both CrashReporter.cpp and MumbleApplication.cpp.
This function is needed in CrashReporter.cpp because MinGW doesn't have "_wdupenv_s", the function we are currently using.
compiler.pri: Use the Unix block for win32-g++ too
So that we use the G++ features when compiling with MinGW.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:mingw-v2 branch from 1034f85 to cb1c1b4 Mar 17, 2017

@mkrautz mkrautz merged commit 10079ed into mumble-voip:master Mar 17, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.