From 309f6fc9bcb48a68b692b2f4707b5fea7eaf1c60 Mon Sep 17 00:00:00 2001 From: Billy O'Neal Date: Fri, 26 Jun 2020 12:16:17 -0700 Subject: [PATCH] [vcpkg] Delete unused --purge-tombstones and introduce BufferedPrint class (#12049) * Introduce buffered_print class to manage buffered write patterns like in the ci command. * Remove --purge-tombstones option. * Law of demeter. * Make buffered_print imobile. * buffered_print => BufferedPrint * Fix merge conflict. --- toolsrc/include/vcpkg/base/system.print.h | 23 ++++ toolsrc/include/vcpkg/binarycaching.h | 3 +- toolsrc/src/vcpkg/binarycaching.cpp | 11 +- toolsrc/src/vcpkg/commands.ci.cpp | 137 ++++++++++------------ 4 files changed, 90 insertions(+), 84 deletions(-) diff --git a/toolsrc/include/vcpkg/base/system.print.h b/toolsrc/include/vcpkg/base/system.print.h index 890c13667..abe685cf5 100644 --- a/toolsrc/include/vcpkg/base/system.print.h +++ b/toolsrc/include/vcpkg/base/system.print.h @@ -41,4 +41,27 @@ namespace vcpkg::System { ::vcpkg::System::details::print(Strings::concat_or_view(args...)); } + + class BufferedPrint + { + ::std::string stdout_buffer; + static constexpr ::std::size_t buffer_size_target = 2048; + static constexpr ::std::size_t expected_maximum_print = 256; + static constexpr ::std::size_t alloc_size = buffer_size_target + expected_maximum_print; + + public: + BufferedPrint() { stdout_buffer.reserve(alloc_size); } + BufferedPrint(const BufferedPrint&) = delete; + BufferedPrint& operator=(const BufferedPrint&) = delete; + void append(::vcpkg::StringView nextView) + { + stdout_buffer.append(nextView.data(), nextView.size()); + if (stdout_buffer.size() > buffer_size_target) + { + ::vcpkg::System::details::print(stdout_buffer); + stdout_buffer.clear(); + } + } + ~BufferedPrint() { ::vcpkg::System::details::print(stdout_buffer); } + }; } diff --git a/toolsrc/include/vcpkg/binarycaching.h b/toolsrc/include/vcpkg/binarycaching.h index d343d6c42..61af79a3f 100644 --- a/toolsrc/include/vcpkg/binarycaching.h +++ b/toolsrc/include/vcpkg/binarycaching.h @@ -40,8 +40,7 @@ namespace vcpkg /// Requests the result of `try_restore()` without actually downloading the package. Used by CI to determine /// missing packages. virtual RestoreResult precheck(const VcpkgPaths& paths, - const Dependencies::InstallPlanAction& action, - bool purge_tombstones) = 0; + const Dependencies::InstallPlanAction& action) = 0; }; IBinaryProvider& null_binary_provider(); diff --git a/toolsrc/src/vcpkg/binarycaching.cpp b/toolsrc/src/vcpkg/binarycaching.cpp index aa674e2ca..86c9899e3 100644 --- a/toolsrc/src/vcpkg/binarycaching.cpp +++ b/toolsrc/src/vcpkg/binarycaching.cpp @@ -208,8 +208,7 @@ namespace } } RestoreResult precheck(const VcpkgPaths& paths, - const Dependencies::InstallPlanAction& action, - bool purge_tombstones) override + const Dependencies::InstallPlanAction& action) override { const auto& abi_tag = action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi; auto& fs = paths.get_filesystem(); @@ -231,11 +230,7 @@ namespace const fs::path archive_subpath = fs::u8path(abi_tag.substr(0, 2)) / fs::u8path(archive_name); const fs::path archive_tombstone_path = archives_root_dir / fs::u8path("fail") / archive_subpath; - if (purge_tombstones) - { - fs.remove(archive_tombstone_path, ec); // Ignore error - } - else if (fs.exists(archive_tombstone_path)) + if (fs.exists(archive_tombstone_path)) { if (action.build_options.fail_on_tombstone == Build::FailOnTombstone::YES) { @@ -607,7 +602,7 @@ namespace } void push_success(const VcpkgPaths&, const Dependencies::InstallPlanAction&) override {} void push_failure(const VcpkgPaths&, const std::string&, const PackageSpec&) override {} - RestoreResult precheck(const VcpkgPaths&, const Dependencies::InstallPlanAction&, bool) override + RestoreResult precheck(const VcpkgPaths&, const Dependencies::InstallPlanAction&) override { return RestoreResult::missing; } diff --git a/toolsrc/src/vcpkg/commands.ci.cpp b/toolsrc/src/vcpkg/commands.ci.cpp index af9611931..efc7bbc15 100644 --- a/toolsrc/src/vcpkg/commands.ci.cpp +++ b/toolsrc/src/vcpkg/commands.ci.cpp @@ -34,7 +34,6 @@ namespace vcpkg::Commands::CI static constexpr StringLiteral OPTION_DRY_RUN = "--dry-run"; static constexpr StringLiteral OPTION_EXCLUDE = "--exclude"; - static constexpr StringLiteral OPTION_PURGE_TOMBSTONES = "--purge-tombstones"; static constexpr StringLiteral OPTION_XUNIT = "--x-xunit"; static constexpr StringLiteral OPTION_RANDOMIZE = "--x-randomize"; @@ -43,10 +42,9 @@ namespace vcpkg::Commands::CI {OPTION_XUNIT, "File to output results in XUnit format (internal)"}, }}; - static constexpr std::array CI_SWITCHES = {{ + static constexpr std::array CI_SWITCHES = {{ {OPTION_DRY_RUN, "Print out plan without execution"}, {OPTION_RANDOMIZE, "Randomize the install order"}, - {OPTION_PURGE_TOMBSTONES, "Purge failure tombstones and retry building the ports"}, }}; const CommandStructure COMMAND_STRUCTURE = { @@ -256,7 +254,6 @@ namespace vcpkg::Commands::CI const PortFileProvider::PortFileProvider& provider, const CMakeVars::CMakeVarProvider& var_provider, const std::vector& specs, - const bool purge_tombstones, IBinaryProvider& binaryprovider) { auto ret = std::make_unique(); @@ -293,7 +290,7 @@ namespace vcpkg::Commands::CI std::vector install_specs; for (auto&& install_action : action_plan.install_actions) { - install_specs.emplace_back(FullPackageSpec{install_action.spec, install_action.feature_list}); + install_specs.emplace_back(install_action.spec, install_action.feature_list); } var_provider.load_tag_vars(install_specs, provider); @@ -306,81 +303,74 @@ namespace vcpkg::Commands::CI Build::compute_all_abis(paths, action_plan, var_provider, {}); - std::string stdout_buffer; - for (auto&& action : action_plan.install_actions) { - auto p = &action; - ret->abi_map.emplace(action.spec, action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); - ret->features.emplace(action.spec, action.feature_list); - if (auto scfl = p->source_control_file_location.get()) + vcpkg::System::BufferedPrint stdout_print; + for (auto&& action : action_plan.install_actions) { - auto emp = ret->default_feature_provider.emplace(p->spec.name(), *scfl); - emp.first->second.source_control_file->core_paragraph->default_features = p->feature_list; + auto p = &action; + ret->abi_map.emplace(action.spec, action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi); + ret->features.emplace(action.spec, action.feature_list); + if (auto scfl = p->source_control_file_location.get()) + { + auto emp = ret->default_feature_provider.emplace(p->spec.name(), *scfl); + emp.first->second.source_control_file->core_paragraph->default_features = p->feature_list; - p->build_options = build_options; - } + p->build_options = build_options; + } - auto precheck_result = binaryprovider.precheck(paths, action, purge_tombstones); - bool b_will_build = false; + auto precheck_result = binaryprovider.precheck(paths, action); + bool b_will_build = false; - std::string state; + std::string state; - if (Util::Sets::contains(exclusions, p->spec.name())) - { - state = "skip"; - ret->known.emplace(p->spec, BuildResult::EXCLUDED); - will_fail.emplace(p->spec); - } - else if (!supported_for_triplet(var_provider, p)) - { - // This treats unsupported ports as if they are excluded - // which means the ports dependent on it will be cascaded due to missing dependencies - // Should this be changed so instead it is a failure to depend on a unsupported port? - state = "n/a"; - ret->known.emplace(p->spec, BuildResult::EXCLUDED); - will_fail.emplace(p->spec); - } - else if (Util::any_of(p->package_dependencies, - [&](const PackageSpec& spec) { return Util::Sets::contains(will_fail, spec); })) - { - state = "cascade"; - ret->known.emplace(p->spec, BuildResult::CASCADED_DUE_TO_MISSING_DEPENDENCIES); - will_fail.emplace(p->spec); - } - else if (precheck_result == RestoreResult::success) - { - state = "pass"; - ret->known.emplace(p->spec, BuildResult::SUCCEEDED); - } - else if (precheck_result == RestoreResult::build_failed) - { - state = "fail"; - ret->known.emplace(p->spec, BuildResult::BUILD_FAILED); - will_fail.emplace(p->spec); - } - else - { - ret->unknown.push_back(FullPackageSpec{p->spec, {p->feature_list.begin(), p->feature_list.end()}}); - b_will_build = true; - } + if (Util::Sets::contains(exclusions, p->spec.name())) + { + state = "skip"; + ret->known.emplace(p->spec, BuildResult::EXCLUDED); + will_fail.emplace(p->spec); + } + else if (!supported_for_triplet(var_provider, p)) + { + // This treats unsupported ports as if they are excluded + // which means the ports dependent on it will be cascaded due to missing dependencies + // Should this be changed so instead it is a failure to depend on a unsupported port? + state = "n/a"; + ret->known.emplace(p->spec, BuildResult::EXCLUDED); + will_fail.emplace(p->spec); + } + else if (Util::any_of(p->package_dependencies, + [&](const PackageSpec& spec) { return Util::Sets::contains(will_fail, spec); })) + { + state = "cascade"; + ret->known.emplace(p->spec, BuildResult::CASCADED_DUE_TO_MISSING_DEPENDENCIES); + will_fail.emplace(p->spec); + } + else if (precheck_result == RestoreResult::success) + { + state = "pass"; + ret->known.emplace(p->spec, BuildResult::SUCCEEDED); + } + else if (precheck_result == RestoreResult::build_failed) + { + state = "fail"; + ret->known.emplace(p->spec, BuildResult::BUILD_FAILED); + will_fail.emplace(p->spec); + } + else + { + ret->unknown.emplace_back(p->spec, p->feature_list); + b_will_build = true; + } - Strings::append(stdout_buffer, - Strings::format("%40s: %1s %8s: %s\n", - p->spec, - (b_will_build ? "*" : " "), - state, - action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi)); - if (stdout_buffer.size() > 2048) - { - System::print2(stdout_buffer); - stdout_buffer.clear(); + stdout_print.append(Strings::format("%40s: %1s %8s: %s\n", + p->spec, + (b_will_build ? "*" : " "), + state, + action.abi_info.value_or_exit(VCPKG_LINE_INFO).package_abi)); } - } - System::print2(stdout_buffer); - stdout_buffer.clear(); + } // flush stdout_print System::printf("Time to determine pass/fail: %s\n", timer.elapsed()); - return ret; } @@ -411,7 +401,6 @@ namespace vcpkg::Commands::CI } const auto is_dry_run = Util::Sets::contains(options.switches, OPTION_DRY_RUN); - const auto purge_tombstones = Util::Sets::contains(options.switches, OPTION_PURGE_TOMBSTONES); std::vector triplets = Util::fmap( args.command_arguments, [](std::string s) { return Triplet::from_canonical_name(std::move(s)); }); @@ -469,7 +458,6 @@ namespace vcpkg::Commands::CI provider, var_provider, all_default_full_specs, - purge_tombstones, binaryprovider); PortFileProvider::MapPortFileProvider new_default_provider(split_specs->default_feature_provider); @@ -557,8 +545,9 @@ namespace vcpkg::Commands::CI result.summary.print(); } - auto it_xunit = options.settings.find(OPTION_XUNIT); - if (it_xunit != options.settings.end()) + auto& settings = options.settings; + auto it_xunit = settings.find(OPTION_XUNIT); + if (it_xunit != settings.end()) { paths.get_filesystem().write_contents( fs::u8path(it_xunit->second), xunitTestResults.build_xml(), VCPKG_LINE_INFO);