diff --git a/Source/cmFindPackageCommand.cxx b/Source/cmFindPackageCommand.cxx index a47e9c3428..993fa930e4 100644 --- a/Source/cmFindPackageCommand.cxx +++ b/Source/cmFindPackageCommand.cxx @@ -2758,23 +2758,32 @@ bool cmFindPackageCommand::CheckVersion(std::string const& config_file) if (result && hasVersion) { this->VersionFound = version; - std::vector const& versionParts = reader->ParseVersion(); - this->VersionFoundCount = static_cast(versionParts.size()); - switch (this->VersionFoundCount) { - case 4: - this->VersionFoundTweak = versionParts[3]; - CM_FALLTHROUGH; - case 3: - this->VersionFoundPatch = versionParts[2]; - CM_FALLTHROUGH; - case 2: - this->VersionFoundMinor = versionParts[1]; - CM_FALLTHROUGH; - case 1: - this->VersionFoundMajor = versionParts[0]; - CM_FALLTHROUGH; - default: - break; + cm::optional const& + parsedVersion = reader->ParseVersion(); + if (parsedVersion) { + std::vector const& versionParts = + parsedVersion->ReleaseComponents; + + this->VersionFoundCount = + static_cast(versionParts.size()); + switch (std::min(this->VersionFoundCount, 4u)) { + case 4: + this->VersionFoundTweak = versionParts[3]; + CM_FALLTHROUGH; + case 3: + this->VersionFoundPatch = versionParts[2]; + CM_FALLTHROUGH; + case 2: + this->VersionFoundMinor = versionParts[1]; + CM_FALLTHROUGH; + case 1: + this->VersionFoundMajor = versionParts[0]; + CM_FALLTHROUGH; + default: + break; + } + } else { + this->VersionFoundCount = 0; } } this->CpsReader = std::move(reader); diff --git a/Source/cmPackageInfoReader.cxx b/Source/cmPackageInfoReader.cxx index a5349cada7..dd039433e2 100644 --- a/Source/cmPackageInfoReader.cxx +++ b/Source/cmPackageInfoReader.cxx @@ -366,6 +366,62 @@ void AddDefinitions(cmMakefile* makefile, cmTarget* target, } } +cm::optional ParseSimpleVersion( + std::string const& version) +{ + if (version.empty()) { + return cm::nullopt; + } + + cmPackageInfoReader::Pep440Version result; + result.Simple = true; + + cm::string_view remnant{ version }; + for (;;) { + // Find the next part separator. + std::string::size_type const n = remnant.find_first_of(".+-"_s); + if (n == 0) { + // The part is an empty string. + return cm::nullopt; + } + + // Extract the part as a number. + cm::string_view const part = remnant.substr(0, n); + std::string::size_type const l = part.size(); + std::string::size_type p; + unsigned long const value = std::stoul(std::string{ part }, &p); + if (p != l || value > std::numeric_limits::max()) { + // The part was not a valid number or is too big. + return cm::nullopt; + } + result.ReleaseComponents.push_back(static_cast(value)); + + // Have we consumed the entire input? + if (n == std::string::npos) { + return { std::move(result) }; + } + + // Lop off the current part. + char const sep = remnant[n]; + remnant = remnant.substr(n + 1); + if (sep == '+' || sep == '-') { + // If we hit the local label, we're done. + result.LocalLabel = remnant; + return { std::move(result) }; + } + + // We just consumed a '.'; check that there's more. + if (remnant.empty()) { + // A trailing part separator is not allowed. + return cm::nullopt; + } + + // Continue with the remaining input. + } + + // Unreachable. +} + } // namespace std::unique_ptr cmPackageInfoReader::Read( @@ -428,40 +484,22 @@ cm::optional cmPackageInfoReader::GetVersion() const return cm::nullopt; } -std::vector cmPackageInfoReader::ParseVersion() const +cm::optional +cmPackageInfoReader::ParseVersion() const { // Check that we have a version. cm::optional const& version = this->GetVersion(); if (!version) { - return {}; + return cm::nullopt; } - std::vector result; - cm::string_view remnant{ *version }; - // Check if we know how to parse the version. Json::Value const& schema = this->Data["version_schema"]; if (schema.isNull() || cmStrCaseEq(schema.asString(), "simple"_s)) { - // Keep going until we run out of parts. - while (!remnant.empty()) { - std::string::size_type n = remnant.find('.'); - cm::string_view part = remnant.substr(0, n); - if (n == std::string::npos) { - remnant = {}; - } else { - remnant = remnant.substr(n + 1); - } - - unsigned long const value = std::stoul(std::string{ part }, &n); - if (n == 0 || value > std::numeric_limits::max()) { - // The part was not a valid number or is too big. - return {}; - } - result.push_back(static_cast(value)); - } + return ParseSimpleVersion(*version); } - return result; + return cm::nullopt; } std::vector cmPackageInfoReader::GetRequirements() const diff --git a/Source/cmPackageInfoReader.h b/Source/cmPackageInfoReader.h index d31cee9cb8..509a72e5de 100644 --- a/Source/cmPackageInfoReader.h +++ b/Source/cmPackageInfoReader.h @@ -44,10 +44,31 @@ public: std::string GetName() const; cm::optional GetVersion() const; + // NOTE: The eventual intent is for CPS to support multiple version schemas, + // and in particular, we expect to want to support "simple", "custom", "rpm", + // "dpkg" and "pep440". Additionally, we desire to be able to parse each of + // these to the maximum extent possible; in particular, we want to be able + // to decompose "simple" and "pep440" versions into components represented + // as numeric types rather than strings, which is not possible with the "rpm" + // and "dpkg" schemas. Therefore, we require different data structures to + // represent different version schemas. + + struct Pep440Version + { + // NOTE: This structure is currently incomplete as we only support the + // "simple" schema at this time. + bool Simple; // "simple" can be represented as a subset of "pep440" + std::vector ReleaseComponents; + cm::optional LocalLabel; + }; + + // FIXME: Return a sum type (e.g. {cm,std}::variant) of possible versions + // when we support more than just the "simple" (and possibly "pep440") + // schema(s). /// If the package uses the 'simple' version scheme, obtain the version as - /// a numeric tuple. Returns an empty vector for other schemes or if no - /// version is specified. - std::vector ParseVersion() const; + /// a numeric tuple and optional trailing string. Returns a disengaged + /// optional for other schemes or if no version is specified. + cm::optional ParseVersion() const; std::vector GetRequirements() const; std::vector GetComponentNames() const; diff --git a/Tests/FindPackageCpsTest/CMakeLists.txt b/Tests/FindPackageCpsTest/CMakeLists.txt index 58de268df0..dfb59e8583 100644 --- a/Tests/FindPackageCpsTest/CMakeLists.txt +++ b/Tests/FindPackageCpsTest/CMakeLists.txt @@ -21,24 +21,53 @@ set(CMAKE_FIND_FRAMEWORK FIRST) add_executable(FindPackageCpsTest FindPackageTest.cxx) +############################################################################### + +function(expect PACKAGE VAR OP VALUE WHAT) + if(NOT ${PACKAGE}_${VAR} ${OP} ${VALUE}) + message(SEND_ERROR "${PACKAGE} wrong ${WHAT} ${${PACKAGE}_${VAR}} !") + endif() +endfunction() + +function(test_version PACKAGE LITERAL COUNT MAJOR MINOR PATCH TWEAK) + if(NOT ${PACKAGE}_FOUND) + message(SEND_ERROR "${PACKAGE} not found !") + else() + expect(${PACKAGE} VERSION STREQUAL "${LITERAL}" "version") + expect(${PACKAGE} VERSION_COUNT EQUAL ${COUNT} "version count") + expect(${PACKAGE} VERSION_MAJOR EQUAL ${MAJOR} "major version") + expect(${PACKAGE} VERSION_MINOR EQUAL ${MINOR} "minor version") + expect(${PACKAGE} VERSION_PATCH EQUAL ${PATCH} "patch version") + expect(${PACKAGE} VERSION_TWEAK EQUAL ${TWEAK} "tweak version") + endif() +endfunction() + +function(test_unparsed_version PACKAGE VERSION) + find_package(${PACKAGE} CONFIG) + test_version(${PACKAGE} "${VERSION}" 0 0 0 0 0) +endfunction() + ############################################################################### # Test a basic package search. # It should NOT find the package's CMake package file. find_package(Sample CONFIG) -if(NOT Sample_FOUND) - message(SEND_ERROR "Sample not found !") -elseif(NOT Sample_VERSION STREQUAL "2.10.11") - message(SEND_ERROR "Sample wrong version ${Sample_VERSION} !") -elseif(NOT Sample_VERSION_MAJOR EQUAL 2) - message(SEND_ERROR "Sample wrong major version ${Sample_VERSION_MAJOR} !") -elseif(NOT Sample_VERSION_MINOR EQUAL 10) - message(SEND_ERROR "Sample wrong minor version ${Sample_VERSION_MINOR} !") -elseif(NOT Sample_VERSION_PATCH EQUAL 11) - message(SEND_ERROR "Sample wrong patch version ${Sample_VERSION_PATCH} !") -elseif(NOT Sample_VERSION_TWEAK EQUAL 0) - message(SEND_ERROR "Sample wrong tweak version ${Sample_VERSION_TWEAK} !") -endif() +test_version(Sample "2.10.11" 3 2 10 11 0) + +############################################################################### +# Test some more complicated version parsing. + +find_package(LongVersion CONFIG) +test_version(LongVersion "1.1.2.3.5.8+fibonacci" 6 1 1 2 3) + +find_package(EmptyMarker CONFIG) +test_version(EmptyMarker "1.1+" 2 1 1 0 0) + +test_unparsed_version(BadVersion1 "1..1") +test_unparsed_version(BadVersion2 "1.1a.0") +test_unparsed_version(BadVersion3 "1.1.") +test_unparsed_version(BadVersion4 "+42") +test_unparsed_version(CustomVersion "VII") ############################################################################### # Test glob sorting. diff --git a/Tests/FindPackageCpsTest/cps/badversion1.cps b/Tests/FindPackageCpsTest/cps/badversion1.cps new file mode 100644 index 0000000000..05acfa1cea --- /dev/null +++ b/Tests/FindPackageCpsTest/cps/badversion1.cps @@ -0,0 +1,8 @@ +{ + "cps_version": "0.13", + "name": "BadVersion1", + "version": "1..1", + "version_schema": "simple", + "cps_path": "@prefix@/cps", + "components": {} +} diff --git a/Tests/FindPackageCpsTest/cps/badversion2.cps b/Tests/FindPackageCpsTest/cps/badversion2.cps new file mode 100644 index 0000000000..99e683dbb9 --- /dev/null +++ b/Tests/FindPackageCpsTest/cps/badversion2.cps @@ -0,0 +1,8 @@ +{ + "cps_version": "0.13", + "name": "BadVersion2", + "version": "1.1a.0", + "version_schema": "simple", + "cps_path": "@prefix@/cps", + "components": {} +} diff --git a/Tests/FindPackageCpsTest/cps/badversion3.cps b/Tests/FindPackageCpsTest/cps/badversion3.cps new file mode 100644 index 0000000000..97e670ddac --- /dev/null +++ b/Tests/FindPackageCpsTest/cps/badversion3.cps @@ -0,0 +1,8 @@ +{ + "cps_version": "0.13", + "name": "BadVersion3", + "version": "1.1.", + "version_schema": "simple", + "cps_path": "@prefix@/cps", + "components": {} +} diff --git a/Tests/FindPackageCpsTest/cps/badversion4.cps b/Tests/FindPackageCpsTest/cps/badversion4.cps new file mode 100644 index 0000000000..2574866c77 --- /dev/null +++ b/Tests/FindPackageCpsTest/cps/badversion4.cps @@ -0,0 +1,8 @@ +{ + "cps_version": "0.13", + "name": "BadVersion4", + "version": "+42", + "version_schema": "simple", + "cps_path": "@prefix@/cps", + "components": {} +} diff --git a/Tests/FindPackageCpsTest/cps/customversion.cps b/Tests/FindPackageCpsTest/cps/customversion.cps new file mode 100644 index 0000000000..30aa29dd60 --- /dev/null +++ b/Tests/FindPackageCpsTest/cps/customversion.cps @@ -0,0 +1,8 @@ +{ + "cps_version": "0.13", + "name": "CustomVersion", + "version": "VII", + "version_schema": "roman", + "cps_path": "@prefix@/cps", + "components": {} +} diff --git a/Tests/FindPackageCpsTest/cps/emptymarker.cps b/Tests/FindPackageCpsTest/cps/emptymarker.cps new file mode 100644 index 0000000000..2099c92cc5 --- /dev/null +++ b/Tests/FindPackageCpsTest/cps/emptymarker.cps @@ -0,0 +1,8 @@ +{ + "cps_version": "0.13", + "name": "EmptyMarker", + "version": "1.1+", + "version_schema": "simple", + "cps_path": "@prefix@/cps", + "components": {} +} diff --git a/Tests/FindPackageCpsTest/cps/longversion.cps b/Tests/FindPackageCpsTest/cps/longversion.cps new file mode 100644 index 0000000000..300952d282 --- /dev/null +++ b/Tests/FindPackageCpsTest/cps/longversion.cps @@ -0,0 +1,8 @@ +{ + "cps_version": "0.13", + "name": "LongVersion", + "version": "1.1.2.3.5.8+fibonacci", + "version_schema": "simple", + "cps_path": "@prefix@/cps", + "components": {} +}