Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Support for Versions with Preambles #4558

Merged
merged 5 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/AppInstallerCLITests/Versions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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("");
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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]")
Expand Down
4 changes: 2 additions & 2 deletions src/AppInstallerCommonCore/Manifest/ManifestCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion src/AppInstallerCommonCore/Public/winget/ManifestCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions src/AppInstallerSharedLib/Public/AppInstallerVersions.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
15 changes: 15 additions & 0 deletions src/AppInstallerSharedLib/Versions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)
Expand Down Expand Up @@ -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;

Expand Down
Loading