Enable djinterop on Windows CI builds #3155
Conversation
| @@ -49,6 +49,9 @@ endif() | |||
| ####################################################################### | |||
|
|
|||
| set(CMAKE_CXX_STANDARD 17) | |||
| if(MSVC) | |||
| set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /Zc:__cplusplus") | |||
There was a problem hiding this comment.
This is required for the current version of libdjinterop (which looks only at __cplusplus and doesn't use any _MSVC_LANG hacks), but may be considered a good idea to enable anyway.
FWIW, if used on an older version of MSVC, this prints an "unrecognised option" message and carries on anyway.
There was a problem hiding this comment.
Shouldn't it suffice to add this to the libdjinterop CMakeLists.txt? Or is this required for libdjinterop includes? If so, will building Mixxx still work if an older MSVC version is used that doesn't support the flag?
There was a problem hiding this comment.
This flag is already set in libdjinterop's own CMakeLists.txt file, but it is indeed also required to use libdjinterop's public includes, which is why it's required in Mixxx in order to use libdjinterop.
The /Zc:__cplusplus flag is supported from VS 2017 onwards, which appears to be the minimum-supported version to build Mixxx (according to the wiki). That was my reasoning for adding the flag for the whole MSVC build - hope this is ok.
|
|
||
| set(DJINTEROP_INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/lib/libdjinterop-install") | ||
| set(DJINTEROP_LIBRARY "lib/${CMAKE_STATIC_LIBRARY_PREFIX}djinterop${CMAKE_STATIC_LIBRARY_SUFFIX}") |
There was a problem hiding this comment.
Seems reasonable since CMAKE_INSTALL_LIBDIR refers to the standard system location while the local builds use just "lib".
mr-smidge
force-pushed
the
enh/djinterop-win-ci
branch
2 times, most recently
from
91b5827
to
71213a5
Compare
|
The Ubuntu2004 build timed out after 1 hour - please could this be restarted? (I don't have appropriate permissions to do so.) FWIW, the OSX/Scons build also timed out, but I understand this is considered less important as the CMake build passed just fine. Many thanks! |
Unfortunately, AppVeyour builds cannot be restarted afaik. |
mr-smidge
force-pushed
the
enh/djinterop-win-ci
branch
from
71213a5
to
e84d713
Compare
I've force-pushed a no-op update to kick the builds off again. Fingers crossed... |
|
All CMake and AppVeyor builds are now green. |
|
LGTM |
|
Now the master build got stuck at the exact same step: It might be a coincidence, but it is suspicious. @Holzhaus Did we miss something? |
I notice that subsequent master builds now appear ok. There don't seem to be many useful timestamps in the console output to see where it's slow, but if it turns out to be due to libdjinterop, I'm happy to at least take a look. |
This PR enables building of the
ENGINEPRIMEfeature on Windows builds.It also:
/Zc:__cplusplusflag for the whole build when using MSVC - see this post on the MS C++ Team Blog.