[vcpkg] Refactor parsing together and flatten featurespec usages

10 files changed, 237 insertions, 138 deletions

diff --git a/toolsrc/include/PackageSpec.h b/toolsrc/include/PackageSpec.h
index 8e986b4d6..8b485316f 100644
--- a/toolsrc/include/PackageSpec.h
+++ b/toolsrc/include/PackageSpec.h
@@ -33,11 +33,31 @@ namespace vcpkg
         Triplet m_triplet;
     };
 
+    struct FeatureSpec
+    {
+        FeatureSpec(const PackageSpec& spec, const std::string& feature) : m_spec(spec), m_feature(feature) {}
+
+        const std::string& name() const { return m_spec.name(); }
+        const std::string& feature() const { return m_feature; }
+        const Triplet& triplet() const { return m_spec.triplet(); }
+
+        const PackageSpec& spec() const { return m_spec; }
+
+        static std::vector<FeatureSpec> from_strings_and_triplet(const std::vector<std::string>& depends,
+                                                                 const Triplet& t);
+
+    private:
+        PackageSpec m_spec;
+        std::string m_feature;
+    };
+
     struct FullPackageSpec
     {
         PackageSpec package_spec;
         std::vector<std::string> features;
 
+        static std::vector<FeatureSpec> to_feature_specs(const std::vector<FullPackageSpec>& specs);
+
         static ExpectedT<FullPackageSpec, PackageSpecParseResult> from_string(const std::string& spec_as_string,
                                                                               const Triplet& default_triplet);
     };
diff --git a/toolsrc/include/vcpkg_Dependencies.h b/toolsrc/include/vcpkg_Dependencies.h
index 3fee8ef33..37d3a59e5 100644
--- a/toolsrc/include/vcpkg_Dependencies.h
+++ b/toolsrc/include/vcpkg_Dependencies.h
@@ -30,19 +30,12 @@ namespace vcpkg::Dependencies
 
 namespace vcpkg::Dependencies
 {
-    struct FeatureSpec
-    {
-        PackageSpec spec;
-        std::string feature_name;
-    };
-
     struct FeatureNodeEdges
     {
         std::vector<FeatureSpec> remove_edges;
         std::vector<FeatureSpec> build_edges;
         bool plus = false;
     };
-    std::vector<FeatureSpec> to_feature_specs(const std::vector<std::string>& depends, const Triplet& t);
 
     struct Cluster
     {
@@ -210,6 +203,6 @@ namespace vcpkg::Dependencies
                            std::unordered_map<PackageSpec, Cluster>& pkg_to_cluster,
                            GraphPlan& graph_plan);
     std::vector<AnyAction> create_feature_install_plan(const std::unordered_map<PackageSpec, SourceControlFile>& map,
-                                                       const std::vector<FullPackageSpec>& specs,
+                                                       const std::vector<FeatureSpec>& specs,
                                                        const StatusParagraphs& status_db);
 }
diff --git a/toolsrc/src/PackageSpec.cpp b/toolsrc/src/PackageSpec.cpp
index 475b3e3f6..a5d40e998 100644
--- a/toolsrc/src/PackageSpec.cpp
+++ b/toolsrc/src/PackageSpec.cpp
@@ -13,6 +13,48 @@ namespace vcpkg
         return (c == '-') || isdigit(c) || (isalpha(c) && islower(c)) || (c == '[') || (c == ']');
     }
 
+    std::vector<FeatureSpec> FeatureSpec::from_strings_and_triplet(const std::vector<std::string>& depends,
+                                                                   const Triplet& triplet)
+    {
+        std::vector<FeatureSpec> f_specs;
+        for (auto&& depend : depends)
+        {
+            auto maybe_spec = ParsedSpecifier::from_string(depend);
+            if (auto spec = maybe_spec.get())
+            {
+                Checks::check_exit(VCPKG_LINE_INFO,
+                                   spec->triplet.empty(),
+                                   "error: triplets cannot currently be specified in this context: %s",
+                                   depend);
+                PackageSpec pspec =
+                    PackageSpec::from_name_and_triplet(spec->name, triplet).value_or_exit(VCPKG_LINE_INFO);
+
+                for (auto&& feature : spec->features)
+                    f_specs.push_back(FeatureSpec{pspec, feature});
+
+                if (spec->features.empty()) f_specs.push_back(FeatureSpec{pspec, ""});
+            }
+            else
+            {
+                Checks::exit_with_message(
+                    VCPKG_LINE_INFO, "error while parsing feature list: %s: %s", to_string(maybe_spec.error()), depend);
+            }
+        }
+        return f_specs;
+    }
+
+    std::vector<FeatureSpec> FullPackageSpec::to_feature_specs(const std::vector<FullPackageSpec>& specs)
+    {
+        std::vector<FeatureSpec> ret;
+        for (auto&& spec : specs)
+        {
+            ret.emplace_back(spec.package_spec, "");
+            for (auto&& feature : spec.features)
+                ret.emplace_back(spec.package_spec, feature);
+        }
+        return ret;
+    }
+
     ExpectedT<FullPackageSpec, PackageSpecParseResult> FullPackageSpec::from_string(const std::string& spec_as_string,
                                                                                     const Triplet& default_triplet)
     {
diff --git a/toolsrc/src/commands_install.cpp b/toolsrc/src/commands_install.cpp
index 2965d9025..3fc0e2563 100644
--- a/toolsrc/src/commands_install.cpp
+++ b/toolsrc/src/commands_install.cpp
@@ -14,10 +14,10 @@
 namespace vcpkg::Commands::Install
 {
     using Dependencies::InstallPlanAction;
-    using Dependencies::RequestType;
     using Dependencies::InstallPlanType;
     using Dependencies::RemovePlanAction;
     using Dependencies::RemovePlanType;
+    using Dependencies::RequestType;
 
     InstallDir InstallDir::from_destination_root(const fs::path& destination_root,
                                                  const std::string& destination_subdirectory,
@@ -440,8 +440,8 @@ namespace vcpkg::Commands::Install
                                     .value_or_exit(VCPKG_LINE_INFO);
                 scf_map[pkg_spec] = std::move(*port);
             }
-            std::vector<Dependencies::AnyAction> action_plan =
-                Dependencies::create_feature_install_plan(scf_map, full_specs, status_db);
+            std::vector<Dependencies::AnyAction> action_plan = Dependencies::create_feature_install_plan(
+                scf_map, FullPackageSpec::to_feature_specs(full_specs), status_db);
             // install plan will be empty if it is already installed - need to change this at status paragraph part
             Checks::check_exit(
                 VCPKG_LINE_INFO, !action_plan.empty(), "Install plan cannot be empty for feature packages");
diff --git a/toolsrc/src/test_install_plan.cpp b/toolsrc/src/test_install_plan.cpp
index 347998723..e7c62813e 100644
--- a/toolsrc/src/test_install_plan.cpp
+++ b/toolsrc/src/test_install_plan.cpp
@@ -214,8 +214,10 @@ namespace UnitTest1
                 {{"Feature", "beefeaturethree"}, {"Description", "the third feature for b"}, {"Build-Depends", ""}},
             })};
 
-            auto install_plan = Dependencies::create_feature_install_plan(
-                spec_map.map, {spec_a}, StatusParagraphs(std::move(status_paragraphs)));
+            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(4), install_plan.size());
             remove_plan_check(&install_plan[0], "a");
@@ -248,8 +250,10 @@ namespace UnitTest1
                 {{"Feature", "beefeaturethree"}, {"Description", "the third feature for b"}, {"Build-Depends", ""}},
             })};
 
-            auto install_plan = Dependencies::create_feature_install_plan(
-                spec_map.map, {spec_a}, StatusParagraphs(std::move(status_paragraphs)));
+            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());
             features_check(&install_plan[0], "b", {"beefeatureone", "beefeaturetwo", "core"});
@@ -284,8 +288,10 @@ namespace UnitTest1
                                           }),
                                           {"core"}};
 
-            auto install_plan = Dependencies::create_feature_install_plan(
-                spec_map.map, {spec_c, spec_a}, StatusParagraphs(std::move(status_paragraphs)));
+            auto install_plan =
+                Dependencies::create_feature_install_plan(spec_map.map,
+                                                          FullPackageSpec::to_feature_specs({spec_c, spec_a}),
+                                                          StatusParagraphs(std::move(status_paragraphs)));
 
             Assert::AreEqual(size_t(4), install_plan.size());
             remove_plan_check(&install_plan[0], "a");
@@ -328,8 +334,10 @@ namespace UnitTest1
                                           }),
                                           {"core"}};
 
-            auto install_plan = Dependencies::create_feature_install_plan(
-                spec_map.map, {spec_c}, StatusParagraphs(std::move(status_paragraphs)));
+            auto install_plan =
+                Dependencies::create_feature_install_plan(spec_map.map,
+                                                          FullPackageSpec::to_feature_specs({spec_c}),
+                                                          StatusParagraphs(std::move(status_paragraphs)));
 
             Assert::AreEqual(size_t(1), install_plan.size());
             features_check(&install_plan[0], "c", {"core"});
@@ -356,8 +364,10 @@ namespace UnitTest1
                 {{"Feature", "2"}, {"Description", "the second feature for b"}, {"Build-Depends", ""}},
             })};
 
-            auto install_plan = Dependencies::create_feature_install_plan(
-                spec_map.map, {spec_a}, StatusParagraphs(std::move(status_paragraphs)));
+            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());
             features_check(&install_plan[0], "b", {"core", "2"});
@@ -389,8 +399,10 @@ namespace UnitTest1
                 }),
                 {"1"}};
 
-            auto install_plan = Dependencies::create_feature_install_plan(
-                spec_map.map, {spec_a, spec_b}, StatusParagraphs(std::move(status_paragraphs)));
+            auto install_plan =
+                Dependencies::create_feature_install_plan(spec_map.map,
+                                                          FullPackageSpec::to_feature_specs({spec_a, spec_b}),
+                                                          StatusParagraphs(std::move(status_paragraphs)));
 
             Assert::AreEqual(size_t(3), install_plan.size());
             remove_plan_check(&install_plan[0], "b");
@@ -433,8 +445,10 @@ namespace UnitTest1
                 }),
                 {"1"}};
 
-            auto install_plan = Dependencies::create_feature_install_plan(
-                spec_map.map, {spec_b}, StatusParagraphs(std::move(status_paragraphs)));
+            auto install_plan =
+                Dependencies::create_feature_install_plan(spec_map.map,
+                                                          FullPackageSpec::to_feature_specs({spec_b}),
+                                                          StatusParagraphs(std::move(status_paragraphs)));
 
             Assert::AreEqual(size_t(5), install_plan.size());
             remove_plan_check(&install_plan[0], "x");
@@ -495,10 +509,10 @@ namespace UnitTest1
                                              }),
                                              {"core"}};
 
-            auto install_plan =
-                Dependencies::create_feature_install_plan(spec_map.map,
-                                                          {spec_c_64, spec_a_86, spec_a_64, spec_c_86},
-                                                          StatusParagraphs(std::move(status_paragraphs)));
+            auto install_plan = Dependencies::create_feature_install_plan(
+                spec_map.map,
+                FullPackageSpec::to_feature_specs({spec_c_64, spec_a_86, spec_a_64, spec_c_86}),
+                StatusParagraphs(std::move(status_paragraphs)));
 
             /*Assert::AreEqual(size_t(8), install_plan.size());
             auto iterator_pos = [&](const PackageSpec& spec, size_t start) -> int {
@@ -555,8 +569,10 @@ namespace UnitTest1
                 }),
                 {""}};
 
-            auto install_plan = Dependencies::create_feature_install_plan(
-                spec_map.map, {spec_a}, StatusParagraphs(std::move(status_paragraphs)));
+            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(3), install_plan.size());
             features_check(&install_plan[0], "c", {"core", "1", "2"});
diff --git a/toolsrc/src/tests_package_spec.cpp b/toolsrc/src/tests_package_spec.cpp
new file mode 100644
index 000000000..fa201b372
--- /dev/null
+++ b/toolsrc/src/tests_package_spec.cpp
@@ -0,0 +1,119 @@
+#include "BinaryParagraph.h"
+#include "CppUnitTest.h"
+#include "Paragraphs.h"
+#include "vcpkg_Strings.h"
+
+#pragma comment(lib, "version")
+#pragma comment(lib, "winhttp")
+
+using namespace Microsoft::VisualStudio::CppUnitTestFramework;
+
+namespace Microsoft::VisualStudio::CppUnitTestFramework
+{
+    template<>
+    inline std::wstring ToString<vcpkg::PackageSpecParseResult>(const vcpkg::PackageSpecParseResult& t)
+    {
+        return ToString(static_cast<uint32_t>(t));
+    }
+
+    template<>
+    inline std::wstring ToString<vcpkg::PackageSpec>(const vcpkg::PackageSpec& t)
+    {
+        return ToString(t.to_string());
+    }
+}
+
+namespace Strings = vcpkg::Strings;
+
+namespace UnitTest1
+{
+    using namespace vcpkg;
+
+    class SpecifierConversion : public TestClass<SpecifierConversion>
+    {
+        TEST_METHOD(full_package_spec_to_feature_specs)
+        {
+            auto a_spec = PackageSpec::from_name_and_triplet("a", Triplet::X64_WINDOWS).value_or_exit(VCPKG_LINE_INFO);
+            auto b_spec = PackageSpec::from_name_and_triplet("b", Triplet::X64_WINDOWS).value_or_exit(VCPKG_LINE_INFO);
+
+            auto fspecs = FullPackageSpec::to_feature_specs({{a_spec, {"0", "1"}}, {b_spec, {"2", "3"}}});
+
+            Assert::AreEqual(size_t(6), fspecs.size());
+
+            std::array<const char*, 6> features = {"", "0", "1", "", "2", "3"};
+            std::array<PackageSpec*, 6> specs = {&a_spec, &a_spec, &a_spec, &b_spec, &b_spec, &b_spec};
+
+            for (int i = 0; i < features.size(); ++i)
+            {
+                Assert::AreEqual(features[i], fspecs[i].feature().c_str());
+                Assert::AreEqual(*specs[i], fspecs[i].spec());
+            }
+        }
+    };
+
+    class SpecifierParsing : public TestClass<SpecifierParsing>
+    {
+        TEST_METHOD(parsed_specifier_from_string)
+        {
+            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib");
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
+            auto spec = maybe_spec.get();
+            Assert::AreEqual("zlib", spec->name.c_str());
+            Assert::AreEqual(size_t(0), spec->features.size());
+            Assert::AreEqual("", spec->triplet.c_str());
+        }
+
+        TEST_METHOD(parsed_specifier_from_string_with_triplet)
+        {
+            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib:x64-uwp");
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
+            auto spec = maybe_spec.get();
+            Assert::AreEqual("zlib", spec->name.c_str());
+            Assert::AreEqual("x64-uwp", spec->triplet.c_str());
+        }
+
+        TEST_METHOD(parsed_specifier_from_string_with_colons)
+        {
+            auto ec = vcpkg::ParsedSpecifier::from_string("zlib:x86-uwp:").error();
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::TOO_MANY_COLONS, ec);
+        }
+
+        TEST_METHOD(parsed_specifier_from_string_with_feature)
+        {
+            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib[feature]:x64-uwp");
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
+            auto spec = maybe_spec.get();
+            Assert::AreEqual("zlib", spec->name.c_str());
+            Assert::IsTrue(spec->features.size() == 1);
+            Assert::AreEqual("feature", spec->features.front().c_str());
+            Assert::AreEqual("x64-uwp", spec->triplet.c_str());
+        }
+
+        TEST_METHOD(parsed_specifier_from_string_with_many_features)
+        {
+            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib[0, 1,2]");
+            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
+            auto spec = maybe_spec.get();
+            Assert::AreEqual("zlib", spec->name.c_str());
+            Assert::IsTrue(spec->features.size() == 3);
+            Assert::AreEqual("0", spec->features[0].c_str());
+            Assert::AreEqual("1", spec->features[1].c_str());
+            Assert::AreEqual("2", spec->features[2].c_str());
+            Assert::AreEqual("", spec->triplet.c_str());
+        }
+
+        TEST_METHOD(utf8_to_utf16)
+        {
+            auto str = vcpkg::Strings::to_utf16("abc");
+            Assert::AreEqual(L"abc", str.c_str());
+        }
+
+        TEST_METHOD(utf8_to_utf16_with_whitespace)
+        {
+            auto str = vcpkg::Strings::to_utf16("abc -x86-windows");
+            Assert::AreEqual(L"abc -x86-windows", str.c_str());
+        }
+    };
+
+    TEST_CLASS(Metrics){};
+}
diff --git a/toolsrc/src/tests_paragraph.cpp b/toolsrc/src/tests_paragraph.cpp
index 1fd950e19..47a07e12d 100644
--- a/toolsrc/src/tests_paragraph.cpp
+++ b/toolsrc/src/tests_paragraph.cpp
@@ -384,68 +384,5 @@ namespace UnitTest1
             Assert::AreEqual(size_t(1), pghs.size());
             Assert::AreEqual("a, b, c", pghs[0]["Depends"].c_str());
         }
-
-        TEST_METHOD(parsed_specifier_from_string)
-        {
-            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib");
-            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
-            auto spec = maybe_spec.get();
-            Assert::AreEqual("zlib", spec->name.c_str());
-            Assert::AreEqual(size_t(0), spec->features.size());
-            Assert::AreEqual("", spec->triplet.c_str());
-        }
-
-        TEST_METHOD(parsed_specifier_from_string_with_triplet)
-        {
-            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib:x64-uwp");
-            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
-            auto spec = maybe_spec.get();
-            Assert::AreEqual("zlib", spec->name.c_str());
-            Assert::AreEqual("x64-uwp", spec->triplet.c_str());
-        }
-
-        TEST_METHOD(parsed_specifier_from_string_with_colons)
-        {
-            auto ec = vcpkg::ParsedSpecifier::from_string("zlib:x86-uwp:").error();
-            Assert::AreEqual(vcpkg::PackageSpecParseResult::TOO_MANY_COLONS, ec);
-        }
-
-        TEST_METHOD(parsed_specifier_from_string_with_feature)
-        {
-            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib[feature]:x64-uwp");
-            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
-            auto spec = maybe_spec.get();
-            Assert::AreEqual("zlib", spec->name.c_str());
-            Assert::IsTrue(spec->features.size() == 1);
-            Assert::AreEqual("feature", spec->features.front().c_str());
-            Assert::AreEqual("x64-uwp", spec->triplet.c_str());
-        }
-
-        TEST_METHOD(parsed_specifier_from_string_with_many_features)
-        {
-            auto maybe_spec = vcpkg::ParsedSpecifier::from_string("zlib[0, 1,2]");
-            Assert::AreEqual(vcpkg::PackageSpecParseResult::SUCCESS, maybe_spec.error());
-            auto spec = maybe_spec.get();
-            Assert::AreEqual("zlib", spec->name.c_str());
-            Assert::IsTrue(spec->features.size() == 3);
-            Assert::AreEqual("0", spec->features[0].c_str());
-            Assert::AreEqual("1", spec->features[1].c_str());
-            Assert::AreEqual("2", spec->features[2].c_str());
-            Assert::AreEqual("", spec->triplet.c_str());
-        }
-
-        TEST_METHOD(utf8_to_utf16)
-        {
-            auto str = vcpkg::Strings::to_utf16("abc");
-            Assert::AreEqual(L"abc", str.c_str());
-        }
-
-        TEST_METHOD(utf8_to_utf16_with_whitespace)
-        {
-            auto str = vcpkg::Strings::to_utf16("abc -x86-windows");
-            Assert::AreEqual(L"abc -x86-windows", str.c_str());
-        }
     };
-
-    TEST_CLASS(Metrics){};
 }
diff --git a/toolsrc/src/vcpkg_Dependencies.cpp b/toolsrc/src/vcpkg_Dependencies.cpp
index 799bca439..5226fac24 100644
--- a/toolsrc/src/vcpkg_Dependencies.cpp
+++ b/toolsrc/src/vcpkg_Dependencies.cpp
@@ -358,35 +358,6 @@ namespace vcpkg::Dependencies
         return toposort;
     }
 
-    std::vector<FeatureSpec> to_feature_specs(const std::vector<std::string>& depends, const Triplet& triplet)
-    {
-        std::vector<FeatureSpec> f_specs;
-        for (auto&& depend : depends)
-        {
-            auto maybe_spec = ParsedSpecifier::from_string(depend);
-            if (auto spec = maybe_spec.get())
-            {
-                Checks::check_exit(VCPKG_LINE_INFO,
-                                   spec->triplet.empty(),
-                                   "error: triplets cannot currently be specified in this context: %s",
-                                   depend);
-                PackageSpec pspec =
-                    PackageSpec::from_name_and_triplet(spec->name, triplet).value_or_exit(VCPKG_LINE_INFO);
-
-                for (auto&& feature : spec->features)
-                    f_specs.push_back(FeatureSpec{pspec, feature});
-
-                if (spec->features.empty()) f_specs.push_back(FeatureSpec{pspec, ""});
-            }
-            else
-            {
-                Checks::exit_with_message(
-                    VCPKG_LINE_INFO, "error while parsing feature list: %s: %s", to_string(maybe_spec.error()), depend);
-            }
-        }
-        return f_specs;
-    }
-
     void mark_plus_default(Cluster& cluster,
                            std::unordered_map<PackageSpec, Cluster>& pkg_to_cluster,
                            GraphPlan& graph_plan)
@@ -442,8 +413,8 @@ namespace vcpkg::Dependencies
 
         for (auto&& depend : cluster.edges[updated_feature].build_edges)
         {
-            auto& depend_cluster = pkg_to_cluster[depend.spec];
-            mark_plus(depend.feature_name, depend_cluster, pkg_to_cluster, graph_plan);
+            auto& depend_cluster = pkg_to_cluster[depend.spec()];
+            mark_plus(depend.feature(), depend_cluster, pkg_to_cluster, graph_plan);
             mark_plus_default(depend_cluster, pkg_to_cluster, graph_plan);
             if (&depend_cluster == &cluster) continue;
             graph_plan.install_graph.add_edge({&cluster}, {&depend_cluster});
@@ -462,7 +433,7 @@ namespace vcpkg::Dependencies
             auto& remove_edges_edges = pair.second.remove_edges;
             for (auto&& depend : remove_edges_edges)
             {
-                auto& depend_cluster = pkg_to_cluster[depend.spec];
+                auto& depend_cluster = pkg_to_cluster[depend.spec()];
                 graph_plan.remove_graph.add_edge({&cluster}, {&depend_cluster});
                 depend_cluster.transient_uninstalled = true;
                 mark_minus(depend_cluster, pkg_to_cluster, graph_plan);
@@ -476,7 +447,7 @@ namespace vcpkg::Dependencies
     }
 
     std::vector<AnyAction> create_feature_install_plan(const std::unordered_map<PackageSpec, SourceControlFile>& map,
-                                                       const std::vector<FullPackageSpec>& specs,
+                                                       const std::vector<FeatureSpec>& specs,
                                                        const StatusParagraphs& status_db)
     {
         std::unordered_map<PackageSpec, Cluster> pkg_spec_to_package_node;
@@ -488,14 +459,14 @@ namespace vcpkg::Dependencies
             node.spec = it.first;
             FeatureNodeEdges core_dependencies;
             auto core_depends = filter_dependencies(it.second.core_paragraph->depends, node.spec.triplet());
-            core_dependencies.build_edges = to_feature_specs(core_depends, node.spec.triplet());
+            core_dependencies.build_edges = FeatureSpec::from_strings_and_triplet(core_depends, node.spec.triplet());
             node.edges["core"] = std::move(core_dependencies);
 
             for (const auto& feature : it.second.feature_paragraphs)
             {
                 FeatureNodeEdges added_edges;
                 auto depends = filter_dependencies(feature->depends, node.spec.triplet());
-                added_edges.build_edges = to_feature_specs(depends, node.spec.triplet());
+                added_edges.build_edges = FeatureSpec::from_strings_and_triplet(depends, node.spec.triplet());
                 node.edges.emplace(feature->name, std::move(added_edges));
             }
             node.source_control_file = &it.second;
@@ -508,13 +479,13 @@ namespace vcpkg::Dependencies
             Cluster& cluster = pkg_spec_to_package_node[spec];
 
             cluster.transient_uninstalled = false;
-            auto reverse_edges =
-                to_feature_specs(status_paragraph->package.depends, status_paragraph->package.spec.triplet());
+            auto reverse_edges = FeatureSpec::from_strings_and_triplet(status_paragraph->package.depends,
+                                                                       status_paragraph->package.spec.triplet());
 
             for (auto&& dependency : reverse_edges)
             {
-                auto pkg_node = pkg_spec_to_package_node.find(dependency.spec);
-                auto depends_name = dependency.feature_name;
+                auto pkg_node = pkg_spec_to_package_node.find(dependency.spec());
+                auto depends_name = dependency.feature();
                 if (depends_name == "")
                 {
                     for (auto&& default_feature : status_paragraph->package.default_features)
@@ -541,12 +512,9 @@ namespace vcpkg::Dependencies
         GraphPlan graph_plan;
         for (auto&& spec : specs)
         {
-            Cluster& spec_cluster = pkg_spec_to_package_node[spec.package_spec];
+            Cluster& spec_cluster = pkg_spec_to_package_node[spec.spec()];
             mark_plus_default(spec_cluster, pkg_spec_to_package_node, graph_plan);
-            for (auto&& feature : spec.features)
-            {
-                mark_plus(feature, spec_cluster, pkg_spec_to_package_node, graph_plan);
-            }
+            mark_plus(spec.feature(), spec_cluster, pkg_spec_to_package_node, graph_plan);
         }
 
         Graphs::GraphAdjacencyProvider<ClusterPtr> adjacency_remove_graph(graph_plan.remove_graph.adjacency_list());
diff --git a/toolsrc/vcpkgtest/vcpkgtest.vcxproj b/toolsrc/vcpkgtest/vcpkgtest.vcxproj
index ca66260d4..041e74d31 100644
--- a/toolsrc/vcpkgtest/vcpkgtest.vcxproj
+++ b/toolsrc/vcpkgtest/vcpkgtest.vcxproj
@@ -21,6 +21,7 @@
   <ItemGroup>
     <ClCompile Include="..\src\tests_arguments.cpp" />
     <ClCompile Include="..\src\tests_dependencies.cpp" />
+    <ClCompile Include="..\src\tests_package_spec.cpp" />
     <ClCompile Include="..\src\tests_paragraph.cpp" />
     <ClCompile Include="..\src\test_install_plan.cpp" />
   </ItemGroup>
diff --git a/toolsrc/vcpkgtest/vcpkgtest.vcxproj.filters b/toolsrc/vcpkgtest/vcpkgtest.vcxproj.filters
index e376e59f4..b31dbd951 100644
--- a/toolsrc/vcpkgtest/vcpkgtest.vcxproj.filters
+++ b/toolsrc/vcpkgtest/vcpkgtest.vcxproj.filters
@@ -27,5 +27,8 @@
     <ClCompile Include="..\src\test_install_plan.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
+    <ClCompile Include="..\src\tests_package_spec.cpp">
+      <Filter>Source Files</Filter>
+    </ClCompile>
   </ItemGroup>
 </Project>
\ No newline at end of file