Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make registries distinguish between 'baseline is broken' and 'baseline missing'. #1203

Merged
merged 7 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions include/vcpkg/paragraphparser.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ namespace vcpkg
using ParseExpected = vcpkg::ExpectedT<std::unique_ptr<P>, std::unique_ptr<ParseControlErrorInfo>>;

template<class P>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion:

static constexpr struct ToLocalizedString_t
{
    LocalizedString operator()(std::unique_ptr<ParseControlErrorInfo> p) const;
} ToLocalizedString;

Then, call sites can use this as

return maybe_parse_thing().map_error(ToLocalizedString);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

ExpectedL<P> map_parse_expected_to_localized_string(ParseExpected<P>&& parse_expected)
ExpectedL<std::unique_ptr<P>> map_parse_expected_to_localized_string(ParseExpected<P>&& parse_expected)
BillyONeal marked this conversation as resolved.
Show resolved Hide resolved
{
if (auto value = parse_expected.get())
{
return std::move(**value);
return std::move(*value);
}

return LocalizedString::from_raw(parse_expected.error()->to_string());
Expand Down
25 changes: 19 additions & 6 deletions include/vcpkg/paragraphs.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
#include <vcpkg/fwd/registries.h>

#include <vcpkg/base/expected.h>
#include <vcpkg/base/stringview.h>

#include <vcpkg/sourceparagraph.h>

#include <utility>
#include <vector>

namespace vcpkg::Paragraphs
{
uint64_t get_load_ports_stats();
Expand All @@ -22,11 +26,20 @@ namespace vcpkg::Paragraphs

bool is_port_directory(const ReadOnlyFilesystem& fs, const Path& maybe_directory);

ParseExpected<SourceControlFile> try_load_port(const ReadOnlyFilesystem& fs, const Path& port_directory);
ParseExpected<SourceControlFile> try_load_port_text(const std::string& text,
StringView origin,
bool is_manifest,
MessageSink& warning_sink);
// If an error occurs, the Expected will be in the error state.
// Otherwise, if the port is known, the Optional contains the loaded port information.
// Otherwise, the Optional is disengaged.
ras0219-msft marked this conversation as resolved.
Show resolved Hide resolved
BillyONeal marked this conversation as resolved.
Show resolved Hide resolved
ExpectedL<std::unique_ptr<SourceControlFile>> try_load_port(const ReadOnlyFilesystem& fs,
StringView port_name,
const Path& port_directory);
// Identical to try_load_port, but the port unknown condition is mapped to an error.
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,
ras0219-msft marked this conversation as resolved.
Show resolved Hide resolved
MessageSink& warning_sink);

ExpectedL<BinaryControlFile> try_load_cached_package(const ReadOnlyFilesystem& fs,
const Path& package_dir,
Expand All @@ -35,7 +48,7 @@ namespace vcpkg::Paragraphs
struct LoadResults
{
std::vector<SourceControlFileAndLocation> paragraphs;
std::vector<std::unique_ptr<ParseControlErrorInfo>> errors;
std::vector<std::pair<std::string, LocalizedString>> errors;
};

LoadResults try_load_all_registry_ports(const ReadOnlyFilesystem& fs, const RegistrySet& registries);
Expand Down
19 changes: 12 additions & 7 deletions include/vcpkg/registries.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,19 +76,24 @@ namespace vcpkg
{
virtual StringLiteral kind() const = 0;

// returns nullptr if the port doesn't exist
// If an error occurs, the ExpectedL will be in an error state.
// Otherwise, if the port is known, returns a pointer to RegistryEntry describing the port.
// Otherwise, returns a nullptr unique_ptr.
virtual ExpectedL<std::unique_ptr<RegistryEntry>> get_port_entry(StringView port_name) const = 0;

// appends the names of the ports to the out parameter
// may result in duplicated port names; make sure to Util::sort_unique_erase at the end
// Appends the names of the known ports to the out parameter.
// May result in duplicated port names; make sure to Util::sort_unique_erase at the end
virtual ExpectedL<Unit> append_all_port_names(std::vector<std::string>& port_names) const = 0;

// appends the names of the ports to the out parameter if this can be known without
// Appends the names of the ports to the out parameter if this can be known without
// network access.
// returns true if names were appended, otherwise returns false.
// Returns true iff names were appended.
ras0219-msft marked this conversation as resolved.
Show resolved Hide resolved
virtual ExpectedL<bool> try_append_all_port_names_no_network(std::vector<std::string>& port_names) const = 0;

virtual ExpectedL<Version> get_baseline_version(StringView port_name) const = 0;
// If an error occurs, the ExpectedL will be in an error state.
// Otherwise, if the port is in the baseline, returns the version that baseline denotes.
// Otherwise, the Optional is disengaged.
virtual ExpectedL<Optional<Version>> get_baseline_version(StringView port_name) const = 0;

virtual ~RegistryImplementation() = default;
};
Expand Down Expand Up @@ -130,7 +135,7 @@ namespace vcpkg
// the returned list is sorted by priority.
std::vector<const RegistryImplementation*> registries_for_port(StringView name) const;

ExpectedL<Version> baseline_for_port(StringView port_name) const;
ExpectedL<Optional<Version>> baseline_for_port(StringView port_name) const;

View<Registry> registries() const { return registries_; }

Expand Down
4 changes: 2 additions & 2 deletions include/vcpkg/sourceparagraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ namespace vcpkg
std::string registry_location;
};

void print_error_message(Span<const std::unique_ptr<ParseControlErrorInfo>> error_info_list);
inline void print_error_message(const std::unique_ptr<ParseControlErrorInfo>& error_info_list)
void print_error_message(View<LocalizedString> error_info_list);
ras0219-msft marked this conversation as resolved.
Show resolved Hide resolved
inline void print_error_message(const LocalizedString& error_info_list)
ras0219-msft marked this conversation as resolved.
Show resolved Hide resolved
{
return print_error_message({&error_info_list, 1});
}
Expand Down
6 changes: 3 additions & 3 deletions src/vcpkg-test/manifests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ static ParseExpected<SourceControlFile> test_parse_project_manifest(const Json::
auto res = SourceControlFile::parse_project_manifest_object("<test manifest>", obj, null_sink);
if (!res.has_value() && print == PrintErrors::Yes)
{
print_error_message(res.error());
print_error_message(LocalizedString::from_raw(res.error()->to_string()));
}
return res;
}
Expand All @@ -56,7 +56,7 @@ static ParseExpected<SourceControlFile> test_parse_port_manifest(const Json::Obj
auto res = SourceControlFile::parse_port_manifest_object("<test manifest>", obj, null_sink);
if (!res.has_value() && print == PrintErrors::Yes)
{
print_error_message(res.error());
print_error_message(LocalizedString::from_raw(res.error()->to_string()));
}
return res;
}
Expand Down Expand Up @@ -886,7 +886,7 @@ TEST_CASE ("manifest construct maximum", "[manifests]")
auto res = SourceControlFile::parse_port_manifest_object("<test manifest>", object, null_sink);
if (!res.has_value())
{
print_error_message(res.error());
print_error_message(LocalizedString::from_raw(res.error()->to_string()));
}
REQUIRE(res.has_value());
REQUIRE(*res.get() != nullptr);
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg-test/registries.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace
return !no_network_port_names.empty();
}

ExpectedL<Version> get_baseline_version(StringView) const override
ExpectedL<Optional<Version>> get_baseline_version(StringView) const override
{
return LocalizedString::from_raw("error");
}
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/binaryparagraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ namespace vcpkg
if (const auto err = parser.error_info(this->spec.to_string()))
{
msg::println_error(msgErrorParsingBinaryParagraph, msg::spec = this->spec);
print_error_message(err);
print_error_message(LocalizedString::from_raw(err->to_string()));
Checks::exit_fail(VCPKG_LINE_INFO);
}

Expand Down
14 changes: 7 additions & 7 deletions src/vcpkg/commands.add-version.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -412,25 +412,25 @@ namespace vcpkg
continue;
}

auto maybe_scf = Paragraphs::try_load_port(fs, paths.builtin_ports_directory() / port_name);
if (!maybe_scf)
auto maybe_scf =
Paragraphs::try_load_port_required(fs, port_name, paths.builtin_ports_directory() / port_name);
auto scf = maybe_scf.get();
if (!scf)
{
msg::println_error(msgAddVersionLoadPortFailed, msg::package_name = port_name);
print_error_message(maybe_scf.error());
msg::println(Color::error, maybe_scf.error());
Checks::check_exit(VCPKG_LINE_INFO, !add_all);
continue;
}

const auto& scf = maybe_scf.value(VCPKG_LINE_INFO);

if (!skip_formatting_check)
{
// check if manifest file is property formatted
const auto path_to_manifest = paths.builtin_ports_directory() / port_name / "vcpkg.json";
if (fs.exists(path_to_manifest, IgnoreErrors{}))
{
const auto current_file_content = fs.read_contents(path_to_manifest, VCPKG_LINE_INFO);
const auto json = serialize_manifest(*scf);
const auto json = serialize_manifest(**scf);
const auto formatted_content = Json::stringify(json);
if (current_file_content != formatted_content)
{
Expand All @@ -454,7 +454,7 @@ namespace vcpkg
msg::println_warning(msgAddVersionUncommittedChanges, msg::package_name = port_name);
}

const auto& schemed_version = scf->to_schemed_version();
const auto& schemed_version = (*scf)->to_schemed_version();

auto git_tree_it = git_tree_map.find(port_name);
if (git_tree_it == git_tree_map.end())
Expand Down
7 changes: 4 additions & 3 deletions src/vcpkg/commands.add.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,15 @@ namespace vcpkg

auto maybe_manifest_scf =
SourceControlFile::parse_project_manifest_object(manifest->path, manifest->manifest, stdout_sink);
if (!maybe_manifest_scf)
auto pmanifest_scf = maybe_manifest_scf.get();
if (!pmanifest_scf)
{
print_error_message(maybe_manifest_scf.error());
print_error_message(LocalizedString::from_raw(maybe_manifest_scf.error()->to_string()));
msg::println(Color::error, msg::msgSeeURL, msg::url = docs::manifests_url);
Checks::exit_fail(VCPKG_LINE_INFO);
}

auto& manifest_scf = *maybe_manifest_scf.value(VCPKG_LINE_INFO);
auto& manifest_scf = **pmanifest_scf;
for (const auto& spec : specs)
{
auto dep = Util::find_if(manifest_scf.core_paragraph->dependencies, [&spec](Dependency& dep) {
Expand Down
4 changes: 2 additions & 2 deletions src/vcpkg/commands.autocomplete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ namespace vcpkg
StringView port_name{last_arg.begin(), colon};
StringView triplet_prefix{colon + 1, last_arg.end()};
// TODO: Support autocomplete for ports in --overlay-ports
auto maybe_port =
Paragraphs::try_load_port(paths.get_filesystem(), paths.builtin_ports_directory() / port_name);
auto maybe_port = Paragraphs::try_load_port_required(
paths.get_filesystem(), port_name, paths.builtin_ports_directory() / port_name);
if (!maybe_port)
{
Checks::exit_success(VCPKG_LINE_INFO);
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/commands.build.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1684,7 +1684,7 @@ namespace vcpkg

if (const auto err = parser.error_info("PostBuildInformation"))
{
print_error_message(err);
print_error_message(LocalizedString::from_raw(err->to_string()));
Checks::exit_fail(VCPKG_LINE_INFO);
}

Expand Down
17 changes: 9 additions & 8 deletions src/vcpkg/commands.ci-verify-versions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ namespace
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);
if (!maybe_scf)
auto scf = maybe_scf.get();
if (!scf)
{
return {msg::format_error(msgWhileParsingVersionsForPort,
msg::package_name = port_name,
Expand All @@ -82,12 +83,11 @@ namespace
.append_raw('\n')
.append(msgWhileLoadingPortFromGitTree, msg::commit_sha = treeish)
.append_raw('\n')
.append_raw(maybe_scf.error()->error),
.append(maybe_scf.error()),
expected_right_tag};
}

const auto& scf = maybe_scf.value(VCPKG_LINE_INFO);
auto&& git_tree_version = scf->to_schemed_version();
auto&& git_tree_version = (*scf)->to_schemed_version();
if (version_entry.version.version != git_tree_version.version)
{
return {
Expand Down Expand Up @@ -122,16 +122,17 @@ namespace
}
}

auto maybe_scf = Paragraphs::try_load_port(paths.get_filesystem(), port_path);
if (!maybe_scf)
auto maybe_scf = Paragraphs::try_load_port_required(paths.get_filesystem(), port_name, port_path);
auto scf = maybe_scf.get();
if (!scf)
{
return {msg::format_error(msgWhileLoadingLocalPort, msg::package_name = port_name)
.append_raw('\n')
.append_raw(maybe_scf.error()->error),
.append(maybe_scf.error()),
expected_right_tag};
}

const auto local_port_version = maybe_scf.value(VCPKG_LINE_INFO)->to_schemed_version();
const auto local_port_version = (*scf)->to_schemed_version();

auto versions_end = versions->end();
auto it = std::find_if(versions->begin(), versions_end, [&](const GitVersionDbEntry& entry) {
Expand Down
4 changes: 2 additions & 2 deletions src/vcpkg/commands.format-manifest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ namespace
if (!scf)
{
msg::println_error(msgFailedToParseManifest, msg::path = path_string);
print_error_message(scf.error());
print_error_message(LocalizedString::from_raw(scf.error()->to_string()));
msg::println();
return nullopt;
}
Expand Down Expand Up @@ -82,7 +82,7 @@ namespace
if (!scf_res)
{
msg::println_error(msgFailedToParseControl, msg::path = control_path);
print_error_message(scf_res.error());
print_error_message(LocalizedString::from_raw(scf_res.error()->to_string()));
return {};
}

Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg/commands.install.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1104,7 +1104,7 @@ namespace vcpkg
SourceControlFile::parse_project_manifest_object(manifest->path, manifest->manifest, stdout_sink);
if (!maybe_manifest_scf)
{
print_error_message(maybe_manifest_scf.error());
print_error_message(LocalizedString::from_raw(maybe_manifest_scf.error()->to_string()));
msg::println(msgExtendedDocumentationAtUrl, msg::url = docs::manifests_url);
Checks::exit_fail(VCPKG_LINE_INFO);
}
Expand Down
Loading