From 772afb9d22cb2659d5e55f5845b96fd21b44a412 Mon Sep 17 00:00:00 2001 From: Matt Young Date: Wed, 9 Nov 2022 15:39:23 +0100 Subject: [PATCH 1/3] Improved logging of start-up problems. First steps towards fixing Mac builds --- CMakeLists.txt | 102 ++++++++++++++++++++++------- src/Application.cpp | 11 +++- src/PersistentSettings.cpp | 28 ++++++-- src/database/ObjectStore.cpp | 80 +++++++++++----------- src/model/NamedParameterBundle.cpp | 2 +- translations/bt_ca.ts | 2 +- translations/bt_cs.ts | 2 +- translations/bt_de.ts | 2 +- translations/bt_el.ts | 2 +- translations/bt_en.ts | 2 +- translations/bt_es.ts | 2 +- translations/bt_et.ts | 2 +- translations/bt_eu.ts | 2 +- translations/bt_fr.ts | 2 +- translations/bt_gl.ts | 2 +- translations/bt_hu.ts | 2 +- translations/bt_it.ts | 2 +- translations/bt_lv.ts | 2 +- translations/bt_nb.ts | 2 +- translations/bt_nl.ts | 2 +- translations/bt_pl.ts | 2 +- translations/bt_pt.ts | 2 +- translations/bt_ru.ts | 2 +- translations/bt_sr.ts | 2 +- translations/bt_sv.ts | 2 +- translations/bt_tr.ts | 2 +- translations/bt_zh.ts | 2 +- 27 files changed, 175 insertions(+), 92 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 803cc2a9a..9d34348d4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -467,8 +467,11 @@ if(Qt5_POSITION_INDEPENDENT_CODE) set(CMAKE_POSITION_INDEPENDENT_CODE ON) endif() -# There's apparently a whole bunch of extra work we need to do to use Qt on Windows +# There's apparently a whole bunch of extra work we need to do to use Qt on Windows and Mac if(WIN32) + #==================================================================================================================== + #================================================= Windows Qt Stuff ================================================= + #==================================================================================================================== # .:TBD:. Not sure whether/why we need these additional Qt components on Windows find_package(Qt5MultimediaWidgets REQUIRED) find_package(Qt5OpenGL REQUIRED) @@ -532,6 +535,8 @@ if(WIN32) # find_program(WINDEPLOYQT_EXECUTABLE windeployqt HINTS "${QT5_BIN_DIR}") if(EXISTS ${WINDEPLOYQT_EXECUTABLE}) + # Per https://cmake.org/cmake/help/latest/command/add_executable.html, "IMPORTED executables are useful for + # convenient reference from commands like add_custom_command()". add_executable(Qt5::windeployqt IMPORTED) set_target_properties(Qt5::windeployqt PROPERTIES IMPORTED_LOCATION ${WINDEPLOYQT_EXECUTABLE}) endif() @@ -910,6 +915,10 @@ set_source_files_properties(${QM_FILES} MACOSX_PACKAGE_LOCATION "Resources/translations_qm") if(APPLE) + # The MACOSX_BUNDLE parameter here sets the MACOSX_BUNDLE property on the created target. This means the executable + # is built as an Application Bundle, which makes it a GUI executable that can be launched from the Finder. + # + # TBD: When we move to Qt6, look at qt_add_executable add_executable(${fileName_executable} MACOSX_BUNDLE ${repoDir}/src/main.cpp @@ -937,8 +946,16 @@ if(WIN32 AND MINGW) ############################################################################ # MinGW-specific flags. - # '-Wl,-subsystem,windows' supresses the output command window. - # '-Wl,-s' strips the executable of symbols. + # -Wl,-subsystem,windows - suppresses the output command window. + # -Wl,-enable-stdcall-fixup - If the link finds a symbol that it cannot resolve, it will attempt to do “fuzzy + # linking” by looking for another defined symbol that differs only in the format of + # the symbol name (cdecl vs stdcall) and will resolve that symbol by linking to the + # match (and also print a warning). + # -Wl,-enable-auto-import - Do sophisticated linking of _symbol to __imp__symbol for DATA imports from DLLs, and + # create the necessary thunking symbols when building the import libraries with those + # DATA exports. + # -Wl,-enable-runtime-pseudo-reloc - Fixes some of the problems that can occur with -enable-auto-import + # -mthreads - specifies that MinGW-specific thread support is to be used set_target_properties( ${fileName_executable} PROPERTIES @@ -1199,7 +1216,7 @@ if(WIN32) # # Now, because we're also not assuming that the person running the code will have MSYS2/MinGW installed, we need to # include the DLLs that ship with them and get pulled in by GCC. I am hoping to find a less manual way of doing this - # but have not yet paydirt. + # but have not yet hit paydirt. # set(msys2Dlls libbrotli @@ -1315,6 +1332,30 @@ if(CMAKE_INSTALL_SYSTEM_RUNTIME_LIBS) COMPONENT System) endif() +#======================================================================================================================= +#=============================================== Post-compilation message ============================================== +#======================================================================================================================= +# +# After running `make`, you usually want to run `make install`. This is an easy step to miss for someone compiling from +# source for the first time, so it's common practice to give a reminder about it. This trick of making a special cmake +# target comes from https://stackoverflow.com/questions/25240105/how-to-print-messages-after-make-done-with-cmake. It +# does mean the message also gets printed during `sudo make install` which is, at best, unnecessary. But I think, on +# balance, it's better than not having a message. +# +# Putting the message in a different color is usually helpful because it makes it stand out from all the other CMake +# output. Hard-coding the color here is a small risk in that we don't know how well it will show up on whatever color +# scheme the terminal is using. OTOH, AIUI the color of CMake's own messages is hard-coded, so, if we use one of the +# same colors, we're not making things any worse. +# (BTW, I cannot find reference to cmake_echo_color in the CMake documentation, but it seems to work!) +# +add_custom_target( + FinalMessage ALL + ${CMAKE_COMMAND} -E cmake_echo_color --bold --green "*** Finished compiling ${capitalisedProjectName}. Please run sudo make install to install. ***" + COMMENT "Final Message" +) +add_dependencies(FinalMessage ${fileName_executable}) + + #======================================================================================================================= #====================================================== Packaging ====================================================== #======================================================================================================================= @@ -1504,31 +1545,48 @@ elseif(WIN32) # ) elseif(APPLE) #================================================== Mac Packaging =================================================== - # It seems like there are two CPack generators that could potentially be used for Mac: + # Broadly speaking, what we want for Mac is a a DMG disk image which the end user can mount and from s/he can copy + # the application bundle to her/his Applications folder. (On MacOS, a bundle a directory structure that appears as a + # single entity when viewed in the Finder.) + # + # It seems like there are two CPack generators that could potentially be used for this: # - Bundle (https://cmake.org/cmake/help/latest/cpack_gen/bundle.html) which I think is used in conjunction with # the DragNDrop generator # - DragNDrop (https://cmake.org/cmake/help/latest/cpack_gen/dmg.html) which generates a DMG disk image from which # the end user copies the application to her/his Applications folder - # - IFW (https://cmake.org/cmake/help/latest/cpack_gen/ifw.html) which generates a Qt installer - # Historically it looks like DMG images were what we released. # - # See some info at https://doc.qt.io/qt-6/macos-deployment.html for "official" Qt way to do things, but this is using - # qmake. This page https://gitlab.kitware.com/cmake/community/-/wikis/doc/cpack/PackageGenerators is more helpful. - set(CPACK_GENERATOR "Bundle") - - # Bundle name (displayed in the finder underneath the bundle icon). - set(CPACK_BUNDLE_NAME "${CMAKE_PROJECT_NAME}_${PROJECT_VERSION}") - # Bundle icon (displayed in the /Applications folder, on the dock, etc). - set(CPACK_BUNDLE_ICON "${filesToInstall_macIcons}") - # XML format Information Property List file containing application metadata - # Note that CFBundleVersion in this file should be the same as PROJECT_VERSION in this one - set(CPACK_BUNDLE_PLIST "${filesToInstall_macPropertyList}") - # Path to a startup script. This is a path to an executable or script that will be run whenever an end-user double-clicks the generated bundle in the OSX Finder. Optional. - #set(CPACK_BUNDLE_STARTUP_COMMAND "${fileName_executable}") + # According to + # https://stackoverflow.com/questions/46011440/cmake-cpack-preferred-package-generators-for-different-platforms, it's + # best to "Avoid the Bundle generator, it is older and more limited in what it supports, [compared with] the DMG + # [aka DragNDrop] generator." + # + # There is also the "official" Qt way to do things for Mac at https://doc.qt.io/qt-6/macos-deployment.html. This + # does not use CPack but does offer options for doing things more directly in CMake. (NB: As of January 2021, Qt is + # migrating from qmake to cmake -- see https://www.qt.io/blog/qt-and-cmake-the-past-the-present-and-the-future.) We + # have to be a bit careful about mixing and matching approaches as we don't want to end up eg with a bundle inside a + # bundle. + # + # Another potential option would be the Qt Installation Framework (IFW) -- see + # https://doc.qt.io/qtinstallerframework/ -- for which there is a CPack generator + # (https://cmake.org/cmake/help/latest/cpack_gen/ifw.html). This creates a cross-platform GUI installer using Qt. + # However, there are downsides to having a custom tool for install and upgrade, and, additionally, there's not a huge + # amount of info about how to get the CPack IFW generator working on Mac. + # + # Since we're using CPack for Windows and Linux, it feels like it makes sense to use it for Mac too, we we go with + # the DragNDrop generator. Creation of the bundle is handled prior in CMake, prior to invocation of CPack, by the + # MACOSX_BUNDLE target property etc (per https://cmake.org/cmake/help/latest/prop_tgt/MACOSX_BUNDLE_INFO_PLIST.html). + # + # NOTE If you are on Linux and you want to check the contents of a Mac disk image file called, say, foo.dmg, you can + # extract it into regular files from the command line with: + # 7z x foo.dmg + # + set(CPACK_GENERATOR "DragNDrop") + + # The volume name of the generated disk image. Defaults to CPACK_PACKAGE_FILE_NAME, which we set above to be + # something very specific for the benefit of Linux packages. We choose a simpler/nicer name for the Mac disk image. + set(CPACK_DMG_VOLUME_NAME "${CMAKE_PROJECT_NAME} ${PROJECT_VERSION}") message(STATUS "Building Mac package") - #set(CPACK_GENERATOR "DragNDrop") - #set(CPACK_DMG_FORMAT "UDBZ") endif() # The documentation implies that all the CPACK_* variables need to be set before we include the CPack module, so that's diff --git a/src/Application.cpp b/src/Application.cpp index 76ab26127..b4a8fbfdb 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -114,15 +114,20 @@ namespace { * \brief Ensure our directories exist. */ bool ensureDirectoriesExist() { + // // A missing resource directory is a serious issue, without it we're missing the default DB, sound files & // translations. We could attempt to create it, like the other config/data directories, but an empty resource - // dir is just as bad as a missing one. So, instead, we'll display a little more dire warning, and not try to - // create it. + // dir is just as bad as a missing one. So, instead, we'll display a little more dire warning. + // + // .:TBD:. Maybe we should terminate the app here as it's likely that there's some problem with the install and + // users are going to hit other problems. + // QDir resourceDir = Application::getResourceDir(); bool resourceDirSuccess = resourceDir.exists(); if (!resourceDirSuccess) { QString errMsg{ - QObject::tr("Resource directory \"%1\" is missing. Some features will be unavailable.").arg(resourceDir.path()) + QObject::tr("Resource directory \"%1\" is missing. The software might not operate correctly without this " + "directory and its contents.").arg(resourceDir.path()) }; qCritical() << Q_FUNC_INFO << errMsg; diff --git a/src/PersistentSettings.cpp b/src/PersistentSettings.cpp index 6979e338f..741ce103b 100644 --- a/src/PersistentSettings.cpp +++ b/src/PersistentSettings.cpp @@ -1,6 +1,6 @@ /** * PersistentSettings.cpp is part of Brewtarget, and is copyright the following - * authors 2009-2021: + * authors 2009-2022: * • A.J. Drobnich * • Brian Rower * • Chris Pavetto @@ -36,6 +36,9 @@ */ #include "PersistentSettings.h" +#include + +#include #include #include @@ -89,11 +92,11 @@ namespace { // // This is set by PersistentSettings::initialise() // - QSettings * qSettings = nullptr; + std::unique_ptr qSettings{nullptr}; // These also get set by PersistentSettings::initialise() - QDir configDir; - QDir userDataDir; + QDir configDir{""}; + QDir userDataDir{""}; } @@ -112,11 +115,24 @@ void PersistentSettings::initialise(QString customUserDataDir) { // Pretty sure this needs to be done after calls to QApplication::setOrganizationName() etc in main(), which is why // we don't just use static initialisation. // + // Yes, we are knowingly logging before Logging::initializeLogging() has been called (as the latter needs + // PersistentSettings::initialise() to be called first. This is OK as it just means the log message will go to the + // default Qt-0determined location (eg stderr on Linux). + // + // The value in this logging is that we have seen instances where the software is unable to determine a value for + // configDir, and we would like to diagnose why (eg whether it is some permissions problem). + // + qInfo() << + Q_FUNC_INFO << "Qt-proposed directories for user-specific configuration files are:" << + QStandardPaths::standardLocations(QStandardPaths::AppConfigLocation); configDir.setPath(QStandardPaths::writableLocation(QStandardPaths::AppConfigLocation)); + qInfo() << + Q_FUNC_INFO << "Preferred writeable directory for user-specific configuration files is:" << + configDir.absolutePath(); // Now we can set the full path of the persistent settings file - qSettings = new QSettings{configDir.absoluteFilePath("brewtarget.conf"), - QSettings::IniFormat}; + qSettings = std::make_unique(configDir.absoluteFilePath("brewkenPersistentSettings.conf"), + QSettings::IniFormat); // We've done enough now for calls to contains()/insert()/value() etc to work. Mark that we're initialised so we // can (potentially) use one of those calls to initialise the user data directory. diff --git a/src/database/ObjectStore.cpp b/src/database/ObjectStore.cpp index 0040b1849..5e878229d 100644 --- a/src/database/ObjectStore.cpp +++ b/src/database/ObjectStore.cpp @@ -30,13 +30,14 @@ #include "database/BtSqlQuery.h" #include "database/Database.h" #include "database/DbTransaction.h" +#include "Logging.h" #include "model/NamedParameterBundle.h" // Private implementation details that don't need access to class member variables namespace { /** - * For a given field type, get the native database typename + * \brief For a given field type, get the native database typename */ char const * getDatabaseNativeTypeName(Database const & database, ObjectStore::FieldType const fieldType) { switch (fieldType) { @@ -55,7 +56,6 @@ namespace { return nullptr; // Should never get here } - /** * \brief Create a database table without foreign key constraints (allowing tables to be created in any order) * @@ -198,11 +198,11 @@ namespace { } /** - * Return a string containing all the bound values on a query. This is quite a useful thing to have logged when - * you get an error! + * \brief Return a string containing all the bound values on a query. This is quite a useful thing to have logged + * when you get an error! * - * NB: This can be a long string. It includes newlines, and is intended to be logged with qDebug().noquote() or - * similar. + * NOTE: This can be a long string. It includes newlines, and is intended to be logged with + * qDebug().noquote() or similar. */ QString BoundValuesToString(BtSqlQuery const & sqlQuery) { QString result; @@ -217,12 +217,12 @@ namespace { } /** - * Given a (QVariant-wrapped) string value pulled out of the DB for an enum, look up and return its internal - * numerical enum equivalent + * \brief Given a (QVariant-wrapped) string value pulled out of the DB for an enum, look up and return its internal + * numerical enum equivalent */ int stringToEnum(ObjectStore::TableField const & fieldDefn, QVariant const & valueFromDb) { // It's a coding error if we called this function for a non-enum field - Q_ASSERT(fieldDefn.fieldType == ObjectStore::Enum); + Q_ASSERT(fieldDefn.fieldType == ObjectStore::FieldType::Enum); Q_ASSERT(fieldDefn.enumMapping != nullptr); if (valueFromDb.isNull()) { @@ -245,12 +245,12 @@ namespace { } /** - * Given a (QVariant-wrapped) int value of a native enum, look up and return the corresponding string we use to - * store it in the DB + * \brief Given a (QVariant-wrapped) int value of a native enum, look up and return the corresponding string we use + * to store it in the DB */ QString enumToString(ObjectStore::TableField const & fieldDefn, QVariant const & propertyValue) { // It's a coding error if we called this function for a non-enum field - Q_ASSERT(fieldDefn.fieldType == ObjectStore::Enum); + Q_ASSERT(fieldDefn.fieldType == ObjectStore::FieldType::Enum); Q_ASSERT(fieldDefn.enumMapping != nullptr); auto match = fieldDefn.enumMapping->enumToString(propertyValue.toInt()); @@ -348,8 +348,9 @@ namespace { // // Note that, when we are using bind values, we do NOT want to call the - // BtSqlQuery::BtSqlQuery(const QString &, QSqlDatabase db) version of the BtSqlQuery constructor because that would - // result in the supplied query being executed immediately (ie before we've had a chance to bind parameters). + // BtSqlQuery::BtSqlQuery(const QString &, QSqlDatabase db) version of the BtSqlQuery constructor because that + // would result in the supplied query being executed immediately (ie before we've had a chance to bind + // parameters). // BtSqlQuery sqlQuery{connection}; sqlQuery.prepare(queryString); @@ -391,8 +392,8 @@ namespace { propertyValues.append(theValue); } else { // - // The propertyValuesWrapper QVariant should hold QVector. If it doesn't it's a coding error (because we have a - // property getter that's returning something else). + // The propertyValuesWrapper QVariant should hold QVector. If it doesn't it's a coding error (because we + // have a property getter that's returning something else). // // Note that QVariant::toList() is NOT going to be useful to us here because that ONLY works if the contained // type is QList (aka QVariantList) or QStringList. If your QVariant contains some other list-like @@ -410,9 +411,9 @@ namespace { // Now loop through and bind/run the insert query once for each item in the list int itemNumber = 1; qDebug() << - Q_FUNC_INFO << propertyValues.size() << "value(s) (in" << propertyValuesWrapper.typeName() << ") for property" << - GetJunctionTableDefinitionPropertyName(junctionTable) << "of" << object.metaObject()->className() << - "#" << primaryKey.toInt(); + Q_FUNC_INFO << propertyValues.size() << "value(s) (in" << propertyValuesWrapper.typeName() << + ") for property" << GetJunctionTableDefinitionPropertyName(junctionTable) << "of" << + object.metaObject()->className() << "#" << primaryKey.toInt(); for (int curValue : propertyValues) { sqlQuery.bindValue(thisPrimaryKeyBindName, primaryKey); sqlQuery.bindValue(otherPrimaryKeyBindName, curValue); @@ -452,7 +453,8 @@ namespace { Q_FUNC_INFO << "Deleting property " << GetJunctionTableDefinitionPropertyName(junctionTable) << " in junction table " << junctionTable.tableName; - QString const thisPrimaryKeyBindName = QString{":"} + *GetJunctionTableDefinitionThisPrimaryKeyColumn(junctionTable); + QString const thisPrimaryKeyBindName = + QString{":"} + *GetJunctionTableDefinitionThisPrimaryKeyColumn(junctionTable); // Construct the DELETE query QString queryString{"DELETE FROM "}; @@ -495,7 +497,6 @@ class ObjectStore::impl { return; } - /** * Destructor */ @@ -567,7 +568,8 @@ class ObjectStore::impl { QVariant const primaryKey {this->getPrimaryKey(object)}; // - // First check whether this is a simple property. (If not we look for it in the ones we store in junction tables.) + // First check whether this is a simple property. (If not we look for it in the ones we store in junction + // tables.) // auto matchingFieldDefn = std::find_if( this->primaryTable.tableFields.begin(), @@ -611,7 +613,7 @@ class ObjectStore::impl { ); // It's a coding error if we're trying to update a property that's not in the field definitions Q_ASSERT(fieldDefn != this->primaryTable.tableFields.end()); - if (fieldDefn->fieldType == ObjectStore::Enum) { + if (fieldDefn->fieldType == ObjectStore::FieldType::Enum) { // Enums need to be converted to strings first propertyBindValue = QVariant{enumToString(*fieldDefn, propertyBindValue)}; } else if (fieldDefn->foreignKeyTo) { @@ -644,7 +646,8 @@ class ObjectStore::impl { } } else { // - // The property we've been given isn't a simple property, so look for it in the ones we store in junction tables + // The property we've been given isn't a simple property, so look for it in the ones we store in junction + // tables // auto matchingJunctionTableDefinitionDefn = std::find_if( this->junctionTables.begin(), @@ -663,8 +666,8 @@ class ObjectStore::impl { } // - // As elsewhere, the simplest way to update a junction table is to blat any rows relating to the current object and then - // write out data based on the current property values. + // As elsewhere, the simplest way to update a junction table is to blat any rows relating to the current object + // and then write out data based on the current property values. // qDebug() << Q_FUNC_INFO << "Updating" << object.metaObject()->className() << "property" << propertyName << @@ -730,7 +733,7 @@ class ObjectStore::impl { auto const & fieldDefn = this->primaryTable.tableFields[ii]; QVariant bindValue{object.property(*fieldDefn.propertyName)}; - if (fieldDefn.fieldType == ObjectStore::Enum) { + if (fieldDefn.fieldType == ObjectStore::FieldType::Enum) { // Enums need to be converted to strings first bindValue = QVariant{enumToString(fieldDefn, bindValue)}; } else if (fieldDefn.foreignKeyTo && bindValue.toInt() <= 0) { @@ -827,15 +830,18 @@ class ObjectStore::impl { Database * database; }; - ObjectStore::ObjectStore(TableDefinition const & primaryTable, JunctionTableDefinitions const & junctionTables) : pimpl{ std::make_unique(primaryTable, junctionTables) } { qDebug() << Q_FUNC_INFO << "Construct of object store for primary table" << this->pimpl->primaryTable.tableName; + // We have seen a circumstance where primaryTable.tableName is null, which shouldn't be possible. This is some + // diagnostic to try to find out why. + if (this->pimpl->primaryTable.tableName.isNull()) { + qCritical().noquote() << Q_FUNC_INFO << "Primary table without name. Call stack is:" << Logging::getStackTrace(); + } return; } - // See https://herbsutter.com/gotw/_100/ for why we need to explicitly define the destructor here (and not in the // header file) ObjectStore::~ObjectStore() { @@ -857,7 +863,6 @@ void ObjectStore::logDiagnostics() const { return; } - // Note that we have to pass Database in as a parameter because, ultimately, we're being called from Database::load() // which is called from Database::getInstance(), so we don't want to get in an endless loop. bool ObjectStore::createTables(Database & database, QSqlDatabase & connection) const { @@ -1001,7 +1006,7 @@ void ObjectStore::loadAll(Database * database) { } // Enums need to be converted from their string representation in the DB to a numeric value - if (fieldDefn.fieldType == ObjectStore::Enum) { + if (fieldDefn.fieldType == ObjectStore::FieldType::Enum) { fieldValue = QVariant(stringToEnum(fieldDefn, fieldValue)); //qDebug() << // Q_FUNC_INFO << "Value for property" << fieldDefn.propertyName << "after enum conversion: " << @@ -1161,7 +1166,8 @@ void ObjectStore::loadAll(Database * database) { return; // Continue but abort the transaction on a non-debug build } - // This is useful for debugging but I usually leave it commented out as it generates a lot of logging at start-up + // This is useful for debugging but I usually leave it commented out as it generates a lot of logging at + // start-up // qDebug() << // Q_FUNC_INFO << "Set" << // (junctionTable.assumedNumEntries == ObjectStore::MAX_ONE_ENTRY ? 1 : otherKeys.size()) << @@ -1202,7 +1208,6 @@ QList > ObjectStore::getByIds(QVector const & list return listToReturn; } - int ObjectStore::insert(std::shared_ptr object) { // Start transaction // (By the magic of RAII, this will abort if we return from this function without calling dbTransaction.commit() @@ -1291,7 +1296,7 @@ void ObjectStore::update(std::shared_ptr object) { QVariant bindValue{object->property(*fieldDefn.propertyName)}; // Enums need to be converted to strings first - if (fieldDefn.fieldType == ObjectStore::Enum) { + if (fieldDefn.fieldType == ObjectStore::FieldType::Enum) { bindValue = QVariant{enumToString(fieldDefn, bindValue)}; } @@ -1390,7 +1395,6 @@ void ObjectStore::updateProperty(QObject const & object, BtStringConst const & p return; } - std::shared_ptr ObjectStore::defaultSoftDelete(int id) { // // We assume on soft-delete that there is nothing to do on related objects - eg if a Mash is soft deleted (ie marked @@ -1408,7 +1412,6 @@ std::shared_ptr ObjectStore::defaultSoftDelete(int id) { return object; } -// std::shared_ptr ObjectStore::defaultHardDelete(int id) { // // We assume on hard-delete that the subclass ObjectStore (specifically ObjectStoreTyped) will override this member @@ -1479,7 +1482,6 @@ std::shared_ptr ObjectStore::defaultHardDelete(int id) { return object; } - std::optional< std::shared_ptr > ObjectStore::findFirstMatching( std::function)> const & matchFunction ) const { @@ -1511,7 +1513,9 @@ QList > ObjectStore::findAllMatching( // rest of the code expects it and (b) from Qt 6, QList will become the same as QVector (see // https://www.qt.io/blog/qlist-changes-in-qt-6) QList > results; - std::copy_if(this->pimpl->allObjects.cbegin(), this->pimpl->allObjects.cend(), std::back_inserter(results), matchFunction); + std::copy_if(this->pimpl->allObjects.cbegin(), + this->pimpl->allObjects.cend(), + std::back_inserter(results), matchFunction); return results; } diff --git a/src/model/NamedParameterBundle.cpp b/src/model/NamedParameterBundle.cpp index 6fe6a2677..111cf9dea 100644 --- a/src/model/NamedParameterBundle.cpp +++ b/src/model/NamedParameterBundle.cpp @@ -1,6 +1,6 @@ /* * model/NamedParameterBundle.cpp is part of Brewtarget, and is Copyright the following - * authors 2021 + * authors 2021-2022: * - Matt Young * * Brewtarget is free software: you can redistribute it and/or modify diff --git a/translations/bt_ca.ts b/translations/bt_ca.ts index 945873ad1..753ded6a7 100644 --- a/translations/bt_ca.ts +++ b/translations/bt_ca.ts @@ -3090,7 +3090,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_cs.ts b/translations/bt_cs.ts index 1332e1d8c..6eacb28dc 100644 --- a/translations/bt_cs.ts +++ b/translations/bt_cs.ts @@ -3050,7 +3050,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_de.ts b/translations/bt_de.ts index 55045e270..58785236e 100644 --- a/translations/bt_de.ts +++ b/translations/bt_de.ts @@ -3074,7 +3074,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_el.ts b/translations/bt_el.ts index bf1fe44a9..5a9ce2557 100644 --- a/translations/bt_el.ts +++ b/translations/bt_el.ts @@ -3050,7 +3050,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_en.ts b/translations/bt_en.ts index fa38ffe2e..4ef3e9932 100644 --- a/translations/bt_en.ts +++ b/translations/bt_en.ts @@ -2616,7 +2616,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_es.ts b/translations/bt_es.ts index 049ad3082..8ea2bede6 100644 --- a/translations/bt_es.ts +++ b/translations/bt_es.ts @@ -3090,7 +3090,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_et.ts b/translations/bt_et.ts index 8a4cd682d..17ecbfacd 100644 --- a/translations/bt_et.ts +++ b/translations/bt_et.ts @@ -2667,7 +2667,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_eu.ts b/translations/bt_eu.ts index d15027b3d..c41a7535a 100644 --- a/translations/bt_eu.ts +++ b/translations/bt_eu.ts @@ -2675,7 +2675,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_fr.ts b/translations/bt_fr.ts index 3cebdce32..3f1e4acaf 100644 --- a/translations/bt_fr.ts +++ b/translations/bt_fr.ts @@ -3090,7 +3090,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_gl.ts b/translations/bt_gl.ts index f5f60fc7e..4ef68d4f4 100644 --- a/translations/bt_gl.ts +++ b/translations/bt_gl.ts @@ -2790,7 +2790,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_hu.ts b/translations/bt_hu.ts index ba0657d74..9a0b8781d 100644 --- a/translations/bt_hu.ts +++ b/translations/bt_hu.ts @@ -3078,7 +3078,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_it.ts b/translations/bt_it.ts index 806548afc..1c7d59148 100644 --- a/translations/bt_it.ts +++ b/translations/bt_it.ts @@ -3102,7 +3102,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_lv.ts b/translations/bt_lv.ts index fde7a2767..16cfaae4a 100644 --- a/translations/bt_lv.ts +++ b/translations/bt_lv.ts @@ -2730,7 +2730,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_nb.ts b/translations/bt_nb.ts index 7cb0d6f8d..9d2002e8b 100644 --- a/translations/bt_nb.ts +++ b/translations/bt_nb.ts @@ -3050,7 +3050,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_nl.ts b/translations/bt_nl.ts index 38724d78c..4c073fdbf 100644 --- a/translations/bt_nl.ts +++ b/translations/bt_nl.ts @@ -3085,7 +3085,7 @@ Foutmelding: - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_pl.ts b/translations/bt_pl.ts index 786effd69..2c0ccf69e 100644 --- a/translations/bt_pl.ts +++ b/translations/bt_pl.ts @@ -3050,7 +3050,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_pt.ts b/translations/bt_pt.ts index f1adf3c9f..b6ccecee7 100644 --- a/translations/bt_pt.ts +++ b/translations/bt_pt.ts @@ -3082,7 +3082,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_ru.ts b/translations/bt_ru.ts index d7ed961d7..b15316f35 100644 --- a/translations/bt_ru.ts +++ b/translations/bt_ru.ts @@ -3102,7 +3102,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_sr.ts b/translations/bt_sr.ts index 8de81acb1..49aba8c93 100644 --- a/translations/bt_sr.ts +++ b/translations/bt_sr.ts @@ -2918,7 +2918,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_sv.ts b/translations/bt_sv.ts index 601fe09b2..eb9fab8d8 100644 --- a/translations/bt_sv.ts +++ b/translations/bt_sv.ts @@ -3058,7 +3058,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_tr.ts b/translations/bt_tr.ts index 3f1eedcee..b22724642 100644 --- a/translations/bt_tr.ts +++ b/translations/bt_tr.ts @@ -2671,7 +2671,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. diff --git a/translations/bt_zh.ts b/translations/bt_zh.ts index 65f1e0b35..ca3bdf3bf 100644 --- a/translations/bt_zh.ts +++ b/translations/bt_zh.ts @@ -3042,7 +3042,7 @@ Log file may contain more details. - Resource directory "%1" is missing. Some features will be unavailable. + Resource directory "%1" is missing. The software might not operate correctly without this directory and its contents. From 667b354cd27538fccba3bd70960e76c0d5c8cc70 Mon Sep 17 00:00:00 2001 From: Matt Young Date: Sun, 20 Nov 2022 03:27:02 +0100 Subject: [PATCH 2/3] Attempt to fix Mac build --- .github/workflows/mac.yml | 33 +++- CMakeLists.txt | 320 ++++++++++++++++++++++++++++++-------- 2 files changed, 280 insertions(+), 73 deletions(-) diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index ae8be0a8a..fb16a2ba0 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -27,28 +27,45 @@ jobs: fetch-depth: 0 - name: Install Qt + # Version 5.15.2 is, according to https://github.com/jurplel/install-qt-action, the last Qt 5 LTS + # When we're ready to migrate to Qt 6, we'll need to tweak this uses: jurplel/install-qt-action@v3 with: - version: 5.9.5 + version: 5.15.2 - name: Install dependencies + # # Installing Xalan-C will cause Xerces-C to be installed too (as the former depends on the latter) # .:TBD:. Installing Boost here doesn't seem to give us libboost_stacktrace_backtrace # Also, trying to use the "--cc=clang" option to install boost gives an error ("Error: boost: no bottle # available!") For the moment, we're just using Boost header files on Mac though, so this should be OK. + # + # The `brew doctor` command just checks that Homebrew (https://brew.sh/) is installed OK (expected output is "Your + # system is ready to brew". Having Homebrew installed should imply the Xcode Command Line Tools are also + # installed, but `xcode-select -p` confirms this (expected output "/Library/Developer/CommandLineTools"). As + # elsewhere we use the echo trick to ensure that a non-zero return value from these diagnostic commands is not + # treated as a build script failure. + # + # We use the tree command for diagnosing certain build problems (specifically to see what changes certain parts of + # the build have made to the build directory tree). (If need be, you can also download the entire build directory + # within a day of a failed build running, but you need a decent internet connection for this as it's quite large.) + # run: | + echo "Output from brew doctor: $(brew doctor)" + echo "Output from xcode-select -p: $(xcode-select -p)" brew install xalan-c brew install boost + brew install tree - name: Build env: QT_QPA_PLATFORM: offscreen + # Change `make` to `make VERBOSE=1` to get hugely detailed output run: | mkdir build cd build cmake .. make - ls -ltr - name: Prep for tests # If a test fails and we get a core, we'd like to analyse it. This will be easier if we have access to the @@ -77,8 +94,10 @@ jobs: ctest --extra-verbose --output-on-failure 2>&1 - name: Make package + # Change `make package` to `make package VERBOSE=1` to get hugely detailed output run: | cd build + pwd make package ls -ltr @@ -92,17 +111,23 @@ jobs: ${{github.workspace}}/build/brewtarget*.dmg.sha256 retention-days: 7 - - name: Post-processing on cores + - name: Post-processing on any core dump if: ${{ failure() }} # It's all very well capturing core files, but if you don't have a Mac to analyse them on they are not a fat lot # of use. So, if we did get a core, let's at least get a stack trace out of it. + # + # The loop in the last line should run either 0 or 1 times, depending on whether the build failure did or did not + # generate a core file. + # ls -1 | while read ii; do echo "bt" | lldb -c $ii; done run: | + pwd + tree -sh sudo chmod -R +rwx /cores sudo chmod -R +rwx /Library/Logs/DiagnosticReports echo "Contents of /cores directory: $(ls -ltr /cores)" echo "Contents of /Library/Logs/DiagnosticReports directory: $(ls -ltr /Library/Logs/DiagnosticReports)" cd /cores - echo "bt" | lldb -c $(ls -1) + ls -1 | while read ii; do echo "bt" | lldb -c $ii; done - name: Recover Debris Artifacts (aka build output) if: ${{ failure() }} diff --git a/CMakeLists.txt b/CMakeLists.txt index 9d34348d4..ccc8bf6dc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -214,18 +214,6 @@ set(DATA_INSTALL_COMPONENT "Data") # For architecture-dependent binaries set(RUNTIME_INSTALL_COMPONENT "Runtime") -#======================================================================================================================= -#================================================= Mac Bundle Settings ================================================= -#======================================================================================================================= -set(MACOSX_BUNDLE_BUNDLE_NAME "${capitalisedProjectName}") -#set(MACOSX_BUNDLE_GUI_IDENTIFIER) -#set(MACOSX_BUNDLE_INFO_STRING) -#set(MACOSX_BUNDLE_BUNDLE_VERSION) -#set(MACOSX_BUNDLE_SHORT_VERSION_STRING) -set(MACOSX_BUNDLE_LONG_VERSION_STRING ${PROJECT_VERSION}) -set(MACOSX_BUNDLE_ICON_FILE "BrewtargetIcon.icns") -set(MACOSX_BUNDLE_COPYRIGHT "GPLv3") - #======================================================================================================================= #============================================== Compiler settings & flags ============================================== #======================================================================================================================= @@ -351,16 +339,6 @@ MACRO (ADD_PCH_RULE _header_filename _src_list) DEPENDS ${_header}) ENDMACRO(ADD_PCH_RULE _src_list _header_filename) -# See discussion at https://stackoverflow.com/questions/1138301/precompiled-headers-do-we-really-need-them about the -# pros and cons of using pre-compiles headers. For now we keep them turned off. -#if( UNIX AND NOT APPLE ) -# SET( precompiled_h "equipment.h" "fermentable.h" "hop.h" "instruction.h" "mash.h" "mashstep.h" "misc.h" "recipe.h" "style.h" "unit.h" "water.h" "yeast.h" "database.h" "brewtarget.h" ) -# FOREACH( header ${precompiled_h} ) -# ADD_PCH_RULE( ${header} brewtarget_SRCS ) -# ENDFOREACH() -#endif() - - #======================================================================================================================= #=================================================== Set build type ==================================================== #======================================================================================================================= @@ -543,6 +521,26 @@ if(WIN32) # International Components for Unicode file(GLOB IcuDlls "${QT5_BIN_DIR}/libicu*.dll") +elseif(APPLE) + #==================================================================================================================== + #=================================================== Mac Qt Stuff =================================================== + #==================================================================================================================== + + # The macdeployqt executable shipped with Qt does for Mac what windeployqt does for Windows -- see + # https://doc.qt.io/qt-6/macos-deployment.html#the-mac-deployment-tool + # + # At first glance, you might thanks that, with a few name changes, we might share all the CMake code for macdeployqt + # and windeployqt. However, as you will see below, the two programs share _only_ a top-level goal ("automate the + # process of creating a deployable [folder / applicaiton bundle] that contains [the necessary Qt dependencies]" - ie + # so that the end user does not have to install Qt to run our software). They have completely different + # implementations and command line options, so it would be unhelpful to try to treat them identically. + find_program(MACDEPLOYQT_EXECUTABLE macdeployqt HINTS "${QT5_BIN_DIR}") + if(EXISTS ${MACDEPLOYQT_EXECUTABLE}) + # Per https://cmake.org/cmake/help/latest/command/add_executable.html, "IMPORTED executables are useful for + # convenient reference from commands like add_custom_command()". + add_executable(Qt5::macdeployqt IMPORTED) + set_target_properties(Qt5::macdeployqt PROPERTIES IMPORTED_LOCATION ${MACDEPLOYQT_EXECUTABLE}) + endif() endif() @@ -720,7 +718,7 @@ configure_file(src/config.in src/config.h) #======================================================================================================================= # We don't need to list the embedded resource files themselves here, just the "Resource Collection File" that lists what # they are. -set(filesToCompile_qrc "${CMAKE_CURRENT_SOURCE_DIR}/brewtarget.qrc") +set(filesToCompile_qrc "${CMAKE_CURRENT_SOURCE_DIR}/${PROJECT_NAME}.qrc") #======================================================================================================================= #=========================================== Files included with the program =========================================== @@ -739,10 +737,10 @@ set(filesToInstall_data ${repoDir}/data/default_db.sqlite ${repoDir}/doc/manual-en.pdf) # Desktop files to install. -set(filesToInstall_desktop ${repoDir}/brewtarget.desktop) +set(filesToInstall_desktop ${repoDir}/${PROJECT_NAME}.desktop) # Icon files to install. -set(filesToInstall_icons ${repoDir}/images/brewtarget.svg) +set(filesToInstall_icons ${repoDir}/images/${PROJECT_NAME}.svg) # This is the list of translation files to update (from translatable strings in the source code) and from which the # binary .qm files will be generated and shipped. Note that src/OptionDialog.cpp controls which languages are shown to @@ -898,23 +896,28 @@ add_library(btobjlib ${filesToCompile_cpp} ${filesToCompile_qrc}) -set_source_files_properties(${filesToInstall_macIcons} - PROPERTIES - MACOSX_PACKAGE_LOCATION "Resources") -set_source_files_properties(${filesToInstall_data} - PROPERTIES - MACOSX_PACKAGE_LOCATION "Resources") -set_source_files_properties(${filesToInstall_docs} - PROPERTIES - MACOSX_PACKAGE_LOCATION "Resources/en.lproj") -set_source_files_properties(${filesToInstall_sounds} - PROPERTIES - MACOSX_PACKAGE_LOCATION "Resources/sounds") -set_source_files_properties(${QM_FILES} - PROPERTIES - MACOSX_PACKAGE_LOCATION "Resources/translations_qm") + if(APPLE) + # + # We have to tell CMake what things other than the executable etc to include in the Mac Applicaiton Bundle + # + set_source_files_properties(${filesToInstall_macIcons} + PROPERTIES + MACOSX_PACKAGE_LOCATION "Resources") + set_source_files_properties(${filesToInstall_data} + PROPERTIES + MACOSX_PACKAGE_LOCATION "Resources") + set_source_files_properties(${filesToInstall_docs} + PROPERTIES + MACOSX_PACKAGE_LOCATION "Resources/en.lproj") + set_source_files_properties(${filesToInstall_sounds} + PROPERTIES + MACOSX_PACKAGE_LOCATION "Resources/sounds") + set_source_files_properties(${QM_FILES} + PROPERTIES + MACOSX_PACKAGE_LOCATION "Resources/translations_qm") + # The MACOSX_BUNDLE parameter here sets the MACOSX_BUNDLE property on the created target. This means the executable # is built as an Application Bundle, which makes it a GUI executable that can be launched from the Finder. # @@ -929,6 +932,52 @@ if(APPLE) ${filesToInstall_docs} ${filesToInstall_sounds} $) + + #==================================================================================================================== + #================================================ Mac Bundle Settings =============================================== + #==================================================================================================================== + # See https://cmake.org/cmake/help/latest/prop_tgt/MACOSX_BUNDLE_INFO_PLIST.html for the CMake doco and + # https://developer.apple.com/documentation/bundleresources/information_property_list/bundle_configuration for the + # Apple documentation + # + + # Sets the CFBundleName bundle property. "This name can contain up to 15 characters. The system may display it to + # users if CFBundleDisplayName isn't set." (I can't see a way in CMake for us to set CFBundleDisplayName.) + set(MACOSX_BUNDLE_BUNDLE_NAME "${capitalisedProjectName}") + + # Sets the CFBundleIdentifier bundle property which is a case-insensitive string that "uniquely identifies a single + # app throughout the system ... Typically ... a reverse-DNS format". It is used when "applying specified + # preferences" (whatever that means), to "locate an app capable of opening a particular file", and for validating an + # app's signature. + set(MACOSX_BUNDLE_GUI_IDENTIFIER "com.${PROJECT_NAME}.${fileName_executable}") + + # We do not set MACOSX_BUNDLE_INFO_STRING, which sets the CFBundleGetInfoString bundle property, as AFAICT + # CFBundleGetInfoString is obsolete. + + # Sets the CFBundleVersion, which is "a machine-readable string composed of one to three period-separated integers, + # such as 10.14.1. The string can only contain numeric characters (0-9) and periods." + set(MACOSX_BUNDLE_BUNDLE_VERSION ${PROJECT_VERSION}) + + # + # This sets the bundle property key CFBundleShortVersionString, which is "a user-visible string for the version of + # the bundle. The required format is three period-separated integers, such as 10.14.1. The string can only contain + # numeric characters (0-9) and periods." + # + # One might think that CFBundleShortVersionString and CFBundleVersion are so similar as not to merit being separate + # keys, but it is not for us to question whether the left hand of Apple knows what the right hand is doing. + # + # Confusingly there is also a CMake target property called MACOSX_BUNDLE_LONG_VERSION_STRING which claims to set a + # CFBundleLongVersionString bundle property key. However, it seems this bundle property key is long obsolete. + # + set(MACOSX_BUNDLE_SHORT_VERSION_STRING ${PROJECT_VERSION}) + + # Sets the CFBundleIconFile bundle property key + set(MACOSX_BUNDLE_ICON_FILE "BrewkenIcon.icns") + + # Sets the NSHumanReadableCopyright bundle property key, which is "a human-readable copyright notice for the bundle". + set(MACOSX_BUNDLE_COPYRIGHT + "Copyright 2009-2022. Distributed under the terms of the GNU General Public License (version 3).") + else() add_executable(${fileName_executable} ${repoDir}/src/main.cpp @@ -938,6 +987,8 @@ else() $) endif() +#======================================================================================================================= +#======================================================================================================================= # Windows-specific library linking if(WIN32 AND MINGW) ############################################################################ @@ -981,6 +1032,55 @@ if(WIN32 AND MINGW) DEPENDS ${fileName_executable} ) endif() +elseif(APPLE) + # + # We want to achieve the same goal as with Qt5::windeployqt above, but note that the Qt5::macdeployqt functions + # rather differently, with different options, so our CMake code here is correspondingly different + # + # Note that although macdeployqt can create the disk image file, we do that part with CPack to be consistent with our + # approach to packaging on other platforms + # + # Using the MACOSX_BUNDLE paramater on add_executable() above will have created a directory tree akin to the + # following: + # + # ${fileName_executable}.app # Top level folder that the desktop user sees as "the application" + # └── Contents + # ├── Info.plist + # ├── MacOS + # │ └── ${fileName_executable} # The main executable + # └── Resources + # ├── en.lproj + # ├── sounds + # └── translations_qm + # + # Running macdeployqt will add "Frameworks" and "Plugins" directory trees inside the "Contents" directory, and + # populate these with the various Qt shared libraries etc (including database drivers) needed for the code to run. + # It will also copy some, but not all, of the other shared libraries we need. + # + # We still need to: + # - Copy across some non-Qt libraries we need (part of Xalan that macdeployqt does not detect) + # - Copy sound files into Contents/Resources/sounds and translation files into Contents/Resources/translations_qm + # - Copy default DB and default data into Contents/Resources (which is also where the user manual lives for + # Brewtarget) + # The last two should happen courtesy of the set_source_files_properties(... MACOSX_PACKAGE_LOCATION ...) commands + # above. (They won't show up in the diagnostic here because that copying happens _after_ building the executable. + # + if(TARGET Qt5::macdeployqt) + # Note that macdeployqt needs any options flags to be _after_ the name of the app. The only flag we use is: + # -verbose=<0-3> 0 = no output, 1 = error/warning (default), 2 = normal, 3 = debug + add_custom_command(TARGET ${fileName_executable} + POST_BUILD + # Uncomment the echo and tree lines to provide extra diagnostics. These are useful eg if you don't own a + # Mac, and are relying on GitHub actions to do Mac builds. (Just make sure tree is installed, usually via + # brew install tree. +# COMMAND echo "Contents of ${fileName_executable}.app before running macdeployqt" +# COMMAND tree -sh ${fileName_executable}.app + COMMAND Qt5::macdeployqt "$.app" -verbose=2 +# COMMAND echo "Contents of ${fileName_executable}.app after running macdeployqt" +# COMMAND tree -sh ${fileName_executable}.app + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} + ) + endif() endif() add_dependencies(${fileName_executable} translationsTarget) @@ -1149,6 +1249,9 @@ install(TARGETS ${fileName_executable} RUNTIME DESTINATION ${installSubDir_bin} COMPONENT ${RUNTIME_INSTALL_COMPONENT}) +#======================================================================================================================= +#==================================== Shipping shared libraries on Windows and Mac ===================================== +#======================================================================================================================= if(WIN32) # # Shared libraries (DLLs) are a bit complicated on Windows. Because there is not really a standard package @@ -1250,6 +1353,101 @@ if(WIN32) TARGET ${fileName_executable} POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy_if_different $ $ ) +elseif(APPLE) + # + # For similar reasons to Windows, the Mac Bundle needs to have shared libraries copied into it. Mostly this is done + # automatically with macdeployqt, which picks up Qt shared libraries, and other obvious dependencies such as + # libxerces-c.dylib and libxalan-c.dylib, and puts them in the Contents/Frameworks folder inside the bundle. + # + # However, where a library depends on another library, this is not picked up by macdeployqt. In particular, + # libxalan-c.dylib depends on libxalanMsg.dylib. The latter does not show up in XalanC_LIBRARIES either. So we + # have to take a more manual approach. + # + # Firstly, as https://stackoverflow.com/questions/63310608/find-library-cannot-find-basic-shared-library-macos-clion + # explains you must remove the "lib" prefix from the library name before asking CMake to find it. + # + # Then, once you have the path to the library, you have to deal with the case that it will probably be a symbolic + # link, so you need to dereference it rather than let CMake just copy the symlink -- see + # https://stackoverflow.com/questions/11487638/cmake-fileinstall-files-destination-dir-with-symbolic-links. (Except + # you also need to pay attention that the get_filename_component() command is now "superseded by cmake_path() + # command, except REALPATH now offered by file(REAL_PATH) command and PROGRAM now available in + # separate_arguments(PROGRAM) command". And because it's CMake, you have to pay close attention to which parameters + # are values (eg expanded variables) and which are variable names.) + # + # Subsequently, you discover that you need to rename things because: + # /usr/local/lib/libxalanMsg.dylib is a symbolic link to /usr/local/lib/libxalanMsg.112.dylib + # /usr/local/lib/libxalanMsg.112.dylib is a symbolic link to /usr/local/lib/libxalanMsg.112.0.dylib + # but when otool is run by fixup_bundle, it looks for @rpath/libxalanMsg.112.dylib and will not accept any other + # name for the library. So, we change ".0.dylib" at the end of the library file name to ".dylib". + # + # One day, I hope we will find a more elegant and less brittle way to do all this! + # + find_library(XalanCMessage_LIBRARY NAMES xalanMsg) + file(REAL_PATH "${XalanCMessage_LIBRARY}" XalanCMessage_LIBRARY_resolved) + cmake_path(GET XalanCMessage_LIBRARY_resolved FILENAME XalanCMessage_LIBRARY_withoutPath) + string(REGEX REPLACE "\.0\.dylib$" ".dylib" XalanCMessage_LIBRARY_tweaked "${XalanCMessage_LIBRARY_withoutPath}") + message(STATUS "Found libxalanMsg at ${XalanCMessage_LIBRARY} which resolves to ${XalanCMessage_LIBRARY_resolved}") + message(STATUS "Tweaked libxalanMsg file name from ${XalanCMessage_LIBRARY_withoutPath} to ${XalanCMessage_LIBRARY_tweaked}") + install(FILES ${XalanCMessage_LIBRARY_resolved} + DESTINATION ${CMAKE_BINARY_DIR}/${fileName_executable}.app/Contents/Frameworks/ + RENAME ${XalanCMessage_LIBRARY_tweaked}) + + # + # On MacOS, if you are shipping dynamic libraries with your application, you can tell the application where to look + # for those dynamic libraries via "Runpath Search Path" aka "@rpath". See + # https://gitlab.kitware.com/cmake/community/-/wikis/doc/cmake/RPATH-handling for more details. + # + # + # Setting CMAKE_BUILD_WITH_INSTALL_RPATH to true means "the software is always built with the install path for the + # RPATH and does not need to be relinked when installed". This is recommended at + # https://discourse.cmake.org/t/osx-frameworks-rpath-and-image-not-found/3094/9 + # + # Setting CMAKE_INSTALL_RPATH_USE_LINK_PATH to true "will append to the runtime search path (rpath) of installed + # binaries any directories outside the project that are in the linker search path or contain linked library files". + # + # CMAKE_INSTALL_RPATH is "a semicolon-separated list specifying the rpath to use in installed targets". + # + set(CMAKE_BUILD_WITH_INSTALL_RPATH true) + set(CMAKE_INSTALL_RPATH_USE_LINK_PATH true) + message(STATUS "CMAKE_INSTALL_RPATH = ${CMAKE_INSTALL_RPATH}, End Of Line") + + # + # Having copied the shared libraries into the bundle, we now need to use CMake's fixup_bundle + # (see https://cmake.org/cmake/help/latest/module/BundleUtilities.html) to make sure the application inside the + # bundle looks in the bundle, not the system, for each of these shared libraries. + # + # After this, there is a verify_app command which checks that everything "appears valid". + # + # Note that the CMake doco says "DO NOT USE THESE FUNCTIONS AT CONFIGURE TIME (from CMakeLists.txt)! Instead, invoke + # them from an install(CODE) or install(SCRIPT) rule." Note that this means we have to be careful about which + # variables are available to us. A lot of things are simply not set inside install(CODE). As explained at + # https://stackoverflow.com/questions/73815300/how-do-i-get-project-root-dir-in-the-cmake-install-code-section, there + # are various techniques to inject variables into install(CODE). We use the string(CONFIGURE) approach as it avoids + # too much escaping. + # + # The include(BundleUtilities) line is the bit the CMake documentation doesn't tell you about (or at least, not in as + # obvious a way as one might like). Without it you'll get an error about fixup_bundle being an unknown CMake + # command. + # + # Another thing you have to find out the hard way is that the first parameter to fixup_bundle ("") cannot be a + # relative path. If you pass in "foo.app", then fixup_bundle will look for "/foo.app". + # + # Besides the full path to the bundle to fix, fixup_bundle takes two other parameters -- + # fixup_bundle( ). The parameter "is a list of libraries that must be fixed up, but that + # cannot be determined by otool output analysis (i.e. plugins)". The parameter is undocumented (welcome to + # the joy of CMake) and often appears to be set to empty string in examples on stackoverflow etc. + # + # For diagnostics, it's often helpful to add lines such as the following in the code block below: + # execute_process(COMMAND echo "Pre fixup_bundle - build tree") + # execute_process(COMMAND pwd) + # execute_process(COMMAND tree -sh .) + # + string(CONFIGURE [[ + include(BundleUtilities) + fixup_bundle("${CMAKE_BINARY_DIR}/${fileName_executable}.app" "" "${CMAKE_BINARY_DIR}/${fileName_executable}.app/Contents/Frameworks") + verify_app("${CMAKE_BINARY_DIR}/${fileName_executable}.app") + ]] installCode) + install(CODE "${installCode}") endif() # Install the translations. @@ -1350,12 +1548,11 @@ endif() # add_custom_target( FinalMessage ALL - ${CMAKE_COMMAND} -E cmake_echo_color --bold --green "*** Finished compiling ${capitalisedProjectName}. Please run sudo make install to install. ***" + ${CMAKE_COMMAND} -E cmake_echo_color --bold --green "⭐⭐⭐ Finished compiling ${capitalisedProjectName}. Please run sudo make install to install. ⭐⭐⭐" COMMENT "Final Message" ) add_dependencies(FinalMessage ${fileName_executable}) - #======================================================================================================================= #====================================================== Packaging ====================================================== #======================================================================================================================= @@ -1381,7 +1578,7 @@ add_dependencies(FinalMessage ${fileName_executable}) # - CPACK_PACKAGING_INSTALL_PREFIX as the defaults from each generator should usually be fine # - CPACK_PACKAGE_CONTACT as, AFAICT, it's only used to provide a default value for CPACK_DEBIAN_PACKAGE_MAINTAINER # -set(CPACK_PACKAGE_VENDOR "The Brewtarget Team") +set(CPACK_PACKAGE_VENDOR "The ${capitalisedProjectName} Team") string( CONCAT CPACK_PACKAGE_DESCRIPTION @@ -1398,16 +1595,16 @@ set(CPACK_PACKAGE_HOMEPAGE_URL "https://github.com/Brewtarget/brewtarget") message("CMAKE_PROJECT_NAME = ${CMAKE_PROJECT_NAME}") message("PROJECT_VERSION = ${PROJECT_VERSION}") set(CPACK_PACKAGE_FILE_NAME "${CMAKE_PROJECT_NAME}_${PROJECT_VERSION}_${CMAKE_SYSTEM_PROCESSOR}") -set(CPACK_PACKAGE_INSTALL_DIRECTORY "brewtarget-${PROJECT_VERSION}") +set(CPACK_PACKAGE_INSTALL_DIRECTORY "${capitalisedProjectName}-${PROJECT_VERSION}") if(NOT WIN32) # Setting this on Windows breaks the NSIS packager with some confusing message saying it can't find the file - set(CPACK_PACKAGE_ICON "${repoDir}/images/brewtarget.svg") + set(CPACK_PACKAGE_ICON "${repoDir}/images/${PROJECT_NAME}.svg") endif() set(CPACK_PACKAGE_CHECKSUM "SHA256") set(CPACK_RESOURCE_FILE_LICENSE "${repoDir}/COPYING.GPLv3") set(CPACK_STRIP_FILES true) set(CPACK_VERBATIM_VARIABLES true) -set(CPACK_PACKAGE_EXECUTABLES "${fileName_executable}" "Brewtarget") +set(CPACK_PACKAGE_EXECUTABLES "${fileName_executable}" "${capitalisedProjectName}") # # Variables for Source Package Generators @@ -1418,16 +1615,16 @@ set(CPACK_SOURCE_IGNORE_FILES "/.svn/" "~$" "/CMakeFiles/" "/_CPack_Packages/" - "^brewtarget.*deb$" - "^brewtarget.*rpm$" - "^brewtarget.*tar.*$" + "^${PROJECT_NAME}.*deb$" + "^${PROJECT_NAME}.*rpm$" + "^${PROJECT_NAME}.*tar.*$" "CPack.*" "Makefile" "cmake_install.*" "\\\\.o$" - "/brewtarget.dir/" + "/${PROJECT_NAME}.dir/" "moc_.*" - "qrc_brewtarget.*" + "qrc_${PROJECT_NAME}.*" "ui_.*h" "install_manifest.*" "config\\\\.h") @@ -1505,11 +1702,11 @@ elseif(WIN32) set(CPACK_GENERATOR "NSIS") # The title displayed at the top of the installer. - set(CPACK_NSIS_PACKAGE_NAME "Brewtarget-${PROJECT_VERSION}") + set(CPACK_NSIS_PACKAGE_NAME "${capitalisedProjectName}-${PROJECT_VERSION}") # The display name string that appears in the Windows Apps & features in Control Panel - set(CPACK_NSIS_DISPLAY_NAME "Brewtarget-${PROJECT_VERSION}") + set(CPACK_NSIS_DISPLAY_NAME "${capitalisedProjectName}-${PROJECT_VERSION}") # ? - set(CPACK_PACKAGE_INSTALL_REGISTRY_KEY "Brewtarget-${PROJECT_VERSION}") + set(CPACK_PACKAGE_INSTALL_REGISTRY_KEY "${capitalisedProjectName}-${PROJECT_VERSION}") # URL to a web site providing more information about your application set(CPACK_NSIS_URL_INFO_ABOUT "https://github.com/Brewtarget/brewtarget") # The name of a *.ico file used as the main icon for the generated install program @@ -1539,10 +1736,6 @@ elseif(WIN32) set(CPACK_NSIS_MODIFY_PATH ON) - # Extra start menu items. - #set(CPACK_NSIS_MENU_LINKS - # "bin/${fileName_executable}" "My Brewtarget" - # ) elseif(APPLE) #================================================== Mac Packaging =================================================== # Broadly speaking, what we want for Mac is a a DMG disk image which the end user can mount and from s/he can copy @@ -1645,17 +1838,6 @@ set_property(DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} ${packagesTarBz2Files} ${packagesChecksumFiles}) -# make addframeworks should copy the Qt libraries to the app. -if(APPLE) - set(QT_BINARY_DIR "${_qt5Core_install_prefix}/bin") - add_custom_target( - addframeworks ALL - COMMAND ${QT_BINARY_DIR}/macdeployqt "Brewtarget.app" -dmg - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} - DEPENDS ${fileName_executable} - ) -endif() - if(UNIX AND NOT APPLE) add_custom_target( package_lint From 536235facff72e2d4cf38d87eb44830a0ddacc20 Mon Sep 17 00:00:00 2001 From: Matt Young Date: Sun, 20 Nov 2022 03:27:40 +0100 Subject: [PATCH 3/3] Minor version bump --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ccc8bf6dc..f867e77d0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -101,7 +101,7 @@ cmake_policy(SET CMP0048 NEW) #======================================================================================================================= # It's simplest to keep the project name all lower-case as it means we can use a lot more of the default settings for # Linux packaging (where directory names etc are expected to be all lower-case) -project(brewtarget VERSION 3.0.3 LANGUAGES CXX) +project(brewtarget VERSION 3.0.4 LANGUAGES CXX) message(STATUS "Building ${PROJECT_NAME} version ${PROJECT_VERSION}") message(STATUS "PROJECT_SOURCE_DIR is ${PROJECT_SOURCE_DIR}") # Sometimes we do need the capitalised version of the project name