dolphin
dolphin copied to clipboard
Remove Visual Studio
See #8068 for rationales for removing Visual Studio.
That's not the main point of this PR though, the main point of this PR is to enable vcpkg integration.
Having a package manager on all systems will allow us to slowly thin out Externals and stop shipping all our dependencies in-tree. Not all dependencies will be suited to this, but something is much better than nothing.
In regards to the buildbots, I think dusting off dolphin-emu/sadm#133 should work fine.
Note: to people who have concerns that switching the buildsystem will make things difficult for old PR's, any old PR's will already need to rebase to build due to the android issues, the freebsd issues, etc.
Two things:
- While I agree that removing the VS Project files is a good idea in general, first we need to ensure that the CMake project actually does the same thing as the VS Project files for Windows+MSVC. I'm pretty sure there's still some inconsistency with compile flags and the like there -- and I know that incremental updating of the current build revision is broken, because my local build never updates that correctly unless I do a full wipe.
- vcpkg is a wholly different beast that you're not going to get through in the same breath as removing the VS Project files. Either do that first or leave that for later.
Having Externals is a big advantage in terms of controlling dependencies and how the dependencies code is compiled. We can easily adjust compile flags uniformly across the entire project, something which is possible but not currently implemented in our cmake environment.
I'm strongly against using vcpkg in dolphin.
- vcpkg is a wholly different beast that you're not going to get through in the same breath as removing the VS Project files. Either do that first or leave that for later.
I know, I'm leaving it for a seperate PR. Just putting it as part of my rationale.
I had a look, and all of the flags appear to be the same.
Incremental updating of the build version appears to be broken because CMake does not call make_scmrev.h.js. I can't find any official examples on how to call a javascript file in CMake, so I was planning on converting it to Python. Is there any benefit to it being a JS file?
I assume it's js because that's what VS build scripts support?
I also see no reason why we need to invoke python for this, we have this set up at work with pure CMake.
For reference, Dolphin's CMake files already do this, it's just not properly re-executed when the commit changes:
https://github.com/dolphin-emu/dolphin/blob/1088021e39750c22ba622ab9e5065ffc32cb0e39/CMakeLists.txt#L165-L216 https://github.com/dolphin-emu/dolphin/blob/1088021e39750c22ba622ab9e5065ffc32cb0e39/CMakeLists.txt#L974-L986
Fixed.
Please make the CMakeLists.txt change a separate PR.
See #11035.
Seeing that #11061 is now merged, is this just waiting on buildbots? (i.e. do people have any other changes, comments or complaints)
I'm not convinced the CMake build files use the correct flags yet. Taking a random file:
CMake in Debug mode:
build Source\Core\AudioCommon\CMakeFiles\audiocommon.dir\AudioCommon.cpp.obj: CXX_COMPILER__audiocommon_Debug dolphin\Source\Core\AudioCommon\AudioCommon.cpp || cmake_object_order_depends_target_audiocommon
DEFINES = -DAUTOUPDATE=1 -DDATA_DIR=\"dolphin/out/install/Debug/share/dolphin-emu/\" -DHAS_OPENGL -DHAS_VULKAN -DHAVE_FFMPEG -DHAVE_SDL2=1 -DNOMINMAX -DSFML_STATIC -DUNICODE -DUSE_ANALYTICS=1 -DUSE_UPNP -DWIN32_LEAN_AND_MEAN -D_ARCH_64=1 -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_DEBUG -D_M_X86=1 -D_M_X86_64=1 -D_SCL_SECURE_NO_WARNINGS -D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING -D_UNICODE -D__LIBUSB__ -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS
FLAGS = /DWIN32 /D_WINDOWS /EHsc /Zi /Ob0 /Od /RTC1 -MDd /GR- /W4 /WX /wd4201 /wd4127 /wd4100 /wd4200 /wd4244 /wd4121 /wd4324 /wd4714 /wd4351 /wd4245 /w44263 /w44265 /w44946 /utf-8 /external:anglebrackets /external:W0 /external:templates- /experimental:deterministic /Gy /Oi /GS /permissive- /Zc:inline /Zc:throwingNew /volatile:iso /experimental:newLambdaProcessor /Zc:__cplusplus,externConstexpr,lambda,preprocessor /wd5105 /fp:precise /Yupch.h /FIpch.h /Fpdolphin/Build/Debug/Source/PCH/dolphin.pch /Z7 /Gm- -std:c++latest
INCLUDES = -Idolphin\Externals\curl\include -Idolphin\Externals\SFML\include -Idolphin\Source\Core -Idolphin\Externals\enet\include -Idolphin\External\minizip -Idolphin\Externals\ed25519 -Idolphin\Externals\soundtouch -Idolphin\Externals\mbedtls\include -Idolphin\Externals\libiconv-1.14\include -Idolphin\Externals\discord-rpc\include -Idolphin\Externals\WIL\include -Idolphin\Externals\OpenAL\include -Idolphin\Externals\picojson -Idolphin\Build\Debug\Source\Core -Idolphin\Externals\fmt\include -Idolphin\Externals\minizip\. -Idolphin\Build\Debug\Externals\zlib-ng\zlib-ng -Idolphin\Externals\zlib-ng\zlib-ng -Idolphin\Externals\cubeb\include -Idolphin\Build\Debug\exports -Idolphin\Externals\FreeSurround\include
OBJECT_DIR = Source\Core\AudioCommon\CMakeFiles\audiocommon.dir
OBJECT_FILE_DIR = Source\Core\AudioCommon\CMakeFiles\audiocommon.dir
TARGET_COMPILE_PDB = Source\Core\AudioCommon\CMakeFiles\audiocommon.dir\audiocommon.pdb
TARGET_PDB = Source\Core\AudioCommon\audiocommon.pdb
MSVC in Debug mode:
^DOLPHIN\SOURCE\CORE\AUDIOCOMMON\AUDIOCOMMON.CPP
/c /IDOLPHIN\EXTERNALS\ZSTD\LIB /I"DOLPHIN\EXTERNALS\ZLIB-NG" /IDOLPHIN\EXTERNALS\XXHASH /I"DOLPHIN\EXTERNALS\SPIRV_CROSS\SPIRV-CROSS" /IDOLPHIN\EXTERNALS\SOUNDTOUCH /IDOLPHIN\EXTERNALS\SFML\INCLUDE /IDOLPHIN\EXTERNALS\SDL\SDL\INCLUDE /IDOLPHIN\EXTERNALS\PUGIXML /IDOLPHIN\EXTERNALS\PICOJSON /IDOLPHIN\EXTERNALS\MINIZIP /IDOLPHIN\EXTERNALS\MINIUPNPC\SRC /IDOLPHIN\EXTERNALS\MGBA\MGBA\INCLUDE /IDOLPHIN\EXTERNALS\MBEDTLS\INCLUDE /IDOLPHIN\EXTERNALS\LZO /IDOLPHIN\EXTERNALS\LIBUSB\LIBUSB\LIBUSB /IDOLPHIN\EXTERNALS\LIBSPNG\LIBSPNG\SPNG /IDOLPHIN\EXTERNALS\LIBLZMA\API /IDOLPHIN\EXTERNALS\IMGUI /IDOLPHIN\EXTERNALS\GLSLANG\SPIRV /IDOLPHIN\EXTERNALS\GLSLANG\GLSLANG\PUBLIC /IDOLPHIN\EXTERNALS\GLSLANG\STANDALONE /IDOLPHIN\EXTERNALS\GLSLANG /IDOLPHIN\EXTERNALS\FREESURROUND\INCLUDE /IDOLPHIN\EXTERNALS\FMT\INCLUDE /IDOLPHIN\EXTERNALS\FATFS /IDOLPHIN\EXTERNALS\ENET\INCLUDE /IDOLPHIN\EXTERNALS\ED25519 /I"DOLPHIN\EXTERNALS\DISCORD-RPC\INCLUDE" /IDOLPHIN\EXTERNALS\CURL\INCLUDE /IDOLPHIN\EXTERNALS\CUBEB\INCLUDE /IDOLPHIN\EXTERNALS\CUBEB\MSVC /I"DOLPHIN\EXTERNALS\CPP-OPTPARSE" /IDOLPHIN\EXTERNALS\BZIP2 /IDOLPHIN\EXTERNALS\BOCHS_DISASM /IDOLPHIN\EXTERNALS\WIL\INCLUDE /IDOLPHIN\EXTERNALS\VULKAN\INCLUDE /IDOLPHIN\EXTERNALS\RANGESET\INCLUDE /IDOLPHIN\EXTERNALS\OPENAL\INCLUDE /I"DOLPHIN\EXTERNALS\FFMPEG-BIN\X64\INCLUDE" /IDOLPHIN\SOURCE\CORE\ /Z7 /JMC /nologo /W4 /WX /diagnostics:caret /Od /Oi /D SPIRV_CROSS_EXCEPTIONS_TO_ASSERTIONS /D SFML_STATIC /D SPNG_STATIC /D LZMA_API_STATIC /D CURL_STATICLIB /D HAVE_SDL2 /D AUTOUPDATE /D HAS_LIBMGBA /D HAS_VULKAN /D HAS_OPENGL /D HAVE_FFMPEG /D USE_DISCORD_PRESENCE /D USE_ANALYTICS=1 /D USE_UPNP /D __LIBUSB__ /D _ARCH_64=1 /D _M_X86=1 /D _M_X86_64=1 /D _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING /D _WINSOCK_DEPRECATED_NO_WARNINGS /D _CRT_SECURE_NO_WARNINGS /D _CRT_NONSTDC_NO_WARNINGS /D WIN32_LEAN_AND_MEAN /D NOMINMAX /D WIL_SUPPRESS_EXCEPTIONS /D _DEBUG /D _UNICODE /D UNICODE /Gm- /EHsc /RTC1 /MDd /GS /Gy /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR- /std:c++latest /permissive- /Yu"pch.h" /Fp"DOLPHIN\BUILD\X64\DEBUG\PCH\PCH.PCH" /Fo"DOLPHIN\BUILD\X64\DEBUG\DOLPHINLIB\\" /Fd"DOLPHIN\BUILD\X64\DEBUG\DOLPHINLIB\BIN\DOLPHINLIB.PDB" /external:anglebrackets /external:W0 /external:templates- /Gd /TP /wd4245 /wd4201 /wd4127 /wd4100 /wd4244 /wd4121 /wd4324 /wd4714 /FIPCH.H /FC /w44263 /w44265 /w44946 /pathmap:"C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\\"=v:\ /pathmap:"C:\Program Files (x86)\Windows Kits\10\\"=w:\ /pathmap:"dolphin\\"=d:\ /experimental:deterministic /utf-8 /Zo /Zc:__cplusplus,externConstexpr,lambda,preprocessor,throwingNew /volatile:iso DOLPHIN\SOURCE\CORE\AUDIOCOMMON\AUDIOCOMMON.CPP
Actually untangling that mess, if I didn't make any errors:
-
MSVC defines but CMake doesn't:
SPIRV_CROSS_EXCEPTIONS_TO_ASSERTIONSSPNG_STATICLZMA_API_STATICCURL_STATICLIBHAS_LIBMGBAUSE_DISCORD_PRESENCE_WINSOCK_DEPRECATED_NO_WARNINGSWIL_SUPPRESS_EXCEPTIONS
-
CMake defines but MSVC doesn't:
_CRT_SECURE_NO_DEPRECATE_SCL_SECURE_NO_WARNINGS__STDC_CONSTANT_MACROS__STDC_LIMIT_MACROSWIN32_WINDOWS
-
Other minor define differences that are probably harmless:
- MSVC has
HAVE_SDL2, CMake hasHAVE_SDL2=1. - MSVC has
AUTOUPDATE, CMake hasAUTOUPDATE=1.
- MSVC has
-
Compile flags differences:
- MSVC sets
/c(compile but not link), which is probably implicit in CMake? Harmless, either way. - CMake's debugging info flags are weird (it has both
/Ziand/Z7which are incompatible, and does not have MSVC's/JMCand/Zo), someone probably should look at that. - MSVC sets
/nologo, harmless. - MSVC sets
/diagnostics:caret, harmless. - MSVC sets
/Foand/Fd, harmless. - MSVC sets
/Zc:wchar_tand/Zc:forScope, these are probably meaningful. - CMake sets
/Ob0, disabling inline expansion. I'm guessing this is because I compiled in Debug but MSVC does not set this so maybe should look into this. - CMake sets
/experimental:newLambdaProcessor. I think this is outdated? - MSVC sets
/Gd, does this even matter on x64? - MSVC sets
/TP, harmless. - MSVC sets
/FC, harmless. - MSVC sets a few
/pathmaps. These are meaningful, IIRC we use them to trim the full filepaths out of log output? - CMake sets
/wd4200,/wd4351and/wd5105but MSVC does not, that's a bit odd. Does warning 4351 even exist??
- MSVC sets
Most of the defines happen because our CMake files actually have more localized defines while the MSVC ones just have global ones (eg. the missing SPNG_STATIC and HAS_LIBMGBA, which aren't linked to Common), I think. That's fine. But eg. the WIL_SUPPRESS_EXCEPTIONS I can't find in CMake at all, so that seems wrong.
The compile flags look mostly fine but there's a few that should probably be looked at in more detail, definitely not quite identical.
To be clear, I'm not saying that the current VS flags are the best possible options, but I do think that changes to them should be deliberate, not accidental.
When you say harmles, do you mean I don't need to implement that define or I should implement it? I'm not too familiar with the various meanings and implications of all of the compile flags.
Harmless means you can ignore it.
I've been working on this-- what should I do for CMake defines that MSVC doesn't have? I'm not going for an overhaul of all our defines here, just trying to remove MSVC, so should I just make CMake have the same important defines and flags as MSVC?
CMake's debugging info flags are weird (it has both /Zi and /Z7 which are incompatible
I've fixed that in #11506 (though it's a bit of a mess) among a few other issues.