From df17c63b1120b8ed5a804a8b388be63e8939cd24 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 21 Oct 2024 17:17:55 -0700 Subject: [PATCH] Move merging original_cwd into overlay directories into VcpkgPaths. This work is part of resolving https://github.com/microsoft/vcpkg/issues/30942 / https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1979597 When overlay directories from the config were added in https://github.com/microsoft/vcpkg-tool/pull/743 this added a condition where VcpkgPaths took responsibility to tack on the relative path, transforming those paths that come from the config into absolute paths. I did not realize at the time that this block was repeating machinery already present in IOverlayProvider family. The difference is that, there, it always assumed the prefix would be original_cwd. As part of adding overlay-port-dirs, I needed to add the same kind of prefix handling as overlay-ports get, thus pointing this out to me. It doesn't make sense to keep two independent ways to do this, leaving two options: * Move the prefix stapling into VcpkgPaths (as done in this PR) * Move the prefix stapling related to configs down into OverlayPortProviderImpl I chose to move into VcpkgPaths for several reasons: * Plumbing the information about how to handle config paths correctly into OverlayPortProviderImpl would have added many many function parameters to large parts of the product that are only passing paths information around. * The decision to chdir in VcpkgPaths is a consideration *it* makes, and OverlayPortProviderImpl would be just as happy if the relative paths were left alone. (And in fact I think the right behavior in the future would be to remove this harmful chdir but that is not proposed right now) * Despite making VcpkgPaths do "more work" here, I actually argue that it makes the "VcpkgPaths big ball of mud" problem better, by removing parts of the code that need to consider original_cwd. --- include/vcpkg/commands.find.h | 3 +- include/vcpkg/portfileprovider.h | 7 +-- include/vcpkg/vcpkgpaths.h | 4 +- src/vcpkg/commands.build-external.cpp | 2 +- src/vcpkg/commands.build.cpp | 3 +- src/vcpkg/commands.check-support.cpp | 3 +- src/vcpkg/commands.ci.cpp | 3 +- src/vcpkg/commands.depend-info.cpp | 3 +- src/vcpkg/commands.env.cpp | 3 +- src/vcpkg/commands.export.cpp | 3 +- src/vcpkg/commands.find.cpp | 4 +- src/vcpkg/commands.install.cpp | 11 ++-- src/vcpkg/commands.package-info.cpp | 3 +- src/vcpkg/commands.remove.cpp | 3 +- src/vcpkg/commands.set-installed.cpp | 3 +- src/vcpkg/commands.update.cpp | 3 +- src/vcpkg/commands.upgrade.cpp | 3 +- src/vcpkg/portfileprovider.cpp | 23 +++---- src/vcpkg/vcpkgpaths.cpp | 89 ++++++++++++++------------- 19 files changed, 80 insertions(+), 96 deletions(-) diff --git a/include/vcpkg/commands.find.h b/include/vcpkg/commands.find.h index 9b85cddaf1..f363f60ab0 100644 --- a/include/vcpkg/commands.find.h +++ b/include/vcpkg/commands.find.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -14,7 +15,7 @@ namespace vcpkg bool full_description, bool enable_json, Optional filter, - View overlay_ports); + View overlay_ports); extern const CommandMetadata CommandFindMetadata; void command_find_and_exit(const VcpkgCmdArguments& args, const VcpkgPaths& paths); } diff --git a/include/vcpkg/portfileprovider.h b/include/vcpkg/portfileprovider.h index d8ce691f2e..25a87ebff7 100644 --- a/include/vcpkg/portfileprovider.h +++ b/include/vcpkg/portfileprovider.h @@ -76,12 +76,9 @@ namespace vcpkg std::unique_ptr make_baseline_provider(const RegistrySet& registry_set); std::unique_ptr make_versioned_portfile_provider(const RegistrySet& registry_set); - std::unique_ptr make_overlay_provider(const ReadOnlyFilesystem& fs, - const Path& original_cwd, - View overlay_ports); + std::unique_ptr make_overlay_provider(const ReadOnlyFilesystem& fs, View overlay_ports); std::unique_ptr make_manifest_provider(const ReadOnlyFilesystem& fs, - const Path& original_cwd, - View overlay_ports, + View overlay_ports, const Path& manifest_path, std::unique_ptr&& manifest_scf); } diff --git a/include/vcpkg/vcpkgpaths.h b/include/vcpkg/vcpkgpaths.h index 90fd4d093c..0639a5b8cc 100644 --- a/include/vcpkg/vcpkgpaths.h +++ b/include/vcpkg/vcpkgpaths.h @@ -98,10 +98,10 @@ namespace vcpkg private: const Path triplets; const Path community_triplets; - std::vector overlay_triplets; + std::vector overlay_triplets; public: - std::vector overlay_ports; + std::vector overlay_ports; std::string get_toolver_diagnostics() const; diff --git a/src/vcpkg/commands.build-external.cpp b/src/vcpkg/commands.build-external.cpp index 9ef71a66a6..8637ed7cdb 100644 --- a/src/vcpkg/commands.build-external.cpp +++ b/src/vcpkg/commands.build-external.cpp @@ -48,7 +48,7 @@ namespace vcpkg auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.original_cwd, overlays)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, overlays)); command_build_and_exit_ex(args, paths, host_triplet, build_options, spec, provider, null_build_logs_recorder()); } } diff --git a/src/vcpkg/commands.build.cpp b/src/vcpkg/commands.build.cpp index d1a8a8d494..74a16c021a 100644 --- a/src/vcpkg/commands.build.cpp +++ b/src/vcpkg/commands.build.cpp @@ -107,8 +107,7 @@ namespace vcpkg auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); Checks::exit_with_code(VCPKG_LINE_INFO, command_build_ex(args, paths, diff --git a/src/vcpkg/commands.check-support.cpp b/src/vcpkg/commands.check-support.cpp index 2d76b6a2f5..5207203c6c 100644 --- a/src/vcpkg/commands.check-support.cpp +++ b/src/vcpkg/commands.check-support.cpp @@ -128,8 +128,7 @@ namespace vcpkg auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); auto cmake_vars = CMakeVars::make_triplet_cmake_var_provider(paths); // for each spec in the user-requested specs, check all dependencies diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 115964d7ca..71d4761255 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -376,8 +376,7 @@ namespace vcpkg build_logs_recorder_storage ? *(build_logs_recorder_storage.get()) : null_build_logs_recorder(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(filesystem, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(filesystem, paths.overlay_ports)); auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths); auto& var_provider = *var_provider_storage; diff --git a/src/vcpkg/commands.depend-info.cpp b/src/vcpkg/commands.depend-info.cpp index d89fc4bd8c..85c57a785b 100644 --- a/src/vcpkg/commands.depend-info.cpp +++ b/src/vcpkg/commands.depend-info.cpp @@ -409,8 +409,7 @@ namespace vcpkg auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths); auto& var_provider = *var_provider_storage; diff --git a/src/vcpkg/commands.env.cpp b/src/vcpkg/commands.env.cpp index 92385069cf..7da88ca5e0 100644 --- a/src/vcpkg/commands.env.cpp +++ b/src/vcpkg/commands.env.cpp @@ -80,8 +80,7 @@ namespace vcpkg const ParsedArguments options = args.parse_arguments(CommandEnvMetadata); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths); auto& var_provider = *var_provider_storage; diff --git a/src/vcpkg/commands.export.cpp b/src/vcpkg/commands.export.cpp index a8a9a26330..fe8a08a2ec 100644 --- a/src/vcpkg/commands.export.cpp +++ b/src/vcpkg/commands.export.cpp @@ -603,8 +603,7 @@ namespace vcpkg // Load ports from ports dirs auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); // create the plan std::vector export_plan = create_export_plan(opts.specs, status_db); diff --git a/src/vcpkg/commands.find.cpp b/src/vcpkg/commands.find.cpp index 6e286e1ed9..d40a307193 100644 --- a/src/vcpkg/commands.find.cpp +++ b/src/vcpkg/commands.find.cpp @@ -133,12 +133,12 @@ namespace vcpkg bool full_description, bool enable_json, Optional filter, - View overlay_ports) + View overlay_ports) { Checks::check_exit(VCPKG_LINE_INFO, msg::default_output_stream == OutputStream::StdErr); auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.original_cwd, overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, overlay_ports)); auto source_paragraphs = Util::fmap(provider.load_all_control_files(), [](auto&& port) -> const SourceControlFile* { return port->source_control_file.get(); }); diff --git a/src/vcpkg/commands.install.cpp b/src/vcpkg/commands.install.cpp index 2087e62835..82ca11a748 100644 --- a/src/vcpkg/commands.install.cpp +++ b/src/vcpkg/commands.install.cpp @@ -1240,16 +1240,16 @@ namespace vcpkg auto verprovider = make_versioned_portfile_provider(*registry_set); auto baseprovider = make_baseline_provider(*registry_set); - std::vector extended_overlay_ports; + std::vector extended_overlay_ports; extended_overlay_ports.reserve(paths.overlay_ports.size() + add_builtin_ports_directory_as_overlay); extended_overlay_ports = paths.overlay_ports; if (add_builtin_ports_directory_as_overlay) { - extended_overlay_ports.emplace_back(paths.builtin_ports_directory().native()); + extended_overlay_ports.emplace_back(paths.builtin_ports_directory()); } - auto oprovider = make_manifest_provider( - fs, paths.original_cwd, extended_overlay_ports, manifest->path, std::move(manifest_scf)); + auto oprovider = + make_manifest_provider(fs, extended_overlay_ports, manifest->path, std::move(manifest_scf)); auto install_plan = create_versioned_install_plan(*verprovider, *baseprovider, *oprovider, @@ -1278,8 +1278,7 @@ namespace vcpkg } auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); const std::vector specs = Util::fmap(options.command_arguments, [&](const std::string& arg) { return check_and_get_full_package_spec(arg, default_triplet, paths.get_triplet_db()) diff --git a/src/vcpkg/commands.package-info.cpp b/src/vcpkg/commands.package-info.cpp index 70608eb521..bd94a6235a 100644 --- a/src/vcpkg/commands.package-info.cpp +++ b/src/vcpkg/commands.package-info.cpp @@ -102,8 +102,7 @@ namespace vcpkg Json::Object response; Json::Object results; auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); for (auto&& arg : options.command_arguments) { diff --git a/src/vcpkg/commands.remove.cpp b/src/vcpkg/commands.remove.cpp index 9b53b9cd1b..7e80b73034 100644 --- a/src/vcpkg/commands.remove.cpp +++ b/src/vcpkg/commands.remove.cpp @@ -196,8 +196,7 @@ namespace vcpkg // Load ports from ports dirs auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); specs = Util::fmap(find_outdated_packages(provider, status_db), [](auto&& outdated) { return outdated.spec; }); diff --git a/src/vcpkg/commands.set-installed.cpp b/src/vcpkg/commands.set-installed.cpp index 717858d3c4..75439b1998 100644 --- a/src/vcpkg/commands.set-installed.cpp +++ b/src/vcpkg/commands.set-installed.cpp @@ -314,8 +314,7 @@ namespace vcpkg auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); auto cmake_vars = CMakeVars::make_triplet_cmake_var_provider(paths); Optional pkgsconfig; diff --git a/src/vcpkg/commands.update.cpp b/src/vcpkg/commands.update.cpp index 3aec063456..aaca543a48 100644 --- a/src/vcpkg/commands.update.cpp +++ b/src/vcpkg/commands.update.cpp @@ -68,8 +68,7 @@ namespace vcpkg const StatusParagraphs status_db = database_load_check(fs, paths.installed()); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); const auto outdated_packages = SortedVector( find_outdated_packages(provider, status_db), &OutdatedPackage::compare_by_name); diff --git a/src/vcpkg/commands.upgrade.cpp b/src/vcpkg/commands.upgrade.cpp index a27ea0a6e6..7c6704de96 100644 --- a/src/vcpkg/commands.upgrade.cpp +++ b/src/vcpkg/commands.upgrade.cpp @@ -81,8 +81,7 @@ namespace vcpkg // Load ports from ports dirs auto& fs = paths.get_filesystem(); auto registry_set = paths.make_registry_set(); - PathsPortFileProvider provider(*registry_set, - make_overlay_provider(fs, paths.original_cwd, paths.overlay_ports)); + PathsPortFileProvider provider(*registry_set, make_overlay_provider(fs, paths.overlay_ports)); auto var_provider_storage = CMakeVars::make_triplet_cmake_var_provider(paths); auto& var_provider = *var_provider_storage; diff --git a/src/vcpkg/portfileprovider.cpp b/src/vcpkg/portfileprovider.cpp index c2a4dd4401..8d9c904c20 100644 --- a/src/vcpkg/portfileprovider.cpp +++ b/src/vcpkg/portfileprovider.cpp @@ -211,10 +211,8 @@ namespace vcpkg struct OverlayProviderImpl : IFullOverlayProvider { - OverlayProviderImpl(const ReadOnlyFilesystem& fs, const Path& original_cwd, View overlay_ports) - : m_fs(fs), m_overlay_ports(Util::fmap(overlay_ports, [&original_cwd](const std::string& s) -> Path { - return original_cwd / s; - })) + OverlayProviderImpl(const ReadOnlyFilesystem& fs, View overlay_ports) + : m_fs(fs), m_overlay_ports(overlay_ports.begin(), overlay_ports.end()) { for (auto&& overlay : m_overlay_ports) { @@ -364,11 +362,10 @@ namespace vcpkg struct ManifestProviderImpl : IFullOverlayProvider { ManifestProviderImpl(const ReadOnlyFilesystem& fs, - const Path& original_cwd, - View overlay_ports, + View overlay_ports, const Path& manifest_path, std::unique_ptr&& manifest_scf) - : m_overlay_ports{fs, original_cwd, overlay_ports} + : m_overlay_ports{fs, overlay_ports} , m_manifest_scf_and_location{std::move(manifest_scf), manifest_path} { } @@ -407,21 +404,17 @@ namespace vcpkg return std::make_unique(registry_set); } - std::unique_ptr make_overlay_provider(const ReadOnlyFilesystem& fs, - const Path& original_cwd, - View overlay_ports) + std::unique_ptr make_overlay_provider(const ReadOnlyFilesystem& fs, View overlay_ports) { - return std::make_unique(fs, original_cwd, overlay_ports); + return std::make_unique(fs, overlay_ports); } std::unique_ptr make_manifest_provider(const ReadOnlyFilesystem& fs, - const Path& original_cwd, - View overlay_ports, + View overlay_ports, const Path& manifest_path, std::unique_ptr&& manifest_scf) { - return std::make_unique( - fs, original_cwd, overlay_ports, manifest_path, std::move(manifest_scf)); + return std::make_unique(fs, overlay_ports, manifest_path, std::move(manifest_scf)); } } // namespace vcpkg diff --git a/src/vcpkg/vcpkgpaths.cpp b/src/vcpkg/vcpkgpaths.cpp index 8635927c72..27a4fa65e3 100644 --- a/src/vcpkg/vcpkgpaths.cpp +++ b/src/vcpkg/vcpkgpaths.cpp @@ -98,16 +98,31 @@ namespace return nullopt; } - static std::vector merge_overlays(const std::vector& cli_overlays, - const std::vector& manifest_overlays, - const std::vector& env_overlays) + static void append_overlays(std::vector& result, + const std::vector& overlay_entries, + const Path& relative_root) { - std::vector ret = cli_overlays; - - ret.insert(std::end(ret), std::begin(manifest_overlays), std::end(manifest_overlays)); - ret.insert(std::end(ret), std::begin(env_overlays), std::end(env_overlays)); + for (auto&& entry : overlay_entries) + { + result.push_back(relative_root / entry); + } + } - return ret; + // Merges overlay settings from the 3 major sources in the usual priority order, where command line wins first, then + // manifest, then environment. The parameter order is specifically chosen to group information that comes from the + // manifest together and make parameter order confusion less likely to compile. + static std::vector merge_overlays(const std::vector& cli_overlays, + const std::vector& env_overlays, + const Path& original_cwd, + const std::vector& config_overlays, + const Path& config_directory) + { + std::vector result; + result.reserve(cli_overlays.size() + config_overlays.size() + env_overlays.size()); + append_overlays(result, cli_overlays, original_cwd); + append_overlays(result, config_overlays, config_directory); + append_overlays(result, env_overlays, original_cwd); + return result; } ConfigurationAndSource merge_validate_configs(Optional&& manifest_data, @@ -656,43 +671,33 @@ namespace vcpkg m_pimpl->m_download_manager->asset_cache_configured() ? Debug::println("Asset caching is enabled.") : Debug::println("Asset cache is not configured."); - { - const auto config_path = m_pimpl->m_config_dir / "vcpkg-configuration.json"; - auto maybe_manifest_config = config_from_manifest(m_pimpl->m_manifest_doc); - auto maybe_json_config = - !filesystem.exists(config_path, IgnoreErrors{}) - ? nullopt - : parse_configuration(filesystem.read_contents(config_path, IgnoreErrors{}), config_path, out_sink); - - m_pimpl->m_config = merge_validate_configs(std::move(maybe_manifest_config), - m_pimpl->m_manifest_dir, - std::move(maybe_json_config), - m_pimpl->m_config_dir, - *this); - - auto resolve_relative_to_config = [&](const std::string& overlay_path) { - return (m_pimpl->m_config.directory / overlay_path).native(); - }; - - std::vector overlay_triplet_paths; - std::vector overlay_port_paths; - - if (!m_pimpl->m_config.directory.empty()) - { - auto& config = m_pimpl->m_config.config; - overlay_triplet_paths = Util::fmap(config.overlay_triplets, resolve_relative_to_config); - overlay_port_paths = Util::fmap(config.overlay_ports, resolve_relative_to_config); - } - - overlay_ports = merge_overlays(args.cli_overlay_ports, overlay_port_paths, args.env_overlay_ports); - overlay_triplets = - merge_overlays(args.cli_overlay_triplets, overlay_triplet_paths, args.env_overlay_triplets); - } - - for (const std::string& triplet : this->overlay_triplets) + const auto config_path = m_pimpl->m_config_dir / "vcpkg-configuration.json"; + auto maybe_manifest_config = config_from_manifest(m_pimpl->m_manifest_doc); + auto maybe_json_config = + filesystem.exists(config_path, IgnoreErrors{}) + ? parse_configuration(filesystem.read_contents(config_path, IgnoreErrors{}), config_path, out_sink) + : nullopt; + + m_pimpl->m_config = merge_validate_configs(std::move(maybe_manifest_config), + m_pimpl->m_manifest_dir, + std::move(maybe_json_config), + m_pimpl->m_config_dir, + *this); + overlay_ports = merge_overlays(args.cli_overlay_ports, + args.env_overlay_ports, + original_cwd, + m_pimpl->m_config.config.overlay_ports, + m_pimpl->m_config.directory); + overlay_triplets = merge_overlays(args.cli_overlay_triplets, + args.env_overlay_triplets, + original_cwd, + m_pimpl->m_config.config.overlay_triplets, + m_pimpl->m_config.directory); + for (const auto& triplet : this->overlay_triplets) { m_pimpl->triplets_dirs.emplace_back(filesystem.almost_canonical(triplet, VCPKG_LINE_INFO)); } + m_pimpl->triplets_dirs.emplace_back(triplets); m_pimpl->triplets_dirs.emplace_back(community_triplets); }