From 468e9e70e644eb26258434c9e27e34935eb3e06d Mon Sep 17 00:00:00 2001 From: Robert Schumacher Date: Thu, 16 Nov 2017 19:29:32 -0800 Subject: [PATCH] [vcpkg] Refactor to remove Build::BuildResults -- too similar to ExtendedBuildResult --- toolsrc/include/vcpkg/base/util.h | 15 ++++++ toolsrc/include/vcpkg/build.h | 28 ++++------ toolsrc/include/vcpkg/install.h | 20 ++----- toolsrc/src/vcpkg/build.cpp | 66 +++++++++++++++-------- toolsrc/src/vcpkg/commands.ci.cpp | 8 ++- toolsrc/src/vcpkg/install.cpp | 90 +++++++++++++------------------ 6 files changed, 115 insertions(+), 112 deletions(-) diff --git a/toolsrc/include/vcpkg/base/util.h b/toolsrc/include/vcpkg/base/util.h index 8d78fd6cc..6c05a3a9e 100644 --- a/toolsrc/include/vcpkg/base/util.h +++ b/toolsrc/include/vcpkg/base/util.h @@ -190,4 +190,19 @@ namespace vcpkg::Util std::unique_lock m_lock; T& m_ptr; }; + + namespace Enum + { + template + E to_enum(bool b) + { + return b ? E::YES : E::NO; + } + + template + bool to_bool(E e) + { + return e == E::YES; + } + } } diff --git a/toolsrc/include/vcpkg/build.h b/toolsrc/include/vcpkg/build.h index cbd34c730..1f6782ccf 100644 --- a/toolsrc/include/vcpkg/build.h +++ b/toolsrc/include/vcpkg/build.h @@ -32,30 +32,23 @@ namespace vcpkg::Build YES }; - inline UseHeadVersion to_use_head_version(const bool value) - { - return value ? UseHeadVersion::YES : UseHeadVersion::NO; - } - - inline bool to_bool(const UseHeadVersion value) { return value == UseHeadVersion::YES; } - enum class AllowDownloads { NO = 0, YES }; - inline AllowDownloads to_allow_downloads(const bool value) + enum class CleanBuildtrees { - return value ? AllowDownloads::YES : AllowDownloads::NO; - } - - inline bool to_bool(const AllowDownloads value) { return value == AllowDownloads::YES; } + NO = 0, + YES + }; struct BuildPackageOptions { UseHeadVersion use_head_version; AllowDownloads allow_downloads; + CleanBuildtrees clean_buildtrees; }; enum class BuildResult @@ -69,12 +62,6 @@ namespace vcpkg::Build EXCLUDED, }; - struct BuildResults - { - BuildResult result_code; - std::unique_ptr binary_control_file; - }; - static constexpr std::array BUILD_RESULT_VALUES = { BuildResult::SUCCEEDED, BuildResult::BUILD_FAILED, @@ -108,8 +95,13 @@ namespace vcpkg::Build struct ExtendedBuildResult { + ExtendedBuildResult(BuildResult code); + ExtendedBuildResult(BuildResult code, std::vector&& unmet_deps); + ExtendedBuildResult(BuildResult code, std::unique_ptr&& bcf); + BuildResult code; std::vector unmet_dependencies; + std::unique_ptr binary_control_file; }; struct BuildPackageConfig diff --git a/toolsrc/include/vcpkg/install.h b/toolsrc/include/vcpkg/install.h index e436e2238..28896adee 100644 --- a/toolsrc/include/vcpkg/install.h +++ b/toolsrc/include/vcpkg/install.h @@ -17,17 +17,6 @@ namespace vcpkg::Install inline KeepGoing to_keep_going(const bool value) { return value ? KeepGoing::YES : KeepGoing::NO; } - enum class CleanBuildtrees - { - NO = 0, - YES - }; - - inline CleanBuildtrees to_clean_buildtrees(const bool value) - { - return value ? CleanBuildtrees::YES : CleanBuildtrees::NO; - } - struct SpecSummary { SpecSummary(const PackageSpec& spec, const Dependencies::AnyAction* action); @@ -35,7 +24,7 @@ namespace vcpkg::Install const BinaryParagraph* get_binary_paragraph() const; PackageSpec spec; - Build::BuildResults build_result; + Build::ExtendedBuildResult build_result; std::string timing; const Dependencies::AnyAction* action; @@ -66,9 +55,9 @@ namespace vcpkg::Install const fs::path& listfile() const; }; - Build::BuildResults perform_install_plan_action(const VcpkgPaths& paths, - const Dependencies::InstallPlanAction& action, - StatusParagraphs& status_db); + Build::ExtendedBuildResult perform_install_plan_action(const VcpkgPaths& paths, + const Dependencies::InstallPlanAction& action, + StatusParagraphs& status_db); enum class InstallResult { @@ -85,7 +74,6 @@ namespace vcpkg::Install InstallSummary perform(const std::vector& action_plan, const KeepGoing keep_going, - const CleanBuildtrees clean_buildtrees, const VcpkgPaths& paths, StatusParagraphs& status_db); diff --git a/toolsrc/src/vcpkg/build.cpp b/toolsrc/src/vcpkg/build.cpp index b0ca3501b..e3787a97e 100644 --- a/toolsrc/src/vcpkg/build.cpp +++ b/toolsrc/src/vcpkg/build.cpp @@ -220,26 +220,25 @@ namespace vcpkg::Build tonull); } - static void create_binary_feature_control_file(const SourceParagraph& source_paragraph, - const FeatureParagraph& feature_paragraph, - const Triplet& triplet, - BinaryControlFile& bcf) + static BinaryParagraph create_binary_feature_control_file(const SourceParagraph& source_paragraph, + const FeatureParagraph& feature_paragraph, + const Triplet& triplet) { - BinaryParagraph bpgh(source_paragraph, feature_paragraph, triplet); - bcf.features.emplace_back(std::move(bpgh)); + return BinaryParagraph(source_paragraph, feature_paragraph, triplet); } - static void create_binary_control_file(const SourceParagraph& source_paragraph, - const Triplet& triplet, - const BuildInfo& build_info, - BinaryControlFile& bcf) + static std::unique_ptr create_binary_control_file(const SourceParagraph& source_paragraph, + const Triplet& triplet, + const BuildInfo& build_info) { + auto bcf = std::make_unique(); BinaryParagraph bpgh(source_paragraph, triplet); if (const auto p_ver = build_info.version.get()) { bpgh.version = *p_ver; } - bcf.core_paragraph = std::move(bpgh); + bcf->core_paragraph = std::move(bpgh); + return bcf; } static void write_binary_control_file(const VcpkgPaths& paths, BinaryControlFile bcf) @@ -311,8 +310,9 @@ namespace vcpkg::Build {"CURRENT_PORT_DIR", config.port_dir / "/."}, {"TARGET_TRIPLET", triplet.canonical_name()}, {"VCPKG_PLATFORM_TOOLSET", toolset.version.c_str()}, - {"VCPKG_USE_HEAD_VERSION", to_bool(config.build_package_options.use_head_version) ? "1" : "0"}, - {"_VCPKG_NO_DOWNLOADS", !to_bool(config.build_package_options.allow_downloads) ? "1" : "0"}, + {"VCPKG_USE_HEAD_VERSION", + Util::Enum::to_bool(config.build_package_options.use_head_version) ? "1" : "0"}, + {"_VCPKG_NO_DOWNLOADS", !Util::Enum::to_bool(config.build_package_options.allow_downloads) ? "1" : "0"}, {"GIT", git_exe_path}, {"FEATURES", features}, }); @@ -333,20 +333,18 @@ namespace vcpkg::Build { locked_metrics->track_property("error", "build failed"); locked_metrics->track_property("build_error", spec_string); - return {BuildResult::BUILD_FAILED, {}}; + return BuildResult::BUILD_FAILED; } } const BuildInfo build_info = read_build_info(paths.get_filesystem(), paths.build_info_file_path(spec)); const size_t error_count = PostBuildLint::perform_all_checks(spec, paths, pre_build_info, build_info); - BinaryControlFile bcf; - - create_binary_control_file(config.src, triplet, build_info, bcf); + auto bcf = create_binary_control_file(config.src, triplet, build_info); if (error_count != 0) { - return {BuildResult::POST_BUILD_CHECKS_FAILED, {}}; + return BuildResult::POST_BUILD_CHECKS_FAILED; } if (GlobalState::feature_packages) { @@ -357,18 +355,31 @@ namespace vcpkg::Build for (auto&& f_pgh : config.scf->feature_paragraphs) { if (f_pgh->name == feature) - create_binary_feature_control_file(*config.scf->core_paragraph, *f_pgh, triplet, bcf); + bcf->features.push_back( + create_binary_feature_control_file(*config.scf->core_paragraph, *f_pgh, triplet)); } } } } - write_binary_control_file(paths, bcf); + write_binary_control_file(paths, *bcf); - // const fs::path port_buildtrees_dir = paths.buildtrees / spec.name; - // delete_directory(port_buildtrees_dir); + if (config.build_package_options.clean_buildtrees == CleanBuildtrees::YES) + { + auto& fs = paths.get_filesystem(); + auto buildtrees_dir = paths.buildtrees / spec.name(); + auto buildtree_files = fs.get_files_non_recursive(buildtrees_dir); + for (auto&& file : buildtree_files) + { + if (fs.is_directory(file) && file.filename() != "src") + { + std::error_code ec; + fs.remove_all(file, ec); + } + } + } - return {BuildResult::SUCCEEDED, {}}; + return {BuildResult::SUCCEEDED, std::move(bcf)}; } const std::string& to_string(const BuildResult build_result) @@ -549,4 +560,13 @@ namespace vcpkg::Build return pre_build_info; } + ExtendedBuildResult::ExtendedBuildResult(BuildResult code) : code(code) {} + ExtendedBuildResult::ExtendedBuildResult(BuildResult code, std::unique_ptr&& bcf) + : code(code), binary_control_file(std::move(bcf)) + { + } + ExtendedBuildResult::ExtendedBuildResult(BuildResult code, std::vector&& unmet_deps) + : code(code), unmet_dependencies(std::move(unmet_deps)) + { + } } diff --git a/toolsrc/src/vcpkg/commands.ci.cpp b/toolsrc/src/vcpkg/commands.ci.cpp index 3c1c443f0..8f79b83e1 100644 --- a/toolsrc/src/vcpkg/commands.ci.cpp +++ b/toolsrc/src/vcpkg/commands.ci.cpp @@ -41,7 +41,11 @@ namespace vcpkg::Commands::CI Checks::check_exit(VCPKG_LINE_INFO, !install_plan.empty(), "Install plan cannot be empty"); - const Build::BuildPackageOptions install_plan_options = {Build::UseHeadVersion::NO, Build::AllowDownloads::YES}; + const Build::BuildPackageOptions install_plan_options = { + Build::UseHeadVersion::NO, + Build::AllowDownloads::YES, + Build::CleanBuildtrees::YES, + }; const std::vector action_plan = Util::fmap(install_plan, [&install_plan_options](InstallPlanAction& install_action) { @@ -49,7 +53,7 @@ namespace vcpkg::Commands::CI return Dependencies::AnyAction(std::move(install_action)); }); - return Install::perform(action_plan, Install::KeepGoing::YES, Install::CleanBuildtrees::YES, paths, status_db); + return Install::perform(action_plan, Install::KeepGoing::YES, paths, status_db); } struct TripletAndSummary diff --git a/toolsrc/src/vcpkg/install.cpp b/toolsrc/src/vcpkg/install.cpp index 7edcafb1f..cc006811b 100644 --- a/toolsrc/src/vcpkg/install.cpp +++ b/toolsrc/src/vcpkg/install.cpp @@ -247,11 +247,11 @@ namespace vcpkg::Install } using Build::BuildResult; - using Build::BuildResults; + using Build::ExtendedBuildResult; - BuildResults perform_install_plan_action(const VcpkgPaths& paths, - const InstallPlanAction& action, - StatusParagraphs& status_db) + ExtendedBuildResult perform_install_plan_action(const VcpkgPaths& paths, + const InstallPlanAction& action, + StatusParagraphs& status_db) { const InstallPlanType& plan_type = action.plan_type; const std::string display_name = action.spec.to_string(); @@ -259,7 +259,7 @@ namespace vcpkg::Install GlobalState::feature_packages ? action.displayname() : display_name; const bool is_user_requested = action.request_type == RequestType::USER_REQUESTED; - const bool use_head_version = to_bool(action.build_options.use_head_version); + const bool use_head_version = Util::Enum::to_bool(action.build_options.use_head_version); if (plan_type == InstallPlanType::ALREADY_INSTALLED) { @@ -268,9 +268,22 @@ namespace vcpkg::Install System::Color::warning, "Package %s is already installed -- not building from HEAD", display_name); else System::println(System::Color::success, "Package %s is already installed", display_name); - return {BuildResult::SUCCEEDED, nullptr}; + return BuildResult::SUCCEEDED; } + auto aux_install = [&](const std::string& name, const BinaryControlFile& bcf) -> BuildResult { + System::println("Installing package %s... ", name); + const auto install_result = install_package(paths, bcf, &status_db); + switch (install_result) + { + case InstallResult::SUCCESS: + System::println(System::Color::success, "Installing package %s... done", name); + return BuildResult::SUCCEEDED; + case InstallResult::FILE_CONFLICTS: return BuildResult::FILE_CONFLICTS; + default: Checks::unreachable(VCPKG_LINE_INFO); + } + }; + if (plan_type == InstallPlanType::BUILD_AND_INSTALL) { if (use_head_version) @@ -278,7 +291,7 @@ namespace vcpkg::Install else System::println("Building package %s... ", display_name_with_features); - const auto result = [&]() -> Build::ExtendedBuildResult { + auto result = [&]() -> Build::ExtendedBuildResult { if (GlobalState::feature_packages) { const Build::BuildPackageConfig build_config{ @@ -303,23 +316,15 @@ namespace vcpkg::Install if (result.code != Build::BuildResult::SUCCEEDED) { System::println(System::Color::error, Build::create_error_message(result.code, action.spec)); - return {result.code, nullptr}; + return result; } System::println("Building package %s... done", display_name_with_features); auto bcf = std::make_unique( Paragraphs::try_load_cached_control_package(paths, action.spec).value_or_exit(VCPKG_LINE_INFO)); - System::println("Installing package %s... ", display_name_with_features); - const auto install_result = install_package(paths, *bcf, &status_db); - switch (install_result) - { - case InstallResult::SUCCESS: - System::println(System::Color::success, "Installing package %s... done", display_name); - return {BuildResult::SUCCEEDED, std::move(bcf)}; - case InstallResult::FILE_CONFLICTS: return {BuildResult::FILE_CONFLICTS, nullptr}; - default: Checks::unreachable(VCPKG_LINE_INFO); - } + auto code = aux_install(display_name_with_features, *bcf); + return {code, std::move(bcf)}; } if (plan_type == InstallPlanType::INSTALL) @@ -329,23 +334,15 @@ namespace vcpkg::Install System::println( System::Color::warning, "Package %s is already built -- not building from HEAD", display_name); } - System::println("Installing package %s... ", display_name); - const auto install_result = install_package( - paths, action.any_paragraph.binary_control_file.value_or_exit(VCPKG_LINE_INFO), &status_db); - switch (install_result) - { - case InstallResult::SUCCESS: - System::println(System::Color::success, "Installing package %s... done", display_name); - return {BuildResult::SUCCEEDED, nullptr}; - case InstallResult::FILE_CONFLICTS: return {BuildResult::FILE_CONFLICTS, nullptr}; - default: Checks::unreachable(VCPKG_LINE_INFO); - } + auto code = aux_install(display_name_with_features, + action.any_paragraph.binary_control_file.value_or_exit(VCPKG_LINE_INFO)); + return code; } if (plan_type == InstallPlanType::EXCLUDED) { System::println(System::Color::warning, "Package %s is excluded", display_name); - return {BuildResult::EXCLUDED, nullptr}; + return BuildResult::EXCLUDED; } Checks::unreachable(VCPKG_LINE_INFO); @@ -459,8 +456,7 @@ namespace vcpkg::Install for (const SpecSummary& result : this->results) { - System::println( - " %s: %s: %s", result.spec, Build::to_string(result.build_result.result_code), result.timing); + System::println(" %s: %s: %s", result.spec, Build::to_string(result.build_result.code), result.timing); } std::map summary; @@ -471,7 +467,7 @@ namespace vcpkg::Install for (const SpecSummary& r : this->results) { - summary[r.build_result.result_code]++; + summary[r.build_result.code]++; } System::println("\nSUMMARY"); @@ -483,7 +479,6 @@ namespace vcpkg::Install InstallSummary perform(const std::vector& action_plan, const KeepGoing keep_going, - const CleanBuildtrees clean_buildtrees, const VcpkgPaths& paths, StatusParagraphs& status_db) { @@ -506,23 +501,9 @@ namespace vcpkg::Install if (const auto install_action = action.install_plan.get()) { - Build::BuildResults result = perform_install_plan_action(paths, *install_action, status_db); - if (clean_buildtrees == CleanBuildtrees::YES) - { - auto& fs = paths.get_filesystem(); - auto buildtrees_dir = paths.buildtrees / install_action->spec.name(); - auto buildtree_files = fs.get_files_non_recursive(buildtrees_dir); - for (auto&& file : buildtree_files) - { - if (fs.is_directory(file) && file.filename() != "src") - { - std::error_code ec; - fs.remove_all(file, ec); - } - } - } + auto result = perform_install_plan_action(paths, *install_action, status_db); - if (result.result_code != BuildResult::SUCCEEDED && keep_going == KeepGoing::NO) + if (result.code != BuildResult::SUCCEEDED && keep_going == KeepGoing::NO) { System::println(Build::create_user_troubleshooting_message(install_action->spec)); Checks::exit_fail(VCPKG_LINE_INFO); @@ -669,8 +650,11 @@ namespace vcpkg::Install // create the plan StatusParagraphs status_db = database_load_check(paths); - const Build::BuildPackageOptions install_plan_options = {Build::to_use_head_version(use_head_version), - Build::to_allow_downloads(!no_downloads)}; + const Build::BuildPackageOptions install_plan_options = { + Util::Enum::to_enum(use_head_version), + Util::Enum::to_enum(!no_downloads), + Build::CleanBuildtrees::NO, + }; // Note: action_plan will hold raw pointers to SourceControlFiles from this map std::unordered_map scf_map; @@ -726,7 +710,7 @@ namespace vcpkg::Install Checks::exit_success(VCPKG_LINE_INFO); } - const InstallSummary summary = perform(action_plan, keep_going, Install::CleanBuildtrees::NO, paths, status_db); + const InstallSummary summary = perform(action_plan, keep_going, paths, status_db); System::println("\nTotal elapsed time: %s", summary.total_elapsed_time);