From 5f4af89cff6f32b8f754a6bf920632a898bc868c Mon Sep 17 00:00:00 2001
From: Robert Schumacher <roschuma@microsoft.com>
Date: Mon, 19 Nov 2018 17:16:29 -0800
Subject: [PATCH] [vcpkg] Improve error messages while installing

---
 toolsrc/src/vcpkg/dependencies.cpp | 62 ++++++++++++++----------------
 toolsrc/src/vcpkg/tools.cpp        |  2 -
 2 files changed, 28 insertions(+), 36 deletions(-)

diff --git a/toolsrc/src/vcpkg/dependencies.cpp b/toolsrc/src/vcpkg/dependencies.cpp
index 8fb35b0da..8f2b4eaef 100644
--- a/toolsrc/src/vcpkg/dependencies.cpp
+++ b/toolsrc/src/vcpkg/dependencies.cpp
@@ -488,26 +488,24 @@ namespace vcpkg::Dependencies
         if (plus) return MarkPlusResult::SUCCESS;
         plus = true;
 
+        auto p_source = cluster.source.get();
+        if (!p_source)
+        {
+            Checks::exit_with_message(
+                VCPKG_LINE_INFO, "Error: Cannot find definition for package `%s`.", cluster.spec.name());
+        }
+
         if (feature.empty())
         {
             // Add default features for this package. This is an exact reference, so ignore prevent_default_features.
-            if (auto p_source = cluster.source.get())
+            for (auto&& default_feature : p_source->scf->core_paragraph.get()->default_features)
             {
-                for (auto&& default_feature : p_source->scf->core_paragraph.get()->default_features)
+                auto res = mark_plus(default_feature, cluster, graph, graph_plan, prevent_default_features);
+                if (res != MarkPlusResult::SUCCESS)
                 {
-                    auto res = mark_plus(default_feature, cluster, graph, graph_plan, prevent_default_features);
-                    if (res != MarkPlusResult::SUCCESS)
-                    {
-                        return res;
-                    }
+                    return res;
                 }
             }
-            else
-            {
-                Checks::exit_with_message(VCPKG_LINE_INFO,
-                                          "Error: Unable to install default features because can't find CONTROL for %s",
-                                          cluster.spec);
-            }
 
             // "core" is always required.
             return mark_plus("core", cluster, graph, graph_plan, prevent_default_features);
@@ -515,28 +513,20 @@ namespace vcpkg::Dependencies
 
         if (feature == "*")
         {
-            if (auto p_source = cluster.source.get())
+            for (auto&& fpgh : p_source->scf->feature_paragraphs)
             {
-                for (auto&& fpgh : p_source->scf->feature_paragraphs)
-                {
-                    auto res = mark_plus(fpgh->name, cluster, graph, graph_plan, prevent_default_features);
+                auto res = mark_plus(fpgh->name, cluster, graph, graph_plan, prevent_default_features);
 
-                    Checks::check_exit(VCPKG_LINE_INFO,
-                                       res == MarkPlusResult::SUCCESS,
-                                       "Error: Unable to locate feature %s in %s",
-                                       fpgh->name,
-                                       cluster.spec);
-                }
-
-                auto res = mark_plus("core", cluster, graph, graph_plan, prevent_default_features);
-
-                Checks::check_exit(VCPKG_LINE_INFO, res == MarkPlusResult::SUCCESS);
-            }
-            else
-            {
-                Checks::exit_with_message(
-                    VCPKG_LINE_INFO, "Error: Unable to handle '*' because can't find CONTROL for %s", cluster.spec);
+                Checks::check_exit(VCPKG_LINE_INFO,
+                                   res == MarkPlusResult::SUCCESS,
+                                   "Error: Internal error while installing feature %s in %s",
+                                   fpgh->name,
+                                   cluster.spec);
             }
+
+            auto res = mark_plus("core", cluster, graph, graph_plan, prevent_default_features);
+
+            Checks::check_exit(VCPKG_LINE_INFO, res == MarkPlusResult::SUCCESS);
             return MarkPlusResult::SUCCESS;
         }
 
@@ -643,7 +633,7 @@ namespace vcpkg::Dependencies
     }
 
     /// <summary>Figure out which actions are required to install features specifications in `specs`.</summary>
-    /// <param name="map">Map of all source files in the current environment.</param>
+    /// <param name="map">Map of all source control files in the current environment.</param>
     /// <param name="specs">Feature specifications to resolve dependencies for.</param>
     /// <param name="status_db">Status of installed packages in the current environment.</param>
     std::vector<AnyAction> create_feature_install_plan(const std::unordered_map<std::string, SourceControlFile>& map,
@@ -666,7 +656,11 @@ namespace vcpkg::Dependencies
 
         auto res = mark_plus(spec.feature(), spec_cluster, *m_graph, *m_graph_plan, prevent_default_features);
 
-        Checks::check_exit(VCPKG_LINE_INFO, res == MarkPlusResult::SUCCESS, "Error: Unable to locate feature %s", spec);
+        Checks::check_exit(VCPKG_LINE_INFO,
+                           res == MarkPlusResult::SUCCESS,
+                           "Error: `%s` is not a feature of package `%s`",
+                           spec.feature(),
+                           spec.name());
 
         m_graph_plan->install_graph.add_vertex(ClusterPtr{&spec_cluster});
     }
diff --git a/toolsrc/src/vcpkg/tools.cpp b/toolsrc/src/vcpkg/tools.cpp
index e28291428..f4ee2d653 100644
--- a/toolsrc/src/vcpkg/tools.cpp
+++ b/toolsrc/src/vcpkg/tools.cpp
@@ -193,7 +193,6 @@ namespace vcpkg
             System::println("Downloading %s...", tool_name);
             System::println("  %s -> %s", tool_data.url, tool_data.download_path.string());
             Downloads::download_file(fs, tool_data.url, tool_data.download_path, tool_data.sha512);
-            System::println("Downloading %s... done.", tool_name);
         }
         else
         {
@@ -204,7 +203,6 @@ namespace vcpkg
         {
             System::println("Extracting %s...", tool_name);
             Archives::extract_archive(paths, tool_data.download_path, tool_data.tool_dir_path);
-            System::println("Extracting %s... done.", tool_name);
         }
         else
         {