From 6b8e0f7ae4bc032cd926e27547d6359da92a0f30 Mon Sep 17 00:00:00 2001 From: Jared Van Bortel Date: Tue, 30 Jul 2024 13:20:52 -0400 Subject: [PATCH] chat: fix comparison of versions with suffixes (#2772) Pre-release and post-release suffixes are now interpreted correctly. Also fix comparison of incomplete versions. Signed-off-by: Jared Van Bortel --- gpt4all-chat/download.cpp | 83 ++++++++++++++++++++++++++------------ gpt4all-chat/download.h | 1 + gpt4all-chat/modellist.cpp | 29 ++----------- 3 files changed, 61 insertions(+), 52 deletions(-) diff --git a/gpt4all-chat/download.cpp b/gpt4all-chat/download.cpp index 399e6e7b3ddc..47981d0b73c7 100644 --- a/gpt4all-chat/download.cpp +++ b/gpt4all-chat/download.cpp @@ -5,6 +5,7 @@ #include "network.h" #include +#include #include #include #include @@ -14,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -28,6 +30,7 @@ #include #include +#include #include #include @@ -60,30 +63,58 @@ static bool operator==(const ReleaseInfo& lhs, const ReleaseInfo& rhs) return lhs.version == rhs.version; } -static bool compareVersions(const QString &a, const QString &b) +std::strong_ordering Download::compareAppVersions(const QString &a, const QString &b) { - QRegularExpression regex("(\\d+)"); - QStringList aParts = a.split('.'); - QStringList bParts = b.split('.'); - Q_ASSERT(aParts.size() == 3); - Q_ASSERT(bParts.size() == 3); - - for (int i = 0; i < std::min(aParts.size(), bParts.size()); ++i) { - QRegularExpressionMatch aMatch = regex.match(aParts[i]); - QRegularExpressionMatch bMatch = regex.match(bParts[i]); - Q_ASSERT(aMatch.hasMatch() && bMatch.hasMatch()); - if (aMatch.hasMatch() && bMatch.hasMatch()) { - int aInt = aMatch.captured(1).toInt(); - int bInt = bMatch.captured(1).toInt(); - if (aInt > bInt) { - return true; - } else if (aInt < bInt) { - return false; - } - } + static QRegularExpression versionRegex(R"(^(\d+(?:\.\d+){0,2})(-.+)?$)"); + + // When comparing versions, make sure a2 < a10. + QCollator versionCollator(QLocale(QLocale::English, QLocale::UnitedStates)); + versionCollator.setNumericMode(true); + + QRegularExpressionMatch aMatch = versionRegex.match(a); + QRegularExpressionMatch bMatch = versionRegex.match(b); + + Q_ASSERT(aMatch.hasMatch() && bMatch.hasMatch()); // expect valid versions + + // Check for an invalid version. foo < 3.0.0 -> !hasMatch < hasMatch + if (auto diff = aMatch.hasMatch() <=> bMatch.hasMatch(); diff != 0) + return diff; // invalid version compares as lower + + // Compare invalid versions. fooa < foob + if (!aMatch.hasMatch() && !bMatch.hasMatch()) + return versionCollator.compare(a, b) <=> 0; // lexicographic comparison + + // Compare first three components. 3.0.0 < 3.0.1 + QStringList aParts = aMatch.captured(1).split('.'); + QStringList bParts = bMatch.captured(1).split('.'); + for (int i = 0; i < qMax(aParts.size(), bParts.size()); i++) { + bool ok = false; + int aInt = aParts.value(i, "0").toInt(&ok); + Q_ASSERT(ok); + int bInt = bParts.value(i, "0").toInt(&ok); + Q_ASSERT(ok); + if (auto diff = aInt <=> bInt; diff != 0) + return diff; // version with lower component compares as lower } - return aParts.size() > bParts.size(); + // Check for a pre/post-release suffix. 3.0.0-dev0 < 3.0.0-rc1 < 3.0.0 < 3.0.0-post1 + auto getSuffixOrder = [](const QRegularExpressionMatch &match) -> int { + QString suffix = match.captured(2); + return suffix.startsWith("-dev") ? 0 : + suffix.startsWith("-rc") ? 1 : + suffix.isEmpty() ? 2 : + /* some other suffix */ 3; + }; + if (auto diff = getSuffixOrder(aMatch) <=> getSuffixOrder(bMatch); diff != 0) + return diff; // different suffix types + + // Lexicographic comparison of suffix. 3.0.0-rc1 < 3.0.0-rc2 + if (aMatch.hasCaptured(2) && bMatch.hasCaptured(2)) { + if (auto diff = versionCollator.compare(aMatch.captured(2), bMatch.captured(2)); diff != 0) + return diff <=> 0; + } + + return std::strong_ordering::equal; } ReleaseInfo Download::releaseInfo() const @@ -99,11 +130,11 @@ ReleaseInfo Download::releaseInfo() const bool Download::hasNewerRelease() const { const QString currentVersion = QCoreApplication::applicationVersion(); - QList versions = m_releaseMap.keys(); - std::sort(versions.begin(), versions.end(), compareVersions); - if (versions.isEmpty()) - return false; - return compareVersions(versions.first(), currentVersion); + for (const auto &version : m_releaseMap.keys()) { + if (compareAppVersions(version, currentVersion) > 0) + return true; + } + return false; } bool Download::isFirstStart(bool writeVersion) const diff --git a/gpt4all-chat/download.h b/gpt4all-chat/download.h index 1a1195ed89fd..9cb46a9e09a3 100644 --- a/gpt4all-chat/download.h +++ b/gpt4all-chat/download.h @@ -57,6 +57,7 @@ class Download : public QObject public: static Download *globalInstance(); + static std::strong_ordering compareAppVersions(const QString &a, const QString &b); ReleaseInfo releaseInfo() const; bool hasNewerRelease() const; QString latestNews() const { return m_latestNews; } diff --git a/gpt4all-chat/modellist.cpp b/gpt4all-chat/modellist.cpp index 7d2d349a1cda..580b615ff4e6 100644 --- a/gpt4all-chat/modellist.cpp +++ b/gpt4all-chat/modellist.cpp @@ -1,5 +1,6 @@ #include "modellist.h" +#include "download.h" #include "mysettings.h" #include "network.h" @@ -33,7 +34,6 @@ #include #include -#include #include #include #include @@ -1442,29 +1442,6 @@ void ModelList::updateDataForSettings() emit dataChanged(index(0, 0), index(m_models.size() - 1, 0)); } -static std::strong_ordering compareVersions(const QString &a, const QString &b) -{ - QRegularExpression regex("(\\d+)"); - QStringList aParts = a.split('.'); - QStringList bParts = b.split('.'); - Q_ASSERT(aParts.size() == 3); - Q_ASSERT(bParts.size() == 3); - - for (int i = 0; i < std::min(aParts.size(), bParts.size()); ++i) { - QRegularExpressionMatch aMatch = regex.match(aParts[i]); - QRegularExpressionMatch bMatch = regex.match(bParts[i]); - Q_ASSERT(aMatch.hasMatch() && bMatch.hasMatch()); - if (aMatch.hasMatch() && bMatch.hasMatch()) { - int aInt = aMatch.captured(1).toInt(); - int bInt = bMatch.captured(1).toInt(); - if (auto diff = aInt <=> bInt; diff != 0) - return diff; - } - } - - return aParts.size() <=> bParts.size(); -} - void ModelList::parseModelsJsonFile(const QByteArray &jsonData, bool save) { QJsonParseError err; @@ -1516,11 +1493,11 @@ void ModelList::parseModelsJsonFile(const QByteArray &jsonData, bool save) continue; // If the current version is strictly less than required version, then skip - if (!requiresVersion.isEmpty() && compareVersions(currentVersion, requiresVersion) < 0) + if (!requiresVersion.isEmpty() && Download::compareAppVersions(currentVersion, requiresVersion) < 0) continue; // If the version removed is less than or equal to the current version, then skip - if (!versionRemoved.isEmpty() && compareVersions(versionRemoved, currentVersion) <= 0) + if (!versionRemoved.isEmpty() && Download::compareAppVersions(versionRemoved, currentVersion) <= 0) continue; modelFilesize = ModelList::toFileSize(modelFilesize.toULongLong());