From ab153a24ec2d5963192db63076bc0be1afa3ad95 Mon Sep 17 00:00:00 2001 From: alcroito Date: Tue, 8 Mar 2022 12:30:17 +0100 Subject: [PATCH] Run macdeployqt only once at install time Instead of running macdeployqt once for each extra helper binary and plugin (plus the main run for the application itself), collect all the binaries that should be processed and run macdeloyqt only once after all the binaries have been installed. This also moves the main app macdeployqt call from a POST_BUILD step to an install(CODE) step, making increment rebuilds of the app faster. To ensure that macdeployqt is called after all the binaries are installed, a new post_install subdirectory is needed to circumvent CMake's limitation regarding the default order of installation. CMake first runs the current directory install() calls and then it's child subdirectory ones. Because we want macdeployqt to be the last install() call, it needs to be done inside a subdirectory that is added last. Note due to a bug in macdeployqt, the deployed app inside the .dmg file will fail to run on arm macs, due to broken code signature. See https://bugreports.qt.io/browse/QTBUG-101696 for details. For the final release, the release-tool should take care of proper resigning. --- CMakeLists.txt | 3 +- cmake/KPXCHelpers.cmake | 33 ----------------- cmake/KPXCMacDeployHelpers.cmake | 61 ++++++++++++++++++++++++++++++++ src/CMakeLists.txt | 10 +++--- src/autotype/mac/CMakeLists.txt | 4 +-- src/cli/CMakeLists.txt | 5 ++- src/post_install/CMakeLists.txt | 13 +++++++ src/proxy/CMakeLists.txt | 4 +-- 8 files changed, 86 insertions(+), 47 deletions(-) delete mode 100644 cmake/KPXCHelpers.cmake create mode 100644 cmake/KPXCMacDeployHelpers.cmake create mode 100644 src/post_install/CMakeLists.txt diff --git a/CMakeLists.txt b/CMakeLists.txt index 7835b8a08..baa399de7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -33,7 +33,7 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${CMAKE_CURRENT_SOURCE_DIR}/cmake) # Support Visual Studio Code include(CMakeToolsHelpers OPTIONAL) include(FeatureSummary) -include(KPXCHelpers) +include(KPXCMacDeployHelpers) include(CheckCCompilerFlag) include(CheckCXXCompilerFlag) @@ -476,6 +476,7 @@ if(APPLE) message(FATAL_ERROR "macdeployqt is required to build on macOS") endif() message(STATUS "Using macdeployqt: ${MACDEPLOYQT_EXE}") + set(MACDEPLOYQT_EXTRA_BINARIES "") elseif(WIN32) find_program(WINDEPLOYQT_EXE windeployqt HINTS ${Qt5_PREFIX}/bin ${Qt5_PREFIX}/tools/qt5/bin ENV PATH) if(NOT WINDEPLOYQT_EXE) diff --git a/cmake/KPXCHelpers.cmake b/cmake/KPXCHelpers.cmake deleted file mode 100644 index b7896401a..000000000 --- a/cmake/KPXCHelpers.cmake +++ /dev/null @@ -1,33 +0,0 @@ -function(kpxc_run_macdeployqt_on_installed_helper_binary app_name installed_binary_relative_path) - # Running macdeployqt on a POST_BUILD copied binary is not useful for CPack, because - # the copied binary will be overriden by the corresponding install(TARGETS) command of the same - # binary. - # Thus, we run macdeployqt using install(CODE) on the already installed binary. - # The precondition is that install(TARGETS) has to be called before this function is called. - set(escaped_prefix "\${CMAKE_INSTALL_PREFIX}") - set(app_bundle_name "${app_name}.app") - set(app_bundle_path "${escaped_prefix}/${app_bundle_name}") - set(installed_binary_path "${escaped_prefix}/${installed_binary_relative_path}") - - if(CMAKE_VERSION VERSION_GREATER "3.14") - set(command_echo "COMMAND_ECHO STDOUT") - else() - set(command_echo "") - endif() - - install(CODE - " -execute_process( - COMMAND - ${MACDEPLOYQT_EXE} - ${app_bundle_path} - -executable=${installed_binary_path} -no-plugins 2> /dev/null - ${command_echo} - RESULT_VARIABLE exit_code -) -if(NOT exit_code EQUAL 0) - message(FATAL_ERROR - \"Running macdeployqt on ${installed_binary_path} failed with exit code \${exit_code}.\") -endif() -") -endfunction() diff --git a/cmake/KPXCMacDeployHelpers.cmake b/cmake/KPXCMacDeployHelpers.cmake new file mode 100644 index 000000000..d22051d32 --- /dev/null +++ b/cmake/KPXCMacDeployHelpers.cmake @@ -0,0 +1,61 @@ +# Running macdeployqt on a POST_BUILD copied binaries is pointless when using CPack because +# the copied binaries will be overriden by the corresponding install(TARGETS) commands. +# That's why we run macdeployqt using install(CODE) on the already installed binaries. +# The precondition is that all install(TARGETS) calls have to be called before this function is +# called. +# macdeloyqt is called only once, but it is given all executables that should be processed. +function(kpxc_run_macdeployqt_at_install_time) + set(NO_VALUE_OPTIONS) + set(SINGLE_VALUE_OPTIONS + APP_NAME + ) + set(MULTI_VALUE_OPTIONS + EXTRA_BINARIES + ) + cmake_parse_arguments(PARSE_ARGV 0 ARG + "${NO_VALUE_OPTIONS}" "${SINGLE_VALUE_OPTIONS}" "${MULTI_VALUE_OPTIONS}" + ) + + set(ESCAPED_PREFIX "\${CMAKE_INSTALL_PREFIX}") + set(APP_BUNDLE_NAME "${ARG_APP_NAME}.app") + set(APP_BUNDLE_PATH "${ESCAPED_PREFIX}/${APP_BUNDLE_NAME}") + + # Collect extra binaries and plugins that should be handled by macdpeloyqt. + set(EXTRA_BINARIES "") + foreach(EXTRA_BINARY ${ARG_EXTRA_BINARIES}) + set(INSTALLED_BINARY_PATH "${ESCAPED_PREFIX}/${EXTRA_BINARY}") + list(APPEND EXTRA_BINARIES "-executable=${INSTALLED_BINARY_PATH}") + endforeach() + + list(JOIN EXTRA_BINARIES " " EXTRA_BINARIES_STR) + + if(CMAKE_VERSION VERSION_GREATER "3.14") + set(COMMAND_ECHO "COMMAND_ECHO STDOUT") + else() + set(COMMAND_ECHO "") + endif() + + set(COMMAND_ARGS + ${MACDEPLOYQT_EXE} + ${APP_BUNDLE_PATH} + + # Adjusts dependency rpaths of extra binaries + ${EXTRA_BINARIES_STR} + + # Silences warnings on subsequent re-installations + -always-overwrite + ) + + install(CODE + " +execute_process( + COMMAND ${COMMAND_ARGS} + ${COMMAND_ECHO} + RESULT_VARIABLE EXIT_CODE +) +if(NOT EXIT_CODE EQUAL 0) + message(FATAL_ERROR + \"Running ${COMMAND_ARGS} failed with exit code \${EXIT_CODE}.\") +endif() +") +endfunction() diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f1c5d5d81..38d162ec8 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -409,12 +409,6 @@ if(APPLE AND WITH_APP_BUNDLE) set(CPACK_STRIP_FILES ON) set(CPACK_PACKAGE_FILE_NAME "${PROGNAME}-${KEEPASSXC_VERSION}") include(CPack) - - add_custom_command(TARGET ${PROGNAME} - POST_BUILD - COMMAND ${MACDEPLOYQT_EXE} ${PROGNAME}.app 2> /dev/null - WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src - COMMENT "Deploying app bundle") endif() install(TARGETS ${PROGNAME} @@ -530,3 +524,7 @@ if(WIN32) endif() endif() + +# The install commands in this subdirectory will be executed after all the install commands in the +# current scope are ran. It is required for correct functtioning of macdeployqt. +add_subdirectory(post_install) diff --git a/src/autotype/mac/CMakeLists.txt b/src/autotype/mac/CMakeLists.txt index 1f77ac184..e0df901fd 100644 --- a/src/autotype/mac/CMakeLists.txt +++ b/src/autotype/mac/CMakeLists.txt @@ -14,6 +14,6 @@ if(WITH_APP_BUNDLE) WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src COMMENT "Copying autotype plugin into app bundle") - kpxc_run_macdeployqt_on_installed_helper_binary( - "${PROGNAME}" "${PLUGIN_INSTALL_DIR}/libkeepassxc-autotype-cocoa.so") + set_property(GLOBAL APPEND PROPERTY + _MACDEPLOYQT_EXTRA_BINARIES "${PLUGIN_INSTALL_DIR}/libkeepassxc-autotype-cocoa.so") endif() diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt index b25c6b06d..984427084 100644 --- a/src/cli/CMakeLists.txt +++ b/src/cli/CMakeLists.txt @@ -77,7 +77,6 @@ if(APPLE AND WITH_APP_BUNDLE) COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_BINARY_DIR}/keepassxc-cli ${CLI_APP_DIR}/keepassxc-cli WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src COMMENT "Copying keepassxc-cli into app bundle") - - kpxc_run_macdeployqt_on_installed_helper_binary( - "${PROGNAME}" "${CLI_INSTALL_DIR}/keepassxc-cli") + set_property(GLOBAL APPEND PROPERTY + _MACDEPLOYQT_EXTRA_BINARIES "${CLI_INSTALL_DIR}/keepassxc-cli") endif() diff --git a/src/post_install/CMakeLists.txt b/src/post_install/CMakeLists.txt new file mode 100644 index 000000000..359c891f3 --- /dev/null +++ b/src/post_install/CMakeLists.txt @@ -0,0 +1,13 @@ +# The install commands in this subdirectory will be executed after all the install commands in the +# current scope are ran. It is required for correct functtioning of macdeployqt. + +if(APPLE AND WITH_APP_BUNDLE) + # Run macdeloyqt on the main app and any extra binaries and plugins as specified by the + # _MACDEPLOYQT_EXTRA_BINARIES global property. + # All install(TARGETS) calls should have already been called. + get_property(MACDEPLOYQT_EXTRA_BINARIES GLOBAL PROPERTY _MACDEPLOYQT_EXTRA_BINARIES) + kpxc_run_macdeployqt_at_install_time( + APP_NAME "${PROGNAME}" + EXTRA_BINARIES ${MACDEPLOYQT_EXTRA_BINARIES} + ) +endif() diff --git a/src/proxy/CMakeLists.txt b/src/proxy/CMakeLists.txt index 066d65b81..15137f5b5 100755 --- a/src/proxy/CMakeLists.txt +++ b/src/proxy/CMakeLists.txt @@ -37,8 +37,8 @@ if(WITH_XC_BROWSER) WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src COMMENT "Copying keepassxc-proxy into app bundle") - kpxc_run_macdeployqt_on_installed_helper_binary( - "${PROGNAME}" "${PROXY_INSTALL_DIR}/keepassxc-proxy") + set_property(GLOBAL APPEND PROPERTY + _MACDEPLOYQT_EXTRA_BINARIES "${PROXY_INSTALL_DIR}/keepassxc-proxy") endif() if(WIN32)