From 5de09ab24a74a2873480071f74dcc4938d76f8b9 Mon Sep 17 00:00:00 2001 From: Leander Schulten Date: Sun, 19 Jun 2022 01:23:48 +0200 Subject: [PATCH] don't use triplet file hash. Hash variables instead Fixes https://github.com/microsoft/vcpkg/issues/19984 --- include/vcpkg-test/mockcmakevarprovider.h | 15 ++- include/vcpkg/build.h | 5 +- include/vcpkg/cmakevars.h | 8 +- include/vcpkg/vcpkgpaths.h | 2 +- src/vcpkg-test/mockcmakevarsprovider.cpp | 7 ++ src/vcpkg/base/strings.cpp | 1 - src/vcpkg/build.cpp | 33 +++--- src/vcpkg/cmakevars.cpp | 118 +++++++++++++++++++--- src/vcpkg/vcpkgpaths.cpp | 4 +- 9 files changed, 148 insertions(+), 45 deletions(-) diff --git a/include/vcpkg-test/mockcmakevarprovider.h b/include/vcpkg-test/mockcmakevarprovider.h index 9b17662066..fbe7da78cc 100644 --- a/include/vcpkg-test/mockcmakevarprovider.h +++ b/include/vcpkg-test/mockcmakevarprovider.h @@ -18,12 +18,18 @@ namespace vcpkg::Test dep_info_vars.emplace(spec, SMap{}); } - void load_tag_vars(Span specs, - const PortFileProvider::PortFileProvider& port_provider, - Triplet host_triplet) const override + void load_tag_vars_and_triplet_hash(Span specs, + const PortFileProvider::PortFileProvider& port_provider, + Triplet host_triplet) const override { for (auto&& spec : specs) + { tag_vars.emplace(spec.package_spec, SMap{}); + triplet_hashs.emplace( + spec.package_spec, + Strings::concat("fake hash for ", spec.package_spec.triplet(), " host:", host_triplet)); + } + (void)(port_provider); (void)(host_triplet); } @@ -37,8 +43,11 @@ namespace vcpkg::Test Optional&> get_tag_vars( const PackageSpec& spec) const override; + Optional get_triplet_hash(const PackageSpec& spec) const override; + mutable std::unordered_map> dep_info_vars; mutable std::unordered_map> tag_vars; + mutable std::unordered_map triplet_hashs; mutable std::unordered_map> generic_triplet_vars; }; } diff --git a/include/vcpkg/build.h b/include/vcpkg/build.h index c37102bbd1..25fcc843f6 100644 --- a/include/vcpkg/build.h +++ b/include/vcpkg/build.h @@ -386,13 +386,12 @@ namespace vcpkg::Build explicit EnvCache(bool compiler_tracking) : m_compiler_tracking(compiler_tracking) { } const Environment& get_action_env(const VcpkgPaths& paths, const AbiInfo& abi_info); - const std::string& get_triplet_info(const VcpkgPaths& paths, const AbiInfo& abi_info); + const std::string& get_toolchain_abi(const VcpkgPaths& paths, const AbiInfo& abi_info); const CompilerInfo& get_compiler_info(const VcpkgPaths& paths, const AbiInfo& abi_info); private: struct TripletMapEntry { - std::string hash; Cache triplet_infos; Cache triplet_infos_without_compiler; Cache compiler_info; @@ -400,7 +399,7 @@ namespace vcpkg::Build Cache m_triplet_cache; Cache m_toolchain_cache; - const TripletMapEntry& get_triplet_cache(const Filesystem& fs, const Path& p); + const TripletMapEntry& get_triplet_cache(const Path& p); #if defined(_WIN32) struct EnvMapEntry diff --git a/include/vcpkg/cmakevars.h b/include/vcpkg/cmakevars.h index f6b182a4f3..107f3de09e 100644 --- a/include/vcpkg/cmakevars.h +++ b/include/vcpkg/cmakevars.h @@ -27,13 +27,15 @@ namespace vcpkg::CMakeVars virtual Optional&> get_tag_vars( const PackageSpec& spec) const = 0; + virtual Optional get_triplet_hash(const PackageSpec& spec) const = 0; + virtual void load_generic_triplet_vars(Triplet triplet) const = 0; virtual void load_dep_info_vars(Span specs, Triplet host_triplet) const = 0; - virtual void load_tag_vars(Span specs, - const PortFileProvider::PortFileProvider& port_provider, - Triplet host_triplet) const = 0; + virtual void load_tag_vars_and_triplet_hash(Span specs, + const PortFileProvider::PortFileProvider& port_provider, + Triplet host_triplet) const = 0; void load_tag_vars(const vcpkg::Dependencies::ActionPlan& action_plan, const PortFileProvider::PortFileProvider& port_provider, diff --git a/include/vcpkg/vcpkgpaths.h b/include/vcpkg/vcpkgpaths.h index 4f0c1e1474..fd7eed632a 100644 --- a/include/vcpkg/vcpkgpaths.h +++ b/include/vcpkg/vcpkgpaths.h @@ -166,7 +166,7 @@ namespace vcpkg Filesystem& get_filesystem() const; const Environment& get_action_env(const Build::AbiInfo& abi_info) const; - const std::string& get_triplet_info(const Build::AbiInfo& abi_info) const; + const std::string& get_toolchain_abi(const Build::AbiInfo& abi_info) const; const Build::CompilerInfo& get_compiler_info(const Build::AbiInfo& abi_info) const; bool manifest_mode_enabled() const { return get_manifest().has_value(); } diff --git a/src/vcpkg-test/mockcmakevarsprovider.cpp b/src/vcpkg-test/mockcmakevarsprovider.cpp index 77a796fade..0ffab2f06e 100644 --- a/src/vcpkg-test/mockcmakevarsprovider.cpp +++ b/src/vcpkg-test/mockcmakevarsprovider.cpp @@ -25,4 +25,11 @@ namespace vcpkg::Test if (it == tag_vars.end()) return nullopt; return it->second; } + + Optional MockCMakeVarProvider::get_triplet_hash(const PackageSpec& spec) const + { + auto it = triplet_hashs.find(spec); + if (it == triplet_hashs.end()) return nullopt; + return it->second; + } } diff --git a/src/vcpkg/base/strings.cpp b/src/vcpkg/base/strings.cpp index dbfaeb0645..ef3cb5b1c1 100644 --- a/src/vcpkg/base/strings.cpp +++ b/src/vcpkg/base/strings.cpp @@ -20,7 +20,6 @@ using namespace vcpkg; namespace { - DECLARE_AND_REGISTER_MESSAGE(InvalidFormatString, (msg::actual), "{actual} is the provided format string", diff --git a/src/vcpkg/build.cpp b/src/vcpkg/build.cpp index d573985af8..625b0df4bc 100644 --- a/src/vcpkg/build.cpp +++ b/src/vcpkg/build.cpp @@ -669,11 +669,9 @@ namespace vcpkg::Build }); } - const EnvCache::TripletMapEntry& EnvCache::get_triplet_cache(const Filesystem& fs, const Path& p) + const EnvCache::TripletMapEntry& EnvCache::get_triplet_cache(const Path& p) { - return m_triplet_cache.get_lazy(p, [&]() -> TripletMapEntry { - return TripletMapEntry{Hash::get_file_hash(fs, p, Hash::Algorithm::Sha256).value_or_exit(VCPKG_LINE_INFO)}; - }); + return m_triplet_cache.get_lazy(p, [&]() -> TripletMapEntry { return TripletMapEntry{}; }); } const CompilerInfo& EnvCache::get_compiler_info(const VcpkgPaths& paths, const AbiInfo& abi_info) @@ -691,7 +689,7 @@ namespace vcpkg::Build auto&& toolchain_hash = get_toolchain_cache(m_toolchain_cache, abi_info.pre_build_info->toolchain_file(), fs); - auto&& triplet_entry = get_triplet_cache(fs, triplet_file_path); + auto&& triplet_entry = get_triplet_cache(triplet_file_path); return triplet_entry.compiler_info.get_lazy(toolchain_hash, [&]() -> CompilerInfo { if (m_compiler_tracking) @@ -705,7 +703,7 @@ namespace vcpkg::Build }); } - const std::string& EnvCache::get_triplet_info(const VcpkgPaths& paths, const AbiInfo& abi_info) + const std::string& EnvCache::get_toolchain_abi(const VcpkgPaths& paths, const AbiInfo& abi_info) { const auto& fs = paths.get_filesystem(); Checks::check_exit(VCPKG_LINE_INFO, abi_info.pre_build_info != nullptr); @@ -713,20 +711,19 @@ namespace vcpkg::Build auto&& toolchain_hash = get_toolchain_cache(m_toolchain_cache, abi_info.pre_build_info->toolchain_file(), fs); - auto&& triplet_entry = get_triplet_cache(fs, triplet_file_path); + auto&& triplet_entry = get_triplet_cache(triplet_file_path); if (m_compiler_tracking && !abi_info.pre_build_info->disable_compiler_tracking) { return triplet_entry.triplet_infos.get_lazy(toolchain_hash, [&]() -> std::string { auto& compiler_info = get_compiler_info(paths, abi_info); - return Strings::concat(triplet_entry.hash, '-', toolchain_hash, '-', compiler_info.hash); + return Strings::concat(toolchain_hash, '-', compiler_info.hash); }); } else { - return triplet_entry.triplet_infos_without_compiler.get_lazy(toolchain_hash, [&]() -> std::string { - return Strings::concat(triplet_entry.hash, '-', toolchain_hash); - }); + return triplet_entry.triplet_infos_without_compiler.get_lazy( + toolchain_hash, [&]() -> std::string { return Strings::concat(toolchain_hash); }); } } @@ -1221,7 +1218,7 @@ namespace vcpkg::Build struct AbiTagAndFiles { - const std::string* triplet_abi; + std::string triplet_abi; std::string tag; Path tag_file; @@ -1232,6 +1229,7 @@ namespace vcpkg::Build static Optional compute_abi_tag(const VcpkgPaths& paths, const Dependencies::InstallPlanAction& action, + const CMakeVars::CMakeVarProvider& var_provider, Span dependency_abis) { auto& fs = paths.get_filesystem(); @@ -1263,9 +1261,10 @@ namespace vcpkg::Build std::vector abi_tag_entries(dependency_abis.begin(), dependency_abis.end()); const auto& abi_info = action.abi_info.value_or_exit(VCPKG_LINE_INFO); - const auto& triplet_abi = paths.get_triplet_info(abi_info); - abi_tag_entries.emplace_back("triplet", triplet.canonical_name()); + const auto& toolchain_abi = paths.get_toolchain_abi(abi_info); + std::string triplet_abi = var_provider.get_triplet_hash(action.spec).value_or_exit(VCPKG_LINE_INFO); abi_tag_entries.emplace_back("triplet_abi", triplet_abi); + abi_tag_entries.emplace_back("toolchain_abi", toolchain_abi); abi_entries_from_abi_info(abi_info, abi_tag_entries); // If there is an unusually large number of files in the port then @@ -1368,7 +1367,7 @@ namespace vcpkg::Build fs.write_contents(abi_file_path, full_abi_info, VCPKG_LINE_INFO); return AbiTagAndFiles{ - &triplet_abi, + triplet_abi, Hash::get_file_hash(fs, abi_file_path, Hash::Algorithm::Sha256).value_or_exit(VCPKG_LINE_INFO), abi_file_path, std::move(files), @@ -1430,11 +1429,11 @@ namespace vcpkg::Build paths, action.spec.triplet(), var_provider.get_tag_vars(action.spec).value_or_exit(VCPKG_LINE_INFO)); abi_info.toolset = paths.get_toolset(*abi_info.pre_build_info); - auto maybe_abi_tag_and_file = compute_abi_tag(paths, action, dependency_abis); + auto maybe_abi_tag_and_file = compute_abi_tag(paths, action, var_provider, dependency_abis); if (auto p = maybe_abi_tag_and_file.get()) { abi_info.compiler_info = paths.get_compiler_info(abi_info); - abi_info.triplet_abi = *p->triplet_abi; + abi_info.triplet_abi = std::move(p->triplet_abi); abi_info.package_abi = std::move(p->tag); abi_info.abi_tag_file = std::move(p->tag_file); abi_info.relative_port_files = std::move(p->files); diff --git a/src/vcpkg/cmakevars.cpp b/src/vcpkg/cmakevars.cpp index 415cefa47a..997326113c 100644 --- a/src/vcpkg/cmakevars.cpp +++ b/src/vcpkg/cmakevars.cpp @@ -26,7 +26,7 @@ namespace vcpkg::CMakeVars install_package_specs.emplace_back(FullPackageSpec{action.spec, action.feature_list}); } - load_tag_vars(install_package_specs, port_provider, host_triplet); + load_tag_vars_and_triplet_hash(install_package_specs, port_provider, host_triplet); } const std::unordered_map& CMakeVarProvider::get_or_load_dep_info_vars( @@ -53,9 +53,9 @@ namespace vcpkg::CMakeVars void load_dep_info_vars(View specs, Triplet host_triplet) const override; - void load_tag_vars(View specs, - const PortFileProvider::PortFileProvider& port_provider, - Triplet host_triplet) const override; + void load_tag_vars_and_triplet_hash(View specs, + const PortFileProvider::PortFileProvider& port_provider, + Triplet host_triplet) const override; Optional&> get_generic_triplet_vars( Triplet triplet) const override; @@ -66,18 +66,22 @@ namespace vcpkg::CMakeVars Optional&> get_tag_vars( const PackageSpec& spec) const override; - public: + Optional get_triplet_hash(const PackageSpec& spec) const override; + + private: Path create_tag_extraction_file( const View> spec_abi_settings) const; Path create_dep_info_extraction_file(const View specs) const; void launch_and_split(const Path& script_path, - std::vector>>& vars) const; + std::vector>>& vars, + Optional&> triplet_hashes) const; const VcpkgPaths& paths; mutable std::unordered_map> dep_resolution_vars; mutable std::unordered_map> tag_vars; + mutable std::unordered_map triplet_hashes; mutable std::unordered_map> generic_triplet_vars; }; } @@ -113,8 +117,23 @@ namespace vcpkg::CMakeVars Strings::append(extraction_file, R"( set(CMAKE_CURRENT_LIST_FILE "${_vcpkg_triplet_file_BACKUP_CURRENT_LIST_FILE}") +unset(_vcpkg_triplet_file_BACKUP_CURRENT_LIST_FILE) get_filename_component(CMAKE_CURRENT_LIST_DIR "${CMAKE_CURRENT_LIST_FILE}" DIRECTORY) endmacro() + +macro(print_variables start_uuid) + get_cmake_property(z_vcpkg_variable_names VARIABLES) + set(z_vcpkg_output "") + foreach (z_vcpkg_variable_name ${z_vcpkg_variable_names}) + string(APPEND z_vcpkg_output "${z_vcpkg_variable_name}=${${z_vcpkg_variable_name}}56732a63-363f-4326-8eac-738ed6390780") + endforeach() + string(REPLACE "\r" "\\r" z_vcpkg_output "${z_vcpkg_output}") + string(REPLACE "\n" "\\n" z_vcpkg_output "${z_vcpkg_output}") + message("${start_uuid}${z_vcpkg_output}") + unset(z_vcpkg_variable_names) + unset(z_vcpkg_variable_name) + unset(z_vcpkg_output) +endmacro() )"); return extraction_file; } @@ -137,7 +156,9 @@ endmacro() function(vcpkg_get_tags PORT FEATURES VCPKG_TRIPLET_ID VCPKG_ABI_SETTINGS_FILE) message("d8187afd-ea4a-4fc3-9aa4-a6782e1ed9af") + print_variables("15fe3605-2429-46b9-b6b1-c5c008621f79") vcpkg_triplet_file(${VCPKG_TRIPLET_ID}) + print_variables("9dbc61d9-e274-40b4-b38a-4d2d3fe91341") # GUID used as a flag - "cut here line" message("c35112b6-d1ba-415b-aa5d-81de856ef8eb @@ -255,14 +276,19 @@ endfunction() "{command_line}\n" "failed with the following results:"); - void TripletCMakeVarProvider::launch_and_split( - const Path& script_path, std::vector>>& vars) const + void TripletCMakeVarProvider::launch_and_split(const Path& script_path, + std::vector>>& vars, + Optional&> triplet_hashes) const { static constexpr StringLiteral PORT_START_GUID = "d8187afd-ea4a-4fc3-9aa4-a6782e1ed9af"; static constexpr StringLiteral PORT_END_GUID = "8c504940-be29-4cba-9f8f-6cd83e9d87b7"; static constexpr StringLiteral BLOCK_START_GUID = "c35112b6-d1ba-415b-aa5d-81de856ef8eb"; static constexpr StringLiteral BLOCK_END_GUID = "e1e74b5c-18cb-4474-a6bd-5c1c8bc81f3f"; + static constexpr StringLiteral PREVIOUS_VARIABLES_GUID = "15fe3605-2429-46b9-b6b1-c5c008621f79"; + static constexpr StringLiteral AFTER_VARIABLES_GUID = "9dbc61d9-e274-40b4-b38a-4d2d3fe91341"; + static std::string VARIABLE_END_GUID = "56732a63-363f-4326-8eac-738ed6390780"; + const auto cmd_launch_cmake = vcpkg::make_cmake_cmd(paths, script_path, {}); std::vector lines; @@ -288,7 +314,7 @@ endfunction() Checks::check_exit(VCPKG_LINE_INFO, port_start != end && port_end != end, "Failed to parse CMake console output to locate port start/end markers"); - + auto hasher = Hash::get_hasher_for(Hash::Algorithm::Sha256); for (auto var_itr = vars.begin(); port_start != end && var_itr != vars.end(); ++var_itr) { auto block_start = std::find(port_start, port_end, BLOCK_START_GUID); @@ -318,6 +344,51 @@ endfunction() block_end = std::find(block_start, port_end, BLOCK_END_GUID); } + if (triplet_hashes) + { + auto parse_variables = [port_start, port_end](StringView start_guid, auto callback) { + auto variables_line = std::find_if(port_start, port_end, [start_guid](auto& line) { + return Strings::starts_with(line, start_guid); + }); + Checks::check_exit(VCPKG_LINE_INFO, + variables_line != port_end, + "Failed to parse CMake console output to locate variables start marker %s", + start_guid); + size_t start = start_guid.size(); + while (start < variables_line->size()) + { + auto equal_sign = variables_line->find('=', start); + Checks::check_exit( + VCPKG_LINE_INFO, + equal_sign != std::string::npos, + "Expected format is {start_maker}(VARIABLE_NAME=VARIABLE_VALUE{end_marker})+, " + "but was [%s] (start is %d)", + *variables_line, + start); + auto end = variables_line->find(VARIABLE_END_GUID, equal_sign); + Checks::check_exit( + VCPKG_LINE_INFO, + end != std::string::npos, + "Expected format is {start_maker}(VARIABLE_NAME=VARIABLE_VALUE{end_marker})+, but was [%s]", + *variables_line); + callback(StringView(variables_line->c_str() + start, equal_sign - start), + StringView(variables_line->c_str() + equal_sign + 1, end - equal_sign - 1)); + start = end + VARIABLE_END_GUID.size(); + } + }; + std::map previous_variables; + parse_variables(PREVIOUS_VARIABLES_GUID, + [&](auto key, auto value) { previous_variables.emplace(key, value); }); + hasher->clear(); + parse_variables(AFTER_VARIABLES_GUID, [&](StringView key, StringView value) { + auto it = previous_variables.find(key); + if (it != previous_variables.end() && it->second == value) return; + hasher->add_bytes(key.begin(), key.end()); + hasher->add_bytes(value.begin(), value.end()); + }); + triplet_hashes.get()->push_back(hasher->get_hash()); + } + port_start = std::find(port_end, end, PORT_START_GUID); port_end = std::find(port_start, end, PORT_END_GUID); } @@ -330,7 +401,7 @@ endfunction() FullPackageSpec full_spec({"", triplet}, {}); const auto file_path = create_tag_extraction_file(std::array, 1>{ std::pair{&full_spec, ""}}); - launch_and_split(file_path, vars); + launch_and_split(file_path, vars, nullopt); paths.get_filesystem().remove(file_path, VCPKG_LINE_INFO); generic_triplet_vars[triplet].insert(std::make_move_iterator(vars.front().begin()), @@ -346,7 +417,7 @@ endfunction() { print2("Loading dependency information for ", specs.size(), " packages...\n"); } - launch_and_split(file_path, vars); + launch_and_split(file_path, vars, nullopt); paths.get_filesystem().remove(file_path, VCPKG_LINE_INFO); auto var_list_itr = vars.begin(); @@ -362,9 +433,10 @@ endfunction() } } - void TripletCMakeVarProvider::load_tag_vars(View specs, - const PortFileProvider::PortFileProvider& port_provider, - Triplet host_triplet) const + void TripletCMakeVarProvider::load_tag_vars_and_triplet_hash( + View specs, + const PortFileProvider::PortFileProvider& port_provider, + Triplet host_triplet) const { if (specs.size() == 0) return; std::vector> spec_abi_settings; @@ -378,11 +450,14 @@ endfunction() } std::vector>> vars(spec_abi_settings.size()); + std::vector triplet_hashes_vec; + triplet_hashes_vec.reserve(spec_abi_settings.size()); const auto file_path = create_tag_extraction_file(spec_abi_settings); - launch_and_split(file_path, vars); + launch_and_split(file_path, vars, triplet_hashes_vec); paths.get_filesystem().remove(file_path, VCPKG_LINE_INFO); auto var_list_itr = vars.begin(); + auto triplet_hashes_iter = triplet_hashes_vec.begin(); for (const auto& spec_abi_setting : spec_abi_settings) { const FullPackageSpec& spec = *spec_abi_setting.first; @@ -393,6 +468,8 @@ endfunction() ctxt.emplace("Z_VCPKG_IS_NATIVE", host_triplet == spec.package_spec.triplet() ? "1" : "0"); tag_vars.emplace(spec.package_spec, std::move(ctxt)); + triplet_hashes.emplace(spec.package_spec, std::move(*triplet_hashes_iter)); + ++triplet_hashes_iter; } } @@ -431,4 +508,15 @@ endfunction() return nullopt; } + + Optional TripletCMakeVarProvider::get_triplet_hash(const PackageSpec& spec) const + { + auto find_itr = triplet_hashes.find(spec); + if (find_itr != triplet_hashes.end()) + { + return find_itr->second; + } + + return nullopt; + } } diff --git a/src/vcpkg/vcpkgpaths.cpp b/src/vcpkg/vcpkgpaths.cpp index 958a2f0e0a..3287c8c316 100644 --- a/src/vcpkg/vcpkgpaths.cpp +++ b/src/vcpkg/vcpkgpaths.cpp @@ -1414,9 +1414,9 @@ namespace vcpkg return m_pimpl->m_env_cache.get_action_env(*this, abi_info); } - const std::string& VcpkgPaths::get_triplet_info(const Build::AbiInfo& abi_info) const + const std::string& VcpkgPaths::get_toolchain_abi(const Build::AbiInfo& abi_info) const { - return m_pimpl->m_env_cache.get_triplet_info(*this, abi_info); + return m_pimpl->m_env_cache.get_toolchain_abi(*this, abi_info); } const Build::CompilerInfo& VcpkgPaths::get_compiler_info(const Build::AbiInfo& abi_info) const