Skip to content

Commit

Permalink
Split functions to avoid 'is_manifest' control coupling. (#1229)
Browse files Browse the repository at this point in the history
  • Loading branch information
@BillyONeal
BillyONeal committed Oct 17, 2023
1 parent 79ad338 commit d38bf94
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 67 deletions.
6 changes: 3 additions & 3 deletions include/vcpkg/base/message-data.inc.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2500,10 +2500,10 @@ DECLARE_MESSAGE(PortNotInBaseline,
DECLARE_MESSAGE(PortsAdded, (msg::count), "", "The following {count} ports were added:")
DECLARE_MESSAGE(PortsDiffHelp, (), "", "The argument should be a branch/tag/hash to checkout.")
DECLARE_MESSAGE(PortDoesNotExist, (msg::package_name), "", "{package_name} does not exist")
DECLARE_MESSAGE(PortMissingManifest,
(msg::package_name, msg::path),
DECLARE_MESSAGE(PortMissingManifest2,
(msg::package_name),
"",
"{package_name} has no vcpkg.json or CONTROL file in {path}")
"{package_name} port manifest missing (no vcpkg.json or CONTROL file)")
DECLARE_MESSAGE(PortsNoDiff, (), "", "There were no changes in the ports between the two commits.")
DECLARE_MESSAGE(PortsRemoved, (msg::count), "", "The following {count} ports were removed:")
DECLARE_MESSAGE(PortsUpdated, (msg::count), "", "The following {count} ports were updated:")
Expand Down
11 changes: 7 additions & 4 deletions include/vcpkg/paragraphs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,13 @@ namespace vcpkg::Paragraphs
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_required(const ReadOnlyFilesystem& fs,
StringView port_name,
const Path& port_directory);
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_text(const std::string& text,
StringView origin,
bool is_manifest,
MessageSink& warning_sink);
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_project_manifest_text(StringView text,
StringView origin,
MessageSink& warning_sink);
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_manifest_text(StringView text,
StringView origin,
MessageSink& warning_sink);
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_control_file_text(StringView text, StringView origin);

ExpectedL<BinaryControlFile> try_load_cached_package(const ReadOnlyFilesystem& fs,
const Path& package_dir,
Expand Down
4 changes: 2 additions & 2 deletions locales/messages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1388,8 +1388,8 @@
"_PortDependencyConflict.comment": "An example of {package_name} is zlib.",
"PortDoesNotExist": "{package_name} does not exist",
"_PortDoesNotExist.comment": "An example of {package_name} is zlib.",
"PortMissingManifest": "{package_name} has no vcpkg.json or CONTROL file in {path}",
"_PortMissingManifest.comment": "An example of {package_name} is zlib. An example of {path} is /foo/bar.",
"PortMissingManifest2": "{package_name} port manifest missing (no vcpkg.json or CONTROL file)",
"_PortMissingManifest2.comment": "An example of {package_name} is zlib.",
"PortNotInBaseline": "the baseline does not contain an entry for port {package_name}",
"_PortNotInBaseline.comment": "An example of {package_name} is zlib.",
"PortSupportsField": "(supports: \"{supports_expression}\")",
Expand Down
11 changes: 7 additions & 4 deletions src/vcpkg/commands.ci-verify-versions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@ namespace
if (!maybe_file) continue;

const auto& file = maybe_file.value_or_exit(VCPKG_LINE_INFO);
auto maybe_scf =
Paragraphs::try_load_port_text(file, treeish, control_file == "vcpkg.json", stdout_sink);
auto maybe_scf = control_file == "vcpkg.json"
? Paragraphs::try_load_port_manifest_text(file, treeish, stdout_sink)
: Paragraphs::try_load_control_file_text(file, treeish);
auto scf = maybe_scf.get();
if (!scf)
{
Expand Down Expand Up @@ -306,8 +307,10 @@ namespace vcpkg
if (!manifest_exists && !control_exists)
{
msg::write_unlocalized_text_to_stdout(Color::error, fmt::format("FAIL: {}\n", port_name));
errors.emplace(
msg::format(msgPortMissingManifest, msg::package_name = port_name, msg::path = port_path));
errors.emplace(LocalizedString::from_raw(port_path)
.append_raw(": ")
.append(msgErrorMessage)
.append(msgPortMissingManifest2, msg::package_name = port_name));
continue;
}

Expand Down
119 changes: 65 additions & 54 deletions src/vcpkg/paragraphs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

#include <tuple>

using namespace vcpkg;

static std::atomic<uint64_t> g_load_ports_stats(0);

namespace vcpkg
Expand Down Expand Up @@ -379,46 +381,45 @@ namespace vcpkg::Paragraphs
return fs.exists(maybe_directory / "CONTROL", IgnoreErrors{}) ||
fs.exists(maybe_directory / "vcpkg.json", IgnoreErrors{});
}
} // namespace vcpkg::Paragraphs

static ExpectedL<std::unique_ptr<SourceControlFile>> try_load_manifest_text(const std::string& text,
StringView origin,
MessageSink& warning_sink)
namespace
{
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_any_manifest_text(
StringView text,
StringView origin,
MessageSink& warning_sink,
ParseExpected<SourceControlFile> (*do_parse)(StringView, const Json::Object&, MessageSink&))
{
auto res = Json::parse(text, origin);
if (auto val = res.get())
{
if (val->value.is_object())
{
return SourceControlFile::parse_port_manifest_object(
origin, val->value.object(VCPKG_LINE_INFO), warning_sink)
.map_error(ToLocalizedString);
}
StatsTimer timer(g_load_ports_stats);
return Json::parse_object(text, origin).then([&](Json::Object&& object) {
return do_parse(origin, std::move(object), warning_sink).map_error(ToLocalizedString);
});
}
}

return msg::format(msgJsonValueNotObject);
}
namespace vcpkg::Paragraphs
{
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_project_manifest_text(StringView text,
StringView origin,
MessageSink& warning_sink)
{
return try_load_any_manifest_text(text, origin, warning_sink, SourceControlFile::parse_project_manifest_object);
}

return LocalizedString::from_raw(res.error()->to_string());
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_manifest_text(StringView text,
StringView origin,
MessageSink& warning_sink)
{
return try_load_any_manifest_text(text, origin, warning_sink, SourceControlFile::parse_port_manifest_object);
}

ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_text(const std::string& text,
StringView origin,
bool is_manifest,
MessageSink& warning_sink)
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_control_file_text(StringView text, StringView origin)
{
StatsTimer timer(g_load_ports_stats);

if (is_manifest)
{
return try_load_manifest_text(text, origin, warning_sink);
}

auto pghs = parse_paragraphs(StringView{text}, origin);
if (auto vector_pghs = pghs.get())
{
return SourceControlFile::parse_control_file(origin, std::move(*vector_pghs)).map_error(ToLocalizedString);
}

return std::move(pghs).error();
return parse_paragraphs(text, origin).then([&](std::vector<Paragraph>&& vector_pghs) {
return SourceControlFile::parse_control_file(origin, std::move(vector_pghs)).map_error(ToLocalizedString);
});
}

ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port(const ReadOnlyFilesystem& fs,
Expand All @@ -431,39 +432,49 @@ namespace vcpkg::Paragraphs
const auto control_path = port_directory / "CONTROL";
std::error_code ec;
auto manifest_contents = fs.read_contents(manifest_path, ec);
if (ec)
if (!ec)
{
const auto exists = ec != std::errc::no_such_file_or_directory;
if (exists)
{
return msg::format_error(msgFailedToParseManifest, msg::path = manifest_path)
.append_raw("\n")
.append(format_filesystem_call_error(ec, "read_contents", {manifest_path}));
}

if (fs.exists(control_path, IgnoreErrors{}))
{
return get_paragraphs(fs, control_path).then([&](std::vector<Paragraph>&& vector_pghs) {
return SourceControlFile::parse_control_file(control_path, std::move(vector_pghs))
.map_error(ToLocalizedString);
});
return msg::format_error(msgManifestConflict, msg::path = port_directory);
}

if (fs.exists(port_directory, IgnoreErrors{}))
{
return msg::format_error(
msgPortMissingManifest, msg::package_name = port_name, msg::path = port_directory);
}
return try_load_port_manifest_text(manifest_contents, manifest_path, stdout_sink);
}

auto manifest_exists = ec != std::errc::no_such_file_or_directory;
if (manifest_exists)
{
return msg::format_error(msgFailedToParseManifest, msg::path = manifest_path)
.append_raw("\n")
.append(format_filesystem_call_error(ec, "read_contents", {manifest_path}));
}

return std::unique_ptr<SourceControlFile>();
auto control_contents = fs.read_contents(control_path, ec);
if (!ec)
{
return try_load_control_file_text(control_contents, control_path);
}

if (ec != std::errc::no_such_file_or_directory)
{
return LocalizedString::from_raw(port_directory)
.append_raw(": ")
.append(format_filesystem_call_error(ec, "read_contents", {control_path}));
}

if (fs.exists(control_path, IgnoreErrors{}))
if (fs.exists(port_directory, IgnoreErrors{}))
{
return msg::format_error(msgManifestConflict, msg::path = port_directory);
return LocalizedString::from_raw(port_directory)
.append_raw(": ")
.append(msgErrorMessage)
.append(msgPortMissingManifest2, msg::package_name = port_name);
}

return try_load_manifest_text(manifest_contents, manifest_path, stdout_sink);
return LocalizedString::from_raw(port_directory)
.append_raw(": ")
.append(msgErrorMessage)
.append(msgPortDoesNotExist, msg::package_name = port_name);
}

ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port_required(const ReadOnlyFilesystem& fs,
Expand Down

0 comments on commit d38bf94

Please sign in to comment.