[vcpkg] Improve error messages when a feature is requested that doesn't exist.

3 files changed, 54 insertions, 17 deletions

diff --git a/toolsrc/src/PackageSpec.cpp b/toolsrc/src/PackageSpec.cpp
index a5d40e998..a43bc5ff5 100644
--- a/toolsrc/src/PackageSpec.cpp
+++ b/toolsrc/src/PackageSpec.cpp
@@ -13,6 +13,12 @@ namespace vcpkg
         return (c == '-') || isdigit(c) || (isalpha(c) && islower(c)) || (c == '[') || (c == ']');
     }
 
+    std::string FeatureSpec::to_string() const
+    {
+        if (feature().empty()) return spec().to_string();
+        return Strings::format("%s[%s]:%s", name(), feature(), triplet());
+    }
+
     std::vector<FeatureSpec> FeatureSpec::from_strings_and_triplet(const std::vector<std::string>& depends,
                                                                    const Triplet& triplet)
     {
@@ -36,8 +42,10 @@ namespace vcpkg
             }
             else
             {
-                Checks::exit_with_message(
-                    VCPKG_LINE_INFO, "error while parsing feature list: %s: %s", to_string(maybe_spec.error()), depend);
+                Checks::exit_with_message(VCPKG_LINE_INFO,
+                                          "error while parsing feature list: %s: %s",
+                                          vcpkg::to_string(maybe_spec.error()),
+                                          depend);
             }
         }
         return f_specs;
diff --git a/toolsrc/src/test_install_plan.cpp b/toolsrc/src/test_install_plan.cpp
index d7a5014a9..1c415f273 100644
--- a/toolsrc/src/test_install_plan.cpp
+++ b/toolsrc/src/test_install_plan.cpp
@@ -537,9 +537,9 @@ namespace UnitTest1
             remove_plan_check(&install_plan[1], "b");
 
             // TODO: order here may change but A < X, and B anywhere
-            features_check(&install_plan[2], "a", {"core"});
-            features_check(&install_plan[3], "x", {"core"});
-            features_check(&install_plan[4], "b", {"core", "1"});
+            features_check(&install_plan[2], "b", {"core", "1"});
+            features_check(&install_plan[3], "a", {"core"});
+            features_check(&install_plan[4], "x", {"core"});
         }
 
         TEST_METHOD(basic_feature_test_8)
diff --git a/toolsrc/src/vcpkg_Dependencies.cpp b/toolsrc/src/vcpkg_Dependencies.cpp
index 8741e520f..512e65e28 100644
--- a/toolsrc/src/vcpkg_Dependencies.cpp
+++ b/toolsrc/src/vcpkg_Dependencies.cpp
@@ -453,23 +453,30 @@ namespace vcpkg::Dependencies
         return toposort;
     }
 
-    void mark_plus(const std::string& feature, Cluster& cluster, ClusterGraph& pkg_to_cluster, GraphPlan& graph_plan);
+    enum class MarkPlusResult
+    {
+        FEATURE_NOT_FOUND,
+        SUCCESS,
+    };
+
+    MarkPlusResult mark_plus(const std::string& feature,
+                             Cluster& cluster,
+                             ClusterGraph& pkg_to_cluster,
+                             GraphPlan& graph_plan);
     void mark_minus(Cluster& cluster, ClusterGraph& pkg_to_cluster, GraphPlan& graph_plan);
 
-    void mark_plus(const std::string& feature, Cluster& cluster, ClusterGraph& graph, GraphPlan& graph_plan)
+    MarkPlusResult mark_plus(const std::string& feature, Cluster& cluster, ClusterGraph& graph, GraphPlan& graph_plan)
     {
         if (feature == "")
         {
             // Indicates that core was not specified in the reference
             return mark_plus("core", cluster, graph, graph_plan);
         }
+
         auto it = cluster.edges.find(feature);
-        if (it == cluster.edges.end())
-        {
-            Checks::unreachable(VCPKG_LINE_INFO);
-        }
+        if (it == cluster.edges.end()) return MarkPlusResult::FEATURE_NOT_FOUND;
 
-        if (cluster.edges[feature].plus) return;
+        if (cluster.edges[feature].plus) return MarkPlusResult::SUCCESS;
 
         if (cluster.original_features.find(feature) == cluster.original_features.end())
         {
@@ -478,7 +485,7 @@ namespace vcpkg::Dependencies
 
         if (!cluster.transient_uninstalled)
         {
-            return;
+            return MarkPlusResult::SUCCESS;
         }
         cluster.edges[feature].plus = true;
 
@@ -494,16 +501,28 @@ namespace vcpkg::Dependencies
         if (feature != "core")
         {
             // All features implicitly depend on core
-            mark_plus("core", cluster, graph, graph_plan);
+            auto res = mark_plus("core", cluster, graph, graph_plan);
+
+            // Should be impossible for "core" to not exist
+            Checks::check_exit(VCPKG_LINE_INFO, res == MarkPlusResult::SUCCESS);
         }
 
         for (auto&& depend : cluster.edges[feature].build_edges)
         {
             auto& depend_cluster = graph.get(depend.spec());
-            mark_plus(depend.feature(), depend_cluster, graph, graph_plan);
+            auto res = mark_plus(depend.feature(), depend_cluster, graph, graph_plan);
+
+            Checks::check_exit(VCPKG_LINE_INFO,
+                               res == MarkPlusResult::SUCCESS,
+                               "Error: Unable to satisfy dependency %s of %s",
+                               depend,
+                               FeatureSpec(cluster.spec, feature));
+
             if (&depend_cluster == &cluster) continue;
             graph_plan.install_graph.add_edge({&cluster}, {&depend_cluster});
         }
+
+        return MarkPlusResult::SUCCESS;
     }
 
     void mark_minus(Cluster& cluster, ClusterGraph& graph, GraphPlan& graph_plan)
@@ -526,7 +545,13 @@ namespace vcpkg::Dependencies
         cluster.transient_uninstalled = true;
         for (auto&& original_feature : cluster.original_features)
         {
-            mark_plus(original_feature, cluster, graph, graph_plan);
+            auto res = mark_plus(original_feature, cluster, graph, graph_plan);
+            if (res != MarkPlusResult::SUCCESS)
+            {
+                System::println(System::Color::warning,
+                                "Warning: could not reinstall feature %s",
+                                FeatureSpec{cluster.spec, original_feature});
+            }
         }
     }
 
@@ -592,7 +617,11 @@ namespace vcpkg::Dependencies
         {
             Cluster& spec_cluster = graph.get(spec.spec());
             spec_cluster.request_type = RequestType::USER_REQUESTED;
-            mark_plus(spec.feature(), spec_cluster, graph, graph_plan);
+            auto res = mark_plus(spec.feature(), spec_cluster, graph, graph_plan);
+
+            Checks::check_exit(
+                VCPKG_LINE_INFO, res == MarkPlusResult::SUCCESS, "Error: Unable to locate feature %s", spec);
+
             graph_plan.install_graph.add_vertex(ClusterPtr{&spec_cluster});
         }