Skip to content

Commit 85589d0

Browse files
authored
"Pad" shorter versions with empty parts when comparing (#5009)
Cherry Pick #5001 into `release-v1.9` ###### Microsoft Reviewers: [Open in CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/winget-cli/pull/5009)
1 parent 5a646a7 commit 85589d0

File tree

4 files changed

+29
-28
lines changed

4 files changed

+29
-28
lines changed

src/AppInstallerCLITests/SQLiteIndex.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1911,8 +1911,8 @@ TEST_CASE("SQLiteIndex_Search_VersionSorting", "[sqliteindex]")
19111911
{
19121912
{ UtilityVersion("15.0.0"), Channel("") },
19131913
{ UtilityVersion("14.0.0"), Channel("") },
1914-
{ UtilityVersion("13.2.0-bugfix"), Channel("") },
19151914
{ UtilityVersion("13.2.0"), Channel("") },
1915+
{ UtilityVersion("13.2.0-bugfix"), Channel("") },
19161916
{ UtilityVersion("13.0.0"), Channel("") },
19171917
{ UtilityVersion("16.0.0"), Channel("alpha") },
19181918
{ UtilityVersion("15.8.0"), Channel("alpha") },
@@ -1966,8 +1966,8 @@ TEST_CASE("SQLiteIndex_PathString_VersionSorting", "[sqliteindex]")
19661966
{
19671967
{ UtilityVersion("15.0.0"), Channel("") },
19681968
{ UtilityVersion("14.0.0"), Channel("") },
1969-
{ UtilityVersion("13.2.0-bugfix"), Channel("") },
19701969
{ UtilityVersion("13.2.0"), Channel("") },
1970+
{ UtilityVersion("13.2.0-bugfix"), Channel("") },
19711971
{ UtilityVersion("13.0.0"), Channel("") },
19721972
{ UtilityVersion("16.0.0"), Channel("alpha") },
19731973
{ UtilityVersion("15.8.0"), Channel("alpha") },

src/AppInstallerCLITests/Versions.cpp

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,20 @@ TEST_CASE("VersionCompare", "[versions]")
154154
RequireLessThan("0.0.1-beta", "0.0.2-alpha");
155155
RequireLessThan("13.9.8", "14.1");
156156

157+
// Ensure that versions with non-digit characters in their parts are sorted correctly
158+
RequireLessThan("1-rc", "1");
159+
RequireLessThan("1.2-rc", "1.2");
160+
RequireLessThan("1.0-rc", "1.0");
161+
RequireLessThan("1.0.0-rc", "1");
162+
RequireLessThan("22.0.0-rc.1", "22.0.0");
163+
RequireLessThan("22.0.0-rc.1", "22.0.0.1");
164+
RequireLessThan("22.0.0-rc.1", "22.0.0.1-rc");
165+
166+
// Ensure that Sub-RC versions are sorted correctly
167+
RequireLessThan("22.0.0-rc.1", "22.0.0-rc.1.1");
168+
RequireLessThan("22.0.0-rc.1.1", "22.0.0-rc.1.2");
169+
RequireLessThan("22.0.0-rc.1.2", "22.0.0-rc.2");
170+
157171
RequireEqual("1.0", "1.0.0");
158172

159173
// Ensure whitespace doesn't affect equality
@@ -175,15 +189,16 @@ TEST_CASE("VersionAndChannelSort", "[versions]")
175189
{
176190
{ Version("15.0.0"), Channel("") },
177191
{ Version("14.0.0"), Channel("") },
178-
{ Version("13.2.0-bugfix"), Channel("") },
192+
{ Version("13.2.1-bugfix"), Channel("") },
179193
{ Version("13.2.0"), Channel("") },
194+
{ Version("13.2.0-rc"), Channel("") },
180195
{ Version("13.0.0"), Channel("") },
181196
{ Version("16.0.0"), Channel("alpha") },
182197
{ Version("15.8.0"), Channel("alpha") },
183198
{ Version("15.1.0"), Channel("beta") },
184199
};
185200

186-
std::vector<size_t> reorderList = { 4, 2, 1, 7, 6, 3, 5, 0 };
201+
std::vector<size_t> reorderList = { 4, 2, 1, 7, 6, 3, 8, 5, 0 };
187202
REQUIRE(sortedList.size() == reorderList.size());
188203

189204
std::vector<VersionAndChannel> jumbledList;

src/AppInstallerSharedLib/Public/AppInstallerVersions.h

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,11 @@ namespace AppInstaller::Utility
1818
//
1919
// Versions are compared by:
2020
// for each part in each version
21-
// if both sides have no more parts, return equal
22-
// else if one side has no more parts, it is less
23-
// else if integers not equal, return comparison of integers
21+
// if one side has no more parts, perform the remaining comparisons against an empty part
22+
// if integers not equal, return comparison of integers
2423
// else if only one side has a non-empty string part, it is less
2524
// else if string parts not equal, return comparison of strings
26-
// if all parts are same, use approximate comparator if applicable
25+
// if each part has been compared, use approximate comparator if applicable
2726
//
2827
// Note: approximate to another approximate version is invalid.
2928
// approximate to Unknown is invalid.

src/AppInstallerSharedLib/Versions.cpp

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -137,16 +137,12 @@ namespace AppInstaller::Utility
137137
return (thisIsUnknown && !otherIsUnknown);
138138
}
139139

140-
for (size_t i = 0; i < m_parts.size(); ++i)
140+
const Part emptyPart{};
141+
for (size_t i = 0; i < std::max(m_parts.size(), other.m_parts.size()); ++i)
141142
{
142-
if (i >= other.m_parts.size())
143-
{
144-
// All parts equal to this point
145-
break;
146-
}
147-
148-
const Part& partA = m_parts[i];
149-
const Part& partB = other.m_parts[i];
143+
// Whichever version is shorter, we need to pad it with empty parts
144+
const Part& partA = (i >= m_parts.size()) ? emptyPart : m_parts[i];
145+
const Part& partB = (i >= other.m_parts.size()) ? emptyPart : other.m_parts[i];
150146

151147
if (partA < partB)
152148
{
@@ -159,17 +155,8 @@ namespace AppInstaller::Utility
159155
// else parts are equal, so continue to next part
160156
}
161157

162-
// All parts tested were equal
163-
if (m_parts.size() == other.m_parts.size())
164-
{
165-
return ApproximateCompareLessThan(other);
166-
}
167-
else
168-
{
169-
// Else this is only less if there are more parts in other.
170-
return m_parts.size() < other.m_parts.size();
171-
}
172-
158+
// All parts were compared and found to be equal
159+
return ApproximateCompareLessThan(other);
173160
}
174161

175162
bool Version::operator>(const Version& other) const

0 commit comments

Comments
 (0)