From fd9814205217021c0fc6c69f626a488bf7bc7dfb Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Wed, 10 Feb 2021 10:23:50 +0000 Subject: [PATCH 1/3] [vcpkg] Improve various versioning error messages With suggested fixes and additional help links --- include/vcpkg/versions.h | 2 + src/vcpkg/dependencies.cpp | 98 ++++++++++++++++++++++++++++---------- src/vcpkg/help.cpp | 14 ++++++ src/vcpkg/versions.cpp | 24 ++++++++++ 4 files changed, 114 insertions(+), 24 deletions(-) diff --git a/include/vcpkg/versions.h b/include/vcpkg/versions.h index b26c90dfba..074dab0da1 100644 --- a/include/vcpkg/versions.h +++ b/include/vcpkg/versions.h @@ -22,6 +22,8 @@ namespace vcpkg::Versions String }; + void to_string(std::string& out, Scheme scheme); + struct VersionSpec { std::string port_name; diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 1c084846f6..5c8ea894a0 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -1190,7 +1190,7 @@ namespace vcpkg::Dependencies void add_roots(View dep, const PackageSpec& toplevel); - ExpectedS finalize_extract_plan(); + ExpectedS finalize_extract_plan(const PackageSpec& toplevel); private: const IVersionedPortfileProvider& m_ver_provider; @@ -1210,6 +1210,7 @@ namespace vcpkg::Dependencies // "relaxed" versions all share the same column struct VersionSchemeInfo { + Versions::Scheme scheme; const SourceControlFileLocation* scfl = nullptr; Versions::Version version; // This tracks a list of constraint sources for debugging purposes @@ -1258,6 +1259,11 @@ namespace vcpkg::Dependencies Optional dep_to_version(const std::string& name, const DependencyConstraint& dc); + static std::string format_incomparable_versions_message(const PackageSpec& on, + StringView from, + const VersionSchemeInfo& current, + const VersionSchemeInfo& target); + std::vector m_errors; }; @@ -1313,6 +1319,7 @@ namespace vcpkg::Dependencies // not implemented Checks::unreachable(VCPKG_LINE_INFO); } + vsi->scheme = scheme; vermap.emplace(ver, vsi); return *vsi; } @@ -1616,6 +1623,19 @@ namespace vcpkg::Dependencies m_overrides.emplace(name, v); } + static std::string format_missing_baseline_message(const std::string& onto, const PackageSpec& from) + { + return Strings::concat( + "Error: Cannot resolve a minimum constraint for dependency ", + onto, + " from ", + from, + ".\nThe dependency was not found in the baseline, indicating that the package did not " + "exist at that time. This may be fixed by providing an explicit override version via the " + "\"overrides\" field or by updating the baseline.\nSee `vcpkg help versioning` for more " + "information."); + } + void VersionedPackageGraph::add_roots(View deps, const PackageSpec& toplevel) { auto specs = Util::fmap(deps, [&toplevel](const Dependency& d) { @@ -1711,9 +1731,7 @@ namespace vcpkg::Dependencies } else { - m_errors.push_back(Strings::concat("Cannot resolve unversioned dependency from top-level to ", - dep.name, - " without a baseline entry or override.")); + m_errors.push_back(format_missing_baseline_message(dep.name, toplevel)); } } @@ -1728,7 +1746,38 @@ namespace vcpkg::Dependencies } } - ExpectedS VersionedPackageGraph::finalize_extract_plan() + std::string VersionedPackageGraph::format_incomparable_versions_message(const PackageSpec& on, + StringView from, + const VersionSchemeInfo& current, + const VersionSchemeInfo& target) + { + return Strings::concat( + "Error: Version conflict on ", + on, + ": ", + from, + " required ", + target.version, + " but vcpkg could not compare it to ", + current.version, + "\n\nThe two versions used incomparable schemes:\n \"", + current.version, + "\" was of scheme ", + current.scheme, + "\n \"", + target.version, + "\" was of scheme ", + target.scheme, + "\n\nThis can be resolved by adding an explicit override to the preferred version, for example:\n\n", + Strings::format(R"( "overrides": [ + { "name": "%s", "version": "%s" } + ])", + on.name(), + current.version), + "\n\nSee `vcpkg help versioning` for more information."); + } + + ExpectedS VersionedPackageGraph::finalize_extract_plan(const PackageSpec& toplevel) { if (m_errors.size() > 0) { @@ -1747,7 +1796,8 @@ namespace vcpkg::Dependencies std::vector stack; auto push = [&emitted, this, &stack](const PackageSpec& spec, - const Versions::Version& new_ver) -> Optional { + const Versions::Version& new_ver, + const PackageSpec& origin) -> Optional { auto&& node = m_graph[spec]; auto overlay = m_o_provider.get_control_file(spec.name()); auto over_it = m_overrides.find(spec.name()); @@ -1760,7 +1810,16 @@ namespace vcpkg::Dependencies else p_vnode = node.get_node(new_ver); - if (!p_vnode) return Strings::concat("Version was not found during discovery: ", spec, "@", new_ver); + if (!p_vnode) + { + return Strings::concat( + "Error: Version was not found during discovery: ", + spec, + "@", + new_ver, + "\nThis is an internal vcpkg error. Please open an issue on https://github.com/Microsoft/vcpkg " + "with detailed steps to reproduce the problem."); + } auto p = emitted.emplace(spec, nullptr); if (p.second) @@ -1775,12 +1834,7 @@ namespace vcpkg::Dependencies { if (base_node != p_vnode) { - return Strings::concat("Version conflict on ", - spec.name(), - "@", - new_ver, - ": baseline required ", - *baseline.get()); + return format_incomparable_versions_message(spec, "baseline", *p_vnode, *base_node); } } } @@ -1813,11 +1867,7 @@ namespace vcpkg::Dependencies } else { - return Strings::concat("Cannot resolve unconstrained dependency from ", - spec.name(), - " to ", - dep.name, - " without a baseline entry or override."); + return format_missing_baseline_message(dep.name, spec); } } } @@ -1831,7 +1881,7 @@ namespace vcpkg::Dependencies if (p.first->second == nullptr) { return Strings::concat( - "Cycle detected during ", + "Error: Cycle detected during ", spec, ":\n", Strings::join("\n", stack, [](const auto& p) -> const PackageSpec& { return p.ipa.spec; })); @@ -1839,8 +1889,8 @@ namespace vcpkg::Dependencies else if (p.first->second != p_vnode) { // comparable versions should retrieve the same info node - return Strings::concat( - "Version conflict on ", spec.name(), "@", p.first->second->version, ": required ", new_ver); + return format_incomparable_versions_message( + spec, origin.to_string(), *p_vnode, *p.first->second); } return nullopt; } @@ -1848,7 +1898,7 @@ namespace vcpkg::Dependencies for (auto&& root : m_roots) { - if (auto err = push(root.spec, root.ver)) + if (auto err = push(root.spec, root.ver, toplevel)) { return std::move(*err.get()); } @@ -1867,7 +1917,7 @@ namespace vcpkg::Dependencies { auto dep = std::move(back.deps.back()); back.deps.pop_back(); - if (auto err = push(dep.spec, dep.ver)) + if (auto err = push(dep.spec, dep.ver, back.ipa.spec)) { return std::move(*err.get()); } @@ -1890,6 +1940,6 @@ namespace vcpkg::Dependencies for (auto&& o : overrides) vpg.add_override(o.name, {o.version, o.port_version}); vpg.add_roots(deps, toplevel); - return vpg.finalize_extract_plan(); + return vpg.finalize_extract_plan(toplevel); } } diff --git a/src/vcpkg/help.cpp b/src/vcpkg/help.cpp index 0556b4fa06..63c8d1e0c6 100644 --- a/src/vcpkg/help.cpp +++ b/src/vcpkg/help.cpp @@ -90,6 +90,20 @@ namespace vcpkg::Help "be used to handle version conflicts, such as with `version-string` dependencies. They will not be " "considered when transitively depended upon."); tbl.blank(); + tbl.text( + "Vcpkg will select the minimum version found that matches all applicable constraints, including the " + "version from the baseline specified at top-level as well as any \"version>=\" constraints in the graph."); + tbl.blank(); + tbl.text("To keep your libraries up to date, the best approach is to update your baseline reference. This will " + "ensure all packages, including transitive ones, are updated. However if you need to update a package " + "independently, you can use a \"version>=\" constraint."); + tbl.blank(); + tbl.text( + "Additionally, package publishers can use \"version>=\" constraints to ensure that consumers are using at " + "least a certain minimum version of a given dependency. For example, if a library needs an API added " + "to boost-asio in 1.70, a \"version>=\" constraint will ensure transitive users use a sufficient version " + "even in the face of individual version overrides or cross-registry references."); + tbl.blank(); tbl.text("Example manifest:"); tbl.blank(); tbl.text(R"({ diff --git a/src/vcpkg/versions.cpp b/src/vcpkg/versions.cpp index 239c0e7285..00b441fa8a 100644 --- a/src/vcpkg/versions.cpp +++ b/src/vcpkg/versions.cpp @@ -139,6 +139,30 @@ namespace vcpkg::Versions return ret; } + void to_string(std::string& out, Scheme scheme) + { + if (scheme == Scheme::String) + { + out.append("string"); + } + else if (scheme == Scheme::Semver) + { + out.append("semver"); + } + else if (scheme == Scheme::Relaxed) + { + out.append("relaxed"); + } + else if (scheme == Scheme::Date) + { + out.append("date"); + } + else + { + Checks::unreachable(VCPKG_LINE_INFO); + } + } + VerComp compare(const std::string& a, const std::string& b, Scheme scheme) { if (scheme == Scheme::String) From 111398483532183b098462081f9cbf87c74d8eef Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Thu, 11 Feb 2021 07:15:57 +0000 Subject: [PATCH 2/3] Fix tests to accomodate new error messages --- src/vcpkg-test/dependencies.cpp | 44 +++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp index c6241ebe01..b8e27b6d1a 100644 --- a/src/vcpkg-test/dependencies.cpp +++ b/src/vcpkg-test/dependencies.cpp @@ -969,6 +969,18 @@ TEST_CASE ("version install diamond date", "[versionplan]") check_name_and_version(install_plan.install_actions[2], "a", {"2020-01-03", 0}); } +static void CHECK_LINES(const std::string& a, const std::string& b) +{ + auto as = Strings::split(a, '\n'); + auto bs = Strings::split(b, '\n'); + for (size_t i = 0; i < as.size() && i < bs.size(); ++i) + { + INFO(i); + CHECK(as[i] == bs[i]); + } + CHECK(as.size() == bs.size()); +} + TEST_CASE ("version install scheme failure", "[versionplan]") { MockVersionedPortfileProvider vp; @@ -992,7 +1004,21 @@ TEST_CASE ("version install scheme failure", "[versionplan]") toplevel_spec()); REQUIRE(!install_plan.error().empty()); - CHECK(install_plan.error() == "Version conflict on a@1.0.1: baseline required 1.0.0"); + CHECK_LINES( + install_plan.error(), + R"(Error: Version conflict on a:x86-windows: baseline required 1.0.0 but vcpkg could not compare it to 1.0.1 + +The two versions used incomparable schemes: + "1.0.1" was of scheme relaxed + "1.0.0" was of scheme semver + +This can be resolved by adding an explicit override to the preferred version, for example: + + "overrides": [ + { "name": "a", "version": "1.0.1" } + ] + +See `vcpkg help versioning` for more information.)"); } SECTION ("higher baseline") { @@ -1008,7 +1034,21 @@ TEST_CASE ("version install scheme failure", "[versionplan]") toplevel_spec()); REQUIRE(!install_plan.error().empty()); - CHECK(install_plan.error() == "Version conflict on a@1.0.1: baseline required 1.0.2"); + CHECK_LINES( + install_plan.error(), + R"(Error: Version conflict on a:x86-windows: baseline required 1.0.2 but vcpkg could not compare it to 1.0.1 + +The two versions used incomparable schemes: + "1.0.1" was of scheme relaxed + "1.0.2" was of scheme semver + +This can be resolved by adding an explicit override to the preferred version, for example: + + "overrides": [ + { "name": "a", "version": "1.0.1" } + ] + +See `vcpkg help versioning` for more information.)"); } } From fbaab6e36c715d5034daa19ee5e5c14cc259e876 Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Thu, 11 Feb 2021 07:35:46 +0000 Subject: [PATCH 3/3] Resolve MSVC shadowed variable warning --- src/vcpkg/dependencies.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 8ec8a97dd2..aa93ea2407 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -1376,8 +1376,8 @@ namespace vcpkg::Dependencies { Checks::check_exit(VCPKG_LINE_INFO, scfl); ASSUME(scfl != nullptr); - auto scheme = scfl->source_control_file->core_paragraph->version_scheme; - auto r = compare_versions(scheme, version, scheme, new_ver); + auto s = scfl->source_control_file->core_paragraph->version_scheme; + auto r = compare_versions(s, version, s, new_ver); Checks::check_exit(VCPKG_LINE_INFO, r != VerComp::unk); return r == VerComp::lt; }