diff --git a/src/AppInstallerCLITests/SQLiteIndex.cpp b/src/AppInstallerCLITests/SQLiteIndex.cpp index fb381ebb72..0166cebac1 100644 --- a/src/AppInstallerCLITests/SQLiteIndex.cpp +++ b/src/AppInstallerCLITests/SQLiteIndex.cpp @@ -1911,8 +1911,8 @@ TEST_CASE("SQLiteIndex_Search_VersionSorting", "[sqliteindex]") { { UtilityVersion("15.0.0"), Channel("") }, { UtilityVersion("14.0.0"), Channel("") }, - { UtilityVersion("13.2.0-bugfix"), Channel("") }, { UtilityVersion("13.2.0"), Channel("") }, + { UtilityVersion("13.2.0-bugfix"), Channel("") }, { UtilityVersion("13.0.0"), Channel("") }, { UtilityVersion("16.0.0"), Channel("alpha") }, { UtilityVersion("15.8.0"), Channel("alpha") }, @@ -1966,8 +1966,8 @@ TEST_CASE("SQLiteIndex_PathString_VersionSorting", "[sqliteindex]") { { UtilityVersion("15.0.0"), Channel("") }, { UtilityVersion("14.0.0"), Channel("") }, - { UtilityVersion("13.2.0-bugfix"), Channel("") }, { UtilityVersion("13.2.0"), Channel("") }, + { UtilityVersion("13.2.0-bugfix"), Channel("") }, { UtilityVersion("13.0.0"), Channel("") }, { UtilityVersion("16.0.0"), Channel("alpha") }, { UtilityVersion("15.8.0"), Channel("alpha") }, diff --git a/src/AppInstallerCLITests/Versions.cpp b/src/AppInstallerCLITests/Versions.cpp index 54975eda49..5a05be6db1 100644 --- a/src/AppInstallerCLITests/Versions.cpp +++ b/src/AppInstallerCLITests/Versions.cpp @@ -154,6 +154,20 @@ TEST_CASE("VersionCompare", "[versions]") RequireLessThan("0.0.1-beta", "0.0.2-alpha"); RequireLessThan("13.9.8", "14.1"); + // Ensure that versions with non-digit characters in their parts are sorted correctly + RequireLessThan("1-rc", "1"); + RequireLessThan("1.2-rc", "1.2"); + RequireLessThan("1.0-rc", "1.0"); + RequireLessThan("1.0.0-rc", "1"); + RequireLessThan("22.0.0-rc.1", "22.0.0"); + RequireLessThan("22.0.0-rc.1", "22.0.0.1"); + RequireLessThan("22.0.0-rc.1", "22.0.0.1-rc"); + + // Ensure that Sub-RC versions are sorted correctly + RequireLessThan("22.0.0-rc.1", "22.0.0-rc.1.1"); + RequireLessThan("22.0.0-rc.1.1", "22.0.0-rc.1.2"); + RequireLessThan("22.0.0-rc.1.2", "22.0.0-rc.2"); + RequireEqual("1.0", "1.0.0"); // Ensure whitespace doesn't affect equality @@ -175,15 +189,16 @@ TEST_CASE("VersionAndChannelSort", "[versions]") { { Version("15.0.0"), Channel("") }, { Version("14.0.0"), Channel("") }, - { Version("13.2.0-bugfix"), Channel("") }, + { Version("13.2.1-bugfix"), Channel("") }, { Version("13.2.0"), Channel("") }, + { Version("13.2.0-rc"), Channel("") }, { Version("13.0.0"), Channel("") }, { Version("16.0.0"), Channel("alpha") }, { Version("15.8.0"), Channel("alpha") }, { Version("15.1.0"), Channel("beta") }, }; - std::vector reorderList = { 4, 2, 1, 7, 6, 3, 5, 0 }; + std::vector reorderList = { 4, 2, 1, 7, 6, 3, 8, 5, 0 }; REQUIRE(sortedList.size() == reorderList.size()); std::vector jumbledList; diff --git a/src/AppInstallerSharedLib/Public/AppInstallerVersions.h b/src/AppInstallerSharedLib/Public/AppInstallerVersions.h index 9df7cab413..49dd4f1e99 100644 --- a/src/AppInstallerSharedLib/Public/AppInstallerVersions.h +++ b/src/AppInstallerSharedLib/Public/AppInstallerVersions.h @@ -18,12 +18,11 @@ namespace AppInstaller::Utility // // Versions are compared by: // for each part in each version - // if both sides have no more parts, return equal - // else if one side has no more parts, it is less - // else if integers not equal, return comparison of integers + // if one side has no more parts, perform the remaining comparisons against an empty part + // if integers not equal, return comparison of integers // else if only one side has a non-empty string part, it is less // else if string parts not equal, return comparison of strings - // if all parts are same, use approximate comparator if applicable + // if each part has been compared, use approximate comparator if applicable // // Note: approximate to another approximate version is invalid. // approximate to Unknown is invalid. diff --git a/src/AppInstallerSharedLib/Versions.cpp b/src/AppInstallerSharedLib/Versions.cpp index 2c9972c561..f268146f27 100644 --- a/src/AppInstallerSharedLib/Versions.cpp +++ b/src/AppInstallerSharedLib/Versions.cpp @@ -137,16 +137,12 @@ namespace AppInstaller::Utility return (thisIsUnknown && !otherIsUnknown); } - for (size_t i = 0; i < m_parts.size(); ++i) + const Part emptyPart{}; + for (size_t i = 0; i < std::max(m_parts.size(), other.m_parts.size()); ++i) { - if (i >= other.m_parts.size()) - { - // All parts equal to this point - break; - } - - const Part& partA = m_parts[i]; - const Part& partB = other.m_parts[i]; + // Whichever version is shorter, we need to pad it with empty parts + const Part& partA = (i >= m_parts.size()) ? emptyPart : m_parts[i]; + const Part& partB = (i >= other.m_parts.size()) ? emptyPart : other.m_parts[i]; if (partA < partB) { @@ -159,17 +155,8 @@ namespace AppInstaller::Utility // else parts are equal, so continue to next part } - // All parts tested were equal - if (m_parts.size() == other.m_parts.size()) - { - return ApproximateCompareLessThan(other); - } - else - { - // Else this is only less if there are more parts in other. - return m_parts.size() < other.m_parts.size(); - } - + // All parts were compared and found to be equal + return ApproximateCompareLessThan(other); } bool Version::operator>(const Version& other) const