[vcpkg] Feature packages now include user requested packages even if they are already installed.

4 files changed, 144 insertions, 41 deletions

diff --git a/toolsrc/include/vcpkg_Dependencies.h b/toolsrc/include/vcpkg_Dependencies.h
index e6c3c55c9..bfb452596 100644
--- a/toolsrc/include/vcpkg_Dependencies.h
+++ b/toolsrc/include/vcpkg_Dependencies.h
@@ -3,6 +3,7 @@
 #include "StatusParagraphs.h"
 #include "VcpkgPaths.h"
 #include "vcpkg_Graphs.h"
+#include "vcpkg_Util.h"
 #include "vcpkg_optional.h"
 #include <vector>
 
@@ -38,20 +39,21 @@ namespace vcpkg::Dependencies
         ALREADY_INSTALLED
     };
 
-    struct InstallPlanAction
+    struct InstallPlanAction : Util::MoveOnlyBase
     {
         static bool compare_by_name(const InstallPlanAction* left, const InstallPlanAction* right);
 
         InstallPlanAction();
+
+        InstallPlanAction::InstallPlanAction(const PackageSpec& spec,
+                                             const std::unordered_set<std::string>& features,
+                                             const RequestType& request_type);
         InstallPlanAction(const PackageSpec& spec, const AnyParagraph& any_paragraph, const RequestType& request_type);
         InstallPlanAction(const PackageSpec& spec,
                           const SourceControlFile& any_paragraph,
                           const std::unordered_set<std::string>& features,
                           const RequestType& request_type);
-        InstallPlanAction(const InstallPlanAction&) = delete;
-        InstallPlanAction(InstallPlanAction&&) = default;
-        InstallPlanAction& operator=(const InstallPlanAction&) = delete;
-        InstallPlanAction& operator=(InstallPlanAction&&) = default;
+
         std::string displayname() const;
 
         PackageSpec spec;
diff --git a/toolsrc/include/vcpkg_Util.h b/toolsrc/include/vcpkg_Util.h
index a62b48d54..da2f8eb69 100644
--- a/toolsrc/include/vcpkg_Util.h
+++ b/toolsrc/include/vcpkg_Util.h
@@ -62,4 +62,14 @@ namespace vcpkg::Util
             (*output)[key].push_back(&element);
         }
     }
+
+    struct MoveOnlyBase
+    {
+        MoveOnlyBase() = default;
+        MoveOnlyBase(const MoveOnlyBase&) = delete;
+        MoveOnlyBase(MoveOnlyBase&&) = default;
+
+        MoveOnlyBase& operator=(const MoveOnlyBase&) = delete;
+        MoveOnlyBase& operator=(MoveOnlyBase&&) = default;
+    };
 }
\ No newline at end of file
diff --git a/toolsrc/src/test_install_plan.cpp b/toolsrc/src/test_install_plan.cpp
index 939351872..d7a5014a9 100644
--- a/toolsrc/src/test_install_plan.cpp
+++ b/toolsrc/src/test_install_plan.cpp
@@ -6,6 +6,34 @@ using namespace Microsoft::VisualStudio::CppUnitTestFramework;
 
 using namespace vcpkg;
 
+namespace Microsoft::VisualStudio::CppUnitTestFramework
+{
+    template<>
+    inline std::wstring ToString<vcpkg::Dependencies::InstallPlanType>(const vcpkg::Dependencies::InstallPlanType& t)
+    {
+        switch (t)
+        {
+            case Dependencies::InstallPlanType::ALREADY_INSTALLED: return L"ALREADY_INSTALLED";
+            case Dependencies::InstallPlanType::BUILD_AND_INSTALL: return L"BUILD_AND_INSTALL";
+            case Dependencies::InstallPlanType::INSTALL: return L"INSTALL";
+            case Dependencies::InstallPlanType::UNKNOWN: return L"UNKNOWN";
+            default: return ToString((int)t);
+        }
+    }
+
+    template<>
+    inline std::wstring ToString<vcpkg::Dependencies::RequestType>(const vcpkg::Dependencies::RequestType& t)
+    {
+        switch (t)
+        {
+            case Dependencies::RequestType::AUTO_SELECTED: return L"AUTO_SELECTED";
+            case Dependencies::RequestType::USER_REQUESTED: return L"USER_REQUESTED";
+            case Dependencies::RequestType::UNKNOWN: return L"UNKNOWN";
+            default: return ToString((int)t);
+        }
+    }
+}
+
 namespace UnitTest1
 {
     class InstallPlanTests : public TestClass<InstallPlanTests>
@@ -127,6 +155,60 @@ namespace UnitTest1
             Assert::IsTrue(e_pos > g_pos);
         }
 
+        TEST_METHOD(existing_package_scheme)
+        {
+            using Pgh = std::unordered_map<std::string, std::string>;
+            std::vector<std::unique_ptr<StatusParagraph>> status_paragraphs;
+            status_paragraphs.push_back(std::make_unique<StatusParagraph>(Pgh{{"Package", "a"},
+                                                                              {"Version", "1"},
+                                                                              {"Architecture", "x86-windows"},
+                                                                              {"Multi-Arch", "same"},
+                                                                              {"Status", "install ok installed"}}));
+
+            PackageSpecMap spec_map(Triplet::X86_WINDOWS);
+            auto spec_a = FullPackageSpec{spec_map.set_package_map("a", "1", ""), {""}};
+
+            auto install_plan =
+                Dependencies::create_feature_install_plan(spec_map.map,
+                                                          FullPackageSpec::to_feature_specs({spec_a}),
+                                                          StatusParagraphs(std::move(status_paragraphs)));
+
+            Assert::AreEqual(size_t(1), install_plan.size());
+            auto p = install_plan[0].install_plan.get();
+            Assert::IsNotNull(p);
+            Assert::AreEqual("a", p->spec.name().c_str());
+            Assert::AreEqual(Dependencies::InstallPlanType::ALREADY_INSTALLED, p->plan_type);
+            Assert::AreEqual(Dependencies::RequestType::USER_REQUESTED, p->request_type);
+        }
+
+        TEST_METHOD(user_requested_package_scheme)
+        {
+            using Pgh = std::unordered_map<std::string, std::string>;
+            std::vector<std::unique_ptr<StatusParagraph>> status_paragraphs;
+
+            PackageSpecMap spec_map(Triplet::X86_WINDOWS);
+            auto spec_a = FullPackageSpec{spec_map.set_package_map("a", "1", "b"), {""}};
+            auto spec_b = FullPackageSpec{spec_map.set_package_map("b", "1", ""), {""}};
+
+            auto install_plan =
+                Dependencies::create_feature_install_plan(spec_map.map,
+                                                          FullPackageSpec::to_feature_specs({spec_a}),
+                                                          StatusParagraphs(std::move(status_paragraphs)));
+
+            Assert::AreEqual(size_t(2), install_plan.size());
+            auto p = install_plan[0].install_plan.get();
+            Assert::IsNotNull(p);
+            Assert::AreEqual("b", p->spec.name().c_str());
+            Assert::AreEqual(Dependencies::InstallPlanType::BUILD_AND_INSTALL, p->plan_type);
+            Assert::AreEqual(Dependencies::RequestType::AUTO_SELECTED, p->request_type);
+
+            auto p2 = install_plan[1].install_plan.get();
+            Assert::IsNotNull(p2);
+            Assert::AreEqual("a", p2->spec.name().c_str());
+            Assert::AreEqual(Dependencies::InstallPlanType::BUILD_AND_INSTALL, p2->plan_type);
+            Assert::AreEqual(Dependencies::RequestType::USER_REQUESTED, p2->request_type);
+        }
+
         TEST_METHOD(long_install_scheme)
         {
             using Pgh = std::unordered_map<std::string, std::string>;
diff --git a/toolsrc/src/vcpkg_Dependencies.cpp b/toolsrc/src/vcpkg_Dependencies.cpp
index abab359bb..8741e520f 100644
--- a/toolsrc/src/vcpkg_Dependencies.cpp
+++ b/toolsrc/src/vcpkg_Dependencies.cpp
@@ -29,6 +29,7 @@ namespace vcpkg::Dependencies
         std::unordered_set<std::string> original_features;
         bool will_remove = false;
         bool transient_uninstalled = true;
+        RequestType request_type = RequestType::AUTO_SELECTED;
         Cluster() = default;
 
     private:
@@ -39,6 +40,8 @@ namespace vcpkg::Dependencies
     struct ClusterPtr
     {
         Cluster* ptr;
+
+        Cluster* operator->() { return ptr; }
     };
 
     bool operator==(const ClusterPtr& l, const ClusterPtr& r) { return l.ptr == r.ptr; }
@@ -147,50 +150,44 @@ namespace vcpkg::Dependencies
         }
     }
 
-    InstallPlanAction::InstallPlanAction()
-        : spec(), any_paragraph(), plan_type(InstallPlanType::UNKNOWN), request_type(RequestType::UNKNOWN)
-    {
-    }
+    InstallPlanAction::InstallPlanAction() : plan_type(InstallPlanType::UNKNOWN), request_type(RequestType::UNKNOWN) {}
 
     InstallPlanAction::InstallPlanAction(const PackageSpec& spec,
                                          const SourceControlFile& any_paragraph,
                                          const std::unordered_set<std::string>& features,
                                          const RequestType& request_type)
-        : InstallPlanAction()
+        : spec(spec), plan_type(InstallPlanType::BUILD_AND_INSTALL), request_type(request_type), feature_list(features)
     {
-        this->spec = spec;
-        this->request_type = request_type;
-
-        this->plan_type = InstallPlanType::BUILD_AND_INSTALL;
         this->any_paragraph.source_control_file = &any_paragraph;
-        this->feature_list = features;
+    }
+
+    InstallPlanAction::InstallPlanAction(const PackageSpec& spec,
+                                         const std::unordered_set<std::string>& features,
+                                         const RequestType& request_type)
+        : spec(spec), plan_type(InstallPlanType::ALREADY_INSTALLED), request_type(request_type), feature_list(features)
+    {
     }
 
     InstallPlanAction::InstallPlanAction(const PackageSpec& spec,
                                          const AnyParagraph& any_paragraph,
                                          const RequestType& request_type)
-        : InstallPlanAction()
+        : spec(spec), request_type(request_type), any_paragraph(any_paragraph)
     {
-        this->spec = spec;
-        this->request_type = request_type;
         if (auto p = any_paragraph.status_paragraph.get())
         {
             this->plan_type = InstallPlanType::ALREADY_INSTALLED;
-            this->any_paragraph.status_paragraph = *p;
             return;
         }
 
         if (auto p = any_paragraph.binary_paragraph.get())
         {
             this->plan_type = InstallPlanType::INSTALL;
-            this->any_paragraph.binary_paragraph = *p;
             return;
         }
 
         if (auto p = any_paragraph.source_paragraph.get())
         {
             this->plan_type = InstallPlanType::BUILD_AND_INSTALL;
-            this->any_paragraph.source_paragraph = *p;
             return;
         }
 
@@ -594,7 +591,9 @@ namespace vcpkg::Dependencies
         for (auto&& spec : specs)
         {
             Cluster& spec_cluster = graph.get(spec.spec());
+            spec_cluster.request_type = RequestType::USER_REQUESTED;
             mark_plus(spec.feature(), spec_cluster, graph, graph_plan);
+            graph_plan.install_graph.add_vertex(ClusterPtr{&spec_cluster});
         }
 
         Graphs::GraphAdjacencyProvider<ClusterPtr> adjacency_remove_graph(graph_plan.remove_graph.adjacency_list());
@@ -605,36 +604,46 @@ namespace vcpkg::Dependencies
         auto insert_vertex_list = graph_plan.install_graph.vertex_list();
         auto insert_toposort = Graphs::topological_sort(insert_vertex_list, adjacency_install_graph);
 
-        std::vector<AnyAction> install_plan;
+        std::vector<AnyAction> plan;
 
-        for (auto&& like_cluster : remove_toposort)
+        for (auto&& p_cluster : remove_toposort)
         {
-            auto scf = *like_cluster.ptr->source_control_file.get();
-            auto spec = PackageSpec::from_name_and_triplet(scf->core_paragraph->name, like_cluster.ptr->spec.triplet())
+            auto scf = *p_cluster->source_control_file.get();
+            auto spec = PackageSpec::from_name_and_triplet(scf->core_paragraph->name, p_cluster->spec.triplet())
                             .value_or_exit(VCPKG_LINE_INFO);
-            install_plan.emplace_back(RemovePlanAction{
+            plan.emplace_back(RemovePlanAction{
                 std::move(spec),
                 RemovePlanType::REMOVE,
-                RequestType::AUTO_SELECTED,
+                p_cluster->request_type,
             });
         }
 
-        for (auto&& like_cluster : insert_toposort)
+        for (auto&& p_cluster : insert_toposort)
         {
-            if (!like_cluster.ptr->transient_uninstalled) continue;
-
-            auto scf = *like_cluster.ptr->source_control_file.get();
-            auto pkg_spec =
-                PackageSpec::from_name_and_triplet(scf->core_paragraph->name, like_cluster.ptr->spec.triplet())
-                    .value_or_exit(VCPKG_LINE_INFO);
-            install_plan.emplace_back(InstallPlanAction{
-                pkg_spec,
-                *scf,
-                like_cluster.ptr->to_install_features,
-                RequestType::AUTO_SELECTED,
-            });
+            if (p_cluster->transient_uninstalled)
+            {
+                // If it will be transiently uninstalled, we need to issue a full installation command
+                auto pscf = p_cluster->source_control_file.value_or_exit(VCPKG_LINE_INFO);
+                Checks::check_exit(VCPKG_LINE_INFO, pscf != nullptr);
+                plan.emplace_back(InstallPlanAction{
+                    p_cluster->spec,
+                    *pscf,
+                    p_cluster->to_install_features,
+                    p_cluster->request_type,
+                });
+            }
+            else
+            {
+                // If the package isn't transitively installed, still include it if the user explicitly requested it
+                if (p_cluster->request_type != RequestType::USER_REQUESTED) continue;
+                plan.emplace_back(InstallPlanAction{
+                    p_cluster->spec,
+                    p_cluster->original_features,
+                    p_cluster->request_type,
+                });
+            }
         }
 
-        return install_plan;
+        return plan;
     }
 }