From c76e0def147898591d9c35113b95e0b3f2eaa4d4 Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Mon, 31 Jan 2022 15:41:54 -0800 Subject: [PATCH] Insert "--head" defenses into the binary cache and block "default" when calculating feature ABIs. (#336) * Insert defenses into BinaryCache as reasonable and add tests. : Add the missing T& overloads used elsewhere. : * Remove VcpkgPaths from the IBinaryProvider interface; add it as a member of the concrete implementations. * Hoist BinaryConfigParserState (formerly State in binarycaching.cpp) here. It would be nice to make this more shiny, but hiding the definition of this struct wasn't buying us too much, and being unable to test stuff was a worse evil. * create_binary_providers_from_configs_pure now returns ExpectedS; this lets the tests of the parser machinery continue to be unchanged while still moving paths in as a member. binarycaching.cpp: * Push vcpkgpaths as a member into the concrete implementations. * Change/audit all IBinaryProvider::prefetch to support nullptr. We pass in the underlying action plan vector here, and preparing a vector with the ABI-less packages removed would require extra indirection or flattening of the data we don't want to do in a bugfix. * Change/audit all IBinaryProvider::precheck to not support nullptr. That is used only by CI and everything there should have a package ABI. * Change BinaryCache::prefetch to explicitly document that nullptr package_abi is acceptable. * Change BinaryCache::precheck to explicitly error out on nullptr package_abi. vcpkg-test/binarycaching.cpp: * Add tests for nullptr package ABIs passed to BinaryCache. * Block "default" in package ABI as it indicates that the default features have not yet been resolved. * Enforce that "core" is included before calculating ABI hashes. Co-authored-by: Robert Schumacher --- include/vcpkg/base/expected.h | 12 + include/vcpkg/binarycaching.h | 85 +++--- include/vcpkg/fwd/binarycaching.h | 28 ++ src/vcpkg-test/binarycaching.cpp | 62 ++++- src/vcpkg/binarycaching.cpp | 379 +++++++++++++-------------- src/vcpkg/build.cpp | 21 +- src/vcpkg/commands.buildexternal.cpp | 2 +- src/vcpkg/commands.ci.cpp | 4 +- src/vcpkg/commands.setinstalled.cpp | 2 +- src/vcpkg/commands.upgrade.cpp | 2 +- src/vcpkg/install.cpp | 6 +- 11 files changed, 348 insertions(+), 255 deletions(-) create mode 100644 include/vcpkg/fwd/binarycaching.h diff --git a/include/vcpkg/base/expected.h b/include/vcpkg/base/expected.h index 40d94189de..3814a6d35f 100644 --- a/include/vcpkg/base/expected.h +++ b/include/vcpkg/base/expected.h @@ -132,12 +132,24 @@ namespace vcpkg explicit constexpr operator bool() const noexcept { return !m_s.has_error(); } constexpr bool has_value() const noexcept { return !m_s.has_error(); } + const T&& value_or_exit(const LineInfo& line_info) const&& + { + exit_if_error(line_info); + return std::move(*this->m_t.get()); + } + T&& value_or_exit(const LineInfo& line_info) && { exit_if_error(line_info); return std::move(*this->m_t.get()); } + T& value_or_exit(const LineInfo& line_info) & + { + exit_if_error(line_info); + return *this->m_t.get(); + } + const T& value_or_exit(const LineInfo& line_info) const& { exit_if_error(line_info); diff --git a/include/vcpkg/binarycaching.h b/include/vcpkg/binarycaching.h index dfae184df9..75b67209f5 100644 --- a/include/vcpkg/binarycaching.h +++ b/include/vcpkg/binarycaching.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -16,27 +17,6 @@ namespace vcpkg { - enum class RestoreResult - { - unavailable, - restored, - }; - - enum class CacheAvailability - { - unavailable, - available, - }; - - enum class CacheStatusState - { - unknown, // the cache status of the indicated package ABI is unknown - available, // the cache is known to contain the package ABI, but it has not been restored - restored, // the cache contains the ABI and it has been restored to the packages tree - }; - - struct IBinaryProvider; - struct CacheStatus { bool should_attempt_precheck(const IBinaryProvider* sender) const noexcept; @@ -67,57 +47,84 @@ namespace vcpkg /// Attempts to restore the package referenced by `action` into the packages directory. /// Prerequisite: action has a package_abi() - virtual RestoreResult try_restore(const VcpkgPaths& paths, - const Dependencies::InstallPlanAction& action) const = 0; + virtual RestoreResult try_restore(const Dependencies::InstallPlanAction& action) const = 0; /// Called upon a successful build of `action` to store those contents in the binary cache. /// Prerequisite: action has a package_abi() - virtual void push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) const = 0; + virtual void push_success(const Dependencies::InstallPlanAction& action) const = 0; /// Gives the IBinaryProvider an opportunity to batch any downloading or server communication for /// executing `actions`. /// `cache_status` is a vector with the same number of entries of actions, where each index corresponds /// to the action at the same index in `actions`. The provider must mark the cache status as appropriate. - virtual void prefetch(const VcpkgPaths& paths, - View actions, - View cache_status) const = 0; + /// Note: `actions` *might not* have package ABIs (for example, a --head package)! + /// Prerequisite: if `actions[i]` has no package ABI, `cache_status[i]` is nullptr. + virtual void prefetch(View actions, View cache_status) const = 0; /// Checks whether the `actions` are present in the cache, without restoring them. Used by CI to determine /// missing packages. /// `cache_status` is a view with the same number of entries of actions, where each index corresponds /// to the action at the same index in `actions`. The provider must mark the cache status as appropriate. - virtual void precheck(const VcpkgPaths& paths, - View actions, - View cache_status) const = 0; + /// Prerequisite: `actions` have package ABIs. + virtual void precheck(View actions, View cache_status) const = 0; + }; + + struct BinaryConfigParserState + { + bool m_cleared = false; + bool interactive = false; + std::string nugettimeout = "100"; + + std::vector archives_to_read; + std::vector archives_to_write; + + std::vector url_templates_to_get; + std::vector azblob_templates_to_put; + + std::vector gcs_read_prefixes; + std::vector gcs_write_prefixes; + + std::vector aws_read_prefixes; + std::vector aws_write_prefixes; + + std::vector sources_to_read; + std::vector sources_to_write; + + std::vector configs_to_read; + std::vector configs_to_write; + + std::vector secrets; + + void clear(); }; + ExpectedS create_binary_providers_from_configs_pure(const std::string& env_string, + View args); ExpectedS>> create_binary_providers_from_configs( - View args); - ExpectedS>> create_binary_providers_from_configs_pure( - const std::string& env_string, View args); + const VcpkgPaths& paths, View args); struct BinaryCache { BinaryCache() = default; - explicit BinaryCache(const VcpkgCmdArguments& args); + explicit BinaryCache(const VcpkgCmdArguments& args, const VcpkgPaths& paths); void install_providers(std::vector>&& providers); - void install_providers_for(const VcpkgCmdArguments& args); + void install_providers_for(const VcpkgCmdArguments& args, const VcpkgPaths& paths); /// Attempts to restore the package referenced by `action` into the packages directory. - RestoreResult try_restore(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action); + RestoreResult try_restore(const Dependencies::InstallPlanAction& action); /// Called upon a successful build of `action` to store those contents in the binary cache. - void push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action); + void push_success(const Dependencies::InstallPlanAction& action); /// Gives the IBinaryProvider an opportunity to batch any downloading or server communication for /// executing `actions`. - void prefetch(const VcpkgPaths& paths, View actions); + void prefetch(View actions); /// Checks whether the `actions` are present in the cache, without restoring them. Used by CI to determine /// missing packages. /// Returns a vector where each index corresponds to the matching index in `actions`. - std::vector precheck(const VcpkgPaths& paths, View actions); + std::vector precheck(View actions); private: std::unordered_map m_status; diff --git a/include/vcpkg/fwd/binarycaching.h b/include/vcpkg/fwd/binarycaching.h new file mode 100644 index 0000000000..de46295d3f --- /dev/null +++ b/include/vcpkg/fwd/binarycaching.h @@ -0,0 +1,28 @@ +#pragma once + +namespace vcpkg +{ + enum class RestoreResult + { + unavailable, + restored, + }; + + enum class CacheAvailability + { + unavailable, + available, + }; + + enum class CacheStatusState + { + unknown, // the cache status of the indicated package ABI is unknown + available, // the cache is known to contain the package ABI, but it has not been restored + restored, // the cache contains the ABI and it has been restored to the packages tree + }; + + struct CacheStatus; + struct IBinaryProvider; + struct BinaryCache; + struct BinaryConfigParserState; +} diff --git a/src/vcpkg-test/binarycaching.cpp b/src/vcpkg-test/binarycaching.cpp index 2239f323c1..6da6ba9915 100644 --- a/src/vcpkg-test/binarycaching.cpp +++ b/src/vcpkg-test/binarycaching.cpp @@ -18,27 +18,34 @@ using namespace vcpkg; struct KnowNothingBinaryProvider : IBinaryProvider { - RestoreResult try_restore(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override + RestoreResult try_restore(const Dependencies::InstallPlanAction& action) const override { + CHECK(action.has_package_abi()); return RestoreResult::unavailable; } - virtual void push_success(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override { } - virtual void prefetch(const VcpkgPaths&, - View, - View) const override + virtual void push_success(const Dependencies::InstallPlanAction& action) const override { + CHECK(action.has_package_abi()); } - virtual void precheck(const VcpkgPaths&, - View, + + virtual void prefetch(View actions, View cache_status) const override { + REQUIRE(actions.size() == cache_status.size()); + for (size_t idx = 0; idx < cache_status.size(); ++idx) + { + CHECK(actions[idx].has_package_abi() == (cache_status[idx] != nullptr)); + } + } + virtual void precheck(View actions, + View cache_status) const override + { + REQUIRE(actions.size() == cache_status.size()); for (const auto c : cache_status) { - if (c) - { - c->mark_unavailable(this); - } + CHECK(c); + c->mark_unavailable(this); } } }; @@ -359,6 +366,39 @@ Features: a, b } } +TEST_CASE ("Provider nullptr checks", "[BinaryCache]") +{ + // create a binary cache to test + BinaryCache uut; + std::vector> providers; + providers.emplace_back(std::make_unique()); + uut.install_providers(std::move(providers)); + + // create an action plan with an action without a package ABI set + auto pghs = Paragraphs::parse_paragraphs(R"( +Source: someheadpackage +Version: 1.5 +Description: +)", + ""); + REQUIRE(pghs.has_value()); + auto maybe_scf = SourceControlFile::parse_control_file("", std::move(*pghs.get())); + REQUIRE(maybe_scf.has_value()); + SourceControlFileAndLocation scfl{std::move(*maybe_scf.get()), Path()}; + std::vector install_plan; + install_plan.emplace_back(PackageSpec{"someheadpackage", Test::X64_WINDOWS}, + scfl, + Dependencies::RequestType::USER_REQUESTED, + Test::ARM_UWP, + std::map>{}); + Dependencies::InstallPlanAction& ipa_without_abi = install_plan.back(); + + // test that the binary cache does the right thing. See also CHECKs etc. in KnowNothingBinaryProvider + uut.push_success(ipa_without_abi); // should have no effects + CHECK(uut.try_restore(ipa_without_abi) == RestoreResult::unavailable); + uut.prefetch(install_plan); // should have no effects +} + TEST_CASE ("XmlSerializer", "[XmlSerializer]") { XmlSerializer xml; diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index a51de0fcac..d541cb72ff 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -246,11 +246,13 @@ namespace struct ArchivesBinaryProvider : IBinaryProvider { - ArchivesBinaryProvider(std::vector&& read_dirs, + ArchivesBinaryProvider(const VcpkgPaths& paths, + std::vector&& read_dirs, std::vector&& write_dirs, std::vector&& put_url_templates, std::vector&& secrets) - : m_read_dirs(std::move(read_dirs)) + : paths(paths) + , m_read_dirs(std::move(read_dirs)) , m_write_dirs(std::move(write_dirs)) , m_put_url_templates(std::move(put_url_templates)) , m_secrets(std::move(secrets)) @@ -259,9 +261,7 @@ namespace static Path make_archive_subpath(const std::string& abi) { return Path(abi.substr(0, 2)) / (abi + ".zip"); } - void prefetch(const VcpkgPaths& paths, - View actions, - View cache_status) const override + void prefetch(View actions, View cache_status) const override { std::vector to_try_restore_idxs; std::vector to_try_restore; @@ -271,16 +271,17 @@ namespace const auto timer = ElapsedTimer::create_started(); to_try_restore_idxs.clear(); to_try_restore.clear(); - for (size_t idx = 0; idx < actions.size(); ++idx) + for (size_t idx = 0; idx < cache_status.size(); ++idx) { - auto&& action = actions[idx]; - if (action.has_package_abi() && cache_status[idx]->should_attempt_restore(this)) + auto idx_cache_status = cache_status[idx]; + if (idx_cache_status && idx_cache_status->should_attempt_restore(this)) { to_try_restore_idxs.push_back(idx); - to_try_restore.push_back(&action); + to_try_restore.push_back(&actions[idx]); } } - auto results = try_restore_n(paths, to_try_restore, archives_root_dir); + + auto results = try_restore_n(to_try_restore, archives_root_dir); int num_restored = 0; for (size_t n = 0; n < to_try_restore.size(); ++n) { @@ -301,8 +302,7 @@ namespace } } - std::vector try_restore_n(const VcpkgPaths& paths, - View actions, + std::vector try_restore_n(View actions, const Path& archives_root_dir) const { auto& fs = paths.get_filesystem(); @@ -320,7 +320,7 @@ namespace if (fs.exists(archive_path, IgnoreErrors{})) { auto pkg_path = paths.package_dir(spec); - clean_prepare_dir(paths.get_filesystem(), pkg_path); + clean_prepare_dir(fs, pkg_path); jobs.push_back(decompress_archive_cmd(paths, pkg_path, archive_path)); action_idxs.push_back(i); archive_paths.push_back(std::move(archive_path)); @@ -355,14 +355,14 @@ namespace return results; } - RestoreResult try_restore(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) const override + RestoreResult try_restore(const Dependencies::InstallPlanAction& action) const override { // Note: this method is almost never called -- it will only be called if another provider promised to // restore a package but then failed at runtime auto p_action = &action; for (const auto& archives_root_dir : m_read_dirs) { - if (try_restore_n(paths, {&p_action, 1}, archives_root_dir)[0] == RestoreResult::restored) + if (try_restore_n({&p_action, 1}, archives_root_dir)[0] == RestoreResult::restored) { print2("Restored from ", archives_root_dir.native(), "\n"); return RestoreResult::restored; @@ -371,7 +371,7 @@ namespace return RestoreResult::unavailable; } - void push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) const override + void push_success(const Dependencies::InstallPlanAction& action) const override { if (m_write_dirs.empty() && m_put_url_templates.empty()) { @@ -434,21 +434,19 @@ namespace } } - void precheck(const VcpkgPaths& paths, - View actions, - View cache_status) const override + void precheck(View actions, View cache_status) const override { auto& fs = paths.get_filesystem(); for (size_t idx = 0; idx < actions.size(); ++idx) { const auto& action = actions[idx]; - const auto abi_tag = action.package_abi().get(); - if (!abi_tag || !cache_status[idx]->should_attempt_precheck(this)) + const auto& abi_tag = action.package_abi().value_or_exit(VCPKG_LINE_INFO); + if (!cache_status[idx]->should_attempt_precheck(this)) { continue; } - const auto archive_subpath = make_archive_subpath(*abi_tag); + const auto archive_subpath = make_archive_subpath(abi_tag); bool any_available = false; for (auto&& archives_root_dir : m_read_dirs) { @@ -471,6 +469,7 @@ namespace } private: + const VcpkgPaths& paths; std::vector m_read_dirs; std::vector m_write_dirs; std::vector m_put_url_templates; @@ -478,18 +477,19 @@ namespace }; struct HttpGetBinaryProvider : IBinaryProvider { - HttpGetBinaryProvider(std::vector&& url_templates) : m_url_templates(std::move(url_templates)) { } + HttpGetBinaryProvider(const VcpkgPaths& paths, std::vector&& url_templates) + : paths(paths), m_url_templates(std::move(url_templates)) + { + } - RestoreResult try_restore(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override + RestoreResult try_restore(const Dependencies::InstallPlanAction&) const override { return RestoreResult::unavailable; } - void push_success(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override { } + void push_success(const Dependencies::InstallPlanAction&) const override { } - void prefetch(const VcpkgPaths& paths, - View actions, - View cache_status) const override + void prefetch(View actions, View cache_status) const override { const auto timer = ElapsedTimer::create_started(); auto& fs = paths.get_filesystem(); @@ -500,18 +500,19 @@ namespace { url_paths.clear(); url_indices.clear(); - for (size_t idx = 0; idx < actions.size(); ++idx) + for (size_t idx = 0; idx < cache_status.size(); ++idx) { - auto&& action = actions[idx]; - auto abi = action.package_abi(); - if (!abi || !cache_status[idx]->should_attempt_restore(this)) + auto this_cache_status = cache_status[idx]; + if (!this_cache_status || !this_cache_status->should_attempt_restore(this)) { continue; } + auto&& action = actions[idx]; clean_prepare_dir(fs, paths.package_dir(action.spec)); - url_paths.emplace_back(Strings::replace_all(url_template, "", *abi.get()), - make_temp_archive_path(paths.buildtrees(), action.spec)); + auto uri = Strings::replace_all( + url_template, "", action.package_abi().value_or_exit(VCPKG_LINE_INFO)); + url_paths.emplace_back(std::move(uri), make_temp_archive_path(paths.buildtrees(), action.spec)); url_indices.push_back(idx); } @@ -555,9 +556,7 @@ namespace ". Use --debug for more information.\n"); } - void precheck(const VcpkgPaths&, - View actions, - View cache_status) const override + void precheck(View actions, View cache_status) const override { std::vector actions_present{actions.size()}; std::vector urls; @@ -568,13 +567,13 @@ namespace url_indices.clear(); for (size_t idx = 0; idx < actions.size(); ++idx) { - auto abi = actions[idx].package_abi().get(); - if (!abi || !cache_status[idx]->should_attempt_precheck(this)) + const auto& abi = actions[idx].package_abi().value_or_exit(VCPKG_LINE_INFO); + if (!cache_status[idx]->should_attempt_precheck(this)) { continue; } - urls.push_back(Strings::replace_all(std::string(url_template), "", *abi)); + urls.push_back(Strings::replace_all(std::string(url_template), "", abi)); url_indices.push_back(idx); } @@ -604,6 +603,7 @@ namespace } } + const VcpkgPaths& paths; std::vector m_url_templates; }; @@ -620,13 +620,15 @@ namespace struct NugetBinaryProvider : IBinaryProvider { - NugetBinaryProvider(std::vector&& read_sources, + NugetBinaryProvider(const VcpkgPaths& paths, + std::vector&& read_sources, std::vector&& write_sources, std::vector&& read_configs, std::vector&& write_configs, std::string&& timeout, bool interactive) - : m_read_sources(std::move(read_sources)) + : paths(paths) + , m_read_sources(std::move(read_sources)) , m_write_sources(std::move(write_sources)) , m_read_configs(std::move(read_configs)) , m_write_configs(std::move(write_configs)) @@ -711,9 +713,7 @@ namespace fs.write_contents(packages_config, xml.buf, VCPKG_LINE_INFO); } - void prefetch(const VcpkgPaths& paths, - View actions, - View cache_status) const override + void prefetch(View actions, View cache_status) const override { if (m_read_sources.empty() && m_read_configs.empty()) { @@ -724,16 +724,16 @@ namespace auto& fs = paths.get_filesystem(); std::vector attempts; - for (size_t idx = 0; idx < actions.size(); ++idx) + for (size_t idx = 0; idx < cache_status.size(); ++idx) { - auto&& action = actions[idx]; - auto abi = action.package_abi().get(); - if (!abi || !cache_status[idx]->should_attempt_restore(this)) + auto this_cache_status = cache_status[idx]; + if (!this_cache_status || !this_cache_status->should_attempt_restore(this)) { continue; } - auto& spec = action.spec; + const auto& action = actions[idx]; + const auto& spec = action.spec; fs.remove_all(paths.package_dir(spec), VCPKG_LINE_INFO); attempts.push_back({spec, make_nugetref(action, get_nuget_prefix()), idx}); } @@ -859,12 +859,12 @@ namespace ". Use --debug for more information.\n"); } - RestoreResult try_restore(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override + RestoreResult try_restore(const Dependencies::InstallPlanAction&) const override { return RestoreResult::unavailable; } - void push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) const override + void push_success(const Dependencies::InstallPlanAction& action) const override { if (m_write_sources.empty() && m_write_configs.empty()) { @@ -875,7 +875,8 @@ namespace NugetReference nuget_ref = make_nugetref(action, get_nuget_prefix()); auto nuspec_path = paths.buildtrees() / spec.name() / (spec.triplet().to_string() + ".nuspec"); - paths.get_filesystem().write_contents( + auto& fs = paths.get_filesystem(); + fs.write_contents( nuspec_path, generate_nuspec(paths.package_dir(spec), action, nuget_ref), VCPKG_LINE_INFO); const auto& nuget_exe = paths.get_tool_exe("nuget"); @@ -958,12 +959,14 @@ namespace } } - paths.get_filesystem().remove(nupkg_path, IgnoreErrors{}); + fs.remove(nupkg_path, IgnoreErrors{}); } - void precheck(const VcpkgPaths&, View, View) const override { } + void precheck(View, View) const override { } private: + const VcpkgPaths& paths; + std::vector m_read_sources; std::vector m_write_sources; @@ -1013,8 +1016,10 @@ namespace struct GcsBinaryProvider : IBinaryProvider { - GcsBinaryProvider(std::vector&& read_prefixes, std::vector&& write_prefixes) - : m_read_prefixes(std::move(read_prefixes)), m_write_prefixes(std::move(write_prefixes)) + GcsBinaryProvider(const VcpkgPaths& paths, + std::vector&& read_prefixes, + std::vector&& write_prefixes) + : paths(paths), m_read_prefixes(std::move(read_prefixes)), m_write_prefixes(std::move(write_prefixes)) { } @@ -1023,9 +1028,7 @@ namespace return Strings::concat(prefix, abi, ".zip"); } - void prefetch(const VcpkgPaths& paths, - View actions, - View cache_status) const override + void prefetch(View actions, View cache_status) const override { auto& fs = paths.get_filesystem(); @@ -1037,17 +1040,17 @@ namespace std::vector> url_paths; std::vector url_indices; - for (size_t idx = 0; idx < actions.size(); ++idx) + for (size_t idx = 0; idx < cache_status.size(); ++idx) { - auto&& action = actions[idx]; - auto abi = action.package_abi().get(); - if (!abi || !cache_status[idx]->should_attempt_restore(this)) + const auto this_cache_status = cache_status[idx]; + if (!this_cache_status || !this_cache_status->should_attempt_restore(this)) { continue; } + auto&& action = actions[idx]; clean_prepare_dir(fs, paths.package_dir(action.spec)); - url_paths.emplace_back(make_gcs_path(prefix, *abi), + url_paths.emplace_back(make_gcs_path(prefix, action.package_abi().value_or_exit(VCPKG_LINE_INFO)), make_temp_archive_path(paths.buildtrees(), action.spec)); url_indices.push_back(idx); } @@ -1091,12 +1094,12 @@ namespace ". Use --debug for more information.\n"); } - RestoreResult try_restore(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override + RestoreResult try_restore(const Dependencies::InstallPlanAction&) const override { return RestoreResult::unavailable; } - void push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) const override + void push_success(const Dependencies::InstallPlanAction& action) const override { if (m_write_prefixes.empty()) return; const auto& abi = action.package_abi().value_or_exit(VCPKG_LINE_INFO); @@ -1116,9 +1119,7 @@ namespace print2("Uploaded binaries to ", upload_count, " GCS remotes.\n"); } - void precheck(const VcpkgPaths&, - View actions, - View cache_status) const override + void precheck(View actions, View cache_status) const override { std::vector actions_availability{actions.size()}; for (const auto& prefix : m_read_prefixes) @@ -1126,13 +1127,13 @@ namespace for (size_t idx = 0; idx < actions.size(); ++idx) { auto&& action = actions[idx]; - const auto abi = action.package_abi().get(); - if (!abi || !cache_status[idx]->should_attempt_precheck(this)) + const auto& abi = action.package_abi().value_or_exit(VCPKG_LINE_INFO); + if (!cache_status[idx]->should_attempt_precheck(this)) { continue; } - if (gsutil_stat(make_gcs_path(prefix, *abi))) + if (gsutil_stat(make_gcs_path(prefix, abi))) { actions_availability[idx] = CacheAvailability::available; cache_status[idx]->mark_available(this); @@ -1151,6 +1152,8 @@ namespace } private: + const VcpkgPaths& paths; + std::vector m_read_prefixes; std::vector m_write_prefixes; }; @@ -1195,8 +1198,10 @@ namespace struct AwsBinaryProvider : IBinaryProvider { - AwsBinaryProvider(std::vector&& read_prefixes, std::vector&& write_prefixes) - : m_read_prefixes(std::move(read_prefixes)), m_write_prefixes(std::move(write_prefixes)) + AwsBinaryProvider(const VcpkgPaths& paths, + std::vector&& read_prefixes, + std::vector&& write_prefixes) + : paths(paths), m_read_prefixes(std::move(read_prefixes)), m_write_prefixes(std::move(write_prefixes)) { } @@ -1205,9 +1210,7 @@ namespace return Strings::concat(prefix, abi, ".zip"); } - void prefetch(const VcpkgPaths& paths, - View actions, - View cache_status) const override + void prefetch(View actions, View cache_status) const override { auto& fs = paths.get_filesystem(); @@ -1219,17 +1222,17 @@ namespace std::vector> url_paths; std::vector url_indices; - for (size_t idx = 0; idx < actions.size(); ++idx) + for (size_t idx = 0; idx < cache_status.size(); ++idx) { - auto&& action = actions[idx]; - auto abi = action.package_abi().get(); - if (!abi || !cache_status[idx]->should_attempt_restore(this)) + const auto this_cache_status = cache_status[idx]; + if (!this_cache_status || !this_cache_status->should_attempt_restore(this)) { continue; } + auto&& action = actions[idx]; clean_prepare_dir(fs, paths.package_dir(action.spec)); - url_paths.emplace_back(make_aws_path(prefix, *abi), + url_paths.emplace_back(make_aws_path(prefix, action.package_abi().value_or_exit(VCPKG_LINE_INFO)), make_temp_archive_path(paths.buildtrees(), action.spec)); url_indices.push_back(idx); } @@ -1272,12 +1275,12 @@ namespace msg::elapsed = timer.elapsed().as().count()); } - RestoreResult try_restore(const VcpkgPaths&, const Dependencies::InstallPlanAction&) const override + RestoreResult try_restore(const Dependencies::InstallPlanAction&) const override { return RestoreResult::unavailable; } - void push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) const override + void push_success(const Dependencies::InstallPlanAction& action) const override { if (m_write_prefixes.empty()) return; const auto& abi = action.package_abi().value_or_exit(VCPKG_LINE_INFO); @@ -1297,9 +1300,7 @@ namespace msg::println(msgAwsUploadedPackages, msg::value = upload_count); } - void precheck(const VcpkgPaths& paths, - View actions, - View cache_status) const override + void precheck(View actions, View cache_status) const override { std::vector actions_availability{actions.size()}; for (const auto& prefix : m_read_prefixes) @@ -1307,13 +1308,13 @@ namespace for (size_t idx = 0; idx < actions.size(); ++idx) { auto&& action = actions[idx]; - const auto abi = action.package_abi().get(); - if (!abi || !cache_status[idx]->should_attempt_precheck(this)) + const auto& abi = action.package_abi().value_or_exit(VCPKG_LINE_INFO); + if (!cache_status[idx]->should_attempt_precheck(this)) { continue; } - if (awscli_stat(paths, make_aws_path(prefix, *abi))) + if (awscli_stat(paths, make_aws_path(prefix, abi))) { actions_availability[idx] = CacheAvailability::available; cache_status[idx]->mark_available(this); @@ -1332,6 +1333,8 @@ namespace } private: + const VcpkgPaths& paths; + std::vector m_read_prefixes; std::vector m_write_prefixes; }; @@ -1339,7 +1342,10 @@ namespace namespace vcpkg { - BinaryCache::BinaryCache(const VcpkgCmdArguments& args) { install_providers_for(args); } + BinaryCache::BinaryCache(const VcpkgCmdArguments& args, const VcpkgPaths& paths) + { + install_providers_for(args, paths); + } void BinaryCache::install_providers(std::vector>&& providers) { @@ -1357,19 +1363,21 @@ namespace vcpkg } } - void BinaryCache::install_providers_for(const VcpkgCmdArguments& args) + void BinaryCache::install_providers_for(const VcpkgCmdArguments& args, const VcpkgPaths& paths) { if (args.binary_caching_enabled()) { - install_providers(create_binary_providers_from_configs(args.binary_sources).value_or_exit(VCPKG_LINE_INFO)); + install_providers( + create_binary_providers_from_configs(paths, args.binary_sources).value_or_exit(VCPKG_LINE_INFO)); } } - RestoreResult BinaryCache::try_restore(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) + RestoreResult BinaryCache::try_restore(const Dependencies::InstallPlanAction& action) { const auto abi = action.package_abi().get(); if (!abi) { + // e.g. this is a `--head` package return RestoreResult::unavailable; } @@ -1382,7 +1390,7 @@ namespace vcpkg const auto available = cache_status.get_available_provider(); if (available) { - switch (available->try_restore(paths, action)) + switch (available->try_restore(action)) { case RestoreResult::unavailable: // Even though that provider thought it had it, it didn't; perhaps @@ -1406,7 +1414,7 @@ namespace vcpkg break; } - switch (provider->try_restore(paths, action)) + switch (provider->try_restore(action)) { case RestoreResult::restored: cache_status.mark_restored(); return RestoreResult::restored; case RestoreResult::unavailable: cache_status.mark_unavailable(provider.get()); break; @@ -1417,42 +1425,35 @@ namespace vcpkg return RestoreResult::unavailable; } - void BinaryCache::push_success(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action) + void BinaryCache::push_success(const Dependencies::InstallPlanAction& action) { const auto abi = action.package_abi().get(); if (abi) { for (auto&& provider : m_providers) { - provider->push_success(paths, action); + provider->push_success(action); } m_status[*abi].mark_restored(); } } - static std::vector build_cache_status_vector(View actions, - std::unordered_map& status) + void BinaryCache::prefetch(View actions) { - std::vector results{actions.size()}; + std::vector cache_status{actions.size()}; for (size_t idx = 0; idx < actions.size(); ++idx) { const auto abi = actions[idx].package_abi().get(); if (abi) { - results[idx] = &status[*abi]; + cache_status[idx] = &m_status[*abi]; } } - return results; - } - - void BinaryCache::prefetch(const VcpkgPaths& paths, View actions) - { - auto cache_status = build_cache_status_vector(actions, m_status); for (auto&& provider : m_providers) { - provider->prefetch(paths, actions, cache_status); + provider->prefetch(actions, cache_status); for (auto status : cache_status) { if (status) @@ -1463,13 +1464,23 @@ namespace vcpkg } } - std::vector BinaryCache::precheck(const VcpkgPaths& paths, - View actions) + std::vector BinaryCache::precheck(View actions) { - auto cache_status = build_cache_status_vector(actions, m_status); + std::vector cache_status{actions.size()}; + for (size_t idx = 0; idx < actions.size(); ++idx) + { + auto& action = actions[idx]; + const auto abi = action.package_abi().get(); + Checks::check_exit(VCPKG_LINE_INFO, + abi, + "Error: package %s did not have an abi during ci. This is an internal error.\n", + action.spec); + cache_status[idx] = &m_status[*abi]; + } + for (auto&& provider : m_providers) { - provider->precheck(paths, actions, cache_status); + provider->precheck(actions, cache_status); } std::vector results{actions.size()}; @@ -1568,6 +1579,26 @@ namespace vcpkg default: Checks::unreachable(VCPKG_LINE_INFO); } } + + void BinaryConfigParserState::clear() + { + m_cleared = true; + interactive = false; + nugettimeout = "100"; + archives_to_read.clear(); + archives_to_write.clear(); + url_templates_to_get.clear(); + azblob_templates_to_put.clear(); + gcs_read_prefixes.clear(); + gcs_write_prefixes.clear(); + aws_read_prefixes.clear(); + aws_write_prefixes.clear(); + sources_to_read.clear(); + sources_to_write.clear(); + configs_to_read.clear(); + configs_to_write.clear(); + secrets.clear(); + } } namespace @@ -1611,61 +1642,14 @@ namespace return cachepath; } - struct State - { - bool m_cleared = false; - bool interactive = false; - std::string nugettimeout = "100"; - - std::vector archives_to_read; - std::vector archives_to_write; - - std::vector url_templates_to_get; - std::vector azblob_templates_to_put; - - std::vector gcs_read_prefixes; - std::vector gcs_write_prefixes; - - std::vector aws_read_prefixes; - std::vector aws_write_prefixes; - - std::vector sources_to_read; - std::vector sources_to_write; - - std::vector configs_to_read; - std::vector configs_to_write; - - std::vector secrets; - - void clear() - { - m_cleared = true; - interactive = false; - nugettimeout = "100"; - archives_to_read.clear(); - archives_to_write.clear(); - url_templates_to_get.clear(); - azblob_templates_to_put.clear(); - gcs_read_prefixes.clear(); - gcs_write_prefixes.clear(); - aws_read_prefixes.clear(); - aws_write_prefixes.clear(); - sources_to_read.clear(); - sources_to_write.clear(); - configs_to_read.clear(); - configs_to_write.clear(); - secrets.clear(); - } - }; - struct BinaryConfigParser : ConfigSegmentsParser { - BinaryConfigParser(StringView text, StringView origin, State* state) + BinaryConfigParser(StringView text, StringView origin, BinaryConfigParserState* state) : ConfigSegmentsParser(text, origin), state(state) { } - State* state; + BinaryConfigParserState* state; void parse() { @@ -2102,28 +2086,8 @@ ExpectedS vcpkg::parse_download_configuration(const Optio s.script}; } -ExpectedS>> vcpkg::create_binary_providers_from_configs( - View args) -{ - std::string env_string = get_environment_variable("VCPKG_BINARY_SOURCES").value_or(""); - if (Debug::g_debugging) - { - const auto& cachepath = default_cache_path(); - if (cachepath.has_value()) - { - Debug::print("Default binary cache path is: ", cachepath.value_or_exit(VCPKG_LINE_INFO), '\n'); - } - else - { - Debug::print("No binary cache path. Reason: ", cachepath.error(), '\n'); - } - } - - return create_binary_providers_from_configs_pure(env_string, args); -} - -ExpectedS>> vcpkg::create_binary_providers_from_configs_pure( - const std::string& env_string, View args) +ExpectedS vcpkg::create_binary_providers_from_configs_pure(const std::string& env_string, + View args) { { LockGuardPtr metrics(g_metrics); @@ -2138,7 +2102,7 @@ ExpectedS>> vcpkg::create_binary_pr } } - State s; + BinaryConfigParserState s; BinaryConfigParser default_parser("default,readwrite", "", &s); default_parser.parse(); @@ -2169,22 +2133,50 @@ ExpectedS>> vcpkg::create_binary_pr LockGuardPtr(g_metrics)->track_property("binarycaching-clear", "defined"); } + return s; +} + +ExpectedS>> vcpkg::create_binary_providers_from_configs( + const VcpkgPaths& paths, View args) +{ + std::string env_string = get_environment_variable("VCPKG_BINARY_SOURCES").value_or(""); + if (Debug::g_debugging) + { + const auto& cachepath = default_cache_path(); + if (cachepath.has_value()) + { + Debug::print("Default binary cache path is: ", cachepath.value_or_exit(VCPKG_LINE_INFO), '\n'); + } + else + { + Debug::print("No binary cache path. Reason: ", cachepath.error(), '\n'); + } + } + + auto sRawHolder = create_binary_providers_from_configs_pure(env_string, args); + if (!sRawHolder.has_value()) + { + return sRawHolder.error(); + } + + auto& s = sRawHolder.value_or_exit(VCPKG_LINE_INFO); std::vector> providers; if (!s.gcs_read_prefixes.empty() || !s.gcs_write_prefixes.empty()) { - providers.push_back( - std::make_unique(std::move(s.gcs_read_prefixes), std::move(s.gcs_write_prefixes))); + providers.push_back(std::make_unique( + paths, std::move(s.gcs_read_prefixes), std::move(s.gcs_write_prefixes))); } if (!s.aws_read_prefixes.empty() || !s.aws_write_prefixes.empty()) { - providers.push_back( - std::make_unique(std::move(s.aws_read_prefixes), std::move(s.aws_write_prefixes))); + providers.push_back(std::make_unique( + paths, std::move(s.aws_read_prefixes), std::move(s.aws_write_prefixes))); } if (!s.archives_to_read.empty() || !s.archives_to_write.empty() || !s.azblob_templates_to_put.empty()) { - providers.push_back(std::make_unique(std::move(s.archives_to_read), + providers.push_back(std::make_unique(paths, + std::move(s.archives_to_read), std::move(s.archives_to_write), std::move(s.azblob_templates_to_put), std::move(s.secrets))); @@ -2193,14 +2185,15 @@ ExpectedS>> vcpkg::create_binary_pr if (!s.url_templates_to_get.empty()) { LockGuardPtr(g_metrics)->track_property("binarycaching-url-get", "defined"); - providers.push_back(std::make_unique(std::move(s.url_templates_to_get))); + providers.push_back(std::make_unique(paths, std::move(s.url_templates_to_get))); } if (!s.sources_to_read.empty() || !s.sources_to_write.empty() || !s.configs_to_read.empty() || !s.configs_to_write.empty()) { LockGuardPtr(g_metrics)->track_property("binarycaching-nuget", "defined"); - providers.push_back(std::make_unique(std::move(s.sources_to_read), + providers.push_back(std::make_unique(paths, + std::move(s.sources_to_read), std::move(s.sources_to_write), std::move(s.configs_to_read), std::move(s.configs_to_write), diff --git a/src/vcpkg/build.cpp b/src/vcpkg/build.cpp index bee9cd938c..b63e4b55ad 100644 --- a/src/vcpkg/build.cpp +++ b/src/vcpkg/build.cpp @@ -183,7 +183,7 @@ namespace vcpkg::Build const ParsedArguments options = args.parse_arguments(COMMAND_STRUCTURE); std::string first_arg = args.command_arguments[0]; - BinaryCache binary_cache{args}; + BinaryCache binary_cache{args, paths}; const FullPackageSpec spec = Input::check_and_get_full_package_spec( std::move(first_arg), default_triplet, COMMAND_STRUCTURE.example_text, paths); @@ -1112,8 +1112,21 @@ namespace vcpkg::Build abi_tag_entries.emplace_back("ports.cmake", paths.get_ports_cmake_hash().to_string()); abi_tag_entries.emplace_back("post_build_checks", "2"); - std::vector sorted_feature_list = action.feature_list; - Util::sort(sorted_feature_list); + InternalFeatureSet sorted_feature_list = action.feature_list; + // Check that no "default" feature is present. Default features must be resolved before attempting to calculate + // a package ABI, so the "default" should not have made it here. + static constexpr auto default_literal = StringLiteral{"default"}; + const bool has_no_pseudo_features = std::none_of( + sorted_feature_list.begin(), sorted_feature_list.end(), [](StringView s) { return s == default_literal; }); + Checks::check_exit(VCPKG_LINE_INFO, has_no_pseudo_features); + Util::sort_unique_erase(sorted_feature_list); + + // Check that the "core" feature is present. After resolution into InternalFeatureSet "core" meaning "not + // default" should have already been handled so "core" should be here. + Checks::check_exit( + VCPKG_LINE_INFO, + std::binary_search(sorted_feature_list.begin(), sorted_feature_list.end(), StringLiteral{"core"})); + abi_tag_entries.emplace_back("features", Strings::join(";", sorted_feature_list)); Util::sort(abi_tag_entries); @@ -1290,7 +1303,7 @@ namespace vcpkg::Build if (result.code == BuildResult::SUCCEEDED) { - binary_cache.push_success(paths, action); + binary_cache.push_success(action); } return result; diff --git a/src/vcpkg/commands.buildexternal.cpp b/src/vcpkg/commands.buildexternal.cpp index 978c92fbbf..fb69c12c51 100644 --- a/src/vcpkg/commands.buildexternal.cpp +++ b/src/vcpkg/commands.buildexternal.cpp @@ -24,7 +24,7 @@ namespace vcpkg::Commands::BuildExternal { const ParsedArguments options = args.parse_arguments(COMMAND_STRUCTURE); - BinaryCache binary_cache{args}; + BinaryCache binary_cache{args, paths}; const FullPackageSpec spec = Input::check_and_get_full_package_spec( std::string(args.command_arguments.at(0)), default_triplet, COMMAND_STRUCTURE.example_text, paths); diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 8aaea33132..3bc91fcf90 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -481,7 +481,7 @@ namespace vcpkg::Commands::CI const ParsedArguments options = args.parse_arguments(COMMAND_STRUCTURE); const auto& settings = options.settings; - BinaryCache binary_cache{args}; + BinaryCache binary_cache{args, paths}; Triplet target_triplet = Triplet::from_canonical_name(std::string(args.command_arguments[0])); ExclusionPredicate is_excluded{ parse_exclusions(settings, OPTION_EXCLUDE), @@ -556,7 +556,7 @@ namespace vcpkg::Commands::CI } auto action_plan = compute_full_plan(paths, provider, var_provider, all_default_full_specs, serialize_options); - const auto precheck_results = binary_cache.precheck(paths, action_plan.install_actions); + const auto precheck_results = binary_cache.precheck(action_plan.install_actions); auto split_specs = compute_action_statuses(is_excluded, var_provider, precheck_results, action_plan); { diff --git a/src/vcpkg/commands.setinstalled.cpp b/src/vcpkg/commands.setinstalled.cpp index 86da98a76c..b792014607 100644 --- a/src/vcpkg/commands.setinstalled.cpp +++ b/src/vcpkg/commands.setinstalled.cpp @@ -153,7 +153,7 @@ namespace vcpkg::Commands::SetInstalled std::string(arg), default_triplet, COMMAND_STRUCTURE.example_text, paths); }); - BinaryCache binary_cache{args}; + BinaryCache binary_cache{args, paths}; const bool dry_run = Util::Sets::contains(options.switches, OPTION_DRY_RUN); diff --git a/src/vcpkg/commands.upgrade.cpp b/src/vcpkg/commands.upgrade.cpp index 36e8f83903..bc28643b62 100644 --- a/src/vcpkg/commands.upgrade.cpp +++ b/src/vcpkg/commands.upgrade.cpp @@ -60,7 +60,7 @@ namespace vcpkg::Commands::Upgrade ? Dependencies::UnsupportedPortAction::Warn : Dependencies::UnsupportedPortAction::Error; - BinaryCache binary_cache{args}; + BinaryCache binary_cache{args, paths}; StatusParagraphs status_db = database_load_check(paths.get_filesystem(), paths.installed()); // Load ports from ports dirs diff --git a/src/vcpkg/install.cpp b/src/vcpkg/install.cpp index 9dc4c716ca..18938b1791 100644 --- a/src/vcpkg/install.cpp +++ b/src/vcpkg/install.cpp @@ -329,7 +329,7 @@ namespace vcpkg::Install if (plan_type == InstallPlanType::BUILD_AND_INSTALL) { std::unique_ptr bcf; - auto restore = binary_cache.try_restore(paths, action); + auto restore = binary_cache.try_restore(action); if (restore == RestoreResult::restored) { auto maybe_bcf = Paragraphs::try_load_cached_package(fs, paths.package_dir(action.spec), action.spec); @@ -481,7 +481,7 @@ namespace vcpkg::Install } Build::compute_all_abis(paths, action_plan, var_provider, status_db); - binary_cache.prefetch(paths, action_plan.install_actions); + binary_cache.prefetch(action_plan.install_actions); for (auto&& action : action_plan.install_actions) { TrackedPackageInstallGuard this_install(action_index++, action_count, results, action.spec); @@ -889,7 +889,7 @@ namespace vcpkg::Install BinaryCache binary_cache; if (!only_downloads) { - binary_cache.install_providers_for(args); + binary_cache.install_providers_for(args, paths); } auto& fs = paths.get_filesystem();