From 532ac7e33008fdd2893ba7b91525ec128ee1e131 Mon Sep 17 00:00:00 2001 From: LordGrey <48840279+Lord-Grey@users.noreply.github.com> Date: Sat, 16 Oct 2021 13:54:18 +0200 Subject: [PATCH] Fix Database migration version handling (#1356) --- .github/workflows/apt.yml | 12 +- .github/workflows/nightly.yml | 6 +- debian/control.in | 2 +- debian/rules | 3 +- include/utils/version.hpp | 4 +- libsrc/hyperion/SettingsManager.cpp | 358 +++++++++++++++------------- test/CMakeLists.txt | 3 + test/TestVersions.cpp | 50 ++++ 8 files changed, 267 insertions(+), 171 deletions(-) create mode 100644 test/TestVersions.cpp diff --git a/.github/workflows/apt.yml b/.github/workflows/apt.yml index 2bb2b978..7525cb82 100644 --- a/.github/workflows/apt.yml +++ b/.github/workflows/apt.yml @@ -27,10 +27,15 @@ jobs: with: submodules: true + - name: Extract major.minor.patch from file .version + run: | + tr -d '\n' < .version > temp && mv temp .version + VERSION=$(tr -d '\n' < .version) + echo "MAJOR_MINOR_PATCH=$(echo "${VERSION%.*}.$((${VERSION##*.}))")" >> $GITHUB_ENV + - name: Build package shell: bash run: | - tr -d '\n' < .version > temp && mv temp .version mkdir -p "${GITHUB_WORKSPACE}/deploy" docker run --rm \ -v "${GITHUB_WORKSPACE}/deploy:/deploy" \ @@ -40,9 +45,8 @@ jobs: mkdir -p debian/source && echo '3.0 (quilt)' > debian/source/format && \ dch --create --distribution $(echo ${{ matrix.distribution }} | tr '[:upper:]' '[:lower:]') --package 'hyperion' -v '$(cat .version)~$(echo ${{ matrix.distribution }} | tr '[:upper:]' '[:lower:]')' '${{ github.event.commits[0].message }}' && \ cp -fr LICENSE debian/copyright && \ - sed 's/@BUILD_DEPENDS@/${{ matrix.build-depends }}/g; s/@DEPENDS@/${{ matrix.package-depends }}/g; s/@ARCHITECTURE@/${{ matrix.architecture }}/g' debian/control.in > debian/control && \ - tar cf ../hyperion_2.0.0.orig.tar . && \ - xz -9 ../hyperion_2.0.0.orig.tar && \ + sed 's/@BUILD_DEPENDS@/${{ matrix.build-depends }}/g; s/@DEPENDS@/${{ matrix.package-depends }}/g; s/@ARCHITECTURE@/${{ matrix.architecture }}/g; s/@STANDARDS_VERSION@/${{ env.MAJOR_MINOR_PATCH }}/g' debian/control.in > debian/control && \ + tar -cJf ../hyperion_$(cat .version)~$(echo ${{ matrix.distribution }} | tr '[:upper:]' '[:lower:]').orig.tar.xz . && \ debuild --no-lintian -uc -us && \ cp ../hyperion_* /deploy" diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index e43132d9..163be718 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -54,6 +54,7 @@ jobs: id: vars run: | VERSION="$(tr -d '\n' < .version)+nightly$(date '+%Y%m%d')$(git rev-parse --short HEAD)" + echo "MAJOR_MINOR_PATCH=$(echo "${VERSION%.*}.$((${VERSION##*.}))")" >> $GITHUB_ENV echo "$VERSION" > .version - name: Build package @@ -68,9 +69,8 @@ jobs: mkdir -p debian/source && echo '3.0 (quilt)' > debian/source/format && \ dch --create --distribution $(echo ${{ matrix.distribution }} | tr '[:upper:]' '[:lower:]') --package 'hyperion' -v '$(cat .version)~$(echo ${{ matrix.distribution }} | tr '[:upper:]' '[:lower:]')' '${{ github.event.commits[0].message }}' && \ cp -fr LICENSE debian/copyright && \ - sed 's/@BUILD_DEPENDS@/${{ matrix.build-depends }}/g; s/@DEPENDS@/${{ matrix.package-depends }}/g; s/@ARCHITECTURE@/${{ matrix.architecture }}/g' debian/control.in > debian/control && \ - tar cf ../hyperion_2.0.0.orig.tar . && \ - xz -9 ../hyperion_2.0.0.orig.tar && \ + sed 's/@BUILD_DEPENDS@/${{ matrix.build-depends }}/g; s/@DEPENDS@/${{ matrix.package-depends }}/g; s/@ARCHITECTURE@/${{ matrix.architecture }}/g; s/@STANDARDS_VERSION@/${{ env.MAJOR_MINOR_PATCH }}/g' debian/control.in > debian/control && \ + tar -cJf ../hyperion_$(cat .version)~$(echo ${{ matrix.distribution }} | tr '[:upper:]' '[:lower:]').orig.tar.xz . && \ debuild --no-lintian -uc -us && \ cp ../hyperion_* /deploy" diff --git a/debian/control.in b/debian/control.in index 5a75ef55..e52b6935 100644 --- a/debian/control.in +++ b/debian/control.in @@ -2,7 +2,7 @@ Source: hyperion Section: devel Priority: optional Build-Depends: @BUILD_DEPENDS@ -Standards-Version: 2.0.0 +Standards-Version: @STANDARDS_VERSION@ Maintainer: Hyperion Project Homepage: https://hyperion-project.org/ diff --git a/debian/rules b/debian/rules index 1c9608c8..aee926b0 100755 --- a/debian/rules +++ b/debian/rules @@ -14,6 +14,7 @@ binary-indep: binary-arch: cd $(BUILDDIR); cmake -P cmake_install.cmake + rm -rf debian/tmp/usr/include debian/tmp/usr/lib debian/tmp/usr/bin/flatc mkdir debian/tmp/DEBIAN cp cmake/package-scripts/postinst debian/tmp/DEBIAN chmod 0775 debian/tmp/DEBIAN/postinst @@ -23,7 +24,7 @@ binary-arch: chmod 0775 debian/tmp/DEBIAN/prerm dpkg-gencontrol -phyperion dpkg --build debian/tmp .. - rm -rf debian/tmp + rm -rf debian/tmp $(BUILDDIR) clean: rm -rf $(BUILDDIR) diff --git a/include/utils/version.hpp b/include/utils/version.hpp index 1e4d020d..11c3388c 100644 --- a/include/utils/version.hpp +++ b/include/utils/version.hpp @@ -406,12 +406,12 @@ namespace semver { friend bool operator== (version &lft, version &rgt) { - return lft.getVersion().compare(rgt.getVersion()) == 0; + return !(lft != rgt); } friend bool operator!= (version &lft, version &rgt) { - return !(lft == rgt); + return (lft > rgt) || (lft < rgt); } friend bool operator> (version &lft, version &rgt) diff --git a/libsrc/hyperion/SettingsManager.cpp b/libsrc/hyperion/SettingsManager.cpp index 7351825e..c3540121 100644 --- a/libsrc/hyperion/SettingsManager.cpp +++ b/libsrc/hyperion/SettingsManager.cpp @@ -255,6 +255,21 @@ bool SettingsManager::saveSettings(QJsonObject config, bool correct) return rc; } +inline QString fixVersion (const QString& version) +{ + QString newVersion; + //Try fixing version number, remove dot separated pre-release identifiers not supported + QRegularExpression regEx("(\\d+\\.\\d+\\.\\d+-?[a-zA-Z-\\d]*\\.?[\\d]*)", QRegularExpression::CaseInsensitiveOption | QRegularExpression::MultilineOption); + QRegularExpressionMatch match; + + match = regEx.match(version); + if (match.hasMatch()) + { + newVersion = match.captured(1); + } + return newVersion; +} + bool SettingsManager::resolveConfigVersion(QJsonObject& config) { bool isValid = false; @@ -267,6 +282,14 @@ bool SettingsManager::resolveConfigVersion(QJsonObject& config) if ( !configVersion.isEmpty() ) { isValid = _configVersion.setVersion(configVersion.toStdString()); + if (!isValid) + { + isValid = _configVersion.setVersion( fixVersion(configVersion).toStdString() ); + if (isValid) + { + Info(_log, "Invalid config version [%s] fixed. Updated to [%s]", QSTRING_CSTR(configVersion), _configVersion.getVersion().c_str()); + } + } } else { @@ -277,6 +300,14 @@ bool SettingsManager::resolveConfigVersion(QJsonObject& config) if ( !previousVersion.isEmpty() && isValid ) { isValid = _previousVersion.setVersion(previousVersion.toStdString()); + if (!isValid) + { + isValid = _previousVersion.setVersion( fixVersion(previousVersion).toStdString() ); + if (isValid) + { + Info(_log, "Invalid previous version [%s] fixed. Updated to [%s]", QSTRING_CSTR(previousVersion), _previousVersion.getVersion().c_str()); + } + } } else { @@ -291,199 +322,206 @@ bool SettingsManager::handleConfigUpgrade(QJsonObject& config) { bool migrated = false; - resolveConfigVersion(config); - - //Do only migrate, if configuration is not up to date - if (_previousVersion < _configVersion) + //Only migrate, if valid versions are available + if ( !resolveConfigVersion(config) ) { - //Migration steps for versions <= alpha 9 - semver::version targetVersion {"2.0.0-alpha.9"}; - if (_previousVersion <= targetVersion ) + Warning(_log, "Invalid version information found in configuration. No database migration executed."); + } + else + { + //Do only migrate, if configuration is not up to date + if (_previousVersion < _configVersion) { - Info(_log, "Instance [%u]: Migrate LED Layout from current version [%s] to version [%s] or later", _instance, _previousVersion.getVersion().c_str(), targetVersion.getVersion().c_str()); - - // LED LAYOUT UPGRADE - // from { hscan: { minimum: 0.2, maximum: 0.3 }, vscan: { minimum: 0.2, maximum: 0.3 } } - // from { h: { min: 0.2, max: 0.3 }, v: { min: 0.2, max: 0.3 } } - // to { hmin: 0.2, hmax: 0.3, vmin: 0.2, vmax: 0.3} - if(config.contains("leds")) + //Migration steps for versions <= alpha 9 + semver::version targetVersion {"2.0.0-alpha.9"}; + if (_previousVersion <= targetVersion ) { - const QJsonArray ledarr = config["leds"].toArray(); - const QJsonObject led = ledarr[0].toObject(); + Info(_log, "Instance [%u]: Migrate LED Layout from current version [%s] to version [%s] or later", _instance, _previousVersion.getVersion().c_str(), targetVersion.getVersion().c_str()); - if(led.contains("hscan") || led.contains("h")) + // LED LAYOUT UPGRADE + // from { hscan: { minimum: 0.2, maximum: 0.3 }, vscan: { minimum: 0.2, maximum: 0.3 } } + // from { h: { min: 0.2, max: 0.3 }, v: { min: 0.2, max: 0.3 } } + // to { hmin: 0.2, hmax: 0.3, vmin: 0.2, vmax: 0.3} + if(config.contains("leds")) { - const bool whscan = led.contains("hscan"); - QJsonArray newLedarr; + const QJsonArray ledarr = config["leds"].toArray(); + const QJsonObject led = ledarr[0].toObject(); - for(const auto & entry : ledarr) + if(led.contains("hscan") || led.contains("h")) { - const QJsonObject led = entry.toObject(); - QJsonObject hscan; - QJsonObject vscan; - QJsonValue hmin; - QJsonValue hmax; - QJsonValue vmin; - QJsonValue vmax; - QJsonObject nL; + const bool whscan = led.contains("hscan"); + QJsonArray newLedarr; - if(whscan) + for(const auto & entry : ledarr) { - hscan = led["hscan"].toObject(); - vscan = led["vscan"].toObject(); - hmin = hscan["minimum"]; - hmax = hscan["maximum"]; - vmin = vscan["minimum"]; - vmax = vscan["maximum"]; - } - else - { - hscan = led["h"].toObject(); - vscan = led["v"].toObject(); - hmin = hscan["min"]; - hmax = hscan["max"]; - vmin = vscan["min"]; - vmax = vscan["max"]; - } - // append to led object - nL["hmin"] = hmin; - nL["hmax"] = hmax; - nL["vmin"] = vmin; - nL["vmax"] = vmax; - newLedarr.append(nL); - } - // replace - config["leds"] = newLedarr; - migrated = true; - Info(_log,"Instance [%u]: LED Layout migrated", _instance); - } - } - - if(config.contains("ledConfig")) - { - QJsonObject oldLedConfig = config["ledConfig"].toObject(); - if ( !oldLedConfig.contains("classic")) - { - QJsonObject newLedConfig; - newLedConfig.insert("classic", oldLedConfig ); - QJsonObject defaultMatrixConfig {{"ledshoriz", 1} - ,{"ledsvert", 1} - ,{"cabling","snake"} - ,{"start","top-left"} - }; - newLedConfig.insert("matrix", defaultMatrixConfig ); - - config["ledConfig"] = newLedConfig; - migrated = true; - Info(_log,"Instance [%u]: LED-Config migrated", _instance); - } - } - - // LED Hardware count is leading for versions after alpha 9 - // Setting Hardware LED count to number of LEDs configured via layout, if layout number is greater than number of hardware LEDs - if (config.contains("device")) - { - QJsonObject newDeviceConfig = config["device"].toObject(); - - if (newDeviceConfig.contains("hardwareLedCount")) - { - int hwLedcount = newDeviceConfig["hardwareLedCount"].toInt(); - if (config.contains("leds")) - { - const QJsonArray ledarr = config["leds"].toArray(); - int layoutLedCount = ledarr.size(); - - if (hwLedcount < layoutLedCount ) - { - Warning(_log, "Instance [%u]: HwLedCount/Layout mismatch! Setting Hardware LED count to number of LEDs configured via layout", _instance); - hwLedcount = layoutLedCount; - newDeviceConfig["hardwareLedCount"] = hwLedcount; - migrated = true; + const QJsonObject led = entry.toObject(); + QJsonObject hscan; + QJsonObject vscan; + QJsonValue hmin; + QJsonValue hmax; + QJsonValue vmin; + QJsonValue vmax; + QJsonObject nL; + + if(whscan) + { + hscan = led["hscan"].toObject(); + vscan = led["vscan"].toObject(); + hmin = hscan["minimum"]; + hmax = hscan["maximum"]; + vmin = vscan["minimum"]; + vmax = vscan["maximum"]; + } + else + { + hscan = led["h"].toObject(); + vscan = led["v"].toObject(); + hmin = hscan["min"]; + hmax = hscan["max"]; + vmin = vscan["min"]; + vmax = vscan["max"]; + } + // append to led object + nL["hmin"] = hmin; + nL["hmax"] = hmax; + nL["vmin"] = vmin; + nL["vmax"] = vmax; + newLedarr.append(nL); } + // replace + config["leds"] = newLedarr; + migrated = true; + Info(_log,"Instance [%u]: LED Layout migrated", _instance); } } - if (newDeviceConfig.contains("type")) + if(config.contains("ledConfig")) { - QString type = newDeviceConfig["type"].toString(); - if (type == "atmoorb" || type == "fadecandy" || type == "philipshue" ) + QJsonObject oldLedConfig = config["ledConfig"].toObject(); + if ( !oldLedConfig.contains("classic")) { - if (newDeviceConfig.contains("output")) - { - newDeviceConfig["host"] = newDeviceConfig["output"].toString(); - newDeviceConfig.remove("output"); - migrated = true; - } + QJsonObject newLedConfig; + newLedConfig.insert("classic", oldLedConfig ); + QJsonObject defaultMatrixConfig {{"ledshoriz", 1} + ,{"ledsvert", 1} + ,{"cabling","snake"} + ,{"start","top-left"} + }; + newLedConfig.insert("matrix", defaultMatrixConfig ); + + config["ledConfig"] = newLedConfig; + migrated = true; + Info(_log,"Instance [%u]: LED-Config migrated", _instance); } } - if (migrated) + // LED Hardware count is leading for versions after alpha 9 + // Setting Hardware LED count to number of LEDs configured via layout, if layout number is greater than number of hardware LEDs + if (config.contains("device")) { - config["device"] = newDeviceConfig; - Debug(_log, "LED-Device records migrated"); - } - } + QJsonObject newDeviceConfig = config["device"].toObject(); - if (config.contains("grabberV4L2")) - { - QJsonObject newGrabberV4L2Config = config["grabberV4L2"].toObject(); + if (newDeviceConfig.contains("hardwareLedCount")) + { + int hwLedcount = newDeviceConfig["hardwareLedCount"].toInt(); + if (config.contains("leds")) + { + const QJsonArray ledarr = config["leds"].toArray(); + int layoutLedCount = ledarr.size(); - if (newGrabberV4L2Config.contains("encoding_format")) - { - newGrabberV4L2Config.remove("encoding_format"); - newGrabberV4L2Config["grabberV4L2"] = newGrabberV4L2Config; - migrated = true; + if (hwLedcount < layoutLedCount ) + { + Warning(_log, "Instance [%u]: HwLedCount/Layout mismatch! Setting Hardware LED count to number of LEDs configured via layout", _instance); + hwLedcount = layoutLedCount; + newDeviceConfig["hardwareLedCount"] = hwLedcount; + migrated = true; + } + } + } + + if (newDeviceConfig.contains("type")) + { + QString type = newDeviceConfig["type"].toString(); + if (type == "atmoorb" || type == "fadecandy" || type == "philipshue" ) + { + if (newDeviceConfig.contains("output")) + { + newDeviceConfig["host"] = newDeviceConfig["output"].toString(); + newDeviceConfig.remove("output"); + migrated = true; + } + } + } + + if (migrated) + { + config["device"] = newDeviceConfig; + Debug(_log, "LED-Device records migrated"); + } } - //Add new element enable - if (!newGrabberV4L2Config.contains("enable")) + if (config.contains("grabberV4L2")) { - newGrabberV4L2Config["enable"] = false; - migrated = true; - } - config["grabberV4L2"] = newGrabberV4L2Config; - Debug(_log, "GrabberV4L2 records migrated"); - } + QJsonObject newGrabberV4L2Config = config["grabberV4L2"].toObject(); - if (config.contains("framegrabber")) - { - QJsonObject newFramegrabberConfig = config["framegrabber"].toObject(); + if (newGrabberV4L2Config.contains("encoding_format")) + { + newGrabberV4L2Config.remove("encoding_format"); + newGrabberV4L2Config["grabberV4L2"] = newGrabberV4L2Config; + migrated = true; + } - //Align element namings with grabberV4L2 - //Rename element type -> device - if (newFramegrabberConfig.contains("type")) - { - newFramegrabberConfig["device"] = newFramegrabberConfig["type"].toString(); - newFramegrabberConfig.remove("type"); - migrated = true; - } - //Rename element frequency_Hz -> fps - if (newFramegrabberConfig.contains("frequency_Hz")) - { - newFramegrabberConfig["fps"] = newFramegrabberConfig["frequency_Hz"].toInt(25); - newFramegrabberConfig.remove("frequency_Hz"); - migrated = true; + //Add new element enable + if (!newGrabberV4L2Config.contains("enable")) + { + newGrabberV4L2Config["enable"] = false; + migrated = true; + } + config["grabberV4L2"] = newGrabberV4L2Config; + Debug(_log, "GrabberV4L2 records migrated"); } - //Rename element display -> input - if (newFramegrabberConfig.contains("display")) + if (config.contains("framegrabber")) { - newFramegrabberConfig["input"] = newFramegrabberConfig["display"]; - newFramegrabberConfig.remove("display"); - migrated = true; - } + QJsonObject newFramegrabberConfig = config["framegrabber"].toObject(); - //Add new element enable - if (!newFramegrabberConfig.contains("enable")) - { - newFramegrabberConfig["enable"] = false; - migrated = true; - } + //Align element namings with grabberV4L2 + //Rename element type -> device + if (newFramegrabberConfig.contains("type")) + { + newFramegrabberConfig["device"] = newFramegrabberConfig["type"].toString(); + newFramegrabberConfig.remove("type"); + migrated = true; + } + //Rename element frequency_Hz -> fps + if (newFramegrabberConfig.contains("frequency_Hz")) + { + newFramegrabberConfig["fps"] = newFramegrabberConfig["frequency_Hz"].toInt(25); + newFramegrabberConfig.remove("frequency_Hz"); + migrated = true; + } - config["framegrabber"] = newFramegrabberConfig; - Debug(_log, "Framegrabber records migrated"); + //Rename element display -> input + if (newFramegrabberConfig.contains("display")) + { + newFramegrabberConfig["input"] = newFramegrabberConfig["display"]; + newFramegrabberConfig.remove("display"); + migrated = true; + } + + //Add new element enable + if (!newFramegrabberConfig.contains("enable")) + { + newFramegrabberConfig["enable"] = false; + migrated = true; + } + + config["framegrabber"] = newFramegrabberConfig; + Debug(_log, "Framegrabber records migrated"); + } } } } + return migrated; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index fc37a03f..f34a5f7f 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -36,6 +36,9 @@ if(ENABLE_X11) target_link_libraries(test_x11performance ${X11_LIBRARIES} Qt5::Widgets) endif(ENABLE_X11) +add_executable(test_versions TestVersions.cpp) +target_link_libraries(test_versions Qt5::Core) + ######### These tests are broken. May they fix someone ########## # add_executable(test_image2ledsmap TestImage2LedsMap.cpp) diff --git a/test/TestVersions.cpp b/test/TestVersions.cpp new file mode 100644 index 00000000..ee996bcd --- /dev/null +++ b/test/TestVersions.cpp @@ -0,0 +1,50 @@ + +// STL includes +#include + +// QT includes +#include + +#include +using namespace semver; + +void test(const QString& a, const QString& b, char exp) +{ + semver::version verA (a.toStdString()); + semver::version verB (b.toStdString()); + + std::cout << "[" << a.toStdString() << "] : " << (verA.isValid() ? "" : "not ") << "valid" << std::endl; + std::cout << "[" << b.toStdString() << "] : " << (verB.isValid() ? "" : "not ") << "valid" << std::endl; + + if ( verA.isValid() && verB.isValid()) + { + std::cout << "[" << a.toStdString() << "] < [" << b.toStdString() << "]: " << (verA < verB) << " " << ((exp == '<') ? ((verA < verB) ? "OK" : "NOK") : "") << std::endl; + std::cout << "[" << a.toStdString() << "] > [" << b.toStdString() << "]: " << (verA > verB) << " " << ((exp == '>') ? ((verA > verB) ? "OK" : "NOK") : "") << std::endl; + std::cout << "[" << a.toStdString() << "] = [" << b.toStdString() << "]: " << (verA == verB) << " " << ((exp == '=') ? ((verA == verB) ? "OK" : "NOK") : "") << std::endl; + } + std::cout << "" << std::endl; +}; + +int main() +{ + test ("2.12.0", "2.12.0", '='); + test ("2.0.0-alpha.11", "2.12.0", '<'); + test ("2.11.1", "2.12.0", '<'); + test ("2.12.0", "2.12.1", '<'); + + test ("2.12.0+PR4711", "2.12.0+PR4712", '='); + test ("2.12.0+nighly20211012ac1dad7", "2.12.0+nighly20211012ac1dad8", '='); + + test ("2.0.0+nighly20211012ac1dad7", "2.0.12+nighly20211012ac1dad8", '<'); + + test ("2.0.0-alpha.11+nighly20211012ac1dad7", "2.0.0-alpha.11+nighly20211012ac1dad8", '='); + test ("2.0.0-alpha.11+PR1354", "2.0.0-alpha.11+PR1355", '='); + + test ("2.0.0-alpha.11+nighly20211012ac1dad7", "2.0.12+nighly20211012ac1dad8", '<'); + test ("2.0.0-alpha.11+nighly20211012ac1dad7", "2.0.12", '<'); + test ("2.0.0-alpha-11", "2.12.0", '<'); + + test ("2.0.0-alpha.10.1", "2.0.0-alpha.10", '>'); + + return 0; +}