commit 38aac878ab34d0176b34faf6946d7a0b55d4ce97
parent 57907ac7fa681703a4536e3b4deca8e17ee875d1
Author: Kaleb Luedtke <jluedtk@jci.com>
Date: Thu, 20 Jun 2024 11:03:53 -0500
Improve Support for Versions with Preambles (#4558)
- [x] This pull request is related to an issue.
- #2394
Diffstat:
5 files changed, 59 insertions(+), 3 deletions(-)
diff --git a/src/AppInstallerCLITests/Versions.cpp b/src/AppInstallerCLITests/Versions.cpp
@@ -56,6 +56,19 @@ TEST_CASE("VersionParseWithWhitespace", "[versions]")
}
}
+TEST_CASE("VersionParseWithPreamble", "[versions]")
+{
+ Version version("v1.2.3.4");
+ const auto& parts = version.GetParts();
+ REQUIRE(parts.size() == 4);
+ for (size_t i = 0; i < parts.size(); ++i)
+ {
+ INFO(i);
+ REQUIRE(parts[i].Integer == static_cast<uint64_t>(i + 1));
+ REQUIRE(parts[i].Other == "");
+ }
+}
+
TEST_CASE("VersionParseCorner", "[versions]")
{
Version version1("");
@@ -91,6 +104,14 @@ TEST_CASE("VersionParseCorner", "[versions]")
REQUIRE(parts[0].Other == "");
REQUIRE(parts[1].Integer == 1);
REQUIRE(parts[1].Other == "");
+
+ Version version7("v1.2a");
+ parts = version7.GetParts();
+ REQUIRE(parts.size() == 2);
+ REQUIRE(parts[0].Integer == 1);
+ REQUIRE(parts[0].Other == "");
+ REQUIRE(parts[1].Integer == 2);
+ REQUIRE(parts[1].Other == "a");
}
void RequireLessThan(std::string_view a, std::string_view b)
@@ -134,9 +155,18 @@ TEST_CASE("VersionCompare", "[versions]")
RequireLessThan("13.9.8", "14.1");
RequireEqual("1.0", "1.0.0");
+
// Ensure whitespace doesn't affect equality
RequireEqual("1.0", "1.0 ");
RequireEqual("1.0", "1. 0");
+
+ // Ensure versions with preambles are sorted correctly
+ RequireEqual("1.0", "Version 1.0");
+ RequireEqual("foo1", "bar1");
+ RequireLessThan("v0.0.1", "0.0.2");
+ RequireLessThan("v0.0.1", "v0.0.2");
+ RequireLessThan("1.a2", "1.b1");
+ RequireLessThan("alpha", "beta");
}
TEST_CASE("VersionAndChannelSort", "[versions]")
diff --git a/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp b/src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp
@@ -80,7 +80,7 @@ namespace AppInstaller::Manifest
}
}
- for (const Version& ext : m_extensions)
+ for (const RawVersion& ext : m_extensions)
{
if (ext.GetParts().empty() || ext.GetParts()[0].Integer != 0)
{
@@ -105,7 +105,7 @@ namespace AppInstaller::Manifest
bool ManifestVer::HasExtension(std::string_view extension) const
{
- for (const Version& ext : m_extensions)
+ for (const RawVersion& ext : m_extensions)
{
const auto& parts = ext.GetParts();
if (!parts.empty() && parts[0].Integer == 0 && parts[0].Other == extension)
diff --git a/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h b/src/AppInstallerCommonCore/Public/winget/ManifestCommon.h
@@ -15,6 +15,7 @@ namespace AppInstaller::Manifest
using string_t = Utility::NormalizedString;
using namespace std::string_view_literals;
+ using Utility::RawVersion;
// The maximum supported major version known about by this code.
constexpr uint64_t s_MaxSupportedMajorVersion = 1;
@@ -77,7 +78,7 @@ namespace AppInstaller::Manifest
bool HasExtension(std::string_view extension) const;
private:
- std::vector<Version> m_extensions;
+ std::vector<RawVersion> m_extensions;
};
enum class InstallerTypeEnum
diff --git a/src/AppInstallerSharedLib/Public/AppInstallerVersions.h b/src/AppInstallerSharedLib/Public/AppInstallerVersions.h
@@ -119,12 +119,22 @@ namespace AppInstaller::Utility
std::string m_version;
std::vector<Part> m_parts;
+ bool m_trimPrefix = true;
ApproximateComparator m_approximateComparator = ApproximateComparator::None;
// Remove trailing empty parts (0 or empty)
void Trim();
};
+ // Version that does not have leading non-digit characters trimmed
+ struct RawVersion : protected Version
+ {
+ RawVersion() { m_trimPrefix = false; }
+ RawVersion(std::string version, std::string_view splitChars = DefaultSplitChars);
+
+ using Version::GetParts;
+ };
+
// Four parts version number: 16-bits.16-bits.16-bits.16-bits
struct UInt64Version : public Version
{
diff --git a/src/AppInstallerSharedLib/Versions.cpp b/src/AppInstallerSharedLib/Versions.cpp
@@ -8,6 +8,7 @@ namespace AppInstaller::Utility
{
using namespace std::string_view_literals;
+ static constexpr std::string_view s_Digit_Characters = "0123456789"sv;
static constexpr std::string_view s_Version_Part_Latest = "Latest"sv;
static constexpr std::string_view s_Version_Part_Unknown = "Unknown"sv;
@@ -19,6 +20,12 @@ namespace AppInstaller::Utility
Assign(std::move(version), splitChars);
}
+ RawVersion::RawVersion(std::string version, std::string_view splitChars)
+ {
+ m_trimPrefix = false;
+ Assign(std::move(version), splitChars);
+ }
+
Version::Version(Version baseVersion, ApproximateComparator approximateComparator) : Version(std::move(baseVersion))
{
if (approximateComparator == ApproximateComparator::None)
@@ -56,6 +63,14 @@ namespace AppInstaller::Utility
baseVersion = m_version.substr(s_Approximate_Greater_Than.length(), m_version.length() - s_Approximate_Greater_Than.length());
}
+ // If there is a digit before the split character, or no split characters exist, trim off all leading non-digit characters
+ size_t digitPos = baseVersion.find_first_of(s_Digit_Characters);
+ size_t splitPos = baseVersion.find_first_of(splitChars);
+ if (m_trimPrefix && digitPos != std::string::npos && (splitPos == std::string::npos || digitPos < splitPos))
+ {
+ baseVersion.erase(0, digitPos);
+ }
+
// Then parse the base version
size_t pos = 0;
