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 6 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 unique_ptr contains the loaded port information.
// Otherwise, the unique_ptr is nullptr.
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 checked without network access.
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
7 changes: 2 additions & 5 deletions include/vcpkg/sourceparagraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +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)
{
return print_error_message({&error_info_list, 1});
}
void print_error_message(const LocalizedString& message);
void print_error_message(const std::unique_ptr<ParseControlErrorInfo>& error_info_list);

std::string parse_spdx_license_expression(StringView sv, ParseMessages& messages);

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
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
5 changes: 3 additions & 2 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());
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
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
1 change: 0 additions & 1 deletion src/vcpkg/commands.format-manifest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ namespace
{
msg::println_error(msgFailedToParseManifest, msg::path = path_string);
print_error_message(scf.error());
msg::println();
return nullopt;
}

Expand Down
Loading