From 9aa38dd6871ccaaa4b71f60420d28842721848f0 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Mon, 21 Oct 2024 15:31:39 -0700 Subject: [PATCH] Apply the normal "to_string" pattern in more places. Extracted from https://github.com/microsoft/vcpkg-tool/pull/1514 --- include/vcpkg/base/contractual-constants.h | 10 ++++ include/vcpkg/base/fmt.h | 8 +++ include/vcpkg/base/graphs.h | 1 - include/vcpkg/binaryparagraph.h | 4 +- include/vcpkg/statusparagraph.h | 18 +++--- include/vcpkg/versions.h | 6 +- src/vcpkg/binaryparagraph.cpp | 2 +- src/vcpkg/commands.ci.cpp | 10 ++-- src/vcpkg/commands.export.cpp | 3 +- src/vcpkg/commands.list.cpp | 9 +-- src/vcpkg/dependencies.cpp | 5 -- src/vcpkg/export.prefab.cpp | 7 +-- src/vcpkg/metrics.cpp | 7 +-- src/vcpkg/packagespec.cpp | 9 +-- src/vcpkg/spdx.cpp | 22 +++---- src/vcpkg/statusparagraph.cpp | 68 ++++++++++++---------- src/vcpkg/versions.cpp | 19 +++--- 17 files changed, 108 insertions(+), 100 deletions(-) diff --git a/include/vcpkg/base/contractual-constants.h b/include/vcpkg/base/contractual-constants.h index e637a800db..b6e45a37e2 100644 --- a/include/vcpkg/base/contractual-constants.h +++ b/include/vcpkg/base/contractual-constants.h @@ -558,4 +558,14 @@ namespace vcpkg inline constexpr StringLiteral AbiTagPostBuildChecks = "post_build_checks"; inline constexpr StringLiteral AbiTagPowershell = "powershell"; inline constexpr StringLiteral AbiTagPublicAbiOverride = "public_abi_override"; + + inline constexpr StringLiteral StatusDeinstall = "deinstall"; + inline constexpr StringLiteral StatusError = "error"; + inline constexpr StringLiteral StatusHalfInstalled = "half-installed"; + inline constexpr StringLiteral StatusHold = "hold"; + inline constexpr StringLiteral StatusInstall = "install"; + inline constexpr StringLiteral StatusInstalled = "installed"; + inline constexpr StringLiteral StatusNotInstalled = "not-installed"; + inline constexpr StringLiteral StatusPurge = "purge"; + inline constexpr StringLiteral StatusUnknown = "unknown"; } diff --git a/include/vcpkg/base/fmt.h b/include/vcpkg/base/fmt.h index 416e6dcd38..2129c8c996 100644 --- a/include/vcpkg/base/fmt.h +++ b/include/vcpkg/base/fmt.h @@ -18,3 +18,11 @@ VCPKG_MSVC_WARNING(disable : 6239 4702) #include #include VCPKG_MSVC_WARNING(pop) + +template +std::string adapt_to_string(const T& val) +{ + std::string result; + val.to_string(result); + return result; +} diff --git a/include/vcpkg/base/graphs.h b/include/vcpkg/base/graphs.h index 8c10236efd..311a52bc28 100644 --- a/include/vcpkg/base/graphs.h +++ b/include/vcpkg/base/graphs.h @@ -16,7 +16,6 @@ namespace vcpkg struct AdjacencyProvider { virtual std::vector adjacency_list(const U& vertex) const = 0; - virtual std::string to_string(const V& vertex) const = 0; virtual U load_vertex_data(const V& vertex) const = 0; }; diff --git a/include/vcpkg/binaryparagraph.h b/include/vcpkg/binaryparagraph.h index b9ad5b13c8..f1f2650ebe 100644 --- a/include/vcpkg/binaryparagraph.h +++ b/include/vcpkg/binaryparagraph.h @@ -8,9 +8,7 @@ namespace vcpkg { - /// - /// Built package metadata - /// + // metadata for a package in the 'packages' tree struct BinaryParagraph { BinaryParagraph() = default; diff --git a/include/vcpkg/statusparagraph.h b/include/vcpkg/statusparagraph.h index 4e4fa98f91..63ad9e211f 100644 --- a/include/vcpkg/statusparagraph.h +++ b/include/vcpkg/statusparagraph.h @@ -13,9 +13,8 @@ namespace vcpkg { - /// - /// Installed package metadata - /// + + // metadata for a package's representation in the 'installed' tree struct StatusParagraph { StatusParagraph() noexcept; @@ -30,14 +29,12 @@ namespace vcpkg void serialize(const StatusParagraph& pgh, std::string& out_str); - std::string to_string(InstallState f); - - std::string to_string(Want f); + StringLiteral to_string_literal(InstallState f); + StringLiteral to_string_literal(Want f); struct InstalledPackageView { - InstalledPackageView() noexcept : core(nullptr) { } - + InstalledPackageView() = default; InstalledPackageView(const StatusParagraph* c, std::vector&& fs) : core(c), features(std::move(fs)) { @@ -51,7 +48,7 @@ namespace vcpkg std::vector all_status_paragraphs() const; - const StatusParagraph* core; + const StatusParagraph* core = nullptr; std::vector features; }; @@ -59,3 +56,6 @@ namespace vcpkg const InstalledPaths& installed, const ReadOnlyFilesystem& fs); } + +VCPKG_FORMAT_WITH_TO_STRING_LITERAL_NONMEMBER(vcpkg::InstallState); +VCPKG_FORMAT_WITH_TO_STRING_LITERAL_NONMEMBER(vcpkg::Want); diff --git a/include/vcpkg/versions.h b/include/vcpkg/versions.h index cb3b1a512f..8a54cbf0bc 100644 --- a/include/vcpkg/versions.h +++ b/include/vcpkg/versions.h @@ -48,6 +48,7 @@ namespace vcpkg VersionDiff(const Version& left, const Version& right); std::string to_string() const; + void to_string(std::string& out) const; }; struct VersionMapLess @@ -85,6 +86,7 @@ namespace vcpkg VersionSpec(const std::string& port_name, const std::string& version_string, int port_version); std::string to_string() const; + void to_string(std::string& out) const; friend bool operator==(const VersionSpec& lhs, const VersionSpec& rhs); friend bool operator!=(const VersionSpec& lhs, const VersionSpec& rhs); @@ -97,7 +99,7 @@ namespace vcpkg struct DotVersion { - DotVersion() { } // intentionally disable making this type an aggregate + DotVersion() noexcept { } // intentionally disable making this type an aggregate std::string original_string; std::string version_string; @@ -122,7 +124,7 @@ namespace vcpkg struct DateVersion { - DateVersion() { } // intentionally disable making this type an aggregate + DateVersion() noexcept { } // intentionally disable making this type an aggregate std::string original_string; std::string version_string; diff --git a/src/vcpkg/binaryparagraph.cpp b/src/vcpkg/binaryparagraph.cpp index e997c43e58..e2d02d3615 100644 --- a/src/vcpkg/binaryparagraph.cpp +++ b/src/vcpkg/binaryparagraph.cpp @@ -284,7 +284,7 @@ namespace vcpkg return fmt::format( "\nspec: \"{}\"\nversion: \"{}\"\nport_version: {}\ndescription: [\"{}\"]\nmaintainers: [\"{}\"]\nfeature: " "\"{}\"\ndefault_features: [\"{}\"]\ndependencies: [\"{}\"]\nabi: \"{}\"", - paragraph.spec.to_string(), + paragraph.spec, paragraph.version.text, paragraph.version.port_version, Strings::join(join_str, paragraph.description), diff --git a/src/vcpkg/commands.ci.cpp b/src/vcpkg/commands.ci.cpp index 115964d7ca..24581927f5 100644 --- a/src/vcpkg/commands.ci.cpp +++ b/src/vcpkg/commands.ci.cpp @@ -52,11 +52,11 @@ namespace (void)filesystem.create_directory(target_path, VCPKG_LINE_INFO); if (children.empty()) { - std::string message = - "There are no build logs for " + spec.to_string() + - " build.\n" - "This is usually because the build failed early and outside of a task that is logged.\n" - "See the console output logs from vcpkg for more information on the failure.\n"; + auto message = + fmt::format("There are no build logs for {} build.\n" + "This is usually because the build failed early and outside of a task that is logged.\n" + "See the console output logs from vcpkg for more information on the failure.\n", + spec); filesystem.write_contents(std::move(target_path) / "readme.log", message, VCPKG_LINE_INFO); } else diff --git a/src/vcpkg/commands.export.cpp b/src/vcpkg/commands.export.cpp index a8a9a26330..4032519205 100644 --- a/src/vcpkg/commands.export.cpp +++ b/src/vcpkg/commands.export.cpp @@ -472,8 +472,7 @@ namespace Checks::unreachable(VCPKG_LINE_INFO); } - const std::string display_name = action.spec.to_string(); - msg::println(msgExportingPackage, msg::package_name = display_name); + msg::println(msgExportingPackage, msg::package_name = action.spec); const BinaryParagraph& binary_paragraph = action.core_paragraph().value_or_exit(VCPKG_LINE_INFO); diff --git a/src/vcpkg/commands.list.cpp b/src/vcpkg/commands.list.cpp index ea6373a558..a7ff89e78b 100644 --- a/src/vcpkg/commands.list.cpp +++ b/src/vcpkg/commands.list.cpp @@ -17,19 +17,20 @@ namespace Json::Object obj; for (const StatusParagraph* status_paragraph : installed_packages) { - auto current_spec = status_paragraph->package.spec; - if (obj.contains(current_spec.to_string())) + const auto& current_spec = status_paragraph->package.spec; + const auto current_spec_string = current_spec.to_string(); + if (obj.contains(current_spec_string)) { if (status_paragraph->package.is_feature()) { - Json::Value* value_obj = obj.get(current_spec.to_string()); + Json::Value* value_obj = obj.get(current_spec_string); auto& feature_list = value_obj->object(VCPKG_LINE_INFO)[JsonIdFeatures].array(VCPKG_LINE_INFO); feature_list.push_back(Json::Value::string(status_paragraph->package.feature)); } } else { - Json::Object& library_obj = obj.insert(current_spec.to_string(), Json::Object()); + Json::Object& library_obj = obj.insert(current_spec_string, Json::Object()); library_obj.insert(JsonIdPackageUnderscoreName, Json::Value::string(current_spec.name())); library_obj.insert(JsonIdTriplet, Json::Value::string(current_spec.triplet().to_string())); library_obj.insert(JsonIdVersion, Json::Value::string(status_paragraph->package.version.text)); diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp index 7142a61067..bfdd903138 100644 --- a/src/vcpkg/dependencies.cpp +++ b/src/vcpkg/dependencies.cpp @@ -680,8 +680,6 @@ namespace vcpkg } PackageSpec load_vertex_data(const PackageSpec& s) const override { return s; } - - std::string to_string(const PackageSpec& spec) const override { return spec.to_string(); } }; RemoveAdjacencyProvider p; @@ -746,8 +744,6 @@ namespace vcpkg return ExportPlanAction{spec, request_type}; } - - std::string to_string(const PackageSpec& spec) const override { return spec.to_string(); } }; const std::unordered_set specs_as_set(specs.cbegin(), specs.cend()); @@ -997,7 +993,6 @@ namespace vcpkg { BaseEdgeProvider(const ClusterGraph& parent) : m_parent(parent) { } - std::string to_string(const PackageSpec& spec) const override { return spec.to_string(); } const Cluster* load_vertex_data(const PackageSpec& spec) const override { return &m_parent.find_or_exit(spec, VCPKG_LINE_INFO); diff --git a/src/vcpkg/export.prefab.cpp b/src/vcpkg/export.prefab.cpp index f6f02a461d..22f9fcad01 100644 --- a/src/vcpkg/export.prefab.cpp +++ b/src/vcpkg/export.prefab.cpp @@ -36,12 +36,7 @@ namespace vcpkg::Prefab return paths; } - std::string NdkVersion::to_string() const - { - std::string ret; - this->to_string(ret); - return ret; - } + std::string NdkVersion::to_string() const { return adapt_to_string(*this); } void NdkVersion::to_string(std::string& out) const { out.append("NdkVersion{major=") diff --git a/src/vcpkg/metrics.cpp b/src/vcpkg/metrics.cpp index 008b9f0f34..e29e88f258 100644 --- a/src/vcpkg/metrics.cpp +++ b/src/vcpkg/metrics.cpp @@ -282,12 +282,7 @@ namespace vcpkg last_completed_survey); } - std::string MetricsUserConfig::to_string() const - { - std::string ret; - to_string(ret); - return ret; - } + std::string MetricsUserConfig::to_string() const { return adapt_to_string(*this); } void MetricsUserConfig::try_write(const Filesystem& fs) const { diff --git a/src/vcpkg/packagespec.cpp b/src/vcpkg/packagespec.cpp index eca04f0aa3..53030973c0 100644 --- a/src/vcpkg/packagespec.cpp +++ b/src/vcpkg/packagespec.cpp @@ -70,12 +70,7 @@ namespace namespace vcpkg { - std::string FeatureSpec::to_string() const - { - std::string ret; - this->to_string(ret); - return ret; - } + std::string FeatureSpec::to_string() const { return adapt_to_string(*this); } void FeatureSpec::to_string(std::string& out) const { if (feature().empty()) return spec().to_string(out); @@ -130,7 +125,7 @@ namespace vcpkg std::string PackageSpec::dir() const { return fmt::format("{}_{}", this->m_name, this->m_triplet); } - std::string PackageSpec::to_string() const { return fmt::format("{}:{}", this->name(), this->triplet()); } + std::string PackageSpec::to_string() const { return adapt_to_string(*this); } void PackageSpec::to_string(std::string& s) const { fmt::format_to(std::back_inserter(s), "{}:{}", this->name(), this->triplet()); diff --git a/src/vcpkg/spdx.cpp b/src/vcpkg/spdx.cpp index fd1ae3d842..2e28a3b404 100644 --- a/src/vcpkg/spdx.cpp +++ b/src/vcpkg/spdx.cpp @@ -59,11 +59,11 @@ static StringView extract_cmake_invocation_argument(StringView command, StringVi } static Json::Object make_resource( - std::string spdxid, std::string name, std::string downloadLocation, StringView sha512, StringView filename) + std::string spdxid, StringView name, std::string downloadLocation, StringView sha512, StringView filename) { Json::Object obj; obj.insert(SpdxSpdxId, std::move(spdxid)); - obj.insert(JsonIdName, std::move(name)); + obj.insert(JsonIdName, name.to_string()); if (!filename.empty()) { obj.insert(SpdxPackageFileName, filename); @@ -97,7 +97,7 @@ Json::Value vcpkg::run_resource_heuristics(StringView contents, StringView versi auto sha = extract_cmake_invocation_argument(github, CMakeVariableSHA512); packages.push_back(make_resource(fmt::format("SPDXRef-resource-{}", ++n), - repo.to_string(), + repo, fmt::format("git+https://github.com/{}@{}", repo, ref), sha, {})); @@ -107,8 +107,8 @@ Json::Value vcpkg::run_resource_heuristics(StringView contents, StringView versi { auto url = extract_cmake_invocation_argument(github, CMakeVariableUrl); auto ref = fix_ref_version(extract_cmake_invocation_argument(github, CMakeVariableRef), version_text); - packages.push_back(make_resource( - fmt::format("SPDXRef-resource-{}", ++n), url.to_string(), fmt::format("git+{}@{}", url, ref), {}, {})); + packages.push_back( + make_resource(fmt::format("SPDXRef-resource-{}", ++n), url, fmt::format("git+{}@{}", url, ref), {}, {})); } auto distfile = find_cmake_invocation(contents, "vcpkg_download_distfile"); if (!distfile.empty()) @@ -116,8 +116,8 @@ Json::Value vcpkg::run_resource_heuristics(StringView contents, StringView versi auto url = extract_cmake_invocation_argument(distfile, CMakeVariableUrls); auto filename = extract_cmake_invocation_argument(distfile, CMakeVariableFilename); auto sha = extract_cmake_invocation_argument(distfile, CMakeVariableSHA512); - packages.push_back(make_resource( - fmt::format("SPDXRef-resource-{}", ++n), filename.to_string(), url.to_string(), sha, filename)); + packages.push_back( + make_resource(fmt::format("SPDXRef-resource-{}", ++n), filename, url.to_string(), sha, filename)); } auto sfg = find_cmake_invocation(contents, "vcpkg_from_sourceforge"); if (!sfg.empty()) @@ -126,9 +126,9 @@ Json::Value vcpkg::run_resource_heuristics(StringView contents, StringView versi auto ref = fix_ref_version(extract_cmake_invocation_argument(sfg, CMakeVariableRef), version_text); auto filename = extract_cmake_invocation_argument(sfg, CMakeVariableFilename); auto sha = extract_cmake_invocation_argument(sfg, CMakeVariableSHA512); - auto url = Strings::concat("https://sourceforge.net/projects/", repo, "/files/", ref, '/', filename); - packages.push_back(make_resource( - fmt::format("SPDXRef-resource-{}", ++n), filename.to_string(), std::move(url), sha, filename)); + auto url = fmt::format("https://sourceforge.net/projects/{}/files/{}/{}", repo, ref, filename); + packages.push_back( + make_resource(fmt::format("SPDXRef-resource-{}", ++n), filename, std::move(url), sha, filename)); } return Json::Value::object(std::move(ret)); } @@ -156,7 +156,7 @@ std::string vcpkg::create_spdx_sbom(const InstallPlanAction& action, doc.insert(SpdxDataLicense, SpdxCCZero); doc.insert(SpdxSpdxId, SpdxRefDocument); doc.insert(SpdxDocumentNamespace, std::move(document_namespace)); - doc.insert(JsonIdName, Strings::concat(action.spec.to_string(), '@', cpgh.version.to_string(), ' ', abi)); + doc.insert(JsonIdName, fmt::format("{}@{} {}", action.spec, cpgh.version, abi)); { auto& cinfo = doc.insert(SpdxCreationInfo, Json::Object()); auto& creators = cinfo.insert(JsonIdCreators, Json::Array()); diff --git a/src/vcpkg/statusparagraph.cpp b/src/vcpkg/statusparagraph.cpp index f0a0748ecb..7fa5571aaa 100644 --- a/src/vcpkg/statusparagraph.cpp +++ b/src/vcpkg/statusparagraph.cpp @@ -9,11 +9,13 @@ namespace vcpkg void serialize(const StatusParagraph& pgh, std::string& out_str) { + auto want_literal = to_string_literal(pgh.want); + auto state_literal = to_string_literal(pgh.state); serialize(pgh.package, out_str); out_str.append("Status: ") - .append(to_string(pgh.want)) + .append(want_literal.data(), want_literal.size()) .append(" ok ") - .append(to_string(pgh.state)) + .append(state_literal.data(), state_literal.size()) .push_back('\n'); } @@ -36,11 +38,11 @@ namespace vcpkg ++b; want = [](const std::string& text) { - if (text == "unknown") return Want::UNKNOWN; - if (text == "install") return Want::INSTALL; - if (text == "hold") return Want::HOLD; - if (text == "deinstall") return Want::DEINSTALL; - if (text == "purge") return Want::PURGE; + if (text == StatusUnknown) return Want::UNKNOWN; + if (text == StatusInstall) return Want::INSTALL; + if (text == StatusHold) return Want::HOLD; + if (text == StatusDeinstall) return Want::DEINSTALL; + if (text == StatusPurge) return Want::PURGE; return Want::ERROR_STATE; }(std::string(mark, b)); @@ -48,47 +50,47 @@ namespace vcpkg b += 4; state = [](const std::string& text) { - if (text == "not-installed") return InstallState::NOT_INSTALLED; - if (text == "installed") return InstallState::INSTALLED; - if (text == "half-installed") return InstallState::HALF_INSTALLED; + if (text == StatusNotInstalled) return InstallState::NOT_INSTALLED; + if (text == StatusInstalled) return InstallState::INSTALLED; + if (text == StatusHalfInstalled) return InstallState::HALF_INSTALLED; return InstallState::ERROR_STATE; }(std::string(b, e)); } - std::string to_string(InstallState f) + StringLiteral to_string_literal(InstallState f) { switch (f) { - case InstallState::HALF_INSTALLED: return "half-installed"; - case InstallState::INSTALLED: return "installed"; - case InstallState::NOT_INSTALLED: return "not-installed"; - default: return "error"; + case InstallState::HALF_INSTALLED: return StatusHalfInstalled; + case InstallState::INSTALLED: return StatusInstalled; + case InstallState::NOT_INSTALLED: return StatusNotInstalled; + default: Checks::unreachable(VCPKG_LINE_INFO); } } - std::string to_string(Want f) + StringLiteral to_string_literal(Want f) { switch (f) { - case Want::DEINSTALL: return "deinstall"; - case Want::HOLD: return "hold"; - case Want::INSTALL: return "install"; - case Want::PURGE: return "purge"; - case Want::UNKNOWN: return "unknown"; - default: return "error"; + case Want::DEINSTALL: return StatusDeinstall; + case Want::HOLD: return StatusHold; + case Want::INSTALL: return StatusInstall; + case Want::PURGE: return StatusPurge; + case Want::UNKNOWN: return StatusUnknown; + default: Checks::unreachable(VCPKG_LINE_INFO); } } std::map> InstalledPackageView::feature_dependencies() const { - auto extract_deps = [&](const PackageSpec& spec) { return FeatureSpec{spec, FeatureNameCore.to_string()}; }; + auto extract_deps = [](const PackageSpec& spec) { return FeatureSpec{spec, FeatureNameCore}; }; std::map> deps; - deps.emplace(FeatureNameCore, Util::fmap(core->package.dependencies, extract_deps)); - - for (const StatusParagraph* const& feature : features) + for (const StatusParagraph* feature : features) + { deps.emplace(feature->package.feature, Util::fmap(feature->package.dependencies, extract_deps)); + } return deps; } @@ -97,10 +99,11 @@ namespace vcpkg { InternalFeatureSet ret; ret.emplace_back(FeatureNameCore); - for (const auto& f : features) + for (const StatusParagraph* f : features) { ret.emplace_back(f->package.feature); } + return ret; } @@ -111,18 +114,23 @@ namespace vcpkg // accumulate all features in installed dependencies // Todo: make this unneeded by collapsing all package dependencies into the core package std::vector deps; - for (auto&& feature : features) + for (const StatusParagraph* feature : features) + { for (auto&& dep : feature->package.dependencies) + { deps.push_back(dep); + } + } // Add the core paragraph dependencies to the list for (auto&& dep : core->package.dependencies) + { deps.push_back(dep); + } - auto this_spec = this->spec(); + const auto& this_spec = this->spec(); Util::erase_remove_if(deps, [this_spec](const PackageSpec& pspec) { return pspec == this_spec; }); Util::sort_unique_erase(deps); - return deps; } diff --git a/src/vcpkg/versions.cpp b/src/vcpkg/versions.cpp index e37351f086..00c1dd26e4 100644 --- a/src/vcpkg/versions.cpp +++ b/src/vcpkg/versions.cpp @@ -18,12 +18,7 @@ namespace vcpkg { } - std::string Version::to_string() const - { - std::string result; - to_string(result); - return result; - } + std::string Version::to_string() const { return adapt_to_string(*this); } void Version::to_string(std::string& out) const { @@ -81,9 +76,17 @@ namespace vcpkg VersionDiff::VersionDiff() noexcept : left(), right() { } VersionDiff::VersionDiff(const Version& left, const Version& right) : left(left), right(right) { } - std::string VersionDiff::to_string() const { return fmt::format("{} -> {}", left, right); } + std::string VersionDiff::to_string() const { return adapt_to_string(*this); } + void VersionDiff::to_string(std::string& out) const + { + fmt::format_to(std::back_inserter(out), "{} -> {}", left, right); + } - std::string VersionSpec::to_string() const { return fmt::format("{}@{}", port_name, version); } + std::string VersionSpec::to_string() const { return adapt_to_string(*this); } + void VersionSpec::to_string(std::string& out) const + { + fmt::format_to(std::back_inserter(out), "{}@{}", port_name, version); + } namespace {