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.

Sign up for GitHub

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

REFAC(client): Get rid of launcher and DLL on Windows #6246

Merged
merged 1 commit into from
Nov 4, 2023
Merged

REFAC(client): Get rid of launcher and DLL on Windows #6246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions docs/dev/TheMumbleSourceCode.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ When cloning the repo, the source tree should look something like this:
├── src
│   ├── crypto
│   ├── mumble
│   ├── mumble_exe
│   ├── murmur
│   └── tests
└── themes
Expand Down Expand Up @@ -81,11 +80,8 @@ mainly occupied by various scripts used throughout various tasks surrounding Mum
default configuration file for the Mumble server.

Your main focus should lie on the `src` directory as this is where the bulk of Mumble's source code is living. Directly in `src/` are shared sources
that are used by the Mumble client as well as by the server that live in `src/mumble` and `src/murmur` respectively. The `src/mumble_exe` direcrtory
contains code for the Windows-specific executable. In order to make sense of that you have to know that on Windows we are currently using a bit of an
odd system: The main application is packaged inside a DLL that is loaded by the main executable. This is a historic artifact and currently only
serves to create problems every now and then. In any case though, the mentioned "main executable" whose sole job is loading of that DLL lives in
`mumble_exe`. Generally you don't have to worry about it. The remaining directories within `src` should be somewhat self-explanatory.
that are used by the Mumble client as well as by the server that live in `src/mumble` and `src/murmur` respectively. The remaining directories within
`src` should be somewhat self-explanatory.

The `themes` directory contains all built-in themes that are currently shipped with Mumble. At this point there is only one theme available: the
`Default` theme (note that it contains the Lite and the Dark theme variant). If you want to customize any kind of icons within Mumble, this is the
Expand Down
2 changes: 0 additions & 2 deletions installer/ClientInstaller.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ public ClientInstaller(string version, string arch, Features features) {
"rnnoise.dll",
"speexdsp.dll",
"mumble.exe",
"mumble_app.dll",
};

if (features.overlay) {
Expand All @@ -109,7 +108,6 @@ public ClientInstaller(string version, string arch, Features features) {
"rnnoise.dll",
"speexdsp.dll",
"mumble.exe",
"mumble_app.dll",
};

if (features.overlay) {
Expand Down
93 changes: 61 additions & 32 deletions src/mumble/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ set(MUMBLE_ICON "${CMAKE_SOURCE_DIR}/icons/mumble.ico")
set(MUMBLE_ICNS "${CMAKE_SOURCE_DIR}/icons/mumble.icns")

configure_file("${CMAKE_CURRENT_SOURCE_DIR}/mumble.rc.in" "${MUMBLE_RC}")
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/mumble_dll.rc.in" "${MUMBLE_DLL_RC}")
configure_file("${CMAKE_CURRENT_SOURCE_DIR}/mumble.plist.in" "${MUMBLE_PLIST}")

include(qt-utils)
Expand Down Expand Up @@ -333,42 +332,31 @@ list(APPEND MUMBLE_SOURCES "${CMAKE_BINARY_DIR}/mumble_flags.qrc")

add_library(mumble_client_object_lib OBJECT ${MUMBLE_SOURCES})

if(static AND WIN32)
# On Windows, building a static client means building the main app into a DLL.
add_library(mumble SHARED "main.cpp")
target_compile_definitions(mumble_client_object_lib PUBLIC "MUMBLEAPP_DLL")

set_target_properties(mumble PROPERTIES OUTPUT_NAME "mumble_app")
if(MINGW)
# Remove "lib" prefix.
set_target_properties(mumble PROPERTIES PREFIX "")
endif()

target_sources(mumble_client_object_lib PRIVATE "${MUMBLE_DLL_RC}")

add_subdirectory("${SHARED_SOURCE_DIR}/mumble_exe" "${CMAKE_BINARY_DIR}/mumble_exe")
if(WIN32 AND NOT CMAKE_BUILD_TYPE STREQUAL "Debug")
# We don't want the console to appear in release builds.
add_executable(mumble WIN32 "main.cpp")
else()
add_executable(mumble "main.cpp")
endif()

if(WIN32)
target_sources(mumble_client_object_lib
PRIVATE
"${CMAKE_SOURCE_DIR}/auxiliary_files/mumble.appcompat.manifest"
"${MUMBLE_RC}"
)
if(WIN32)
target_sources(mumble_client_object_lib
PRIVATE
"${CMAKE_SOURCE_DIR}/auxiliary_files/mumble.appcompat.manifest"
"${MUMBLE_RC}"
)

if(elevation)
set_property(TARGET mumble APPEND_STRING PROPERTY LINK_FLAGS " /MANIFESTUAC:\"level=\'asInvoker\' uiAccess=\'true\'\"")
endif()
elseif(APPLE)
set_target_properties(mumble
PROPERTIES
OUTPUT_NAME "Mumble"
MACOSX_BUNDLE TRUE
RESOURCE ${MUMBLE_ICNS}
MACOSX_BUNDLE_INFO_PLIST ${MUMBLE_PLIST}
)
if(elevation)
set_property(TARGET mumble APPEND_STRING PROPERTY LINK_FLAGS " /MANIFESTUAC:\"level=\'asInvoker\' uiAccess=\'true\'\"")
endif()
elseif(APPLE)
set_target_properties(mumble
PROPERTIES
OUTPUT_NAME "Mumble"
MACOSX_BUNDLE TRUE
RESOURCE ${MUMBLE_ICNS}
MACOSX_BUNDLE_INFO_PLIST ${MUMBLE_PLIST}
)
endif()

add_custom_command(
Expand Down Expand Up @@ -1144,3 +1132,44 @@ endif()
target_include_directories(${TARGET_NAME} INTERFACE
$<BUILD_INTERFACE:${AUTOGEN_INCLUDE_DIR}>
)

if(packaging)
set(overlay ON)
set(plugins ON)
Comment on lines +1137 to +1138
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is appropriate here 🤔
These should be defaulted where they are declared and then not overwritten just because we're packaging. I realize that this is what we had in the old code, but I'm not sure this is something we should keep 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe they are forced because the installer code doesn't take into account whether the DLLs exist.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does for the overlay (at least there is a parameter) and maybe it would be best if it also did that for the plugins as well 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, let's do it in a separate pull request.


if(translations)
list(APPEND installer_vars "--all-languages")
endif()


list(APPEND installer_vars
"--version" ${PROJECT_VERSION}
"--arch" "${MUMBLE_TARGET_ARCH}"
)

if(overlay)
list(APPEND installer_vars
"--overlay"
)
endif()

if(g15)
list(APPEND installer_vars
"--g15"
)
endif()

file(COPY
${CMAKE_SOURCE_DIR}/installer/MumbleInstall.cs
${CMAKE_SOURCE_DIR}/installer/ClientInstaller.cs
DESTINATION
${CMAKE_BINARY_DIR}/installer/client
)

add_custom_command(TARGET mumble
POST_BUILD
COMMAND cscs.exe -cd MumbleInstall.cs
COMMAND cscs.exe ClientInstaller.cs ${installer_vars}
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/installer/client
)
endif()
4 changes: 0 additions & 4 deletions src/mumble/Overlay_win.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
#include "../../overlay/overlay_exe/overlay_exe.h"

// Used by the overlay to detect whether we injected into ourselves.
//
// A similar declaration can be found in mumble_exe's Overlay.cpp,
// for the overlay's self-detection checks to continue working in a
// mumble_app.dll world.
extern "C" __declspec(dllexport) void mumbleSelfDetection(){};

// Determine if the current Mumble client is able to host
Expand Down
32 changes: 0 additions & 32 deletions src/mumble/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,7 @@ extern int os_early_init();
extern HWND mumble_mw_hwnd;
#endif // Q_OS_WIN

#if defined(Q_OS_WIN) && !defined(MUMBLEAPP_DLL)
extern "C" __declspec(dllexport) int main(int argc, char **argv) {
#else
int main(int argc, char **argv) {
#endif
int res = 0;

#if defined(Q_OS_WIN)
Expand Down Expand Up @@ -958,31 +954,3 @@ int main(int argc, char **argv) {
}
return res;
}

#if defined(Q_OS_WIN) && defined(MUMBLEAPP_DLL)
extern "C" __declspec(dllexport) int MumbleMain(HINSTANCE instance, HINSTANCE prevInstance, LPSTR cmdArg, int cmdShow) {
Q_UNUSED(instance)
Q_UNUSED(prevInstance)
Q_UNUSED(cmdArg)
Q_UNUSED(cmdShow)

int argc;
wchar_t **argvW = CommandLineToArgvW(GetCommandLineW(), &argc);
if (!argvW) {
return -1;
}

QVector< QByteArray > argvS;
argvS.reserve(argc);

QVector< char * > argvV(argc, nullptr);
for (int i = 0; i < argc; ++i) {
argvS.append(QString::fromWCharArray(argvW[i]).toLocal8Bit());
argvV[i] = argvS.back().data();
}

LocalFree(argvW);

return main(argc, argvV.data());
}
#endif
49 changes: 0 additions & 49 deletions src/mumble/mumble_dll.rc.in
View file
Open in desktop

This file was deleted.

88 changes: 0 additions & 88 deletions src/mumble_exe/CMakeLists.txt
View file
Open in desktop

This file was deleted.

14 changes: 0 additions & 14 deletions src/mumble_exe/Overlay.cpp
View file
Open in desktop

This file was deleted.

Loading