Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be more accepting of unexpected DB data types #781

Merged
merged 12 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion .github/workflows/mac.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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"
Expand Down
91 changes: 75 additions & 16 deletions bt
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
Expand All @@ -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/
Expand Down
55 changes: 34 additions & 21 deletions src/database/ObjectStore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}
Expand Down