Merge pull request Brewtarget#680 from matty0ung/release3.0.1

Improved logging of start-up problems.  Attempt to fix Mac builds.  (They should at least be less broken now!)

.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() }}

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;
 

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 <[email protected]>
  *   • Brian Rower <[email protected]>
  *   • Chris Pavetto <[email protected]>
@@ -36,6 +36,9 @@
  */
 #include "PersistentSettings.h"
 
+#include <memory>
+
+#include <QDebug>
 #include <QSettings>
 #include <QStandardPaths>
 
@@ -89,11 +92,11 @@ namespace {
    //
    // This is set by PersistentSettings::initialise()
    //
-   QSettings * qSettings = nullptr;
+   std::unique_ptr<QSettings> 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<QSettings>(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.