From 5d2ce4ad71e6ced3b6eae92a23bf4bb7d79f86d7 Mon Sep 17 00:00:00 2001 From: Dylan Baker Date: Thu, 3 Oct 2024 15:11:08 -0700 Subject: [PATCH 1/3] 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/3] 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/3] 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" + ] +}