From 5d58eb29cc9ae584335e3380c2645baf05b21222 Mon Sep 17 00:00:00 2001 From: Alexander Karatarakis Date: Mon, 3 Oct 2016 17:45:01 -0700 Subject: [PATCH] [package_spec] Force using the factories that do sanity checks --- toolsrc/include/package_spec.h | 17 ++++++++---- toolsrc/src/commands_installation.cpp | 16 ++++++------ toolsrc/src/lib.cpp | 4 +-- toolsrc/src/package_spec.cpp | 37 ++++++++++++++++++++++----- toolsrc/src/post_build_lint.cpp | 20 +++++++-------- toolsrc/src/test.cpp | 8 +++--- toolsrc/src/vcpkg_Dependencies.cpp | 6 ++--- toolsrc/src/vcpkg_Input.cpp | 2 +- toolsrc/src/vcpkg_paths.cpp | 2 +- 9 files changed, 71 insertions(+), 41 deletions(-) diff --git a/toolsrc/include/package_spec.h b/toolsrc/include/package_spec.h index 942b34adc5..7410145ca9 100644 --- a/toolsrc/include/package_spec.h +++ b/toolsrc/include/package_spec.h @@ -8,12 +8,19 @@ namespace vcpkg { struct package_spec { - static expected from_string(const std::string& spec, const triplet& default_target_triplet); + static expected from_string(const std::string& spec_as_string, const triplet& default_target_triplet); - std::string name; - triplet target_triplet; + static package_spec from_name_and_triplet(const std::string& name, const triplet& target_triplet); + + const std::string& name() const; + + const triplet& target_triplet() const; std::string dir() const; + + private: + std::string m_name; + triplet m_target_triplet; }; std::string to_string(const package_spec& spec); @@ -33,8 +40,8 @@ namespace std size_t operator()(const vcpkg::package_spec& value) const { size_t hash = 17; - hash = hash * 31 + std::hash()(value.name); - hash = hash * 31 + std::hash()(value.target_triplet); + hash = hash * 31 + std::hash()(value.name()); + hash = hash * 31 + std::hash()(value.target_triplet()); return hash; } }; diff --git a/toolsrc/src/commands_installation.cpp b/toolsrc/src/commands_installation.cpp index 521d4df71d..a94fb7e597 100644 --- a/toolsrc/src/commands_installation.cpp +++ b/toolsrc/src/commands_installation.cpp @@ -24,9 +24,9 @@ namespace vcpkg { const fs::path ports_cmake_script_path = paths.ports_cmake; const std::wstring command = Strings::wformat(LR"("%%VS140COMNTOOLS%%..\..\VC\vcvarsall.bat" %s && cmake -DCMD=BUILD -DPORT=%s -DTARGET_TRIPLET=%s "-DCURRENT_PORT_DIR=%s/." -P "%s")", - Strings::utf8_to_utf16(spec.target_triplet.architecture()), - Strings::utf8_to_utf16(spec.name), - Strings::utf8_to_utf16(spec.target_triplet.canonical_name()), + Strings::utf8_to_utf16(spec.target_triplet().architecture()), + Strings::utf8_to_utf16(spec.name()), + Strings::utf8_to_utf16(spec.target_triplet().canonical_name()), port_dir.generic_wstring(), ports_cmake_script_path.generic_wstring()); @@ -53,7 +53,7 @@ namespace vcpkg perform_all_checks(spec, paths); - create_binary_control_file(paths, port_dir, spec.target_triplet); + create_binary_control_file(paths, port_dir, spec.target_triplet()); // const fs::path port_buildtrees_dir = paths.buildtrees / spec.name; // delete_directory(port_buildtrees_dir); @@ -61,7 +61,7 @@ namespace vcpkg static void build_internal(const package_spec& spec, const vcpkg_paths& paths) { - return build_internal(spec, paths, paths.ports / spec.name); + return build_internal(spec, paths, paths.ports / spec.name()); } void install_command(const vcpkg_cmd_arguments& args, const vcpkg_paths& paths, const triplet& default_target_triplet) @@ -85,7 +85,7 @@ namespace vcpkg for (const package_spec& spec : install_plan) { - if (status_db.find_installed(spec.name, spec.target_triplet) != status_db.end()) + if (status_db.find_installed(spec.name(), spec.target_triplet()) != status_db.end()) { System::println(System::color::success, "Package %s is already installed", spec); continue; @@ -133,7 +133,7 @@ namespace vcpkg StatusParagraphs status_db = database_load_check(paths); const package_spec spec = Input::check_and_get_package_spec(args.command_arguments.at(0), default_target_triplet, example.c_str()); - Input::check_triplet(spec.target_triplet, paths); + Input::check_triplet(spec.target_triplet(), paths); std::unordered_set unmet_dependencies = Dependencies::find_unmet_dependencies(paths, spec, status_db); if (!unmet_dependencies.empty()) { @@ -161,7 +161,7 @@ namespace vcpkg expected current_spec = package_spec::from_string(args.command_arguments[0], default_target_triplet); if (auto spec = current_spec.get()) { - Input::check_triplet(spec->target_triplet, paths); + Input::check_triplet(spec->target_triplet(), paths); Environment::ensure_utilities_on_path(paths); const fs::path port_dir = args.command_arguments.at(1); build_internal(*spec, paths, port_dir); diff --git a/toolsrc/src/lib.cpp b/toolsrc/src/lib.cpp index 2127b78501..92451156af 100644 --- a/toolsrc/src/lib.cpp +++ b/toolsrc/src/lib.cpp @@ -135,7 +135,7 @@ static std::string get_fullpkgname_from_listfile(const fs::path& path) static fs::path prefix_path_for_package(const vcpkg_paths& paths, const BinaryParagraph& pgh) { - return paths.package_dir({pgh.name, pgh.target_triplet}); + return paths.package_dir(package_spec::from_name_and_triplet(pgh.name, pgh.target_triplet)); } static void write_update(const vcpkg_paths& paths, const StatusParagraph& p) @@ -316,7 +316,7 @@ static deinstall_plan deinstall_package_plan( void vcpkg::deinstall_package(const vcpkg_paths& paths, const package_spec& spec, StatusParagraphs& status_db) { - auto package_it = status_db.find(spec.name, spec.target_triplet); + auto package_it = status_db.find(spec.name(), spec.target_triplet()); if (package_it == status_db.end()) { System::println(System::color::success, "Package %s is not installed", spec); diff --git a/toolsrc/src/package_spec.cpp b/toolsrc/src/package_spec.cpp index 3da0757fda..57f6179bf8 100644 --- a/toolsrc/src/package_spec.cpp +++ b/toolsrc/src/package_spec.cpp @@ -3,15 +3,15 @@ namespace vcpkg { - expected package_spec::from_string(const std::string& spec, const triplet& default_target_triplet) + expected package_spec::from_string(const std::string& spec_as_string, const triplet& default_target_triplet) { - std::string s(spec); + std::string s(spec_as_string); std::transform(s.begin(), s.end(), s.begin(), ::tolower); auto pos = s.find(':'); if (pos == std::string::npos) { - return package_spec{s, default_target_triplet}; + return from_name_and_triplet(s, default_target_triplet); } auto pos2 = s.find(':', pos + 1); @@ -20,17 +20,40 @@ namespace vcpkg return std::error_code(package_spec_parse_result::too_many_colons); } - return package_spec{s.substr(0, pos), triplet::from_canonical_name(s.substr(pos + 1))}; + const std::string name = s.substr(0, pos); + const triplet target_triplet = triplet::from_canonical_name(s.substr(pos + 1)); + return from_name_and_triplet(name, target_triplet); + } + + package_spec package_spec::from_name_and_triplet(const std::string& name, const triplet& target_triplet) + { + std::string n(name); + std::transform(n.begin(), n.end(), n.begin(), ::tolower); + + package_spec p; + p.m_name = n; + p.m_target_triplet = target_triplet; + return p; + } + + const std::string& package_spec::name() const + { + return this->m_name; + } + + const triplet& package_spec::target_triplet() const + { + return this->m_target_triplet; } std::string package_spec::dir() const { - return Strings::format("%s_%s", this->name, this->target_triplet); + return Strings::format("%s_%s", this->m_name, this->m_target_triplet); } std::string to_string(const package_spec& spec) { - return Strings::format("%s:%s", spec.name, spec.target_triplet); + return Strings::format("%s:%s", spec.name(), spec.target_triplet()); } std::string to_printf_arg(const package_spec& spec) @@ -40,7 +63,7 @@ namespace vcpkg bool operator==(const package_spec& left, const package_spec& right) { - return left.name == right.name && left.target_triplet == right.target_triplet; + return left.name() == right.name() && left.target_triplet() == right.target_triplet(); } std::ostream& operator<<(std::ostream& os, const package_spec& spec) diff --git a/toolsrc/src/post_build_lint.cpp b/toolsrc/src/post_build_lint.cpp index 29710bd142..680f01913d 100644 --- a/toolsrc/src/post_build_lint.cpp +++ b/toolsrc/src/post_build_lint.cpp @@ -113,7 +113,7 @@ namespace vcpkg if (!misplaced_cmake_files.empty()) { - System::println(System::color::warning, "The following cmake files were found outside /share/%s. Please place cmake files in /share/%s.", spec.name, spec.name); + System::println(System::color::warning, "The following cmake files were found outside /share/%s. Please place cmake files in /share/%s.", spec.name(), spec.name()); print_vector_of_files(misplaced_cmake_files); return lint_status::ERROR; } @@ -151,12 +151,12 @@ namespace vcpkg static lint_status check_for_copyright_file(const package_spec& spec, const vcpkg_paths& paths) { - const fs::path copyright_file = paths.packages / spec.dir() / "share" / spec.name / "copyright"; + const fs::path copyright_file = paths.packages / spec.dir() / "share" / spec.name() / "copyright"; if (fs::exists(copyright_file)) { return lint_status::SUCCESS; } - const fs::path current_buildtrees_dir = paths.buildtrees / spec.name; + const fs::path current_buildtrees_dir = paths.buildtrees / spec.name(); const fs::path current_buildtrees_dir_src = current_buildtrees_dir / "src"; std::vector potential_copyright_files; @@ -175,14 +175,14 @@ namespace vcpkg } } - System::println(System::color::warning, "The software license must be available at ${CURRENT_PACKAGES_DIR}/share/%s/copyright .", spec.name); + System::println(System::color::warning, "The software license must be available at ${CURRENT_PACKAGES_DIR}/share/%s/copyright .", spec.name()); if (potential_copyright_files.size() == 1) // if there is only one candidate, provide the cmake lines needed to place it in the proper location { const fs::path found_file = potential_copyright_files[0]; const fs::path relative_path = found_file.string().erase(0, current_buildtrees_dir.string().size() + 1); // The +1 is needed to remove the "/" System::println("\n file(COPY ${CURRENT_BUILDTREES_DIR}/%s DESTINATION ${CURRENT_PACKAGES_DIR}/share/%s)\n" " file(RENAME ${CURRENT_PACKAGES_DIR}/share/%s/%s ${CURRENT_PACKAGES_DIR}/share/%s/copyright)", - relative_path.generic_string(), spec.name, spec.name, found_file.filename().generic_string(), spec.name); + relative_path.generic_string(), spec.name(), spec.name(), found_file.filename().generic_string(), spec.name()); return lint_status::ERROR; } @@ -193,7 +193,7 @@ namespace vcpkg } const fs::path current_packages_dir = paths.packages / spec.dir(); - System::println(" %s/share/%s/copyright", current_packages_dir.generic_string(), spec.name); + System::println(" %s/share/%s/copyright", current_packages_dir.generic_string(), spec.name()); return lint_status::ERROR; } @@ -333,18 +333,18 @@ namespace vcpkg recursive_find_files_with_extension_in_dir(paths.packages / spec.dir() / "debug" / "bin", ".dll", dlls); error_count += check_exports_of_dlls(dlls); - error_count += check_uwp_bit_of_dlls(spec.target_triplet.system(), dlls); - error_count += check_architecture(spec.target_triplet.architecture(), dlls); + error_count += check_uwp_bit_of_dlls(spec.target_triplet().system(), dlls); + error_count += check_architecture(spec.target_triplet().architecture(), dlls); std::vector libs; recursive_find_files_with_extension_in_dir(paths.packages / spec.dir() / "lib", ".lib", libs); recursive_find_files_with_extension_in_dir(paths.packages / spec.dir() / "debug" / "lib", ".lib", libs); - error_count += check_architecture(spec.target_triplet.architecture(), libs); + error_count += check_architecture(spec.target_triplet().architecture(), libs); if (error_count != 0) { - const fs::path portfile = paths.ports / spec.name / "portfile.cmake"; + const fs::path portfile = paths.ports / spec.name() / "portfile.cmake"; System::println(System::color::error, "Found %u error(s). Please correct the portfile:\n %s", error_count, portfile.string()); exit(EXIT_FAILURE); } diff --git a/toolsrc/src/test.cpp b/toolsrc/src/test.cpp index 9bc41fc9ed..ba35391aa9 100644 --- a/toolsrc/src/test.cpp +++ b/toolsrc/src/test.cpp @@ -310,16 +310,16 @@ namespace UnitTest1 { vcpkg::expected spec = vcpkg::package_spec::from_string("zlib", vcpkg::triplet::X86_WINDOWS); Assert::AreEqual(vcpkg::package_spec_parse_result::success, vcpkg::to_package_spec_parse_result(spec.error_code())); - Assert::AreEqual("zlib", spec.get()->name.c_str()); - Assert::AreEqual(vcpkg::triplet::X86_WINDOWS.canonical_name(), spec.get()->target_triplet.canonical_name()); + Assert::AreEqual("zlib", spec.get()->name().c_str()); + Assert::AreEqual(vcpkg::triplet::X86_WINDOWS.canonical_name(), spec.get()->target_triplet().canonical_name()); } TEST_METHOD(package_spec_parse_with_arch) { vcpkg::expected spec = vcpkg::package_spec::from_string("zlib:x64-uwp", vcpkg::triplet::X86_WINDOWS); Assert::AreEqual(vcpkg::package_spec_parse_result::success, vcpkg::to_package_spec_parse_result(spec.error_code())); - Assert::AreEqual("zlib", spec.get()->name.c_str()); - Assert::AreEqual(vcpkg::triplet::X64_UWP.canonical_name(), spec.get()->target_triplet.canonical_name()); + Assert::AreEqual("zlib", spec.get()->name().c_str()); + Assert::AreEqual(vcpkg::triplet::X64_UWP.canonical_name(), spec.get()->target_triplet().canonical_name()); } TEST_METHOD(package_spec_parse_with_multiple_colon) diff --git a/toolsrc/src/vcpkg_Dependencies.cpp b/toolsrc/src/vcpkg_Dependencies.cpp index 6ffb4959d2..9c083a8790 100644 --- a/toolsrc/src/vcpkg_Dependencies.cpp +++ b/toolsrc/src/vcpkg_Dependencies.cpp @@ -20,7 +20,7 @@ namespace vcpkg { namespace Dependencies while (!examine_stack.empty()) { - package_spec spec = examine_stack.back(); + const package_spec spec = examine_stack.back(); examine_stack.pop_back(); if (was_examined.find(spec) != was_examined.end()) @@ -32,8 +32,8 @@ namespace vcpkg { namespace Dependencies for (const std::string& dep_as_string : dependencies_as_string) { - package_spec current_dep = {dep_as_string, spec.target_triplet}; - auto it = status_db.find(current_dep.name, current_dep.target_triplet); + const package_spec current_dep = package_spec::from_name_and_triplet(dep_as_string, spec.target_triplet()); + auto it = status_db.find(current_dep.name(), current_dep.target_triplet()); if (it != status_db.end() && (*it)->want == want_t::install) { continue; diff --git a/toolsrc/src/vcpkg_Input.cpp b/toolsrc/src/vcpkg_Input.cpp index 435d555761..0c03faaa8b 100644 --- a/toolsrc/src/vcpkg_Input.cpp +++ b/toolsrc/src/vcpkg_Input.cpp @@ -44,7 +44,7 @@ namespace vcpkg {namespace Input { for (const package_spec& spec : triplets) { - check_triplet(spec.target_triplet, paths); + check_triplet(spec.target_triplet(), paths); } } }} diff --git a/toolsrc/src/vcpkg_paths.cpp b/toolsrc/src/vcpkg_paths.cpp index 559f719aff..1f9eb0bc50 100644 --- a/toolsrc/src/vcpkg_paths.cpp +++ b/toolsrc/src/vcpkg_paths.cpp @@ -52,7 +52,7 @@ namespace vcpkg fs::path vcpkg_paths::port_dir(const package_spec& spec) const { - return this->ports / spec.name; + return this->ports / spec.name(); } bool vcpkg_paths::is_valid_triplet(const triplet& t) const