From 5d2ce4ad71e6ced3b6eae92a23bf4bb7d79f86d7 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 3 Oct 2024 15:11:08 -0700 Subject: [PATCH 1/5] tests: Allow returns to be regular expressions Which is especially important for writing tests that may have absolute paths in them. --- tests/runner.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/runner.py b/tests/runner.py index 5e2f9e4..6834951 100755 --- a/tests/runner.py +++ b/tests/runner.py @@ -10,6 +10,7 @@ import dataclasses import enum import os +import re import shutil import sys import tempfile @@ -33,6 +34,7 @@ class TestCase(typing.TypedDict): expected: str mode: typing.NotRequired[typing.Literal['pkgconf']] returncode: typing.NotRequired[int] + re: typing.NotRequired[bool] class TestDescription(typing.TypedDict): @@ -64,14 +66,24 @@ class Result: def unordered_compare(out: str, expected: str) -> bool: - if out == expected: - return True - out_parts = out.split() expected_parts = expected.split() return sorted(out_parts) == sorted(expected_parts) +def is_success(rt: int, case_: TestCase, out: str, expected: str) -> bool: + if rt != case_.get('returncode', 0): + return False + + if case_.get('re', False): + return re.search(expected, out) is not None + + if out == expected: + return True + + return unordered_compare(out, expected) + + async def test(args: Arguments, case_: TestCase) -> Result: prefix = args.prefix or SOURCE_DIR @@ -93,7 +105,7 @@ async def test(args: Arguments, case_: TestCase) -> Result: out = bout.decode().strip() err = berr.decode().strip() - success = proc.returncode == case_.get('returncode', 0) and unordered_compare(out, expected) + success = is_success(proc.returncode, case_, out, expected) result = Status.PASS if success else Status.FAIL returncode = proc.returncode except asyncio.TimeoutError: From 2c84240ddced559b6444cd42d1db569bc16547b4 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 3 Oct 2024 12:39:53 -0700 Subject: [PATCH 2/5] version: implement to_string for Schema --- src/cps/version.cpp | 18 ++++++++++++++++-- src/cps/version.hpp | 2 ++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/cps/version.cpp b/src/cps/version.cpp index 7a2c52f..3ee0133 100644 --- a/src/cps/version.cpp +++ b/src/cps/version.cpp @@ -99,13 +99,27 @@ namespace cps::version { } // namespace + std::string to_string(const Schema schema) { + switch (schema) { + case Schema::simple: + return "simple"; + case Schema::dpkg: + return "dpkg"; + case Schema::rpm: + return "rpm"; + case Schema::custom: + return "custom"; + default: + abort(); + }; + } + tl::expected compare(std::string_view left, Operator op, std::string_view right, Schema schema) { switch (schema) { case Schema::simple: return simple_compare(left, op, right); default: - fmt::print(stderr, "Only the simple schema is implemented"); - return "Only the simple schema is implemented."; + return tl::unexpected{fmt::format("The {} schema is not implemented", to_string(schema))}; } } diff --git a/src/cps/version.hpp b/src/cps/version.hpp index d875d47..695adab 100644 --- a/src/cps/version.hpp +++ b/src/cps/version.hpp @@ -28,6 +28,8 @@ namespace cps::version { ge, }; + std::string to_string(const Schema); + /// @brief compare two version strings using the given operator and schema tl::expected compare(std::string_view left, Operator op, std::string_view right, Schema schema); } // namespace cps::version From 6757a7981d73a9d8bd5d13af127d5d5e20f6a0f2 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 3 Oct 2024 13:02:47 -0700 Subject: [PATCH 3/5] search: provide better error messages for why a package wasn't selected This provides messages that better explain why a cps file could not be found, whether there isn't one named in that path, or if there's a version mismatch, or dependency that is missing. --- src/cps/search.cpp | 19 ++++++++++++++++++- tests/cases/cps-config.toml | 3 ++- tests/cases/pkg-config-compat.toml | 3 ++- .../cps-files/lib/cps/has-compat-version.cps | 15 +++++++++++++++ 4 files changed, 37 insertions(+), 3 deletions(-) create mode 100644 tests/cps-files/lib/cps/has-compat-version.cps diff --git a/src/cps/search.cpp b/src/cps/search.cpp index d0ac7fe..a902710 100644 --- a/src/cps/search.cpp +++ b/src/cps/search.cpp @@ -11,6 +11,7 @@ #include "cps/version.hpp" #include +#include #include #include @@ -27,6 +28,8 @@ namespace cps::search { namespace { + using version::to_string; + /// @brief A CPS file, along with the components in that CPS file to /// load class Dependency { @@ -222,10 +225,12 @@ namespace cps::search { tl::expected, std::string> build_node(std::string_view name, const loader::Requirement & requirements, NodeFactory factory, Env env) { const std::vector paths = CPS_TRY(find_paths(name, env)); + std::vector errors{}; for (auto && path : paths) { auto maybe_node = factory.get(name, path); if (!maybe_node) { + errors.emplace_back(fmt::format("No CPS file for {} in path {}", name, path.string())); continue; } auto node = maybe_node.value(); @@ -242,16 +247,26 @@ namespace cps::search { // > If not provided, the CPS will not satisfy any request for // > a specific version of the package. if (!p.version) { + errors.emplace_back( + fmt::format("Tried {}, which does not specify a version, but the user requires version {}", + path.string(), requirements.version.value())); continue; } if (version::compare(p.version.value(), version::Operator::lt, requirements.version.value(), p.version_schema)) { + errors.emplace_back(fmt::format( + "{} has a version of {}, which is less than the required {}, using the schema {}", + path.string(), p.version.value(), requirements.version.value(), + to_string(p.version_schema))); continue; } } if (!std::all_of(requirements.components.begin(), requirements.components.end(), [p](const std::string & c) { return p.components.find(c) != p.components.end(); })) { + // TODO: more fine grained error message + errors.emplace_back(fmt::format("{} does not implement all of the required components '{}'", + path.string(), fmt::join(requirements.components, ", "))); continue; } @@ -259,9 +274,11 @@ namespace cps::search { found.reserve(p.require.size()); for (auto && [n, r] : p.require) { auto && child = build_node(n, r, factory, env); + if (child) { found.emplace_back(child.value()); } else { + errors.emplace_back(child.error()); break; } } @@ -273,7 +290,7 @@ namespace cps::search { return node; } - return tl::unexpected(fmt::format("Could not find a dependency to satisfy {}", name)); + return tl::unexpected(fmt::format("{}:\n {}", name, fmt::join(errors, "\n "))); } tl::expected, std::string> build_node(std::string_view name, diff --git a/tests/cases/cps-config.toml b/tests/cases/cps-config.toml index 5de331e..47a110f 100644 --- a/tests/cases/cps-config.toml +++ b/tests/cases/cps-config.toml @@ -82,7 +82,8 @@ expected = "-I/something -I/opt/include" name = "Requires version, but version not set" cps = "needs-version" args = ["flags", "--modversion", "--print-errors", "--errors-to-stdout"] -expected = "Could not find a dependency to satisfy needs-version" +expected = "Tried /.*/cps/multiple-components.cps, which does not specify a version, but the user requires version 1.0" +re = true returncode = 1 [[case]] diff --git a/tests/cases/pkg-config-compat.toml b/tests/cases/pkg-config-compat.toml index d1c87d1..a8d6ac6 100644 --- a/tests/cases/pkg-config-compat.toml +++ b/tests/cases/pkg-config-compat.toml @@ -50,5 +50,6 @@ expected = "-I/usr/local/include -I/opt/include" name = "Requires version, but version not set" cps = "needs-version" args = ["pkg-config", "--modversion", "--print-errors", "--errors-to-stdout"] -expected = "Could not find a dependency to satisfy needs-version" +expected = "Tried /.*/cps/multiple-components.cps, which does not specify a version, but the user requires version 1.0" returncode = 1 +re = true diff --git a/tests/cps-files/lib/cps/has-compat-version.cps b/tests/cps-files/lib/cps/has-compat-version.cps new file mode 100644 index 0000000..325460e --- /dev/null +++ b/tests/cps-files/lib/cps/has-compat-version.cps @@ -0,0 +1,15 @@ +{ + "name": "has-compat-version", + "cps_version": "0.12.0", + "version": "1.0.700344", + "compat_version": "1.0.0", + "components": { + "default": { + "type": "archive", + "location": "/usr/lib/libfoo.a" + } + }, + "default_components": [ + "default" + ] +} From 889542deff2d61634e0b6d83c33c5785b75a7dd5 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Fri, 4 Oct 2024 09:29:39 -0700 Subject: [PATCH 4/5] search: correctly handle an error in version::compare --- src/cps/search.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/cps/search.cpp b/src/cps/search.cpp index a902710..0b08c71 100644 --- a/src/cps/search.cpp +++ b/src/cps/search.cpp @@ -252,8 +252,13 @@ namespace cps::search { path.string(), requirements.version.value())); continue; } - if (version::compare(p.version.value(), version::Operator::lt, requirements.version.value(), - p.version_schema)) { + auto && v = version::compare(p.version.value(), version::Operator::lt, requirements.version.value(), + p.version_schema); + if (!v) { + errors.emplace_back(fmt::format("{}: {}", path.string(), v.error())); + } + + if (v.value()) { errors.emplace_back(fmt::format( "{} has a version of {}, which is less than the required {}, using the schema {}", path.string(), p.version.value(), requirements.version.value(), From 222f8b97b2419d781db71eb7ad3bc1dfaa8aa010 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Fri, 4 Oct 2024 09:34:50 -0700 Subject: [PATCH 5/5] loader: implement package::compat_version --- src/cps/loader.cpp | 2 ++ src/cps/loader.hpp | 2 +- src/cps/search.cpp | 27 +++++++++++++------ tests/cases/cps-config.toml | 8 +++++- tests/cases/pkg-config-compat.toml | 2 +- .../lib/cps/needs-compat-version.cps | 19 +++++++++++++ 6 files changed, 49 insertions(+), 11 deletions(-) create mode 100644 tests/cps-files/lib/cps/needs-compat-version.cps diff --git a/src/cps/loader.cpp b/src/cps/loader.cpp index 1306cd7..6b8e026 100644 --- a/src/cps/loader.cpp +++ b/src/cps/loader.cpp @@ -309,6 +309,7 @@ namespace cps::loader { auto const name = CPS_TRY(get_required(root, "package", "name")); auto const cps_version = CPS_TRY(get_required(root, "package", "cps_version")); + auto const compat_version = CPS_TRY(get_optional(root, "package", "compat_version")); auto const components = CPS_TRY(get_required(root, "package", "components")); auto const cps_path = CPS_TRY(get_optional(root, "package", "cps_path")); auto const default_components = @@ -330,6 +331,7 @@ namespace cps::loader { .name = std::move(name), .cps_version = std::move(cps_version), .components = std::move(components), + .compat_version = std::move(compat_version), .cps_path = std::move(cps_path), .filename = filename.string(), .default_components = std::move(default_components), diff --git a/src/cps/loader.hpp b/src/cps/loader.hpp index 5b68d4b..b70b039 100644 --- a/src/cps/loader.hpp +++ b/src/cps/loader.hpp @@ -128,7 +128,7 @@ namespace cps::loader { std::string name; std::string cps_version; std::unordered_map components; - // TODO: compat-version + std::optional compat_version; // TODO: configuration // TODO: configurations std::optional cps_path; diff --git a/src/cps/search.cpp b/src/cps/search.cpp index 0b08c71..f288183 100644 --- a/src/cps/search.cpp +++ b/src/cps/search.cpp @@ -237,7 +237,7 @@ namespace cps::search { const loader::Package & p = node->data.package; // If this package doesn't meet the requirements then reject it and continue on. - // The conditions it could fail to meet are: + // The conditions it couldIf we fail to meet are: // 1. the provided version (or Compat-Version) is < the required version // 2. This package lacks required components if (requirements.version) { @@ -246,14 +246,25 @@ namespace cps::search { // // > If not provided, the CPS will not satisfy any request for // > a specific version of the package. - if (!p.version) { - errors.emplace_back( - fmt::format("Tried {}, which does not specify a version, but the user requires version {}", - path.string(), requirements.version.value())); + if (!(p.version && p.compat_version)) { + errors.emplace_back(fmt::format("Tried {}, which does not specify a version or compat_version, " + "but the user requires version {}", + path.string(), requirements.version.value())); continue; } - auto && v = version::compare(p.version.value(), version::Operator::lt, requirements.version.value(), - p.version_schema); + // From the CPS spec, version 0.12.0, for package::compat_version + // + // > Specifies the oldest version of the package with which + // > this version is compatible. This information is used when + // > a consumer requests a specific version. If the version + // > requested is equal to or newer than the compat_version, + // > the package may be used. + // + // > If not specified, the package is not compatible with + // > previous versions (i.e. compat_version is implicitly + // > equal to version). + auto && v = version::compare(p.compat_version.value_or(p.version.value()), version::Operator::lt, + requirements.version.value(), p.version_schema); if (!v) { errors.emplace_back(fmt::format("{}: {}", path.string(), v.error())); } @@ -261,7 +272,7 @@ namespace cps::search { if (v.value()) { errors.emplace_back(fmt::format( "{} has a version of {}, which is less than the required {}, using the schema {}", - path.string(), p.version.value(), requirements.version.value(), + path.string(), p.compat_version.value_or(p.version.value()), requirements.version.value(), to_string(p.version_schema))); continue; } diff --git a/tests/cases/cps-config.toml b/tests/cases/cps-config.toml index 47a110f..660f0e0 100644 --- a/tests/cases/cps-config.toml +++ b/tests/cases/cps-config.toml @@ -82,7 +82,7 @@ expected = "-I/something -I/opt/include" name = "Requires version, but version not set" cps = "needs-version" args = ["flags", "--modversion", "--print-errors", "--errors-to-stdout"] -expected = "Tried /.*/cps/multiple-components.cps, which does not specify a version, but the user requires version 1.0" +expected = "Tried /.*/cps/multiple-components.cps, which does not specify a version or compat_version, but the user requires version 1.0" re = true returncode = 1 @@ -121,3 +121,9 @@ name = "Star components override by name" cps = "full" args = ["flags", "--component", "star_values_override", "--cflags", "--print-errors"] expected = "-fvectorize -I/usr/local/include -I/opt/include -DBAR=2 -DFOO=1 -DOTHER" + +[[case]] +name = "Sets compat_version" +cps = "needs-compat-version" +args = ["flags", "--libs", "--print-errors"] +expected = "-l/usr/lib/libfoo.a" diff --git a/tests/cases/pkg-config-compat.toml b/tests/cases/pkg-config-compat.toml index a8d6ac6..c920c14 100644 --- a/tests/cases/pkg-config-compat.toml +++ b/tests/cases/pkg-config-compat.toml @@ -50,6 +50,6 @@ expected = "-I/usr/local/include -I/opt/include" name = "Requires version, but version not set" cps = "needs-version" args = ["pkg-config", "--modversion", "--print-errors", "--errors-to-stdout"] -expected = "Tried /.*/cps/multiple-components.cps, which does not specify a version, but the user requires version 1.0" +expected = "Tried /.*/cps/multiple-components.cps, which does not specify a version or compat_version, but the user requires version 1.0" returncode = 1 re = true diff --git a/tests/cps-files/lib/cps/needs-compat-version.cps b/tests/cps-files/lib/cps/needs-compat-version.cps new file mode 100644 index 0000000..b431589 --- /dev/null +++ b/tests/cps-files/lib/cps/needs-compat-version.cps @@ -0,0 +1,19 @@ +{ + "name": "needs-components2", + "cps_version": "0.12.0", + "requires": { + "has-compat-version": { + "components": ["default"], + "version": "1.0.0" + } + }, + "components": { + "default": { + "type": "interface", + "requires": ["has-compat-version"] + } + }, + "default_components": [ + "default" + ] +}