diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index 42cc1a96f..a3c7890c7 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -39,6 +39,11 @@ jobs: - uses: actions/checkout@v3 with: fetch-depth: 0 + # + # Tried to install MacPorts from the bt script, but get errors running its configure script, so trying from GitHub + # actions. + # + - uses: melusina-org/setup-macports@v1 # # The `brew doctor` command just checks that Homebrew (https://brew.sh/) is installed OK (expected output is "Your @@ -53,7 +58,7 @@ jobs: run: | echo "Output from brew doctor: $(brew doctor)" echo "Output from xcode-select -p: $(xcode-select -p)" - brew install python@3.11 + brew install python@3.12 echo "Python3 ($(which python3)) version" /usr/bin/env python3 --version echo "Running ./bt -v setup all" diff --git a/bt b/bt index 503cbcb7e..285658593 100755 --- a/bt +++ b/bt @@ -756,6 +756,33 @@ def installDependencies(): case 'Darwin': log.debug('Mac') # + # We install most of the Mac dependencies via Homebrew (https://brew.sh/) using the `brew` command below. + # However, as at 2023-12-01, Homebrew has stopped supplying a package for Xalan-C. So, we install that using + # MacPorts (https://ports.macports.org/), which provides the `port` command. + # + # Note that MacPorts (port) requires sudo but Homebrew (brew) does not. Perhaps more importantly, they two + # package managers install things to different locations: + # - Homebrew packages are installed under /usr/local/Cellar/ with symlinks in /usr/local/opt/ + # - MacPorts packages are installed under /opt/local + # This means we need to have both directories in the include path when we come to compile. Thankfully, both + # CMake and Meson take care of finding a library automatically once given its name. + # + # Note too that package names vary slightly between HomeBrew and MacPorts. Don't assume you can guess one from + # the other, as it's not always evident. + # + + # + # Installing Homebrew is, in theory, somewhat easier and more self-contained than MacPorts as you just run the + # following: + # /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" + # In practice, invoking that command from this script is a bit fiddly to get right. For the moment, we simply + # assume Homebrew is already installed (because it is on the GitHub actions). + # + + # + # We install as many of our dependencies as possible with with Homebrew, and do this first, because some of + # these packages will also be needed for the installation of MacPorts to work. + # # We could make this list shorter if we wanted as, eg, installing Xalan-C will cause Xerces-C to be installed # too (as the former depends on the latter). However, I think it's clearer to explicitly list all the direct # dependencies (eg we do make calls directly into Xerces). @@ -771,22 +798,23 @@ def installDependencies(): # diagnosing certain build problems (eg to see what changes certain parts of the build have made to the build # directory tree) when the build is running as a GitHub action. # - installList = ['boost', - 'cmake', - 'coreutils', - 'doxygen', - 'gcc', - 'git', - 'llvm', - 'meson', - 'ninja', - 'pandoc', - 'tree', - 'qt@5', - 'xalan-c', - 'xerces-c'] - for packageToInstall in installList: - log.debug('Installing ' + packageToInstall) + installListBrew = ['llvm', + 'gcc', + 'cmake', + 'coreutils', + 'boost', + 'doxygen', + 'git', + 'meson', + 'ninja', + 'pandoc', + 'tree', + 'qt@5', +# 'xalan-c', +# 'xerces-c' + ] + for packageToInstall in installListBrew: + log.debug('Installing ' + packageToInstall + ' via Homebrew') abortOnRunFail(subprocess.run(['brew', 'install', packageToInstall])) # # By default, even once Qt5 is installed, Meson will not find it @@ -805,6 +833,37 @@ def installDependencies(): # abortOnRunFail(subprocess.run(['brew', 'link', '--force', 'qt5'])) + # + # In theory, we can now install MacPorts. However, I didn't manage to get the following working. The + # configure script complains about the lack of /usr/local/.//mkspecs/macx-clang. So, for now, we comment this + # bit out and install MacPorts for GitHub actions via the mac.yml script. + # + # This is a source install - per instructions at https://guide.macports.org/#installing.macports.source. In + # principle, we could install a precompiled binary, but it's a bit painful to do programatically as even to + # download the right package you have to know not just the the Darwin version of MacOS you are running, but + # also its "release name" (aka "friendly name"). See + # https://apple.stackexchange.com/questions/333452/how-can-i-find-the-friendly-name-of-the-operating-system-from-the-shell-term + # for various ways to do this if we had to, though we might just as easily copy the info + # from https://en.wikipedia.org/wiki/MacOS#Timeline_of_releases + # +## log.debug('Installing MacPorts') +## abortOnRunFail(subprocess.run(['curl', '-O', 'https://distfiles.macports.org/MacPorts/MacPorts-2.8.1.tar.bz2'])) +## abortOnRunFail(subprocess.run(['tar', 'xf', 'MacPorts-2.8.1.tar.bz2'])) +## abortOnRunFail(subprocess.run(['cd', 'MacPorts-2.8.1/'])) +## abortOnRunFail(subprocess.run(['./configure'])) +## abortOnRunFail(subprocess.run(['make'])) +## abortOnRunFail(subprocess.run(['sudo', 'make', 'install'])) +## abortOnRunFail(subprocess.run(['export', 'PATH=/opt/local/bin:/opt/local/sbin:$PATH'])) + + # + # Now install Xalan-C via MacPorts + # + installListPort = ['xalanc', + 'xercesc3'] + for packageToInstall in installListPort: + log.debug('Installing ' + packageToInstall + ' via MacPorts') + abortOnRunFail(subprocess.run(['sudo', 'port', 'install', packageToInstall])) + # # dmgbuild is a Python package that provides a command line tool to create macOS disk images (aka .dmg # files) -- see https://dmgbuild.readthedocs.io/en/latest/ diff --git a/src/database/ObjectStore.cpp b/src/database/ObjectStore.cpp index be708c8e2..d9c8eb73a 100644 --- a/src/database/ObjectStore.cpp +++ b/src/database/ObjectStore.cpp @@ -695,44 +695,57 @@ class ObjectStore::impl { /// propertyValue.typeName() << "(" << propertyType << ") in column" << fieldDefn.columnName << /// ". This is a known ugliness that we intend to fix one day."; } else { - // It's not a known exception, so it's a coding error. However, we may still be able to recover. - // If we are expecting a boolean and we get a string holding "true" or "false" etc, then we know what to - // do. + // + // It's not a known exception, so it's a coding error. However, we can recover in the following cases: + // - If we are expecting a boolean and we get a string holding "true" or "false" etc, then we know + // what to do. + // - If we are expecting an int and we get a double then we can just ignore the non-integer part of + // the number. + // - If we are expecting an double and we got an int then we can just upcast it. + // bool recovered = false; + QVariant readPropertyValue = propertyValue; if (propertyType == QMetaType::QString && fieldDefn.fieldType == ObjectStore::FieldType::Bool) { // We'll take any reasonable string representation of true/false. For the moment, at least, I'm not // worrying about optional fields here. I think it's pretty rare, if ever, that we'd want an optional // boolean. QString propertyAsLcString = propertyValue.toString().trimmed().toLower(); - bool interpretedValue = false; if (propertyAsLcString == "true" || propertyAsLcString == "t" || propertyAsLcString == "1") { recovered = true; - interpretedValue = true; + propertyValue = QVariant(true); } else if (propertyAsLcString == "false" || propertyAsLcString == "f" || propertyAsLcString == "0") { recovered = true; - interpretedValue = false; - } - if (recovered) { - qWarning() << - Q_FUNC_INFO << "Recovered from unexpected type #" << propertyType << "=" << - propertyValue.typeName() << "in QVariant for property" << fieldDefn.propertyName << - ", field type" << fieldDefn.fieldType << ", value" << propertyValue << ", table" << - primaryTable.tableName << ", column" << fieldDefn.columnName << ". Interpreted value as" << - interpretedValue; - // Now we overwrite the supplied variant with what we worked out it should be, so processing can - // continue. - propertyValue = QVariant(interpretedValue); + propertyValue = QVariant(false); } + } else if (propertyType == QMetaType::Double && fieldDefn.fieldType == ObjectStore::FieldType::Int) { + propertyValue = QVariant(propertyValue.toInt(&recovered)); + } else if (propertyType == QMetaType::Int && fieldDefn.fieldType == ObjectStore::FieldType::Double) { + propertyValue = QVariant(propertyValue.toDouble(&recovered)); } - if (!recovered) { + if (recovered) { + qWarning() << + Q_FUNC_INFO << "Recovered from unexpected type #" << propertyType << "=" << + readPropertyValue.typeName() << "in QVariant for property" << fieldDefn.propertyName << + ", field type" << fieldDefn.fieldType << ", value" << readPropertyValue << ", table" << + primaryTable.tableName << ", column" << fieldDefn.columnName << ". Interpreted value as" << + propertyValue; + } else { + // + // Even in the case where we do not have a reasonable way to interpret the data in this column, we + // should probably NOT terminate the program. We are still discovering lots of cases where folks + // using the software have old Brewtarget databases with quirky values in some columns. It's better + // from a user point of view that the software carries on working even if some (hopefully) obscure + // field could not be read from the DB. + // qCritical() << Q_FUNC_INFO << "Unexpected type #" << propertyType << "=" << propertyValue.typeName() << "in QVariant for property" << fieldDefn.propertyName << ", field type" << fieldDefn.fieldType << ", value" << propertyValue << ", table" << primaryTable.tableName << ", column" << fieldDefn.columnName; - qCritical().noquote() << Q_FUNC_INFO << "Call stack is:" << Logging::getStackTrace(); - // Stop here on debug build - Q_ASSERT(false); + // If, during development or debugging, you want to have the program stop when it cannot interpret + // one of the DB fields, then uncomment the following two lines. +/// qCritical().noquote() << Q_FUNC_INFO << "Call stack is:" << Logging::getStackTrace(); +/// Q_ASSERT(false); } }