Skip to content

Commit

Permalink
Hop Additions seems working. Plus Mac build fix. Plus fix for Brewtar…
Browse files Browse the repository at this point in the history 
…get/brewtarget#780
  • Loading branch information
Matt Young committed Dec 28, 2023
1 parent dee6437 commit 9308c44
Show file tree
Hide file tree
Showing 40 changed files with 961 additions and 357 deletions.
7 changes: 6 additions & 1 deletion .github/workflows/mac.yml
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,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 @@ -840,22 +867,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 @@ -874,6 +902,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
78 changes: 48 additions & 30 deletions src/database/ObjectStore.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,9 +583,11 @@ class ObjectStore::impl {
/**
* Constructor
*/
impl(TypeLookup const & typeLookup,
impl(char const * const className,
TypeLookup const & typeLookup,
TableDefinition const & primaryTable,
JunctionTableDefinitions const & junctionTables) : m_state{ObjectStore::State::NotYetInitialised},
JunctionTableDefinitions const & junctionTables) : m_className{className},
m_state{ObjectStore::State::NotYetInitialised},
typeLookup{typeLookup},
primaryTable{primaryTable},
junctionTables{junctionTables},
Expand Down Expand Up @@ -768,44 +770,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 Expand Up @@ -1232,6 +1247,7 @@ class ObjectStore::impl {
return primaryKeyInDb;
}

char const * const m_className;
ObjectStore::State m_state;
TypeLookup const & typeLookup;
TableDefinition const & primaryTable;
Expand Down Expand Up @@ -1259,10 +1275,11 @@ QString ObjectStore::getDisplayName(ObjectStore::FieldType const fieldType) {
Q_ASSERT(false);
}

ObjectStore::ObjectStore(TypeLookup const & typeLookup,
ObjectStore::ObjectStore(char const * const className,
TypeLookup const & typeLookup,
TableDefinition const & primaryTable,
JunctionTableDefinitions const & junctionTables) :
pimpl{ std::make_unique<impl>(typeLookup, primaryTable, junctionTables) } {
pimpl{ std::make_unique<impl>(className, typeLookup, 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.
Expand Down Expand Up @@ -1839,12 +1856,12 @@ void ObjectStore::updateProperty(QObject const & object, BtStringConst const & p
return;
}

std::shared_ptr<QObject> ObjectStore::defaultSoftDelete(int id) {
std::shared_ptr<QObject> 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
// deleted but remains in the DB) then there isn't actually anything we need to do with its MashSteps.
//
qDebug() << Q_FUNC_INFO << "Soft delete item #" << id;
qDebug() << Q_FUNC_INFO << "Soft delete" << this->pimpl->m_className << "#" << id;
auto object = this->pimpl->allObjects.value(id);
if (this->pimpl->allObjects.contains(id)) {
this->pimpl->allObjects.remove(id);
Expand All @@ -1856,14 +1873,14 @@ std::shared_ptr<QObject> ObjectStore::defaultSoftDelete(int id) {
return object;
}

std::shared_ptr<QObject> ObjectStore::defaultHardDelete(int id) {
std::shared_ptr<QObject> ObjectStore::defaultHardDelete(int id) {
//
// We assume on hard-delete that the subclass ObjectStore (specifically ObjectStoreTyped) will override this member
// function to interact with the object to delete any "owned" objects. It is better to have the rules for that in
// the object model than here in the object store as they can be subtle, and it would be cumbersome to model them
// generically.
//
qDebug() << Q_FUNC_INFO << "Hard delete item #" << id;
qDebug() << Q_FUNC_INFO << "Hard delete" << this->pimpl->m_className << "#" << id;
auto object = this->pimpl->allObjects.value(id);
QSqlDatabase connection = this->pimpl->database->sqlDatabase();
DbTransaction dbTransaction{*this->pimpl->database, connection};
Expand Down Expand Up @@ -1993,10 +2010,11 @@ QList<QObject *> ObjectStore::findAllMatching(std::function<bool(QObject *)> con
QVector<int> ObjectStore::idsOfAllMatching(
std::function<bool(QObject const *)> const & matchFunction
) const {
qDebug() << Q_FUNC_INFO << this->pimpl->m_className;
// It would be nice to use C++20 ranges here, but I couldn't find a way to use them with QHash in such a way that the
// keys of the hash would be accessible in the range. So, for now, we do it the old way.
QVector<int> results;
for (auto hashEntry = this->pimpl->allObjects.cbegin(); hashEntry != this->pimpl->allObjects.cbegin(); ++hashEntry) {
for (auto hashEntry = this->pimpl->allObjects.cbegin(); hashEntry != this->pimpl->allObjects.cend(); ++hashEntry) {
if (matchFunction(hashEntry.value().get())) {
results.append(hashEntry.key());
}
Expand Down
4 changes: 3 additions & 1 deletion src/database/ObjectStore.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,12 +205,14 @@ class ObjectStore : public QObject {
/**
* \brief Constructor sets up mappings but does not read in data from DB
*
* \param className Set by \c ObjectStoreTyped
* \param typeLookup The \c TypeLookup object that, amongst other things allows us to tell whether Qt properties on
* this object type are "optional" (ie wrapped in \c std::optional)
* \param primaryTable First in the list should be the primary key
* \param junctionTables Optional
*/
ObjectStore(TypeLookup const & typeLookup,
ObjectStore(char const * const className,
TypeLookup const & typeLookup,
TableDefinition const & primaryTable,
JunctionTableDefinitions const & junctionTables = JunctionTableDefinitions{});

Expand Down
2 changes: 1 addition & 1 deletion src/database/ObjectStoreTyped.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class ObjectStoreTyped : public ObjectStore {
ObjectStoreTyped(TypeLookup const & typeLookup,
TableDefinition const & primaryTable,
JunctionTableDefinitions const & junctionTables = JunctionTableDefinitions{}) :
ObjectStore(typeLookup, primaryTable, junctionTables) {
ObjectStore(NE::staticMetaObject.className(), typeLookup, primaryTable, junctionTables) {
return;
}

Expand Down
10 changes: 8 additions & 2 deletions src/model/NamedEntity.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -354,8 +354,8 @@ class NamedEntity : public QObject {
* being compared are of the same class (eg we are not comparing a Hop with a Yeast) and that the names match,
* so subclasses do not need to repeat these tests.
*
* We do not currently anticipate sub-sub-classes of \b NamedEntity but if one ever were created, it should
* call its parent's implementation of this function before doing its own class-specific tests.
* A sub-sub-class of \c NamedEntity (eg \c RecipeAdditionHop) should call its parent's implementation of this
* function before doing its own class-specific tests.
* \return \b true if this object is, in all the ways that matter, equal to \b other
*/
virtual bool isEqualTo(NamedEntity const & other) const = 0;
Expand Down Expand Up @@ -646,6 +646,12 @@ S & operator<<(S & stream, NamedEntity const * namedEntity) {
return stream;
}

template<class S>
S & operator<<(S & stream, std::shared_ptr<NamedEntity> namedEntity) {
stream << namedEntity.get();
return stream;
}

/**
* \brief Convenience function for logging, including coping with null pointers
*
Expand Down

0 comments on commit 9308c44

Please sign in to comment.