Multiple macOS fixes and include keepassxc-cli in DMG (#2165)

* Fixes including keepassxc-cli when building KeePassXC dmg on Mac.
resolves #1697
* Fix Qt search path and Properties display on macOS
* Simplify packaging POST_BUILD fixups
* Various fixes to get cli to run on macos
* Correct cli tests on macOS
* Several macOS related GUI test fixes

CMakeLists.txt

@@ -254,11 +254,6 @@ else()
     set(PROGNAME keepassxc)
 endif()
 
-if(APPLE AND WITH_APP_BUNDLE AND "${CMAKE_INSTALL_PREFIX}" STREQUAL "/usr/local")
-    set(CMAKE_INSTALL_PREFIX "/Applications")
-    set(CMAKE_INSTALL_MANDIR "/usr/local/share/man")
-endif()
-
 if(MINGW)
     set(CLI_INSTALL_DIR ".")
     set(PROXY_INSTALL_DIR ".")
@@ -266,9 +261,10 @@ if(MINGW)
     set(PLUGIN_INSTALL_DIR ".")
     set(DATA_INSTALL_DIR "share")
 elseif(APPLE AND WITH_APP_BUNDLE)
-    set(CLI_INSTALL_DIR "/usr/local/bin")
+    set(CMAKE_INSTALL_MANDIR "${PROGNAME}.app/Contents/Resources/man")
-    set(PROXY_INSTALL_DIR "/usr/local/bin")
+    set(CLI_INSTALL_DIR "${PROGNAME}.app/Contents/MacOS")
-    set(BIN_INSTALL_DIR ".")
+    set(PROXY_INSTALL_DIR "${PROGNAME}.app/Contents/MacOS")
+    set(BIN_INSTALL_DIR "${PROGNAME}.app/Contents/MacOS")
     set(PLUGIN_INSTALL_DIR "${PROGNAME}.app/Contents/PlugIns")
     set(DATA_INSTALL_DIR "${PROGNAME}.app/Contents/Resources")
 else()
@@ -306,8 +302,8 @@ set(QT_COMPONENTS Core Network Concurrent Gui Svg Widgets Test LinguistTools)
 if(UNIX AND NOT APPLE)
     find_package(Qt5 COMPONENTS ${QT_COMPONENTS} DBus REQUIRED)
 elseif(APPLE)
-    find_package(Qt5 COMPONENTS ${QT_COMPONENTS} REQUIRED HINTS /usr/local/Cellar/qt/*/lib/cmake ENV PATH)
+    find_package(Qt5 COMPONENTS ${QT_COMPONENTS} REQUIRED HINTS /usr/local/opt/qt/lib/cmake /usr/local/Cellar/qt/*/lib/cmake ENV PATH)
-    find_package(Qt5 COMPONENTS MacExtras HINTS /usr/local/Cellar/qt/*/lib/cmake ENV PATH)
+    find_package(Qt5 COMPONENTS MacExtras HINTS /usr/local/opt/qt/lib/cmake /usr/local/Cellar/qt/*/lib/cmake ENV PATH)
 else()
     find_package(Qt5 COMPONENTS ${QT_COMPONENTS} REQUIRED)
 endif()

src/cli/CMakeLists.txt

@@ -47,6 +47,44 @@ install(TARGETS keepassxc-cli
         BUNDLE DESTINATION . COMPONENT Runtime
         RUNTIME DESTINATION ${CLI_INSTALL_DIR} COMPONENT Runtime)
 
+if(APPLE AND WITH_APP_BUNDLE)
+    add_custom_command(TARGET keepassxc-cli
+                       POST_BUILD
+                       COMMAND ${CMAKE_INSTALL_NAME_TOOL}
+                            -change /usr/local/opt/qt/lib/QtCore.framework/Versions/5/QtCore
+                                "@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore"
+                            -change /usr/local/opt/qt/lib/QtGui.framework/Versions/5/QtGui
+                                "@executable_path/../Frameworks/QtGui.framework/Versions/5/QtGui"
+                            -change /usr/local/opt/qt/lib/QtMacExtras.framework/Versions/5/QtMacExtras
+                                "@executable_path/../Frameworks/QtMacExtras.framework/Versions/5/QtMacExtras"
+                            -change /usr/local/opt/qt/lib/QtConcurrent.framework/Versions/5/QtConcurrent
+                                "@executable_path/../Frameworks/QtConcurrent.framework/Versions/5/QtConcurrent"
+                            -change /usr/local/opt/qt/lib/QtCore.framework/Versions/5/QtCore
+                                "@executable_path/../Frameworks/QtCore.framework/Versions/5/QtCore"
+                            -change /usr/local/opt/qt/lib/QtNetwork.framework/Versions/5/QtNetwork
+                                "@executable_path/../Frameworks/QtNetwork.framework/Versions/5/QtNetwork"
+                            -change /usr/local/opt/qt/lib/QtWidgets.framework/Versions/5/QtWidgets
+                                "@executable_path/../Frameworks/QtWidgets.framework/Versions/5/QtWidgets"
+                            -change /usr/local/opt/qt/lib/QtSvg.framework/Versions/5/QtSvg
+                                "@executable_path/../Frameworks/QtSvg.framework/Versions/5/QtSvg"
+                            -change /usr/local/opt/libgcrypt/lib/libgcrypt.20.dylib
+                                "@executable_path/../Frameworks/libgcrypt.20.dylib"
+                            -change /usr/local/opt/argon2/lib/libargon2.1.dylib
+                                "@executable_path/../Frameworks/libargon2.1.dylib"
+                            -change /usr/local/opt/libgpg-error/lib/libgpg-error.0.dylib
+                                "@executable_path/../Frameworks/libgpg-error.0.dylib"
+                            -change /usr/local/opt/libsodium/lib/libsodium.23.dylib
+                                "@executable_path/../Frameworks/libsodium.23.dylib"
+                            -change /usr/local/opt/qrencode/lib/libqrencode.4.dylib
+                                "@executable_path/../Frameworks/libqrencode.4.dylib"
+                            -change /usr/local/opt/libyubikey/lib/libyubikey.0.dylib
+                                "@executable_path/../Frameworks/libyubikey.0.dylib"
+                            -change /usr/local/opt/ykpers/lib/libykpers-1.1.dylib
+                                "@executable_path/../Frameworks/libykpers-1.1.dylib"
+                       keepassxc-cli
+                       COMMENT "Changing linking of keepassxc-cli")
+endif()
+
 if(APPLE OR UNIX)
     install(FILES keepassxc-cli.1 DESTINATION ${CMAKE_INSTALL_MANDIR}/man1/)
     execute_process(COMMAND mandb -q)

src/cli/keepassxc-cli.cpp

@@ -42,9 +42,7 @@ int main(int argc, char** argv)
     QCoreApplication app(argc, argv);
     QCoreApplication::setApplicationVersion(KEEPASSXC_VERSION);
 
-#ifdef QT_NO_DEBUG
+    Bootstrap::bootstrap();
-    Bootstrap::bootstrapApplication();
-#endif
 
     TextStream out(stdout);
     QStringList arguments;

src/core/Bootstrap.cpp

@@ -62,11 +62,10 @@ namespace Bootstrap
     }
 
     /**
-     * Perform early application bootstrapping such as setting up search paths,
+     * Perform early application bootstrapping that does not rely on a QApplication
-     * configuration OS security properties, and loading translators.
+     * being present.
-     * A QApplication object has to be instantiated before calling this function.
      */
-    void bootstrapApplication()
+    void bootstrap()
     {
 #ifdef QT_NO_DEBUG
         disableCoreDumps();
@@ -74,6 +73,16 @@ namespace Bootstrap
         setupSearchPaths();
         applyEarlyQNetworkAccessManagerWorkaround();
         Translator::installTranslators();
+    }
+
+    /**
+     * Perform early application bootstrapping such as setting up search paths,
+     * configuration OS security properties, and loading translators.
+     * A QApplication object has to be instantiated before calling this function.
+     */
+    void bootstrapApplication()
+    {
+        bootstrap();
         MessageBox::initializeButtonDefs();
 
 #ifdef Q_OS_MACOS

src/core/Bootstrap.h

@@ -22,6 +22,7 @@
 
 namespace Bootstrap
 {
+    void bootstrap();
     void bootstrapApplication();
     void restoreMainWindowState(MainWindow& mainWindow);
     void disableCoreDumps();

src/core/Translator.cpp

@@ -47,7 +47,8 @@ void Translator::installTranslators()
 #ifdef QT_DEBUG
         QString("%1/share/translations").arg(KEEPASSX_BINARY_DIR),
 #endif
-        filePath()->dataPath("translations")};
+        filePath()->dataPath("translations")
+    };
 
     bool translationsLoaded = false;
     for (const QString& path : paths) {

src/gui/EditWidgetProperties.ui

@@ -28,6 +28,9 @@
    </property>
    <item>
     <layout class="QFormLayout" name="formLayout_2">
+     <property name="fieldGrowthPolicy">
+      <enum>QFormLayout::ExpandingFieldsGrow</enum>
+     </property>
      <item row="1" column="0">
       <widget class="QLabel" name="labelCreated">
        <property name="text">

src/gui/dbsettings/DatabaseSettingsWidgetBrowser.ui

@@ -108,7 +108,7 @@
      </property>
      <layout class="QHBoxLayout" name="horizontalLayout_3">
       <item>
-       <widget class="QGroupBox" name="groupBox">
+       <widget class="QGroupBox" name="groupBox_3">
         <layout class="QHBoxLayout" name="horizontalLayout_4">
          <item>
           <widget class="QTableView" name="customDataTable">

src/gui/widgets/PopupHelpWidget.cpp

@@ -30,7 +30,11 @@ PopupHelpWidget::PopupHelpWidget(QWidget* parent)
 {
     Q_ASSERT(parent);
 
+#ifdef Q_OS_MACOS
+    setWindowFlags(Qt::FramelessWindowHint | Qt::Drawer);
+#else
     setWindowFlags(Qt::FramelessWindowHint | Qt::Tool);
+#endif
     hide();
 
     m_appWindow->installEventFilter(this);

src/gui/wizard/NewDatabaseWizard.cpp

@@ -34,6 +34,7 @@ NewDatabaseWizard::NewDatabaseWizard(QWidget* parent)
 {
     setWizardStyle(QWizard::MacStyle);
     setOption(QWizard::WizardOption::HaveHelpButton, false);
+    setOption(QWizard::WizardOption::NoDefaultButton, false);  // Needed for macOS
 
     // clang-format off
     m_pages << new NewDatabaseWizardPageMetaData()

src/proxy/CMakeLists.txt

@@ -32,14 +32,6 @@ if(WITH_XC_BROWSER)
             RUNTIME DESTINATION ${PROXY_INSTALL_DIR} COMPONENT Runtime)
 
     if(APPLE AND WITH_APP_BUNDLE)
-        set(PROXY_BINARY_DIR "${CMAKE_BINARY_DIR}/src/proxy/keepassxc-proxy")
-        set(PROXY_APP_DIR "KeePassXC.app/Contents/MacOS/keepassxc-proxy")
-        add_custom_command(TARGET keepassxc-proxy
-                           POST_BUILD
-                           COMMAND ${CMAKE_COMMAND} -E copy ${PROXY_BINARY_DIR} ${PROXY_APP_DIR}
-                           WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src
-                           COMMENT "Copying keepassxc-proxy inside the application")
-
         add_custom_command(TARGET keepassxc-proxy
                            POST_BUILD
                            COMMAND ${CMAKE_INSTALL_NAME_TOOL}
@@ -51,8 +43,7 @@ if(WITH_XC_BROWSER)
                                     "@executable_path/../Frameworks/QtNetwork.framework/Versions/5/QtNetwork"
                                 -change /usr/local/opt/qt/lib/QtNetwork.framework/Versions/5/QtNetwork
                                     "@executable_path/../Frameworks/QtNetwork.framework/Versions/5/QtNetwork"
-                           ${PROXY_APP_DIR}
+                           keepassxc-proxy
-                           WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/src
                            COMMENT "Changing linking of keepassxc-proxy")
     endif()
     if(MINGW)

tests/TestCli.cpp

@@ -560,26 +560,11 @@ void TestCli::testEstimate()
     QTextStream out(m_stdoutFile.data());
 
     in << input << endl;
-    auto inEnd = in.pos();
     in.seek(0);
-    estimateCmd.execute({"estimate"});
+    estimateCmd.execute({"estimate", "-a"});
-    auto outEnd = out.pos();
     out.seek(0);
     auto result = out.readAll();
-    QVERIFY(result.startsWith("Length " + length));
+    QVERIFY(result.contains("Length " + length));
-    QVERIFY(result.contains("Entropy " + entropy));
-    QVERIFY(result.contains("Log10 " + log10));
-
-    // seek to end of stream
-    in.seek(inEnd);
-    out.seek(outEnd);
-
-    in << input << endl;
-    in.seek(inEnd);
-    estimateCmd.execute({"estimate", "-a"});
-    out.seek(outEnd);
-    result = out.readAll();
-    QVERIFY(result.startsWith("Length " + length));
     QVERIFY(result.contains("Entropy " + entropy));
     QVERIFY(result.contains("Log10 " + log10));
     for (const auto& string : asConst(searchStrings)) {

tests/gui/TestGui.cpp

@@ -344,7 +344,7 @@ void TestGui::testAutoreloadDatabase()
     QVERIFY(m_dbFile->write(unmodifiedMergeDatabase, static_cast<qint64>(unmodifiedMergeDatabase.size())));
     m_dbFile->close();
 
-    Tools::wait(800);
+    QTRY_VERIFY(m_db != m_dbWidget->database());
     m_db = m_dbWidget->database();
 
     // the General group contains one entry from the new db data
@@ -358,16 +358,13 @@ void TestGui::testAutoreloadDatabase()
     // Test rejecting new file in autoreload
     MessageBox::setNextAnswer(MessageBox::No);
     // Overwrite the current temp database with a new file
-    m_dbFile->open();
+    QVERIFY(m_dbFile->open());
     QVERIFY(m_dbFile->write(unmodifiedMergeDatabase, static_cast<qint64>(unmodifiedMergeDatabase.size())));
     m_dbFile->close();
-    Tools::wait(800);
-
-    m_db = m_dbWidget->database();
 
     // Ensure the merge did not take place
     QCOMPARE(m_db->rootGroup()->findChildByName("General")->entries().size(), 0);
-    QVERIFY(m_tabWidget->tabName(m_tabWidget->currentIndex()).endsWith("*"));
+    QTRY_VERIFY(m_tabWidget->tabName(m_tabWidget->currentIndex()).endsWith("*"));
 
     // Reset the state
     cleanup();
@@ -380,13 +377,13 @@ void TestGui::testAutoreloadDatabase()
     testEditEntry();
 
     // This is saying yes to merging the entries
-    MessageBox::setNextAnswer(MessageBox::Yes);
+    MessageBox::setNextAnswer(MessageBox::Merge);
     // Overwrite the current database with the temp data
     QVERIFY(m_dbFile->open());
     QVERIFY(m_dbFile->write(unmodifiedMergeDatabase, static_cast<qint64>(unmodifiedMergeDatabase.size())));
     m_dbFile->close();
-    Tools::wait(800);
 
+    QTRY_VERIFY(m_db != m_dbWidget->database());
     m_db = m_dbWidget->database();
 
     QCOMPARE(m_db->rootGroup()->findChildByName("General")->entries().size(), 1);
@@ -404,6 +401,9 @@ void TestGui::testEditEntry()
     auto* toolBar = m_mainWindow->findChild<QToolBar*>("toolBar");
     auto* entryView = m_dbWidget->findChild<EntryView*>("entryView");
 
+    entryView->setFocus();
+    QVERIFY(entryView->hasFocus());
+
     // Select the first entry in the database
     QModelIndex entryItem = entryView->model()->index(0, 1);
     Entry* entry = entryView->entryFromIndex(entryItem);
@@ -937,6 +937,7 @@ void TestGui::testDeleteEntry()
     auto* toolBar = m_mainWindow->findChild<QToolBar*>("toolBar");
     auto* entryDeleteAction = m_mainWindow->findChild<QAction*>("actionEntryDelete");
     QWidget* entryDeleteWidget = toolBar->widgetForAction(entryDeleteAction);
+    entryView->setFocus();
 
     QCOMPARE(m_dbWidget->currentMode(), DatabaseWidget::Mode::ViewMode);
     clickIndex(entryView->model()->index(1, 1), entryView, Qt::LeftButton);
@@ -996,6 +997,7 @@ void TestGui::testDeleteEntry()
 void TestGui::testCloneEntry()
 {
     auto* entryView = m_dbWidget->findChild<EntryView*>("entryView");
+    entryView->setFocus();
 
     QCOMPARE(entryView->model()->rowCount(), 1);