Fix Database migration version handling (#1356)

.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 && \
+            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 cf ../hyperion_2.0.0.orig.tar . && \
+            tar -cJf ../hyperion_$(cat .version)~$(echo ${{ matrix.distribution }} | tr '[:upper:]' '[:lower:]').orig.tar.xz . && \
-            xz -9 ../hyperion_2.0.0.orig.tar && \
             debuild --no-lintian -uc -us && \
             cp ../hyperion_* /deploy"
 

.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 && \
+            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 cf ../hyperion_2.0.0.orig.tar . && \
+            tar -cJf ../hyperion_$(cat .version)~$(echo ${{ matrix.distribution }} | tr '[:upper:]' '[:lower:]').orig.tar.xz . && \
-            xz -9 ../hyperion_2.0.0.orig.tar && \
             debuild --no-lintian -uc -us && \
             cp ../hyperion_* /deploy"
 

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 <admin@hyperion-project.org>
 Homepage: https://hyperion-project.org/
 

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)

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)

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,8 +322,13 @@ bool SettingsManager::handleConfigUpgrade(QJsonObject& config)
 {
 	bool migrated = false;
 
-	resolveConfigVersion(config);
+	//Only migrate, if valid versions are available
-
+	if ( !resolveConfigVersion(config) )
+	{
+		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)
 		{
@@ -485,5 +521,7 @@ bool SettingsManager::handleConfigUpgrade(QJsonObject& config)
 				}
 			}
 		}
+	}
+
 	return migrated;
 }

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)

test/TestVersions.cpp

@@ -0,0 +1,50 @@
+
+// STL includes
+#include <iostream>
+
+// QT includes
+#include <QString>
+
+#include <utils/version.hpp>
+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;
+}