[vcpkg] Remove extra indirection in IVersionedPortfileProvider/IBaselineProvider (#15010)

* [vcpkg] Remove extra indirection in IVersionedPortfileProvider/IBaselineProvider

* [vcpkg] Address CR comment

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

3 files changed, 117 insertions, 153 deletions

diff --git a/toolsrc/include/vcpkg/portfileprovider.h b/toolsrc/include/vcpkg/portfileprovider.h
index 1ce934c9e..9dbd5ec51 100644
--- a/toolsrc/include/vcpkg/portfileprovider.h
+++ b/toolsrc/include/vcpkg/portfileprovider.h
@@ -41,6 +41,7 @@ namespace vcpkg::PortFileProvider
     struct IVersionedPortfileProvider
     {
         virtual const std::vector<vcpkg::Versions::VersionSpec>& get_port_versions(StringView port_name) const = 0;
+        virtual ~IVersionedPortfileProvider() = default;
 
         virtual ExpectedS<const SourceControlFileLocation&> get_control_file(
             const vcpkg::Versions::VersionSpec& version_spec) const = 0;
@@ -48,38 +49,12 @@ namespace vcpkg::PortFileProvider
 
     struct IBaselineProvider
     {
-        virtual Optional<VersionT> get_baseline_version(StringView port_name) const = 0;
-    };
-
-    namespace details
-    {
-        struct BaselineProviderImpl;
-        struct VersionedPortfileProviderImpl;
-    }
-
-    struct VersionedPortfileProvider : IVersionedPortfileProvider, Util::ResourceBase
-    {
-        explicit VersionedPortfileProvider(const vcpkg::VcpkgPaths& paths);
-        ~VersionedPortfileProvider();
-
-        const std::vector<vcpkg::Versions::VersionSpec>& get_port_versions(StringView port_name) const override;
-
-        ExpectedS<const SourceControlFileLocation&> get_control_file(
-            const vcpkg::Versions::VersionSpec& version_spec) const override;
+        virtual ~IBaselineProvider() = default;
 
-    private:
-        std::unique_ptr<details::VersionedPortfileProviderImpl> m_impl;
+        virtual Optional<VersionT> get_baseline_version(StringView port_name) const = 0;
     };
 
-    struct BaselineProvider : IBaselineProvider, Util::ResourceBase
-    {
-        explicit BaselineProvider(const vcpkg::VcpkgPaths& paths);
-        BaselineProvider(const vcpkg::VcpkgPaths& paths, const std::string& baseline);
-        ~BaselineProvider();
-
-        Optional<VersionT> get_baseline_version(StringView port_name) const override;
-
-    private:
-        std::unique_ptr<details::BaselineProviderImpl> m_impl;
-    };
+    std::unique_ptr<IBaselineProvider> make_baseline_provider(const vcpkg::VcpkgPaths& paths);
+    std::unique_ptr<IBaselineProvider> make_baseline_provider(const vcpkg::VcpkgPaths& paths, StringView baseline);
+    std::unique_ptr<IVersionedPortfileProvider> make_versioned_portfile_provider(const vcpkg::VcpkgPaths& paths);
 }
diff --git a/toolsrc/src/vcpkg/install.cpp b/toolsrc/src/vcpkg/install.cpp
index dd89a7567..a07fb431c 100644
--- a/toolsrc/src/vcpkg/install.cpp
+++ b/toolsrc/src/vcpkg/install.cpp
@@ -840,21 +840,20 @@ namespace vcpkg::Install
 
             if (args.versions_enabled())
             {
-                PortFileProvider::VersionedPortfileProvider verprovider(paths);
-                auto baseprovider = [&]() -> std::unique_ptr<PortFileProvider::BaselineProvider> {
+                auto verprovider = PortFileProvider::make_versioned_portfile_provider(paths);
+                auto baseprovider = [&]() -> std::unique_ptr<PortFileProvider::IBaselineProvider> {
                     if (auto p_baseline = manifest_scf.core_paragraph->extra_info.get("$x-default-baseline"))
                     {
-                        return std::make_unique<PortFileProvider::BaselineProvider>(paths,
-                                                                                    p_baseline->string().to_string());
+                        return PortFileProvider::make_baseline_provider(paths, p_baseline->string().to_string());
                     }
                     else
                     {
-                        return std::make_unique<PortFileProvider::BaselineProvider>(paths);
+                        return PortFileProvider::make_baseline_provider(paths);
                     }
                 }();
 
                 auto install_plan =
-                    Dependencies::create_versioned_install_plan(verprovider,
+                    Dependencies::create_versioned_install_plan(*verprovider,
                                                                 *baseprovider,
                                                                 var_provider,
                                                                 manifest_scf.core_paragraph->dependencies,
diff --git a/toolsrc/src/vcpkg/portfileprovider.cpp b/toolsrc/src/vcpkg/portfileprovider.cpp
index f22464fbe..edc527c24 100644
--- a/toolsrc/src/vcpkg/portfileprovider.cpp
+++ b/toolsrc/src/vcpkg/portfileprovider.cpp
@@ -292,16 +292,15 @@ namespace vcpkg::PortFileProvider
         return ret;
     }
 
-    namespace details
+    namespace
     {
-        struct BaselineProviderImpl
+        struct BaselineProviderImpl : IBaselineProvider, Util::ResourceBase
         {
             BaselineProviderImpl(const VcpkgPaths& paths) : paths(paths) { }
-            BaselineProviderImpl(const VcpkgPaths& paths, const std::string& baseline)
-                : paths(paths), m_baseline(baseline)
+            BaselineProviderImpl(const VcpkgPaths& paths, StringView baseline)
+                : paths(paths), m_baseline(baseline.to_string())
             {
             }
-            ~BaselineProviderImpl() { }
 
             const Optional<std::map<std::string, VersionT, std::less<>>>& get_baseline_cache() const
             {
@@ -348,7 +347,7 @@ namespace vcpkg::PortFileProvider
                 });
             }
 
-            Optional<VersionT> get_baseline_version(StringView port_name) const
+            virtual Optional<VersionT> get_baseline_version(StringView port_name) const override
             {
                 const auto& cache = get_baseline_cache();
                 if (auto p_cache = cache.get())
@@ -382,141 +381,132 @@ namespace vcpkg::PortFileProvider
             mutable std::unique_ptr<PathsPortFileProvider> m_portfile_provider;
         };
 
-        struct VersionedPortfileProviderImpl
+        struct VersionedPortfileProviderImpl : IVersionedPortfileProvider, Util::ResourceBase
         {
-            std::map<std::string, std::vector<VersionSpec>> versions_cache;
-            std::unordered_map<VersionSpec, std::string, VersionSpecHasher> git_tree_cache;
-            std::unordered_map<VersionSpec, SourceControlFileLocation, VersionSpecHasher> control_cache;
-
             VersionedPortfileProviderImpl(const VcpkgPaths& paths) : paths(paths) { }
-            ~VersionedPortfileProviderImpl() { }
-
-            const VcpkgPaths& get_paths() const { return paths; }
-            Files::Filesystem& get_filesystem() const { return paths.get_filesystem(); }
 
-        private:
-            const VcpkgPaths& paths;
-        };
-    }
+            virtual const std::vector<VersionSpec>& get_port_versions(StringView port_name) const override
+            {
+                auto cache_it = versions_cache.find(port_name.to_string());
+                if (cache_it != versions_cache.end())
+                {
+                    return cache_it->second;
+                }
 
-    VersionedPortfileProvider::VersionedPortfileProvider(const VcpkgPaths& paths)
-        : m_impl(std::make_unique<details::VersionedPortfileProviderImpl>(paths))
-    {
-    }
-    VersionedPortfileProvider::~VersionedPortfileProvider() { }
+                auto maybe_versions_file_path = get_versions_json_path(get_paths(), port_name);
+                if (auto versions_file_path = maybe_versions_file_path.get())
+                {
+                    auto maybe_version_entries = parse_versions_file(get_filesystem(), port_name, *versions_file_path);
+                    Checks::check_exit(VCPKG_LINE_INFO,
+                                       maybe_version_entries.has_value(),
+                                       "Error: Couldn't parse versions from file: %s",
+                                       fs::u8string(*versions_file_path));
+                    auto version_entries = maybe_version_entries.value_or_exit(VCPKG_LINE_INFO);
+
+                    auto port = port_name.to_string();
+                    for (auto&& version_entry : version_entries)
+                    {
+                        VersionSpec spec(port, version_entry.version);
+                        versions_cache[port].push_back(spec);
+                        git_tree_cache.emplace(std::move(spec), std::move(version_entry.git_tree));
+                    }
+                    return versions_cache.at(port);
+                }
+                else
+                {
+                    // Fall back to current available version
+                    auto maybe_port = try_load_registry_port(paths, port_name.to_string());
+                    if (auto p = maybe_port.get())
+                    {
+                        auto maybe_error = p->source_control_file->check_against_feature_flags(
+                            p->source_location, paths.get_feature_flags());
 
-    const std::vector<VersionSpec>& VersionedPortfileProvider::get_port_versions(StringView port_name) const
-    {
-        auto cache_it = m_impl->versions_cache.find(port_name.to_string());
-        if (cache_it != m_impl->versions_cache.end())
-        {
-            return cache_it->second;
-        }
+                        if (auto error = maybe_error.get())
+                        {
+                            Checks::exit_with_message(VCPKG_LINE_INFO, "Error: %s", *error);
+                        }
 
-        auto maybe_versions_file_path = get_versions_json_path(m_impl->get_paths(), port_name);
-        if (auto versions_file_path = maybe_versions_file_path.get())
-        {
-            auto maybe_version_entries = parse_versions_file(m_impl->get_filesystem(), port_name, *versions_file_path);
-            Checks::check_exit(VCPKG_LINE_INFO,
-                               maybe_version_entries.has_value(),
-                               "Error: Couldn't parse versions from file: %s",
-                               fs::u8string(*versions_file_path));
-            auto version_entries = maybe_version_entries.value_or_exit(VCPKG_LINE_INFO);
-
-            auto port = port_name.to_string();
-            for (auto&& version_entry : version_entries)
-            {
-                VersionSpec spec(port, version_entry.version);
-                m_impl->versions_cache[port].push_back(spec);
-                m_impl->git_tree_cache.emplace(std::move(spec), std::move(version_entry.git_tree));
+                        VersionSpec vspec(port_name.to_string(),
+                                          VersionT(p->source_control_file->core_paragraph->version,
+                                                   p->source_control_file->core_paragraph->port_version));
+                        control_cache.emplace(vspec, std::move(*p));
+                        return versions_cache.emplace(port_name.to_string(), std::vector<VersionSpec>{vspec})
+                            .first->second;
+                    }
+                    Checks::exit_with_message(
+                        VCPKG_LINE_INFO, "Error: Could not find a definition for port %s", port_name);
+                }
             }
-            return m_impl->versions_cache.at(port);
-        }
-        else
-        {
-            // Fall back to current available version
-            const auto& paths = m_impl->get_paths();
-            auto maybe_port = try_load_registry_port(paths, port_name.to_string());
-            if (auto p = maybe_port.get())
+
+            virtual ExpectedS<const SourceControlFileLocation&> get_control_file(
+                const VersionSpec& version_spec) const override
             {
-                auto maybe_error =
-                    p->source_control_file->check_against_feature_flags(p->source_location, paths.get_feature_flags());
+                // Pre-populate versions cache.
+                get_port_versions(version_spec.port_name);
 
-                if (auto error = maybe_error.get())
+                auto cache_it = control_cache.find(version_spec);
+                if (cache_it != control_cache.end())
                 {
-                    Checks::exit_with_message(VCPKG_LINE_INFO, "Error: %s", *error);
+                    return cache_it->second;
                 }
 
-                VersionSpec vspec(port_name.to_string(),
-                                  VersionT(p->source_control_file->core_paragraph->version,
-                                           p->source_control_file->core_paragraph->port_version));
-                m_impl->control_cache.emplace(vspec, std::move(*p));
-                return m_impl->versions_cache.emplace(port_name.to_string(), std::vector<VersionSpec>{vspec})
-                    .first->second;
-            }
-            Checks::exit_with_message(VCPKG_LINE_INFO, "Error: Could not find a definition for port %s", port_name);
-        }
-    }
-
-    ExpectedS<const SourceControlFileLocation&> VersionedPortfileProvider::get_control_file(
-        const VersionSpec& version_spec) const
-    {
-        // Pre-populate versions cache.
-        get_port_versions(version_spec.port_name);
-
-        auto cache_it = m_impl->control_cache.find(version_spec);
-        if (cache_it != m_impl->control_cache.end())
-        {
-            return cache_it->second;
-        }
+                auto git_tree_cache_it = git_tree_cache.find(version_spec);
+                if (git_tree_cache_it == git_tree_cache.end())
+                {
+                    return Strings::concat("Error: No git object SHA for entry ",
+                                           version_spec.port_name,
+                                           " at version ",
+                                           version_spec.version,
+                                           ".");
+                }
 
-        auto git_tree_cache_it = m_impl->git_tree_cache.find(version_spec);
-        if (git_tree_cache_it == m_impl->git_tree_cache.end())
-        {
-            return Strings::concat("Error: No git object SHA for entry ",
-                                   version_spec.port_name,
-                                   " at version ",
-                                   version_spec.version,
-                                   ".");
-        }
+                const std::string git_tree = git_tree_cache_it->second;
+                auto port_directory = get_paths().git_checkout_port(get_filesystem(), version_spec.port_name, git_tree);
 
-        const std::string git_tree = git_tree_cache_it->second;
-        auto port_directory =
-            m_impl->get_paths().git_checkout_port(m_impl->get_filesystem(), version_spec.port_name, git_tree);
+                auto maybe_control_file = Paragraphs::try_load_port(get_filesystem(), port_directory);
+                if (auto scf = maybe_control_file.get())
+                {
+                    if (scf->get()->core_paragraph->name == version_spec.port_name)
+                    {
+                        return control_cache
+                            .emplace(version_spec,
+                                     SourceControlFileLocation{std::move(*scf), std::move(port_directory)})
+                            .first->second;
+                    }
+                    return Strings::format("Error: Failed to load port from %s: names did not match: '%s' != '%s'",
+                                           fs::u8string(port_directory),
+                                           version_spec.port_name,
+                                           scf->get()->core_paragraph->name);
+                }
 
-        auto maybe_control_file = Paragraphs::try_load_port(m_impl->get_filesystem(), port_directory);
-        if (auto scf = maybe_control_file.get())
-        {
-            if (scf->get()->core_paragraph->name == version_spec.port_name)
-            {
-                return m_impl->control_cache
-                    .emplace(version_spec, SourceControlFileLocation{std::move(*scf), std::move(port_directory)})
-                    .first->second;
+                print_error_message(maybe_control_file.error());
+                return Strings::format(
+                    "Error: Failed to load port %s from %s", version_spec.port_name, fs::u8string(port_directory));
             }
-            return Strings::format("Error: Failed to load port from %s: names did not match: '%s' != '%s'",
-                                   fs::u8string(port_directory),
-                                   version_spec.port_name,
-                                   scf->get()->core_paragraph->name);
-        }
 
-        print_error_message(maybe_control_file.error());
-        return Strings::format(
-            "Error: Failed to load port %s from %s", version_spec.port_name, fs::u8string(port_directory));
+            const VcpkgPaths& get_paths() const { return paths; }
+            Files::Filesystem& get_filesystem() const { return paths.get_filesystem(); }
+
+        private:
+            const VcpkgPaths& paths;
+            mutable std::map<std::string, std::vector<VersionSpec>> versions_cache;
+            mutable std::unordered_map<VersionSpec, std::string, VersionSpecHasher> git_tree_cache;
+            mutable std::unordered_map<VersionSpec, SourceControlFileLocation, VersionSpecHasher> control_cache;
+        };
     }
 
-    BaselineProvider::BaselineProvider(const VcpkgPaths& paths)
-        : m_impl(std::make_unique<details::BaselineProviderImpl>(paths))
+    std::unique_ptr<IBaselineProvider> make_baseline_provider(const vcpkg::VcpkgPaths& paths)
     {
+        return std::make_unique<BaselineProviderImpl>(paths);
     }
 
-    BaselineProvider::BaselineProvider(const VcpkgPaths& paths, const std::string& baseline)
-        : m_impl(std::make_unique<details::BaselineProviderImpl>(paths, baseline))
+    std::unique_ptr<IBaselineProvider> make_baseline_provider(const vcpkg::VcpkgPaths& paths, StringView baseline)
     {
+        return std::make_unique<BaselineProviderImpl>(paths, baseline);
     }
-    BaselineProvider::~BaselineProvider() { }
 
-    Optional<VersionT> BaselineProvider::get_baseline_version(StringView port_name) const
+    std::unique_ptr<IVersionedPortfileProvider> make_versioned_portfile_provider(const vcpkg::VcpkgPaths& paths)
     {
-        return m_impl->get_baseline_version(port_name);
+        return std::make_unique<VersionedPortfileProviderImpl>(paths);
     }
 }