[vcpkg] Improve various versioning error messages (microsoft#7)

* [vcpkg] Improve various versioning error messages

With suggested fixes and additional help links

* Fix tests to accomodate new error messages

* Resolve MSVC shadowed variable warning

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>

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;

src/vcpkg-test/dependencies.cpp

@@ -985,6 +985,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;
@@ -1008,7 +1020,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")
     {
@@ -1024,7 +1050,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.)");
     }
 }
 

src/vcpkg/dependencies.cpp

@@ -1222,7 +1222,7 @@ namespace vcpkg::Dependencies
 
             void add_roots(View<Dependency> dep, const PackageSpec& toplevel);
 
-            ExpectedS<ActionPlan> finalize_extract_plan();
+            ExpectedS<ActionPlan> finalize_extract_plan(const PackageSpec& toplevel);
 
         private:
             const IVersionedPortfileProvider& m_ver_provider;
@@ -1243,6 +1243,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
@@ -1312,6 +1313,11 @@ namespace vcpkg::Dependencies
 
             Optional<Versions::Version> 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<std::string> m_errors;
         };
 
@@ -1367,6 +1373,7 @@ namespace vcpkg::Dependencies
                 // not implemented
                 Checks::unreachable(VCPKG_LINE_INFO);
             }
+            vsi->scheme = scheme;
             vermap.emplace(ver, vsi);
             return *vsi;
         }
@@ -1402,8 +1409,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;
         }
@@ -1654,6 +1661,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<Dependency> deps, const PackageSpec& toplevel)
         {
             auto dep_to_spec = [&toplevel, this](const Dependency& d) {
@@ -1750,9 +1770,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));
                     }
                 }
 
@@ -1767,7 +1785,38 @@ namespace vcpkg::Dependencies
             }
         }
 
-        ExpectedS<ActionPlan> 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<ActionPlan> VersionedPackageGraph::finalize_extract_plan(const PackageSpec& toplevel)
         {
             if (m_errors.size() > 0)
             {
@@ -1786,7 +1835,8 @@ namespace vcpkg::Dependencies
             std::vector<Frame> stack;
 
             auto push = [&emitted, this, &stack](const PackageSpec& spec,
-                                                 const Versions::Version& new_ver) -> Optional<std::string> {
+                                                 const Versions::Version& new_ver,
+                                                 const PackageSpec& origin) -> Optional<std::string> {
                 auto&& node = m_graph[spec];
                 auto overlay = m_o_provider.get_control_file(spec.name());
                 auto over_it = m_overrides.find(spec.name());
@@ -1799,7 +1849,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)
@@ -1814,12 +1873,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);
                                 }
                             }
                         }
@@ -1854,11 +1908,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);
                                 }
                             }
                         }
@@ -1872,24 +1922,24 @@ 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; }));
                     }
                     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;
                 }
             };
 
             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());
                 }
@@ -1908,7 +1958,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());
                         }
@@ -1932,6 +1982,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);
     }
 }

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"({

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)