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.

Sign up for GitHub

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

Plumb error reporting through registries interfaces. #1153

Merged
merged 9 commits into from
Sep 18, 2023
Merged

Plumb error reporting through registries interfaces. #1153

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c780faa
Add maybe_ operations to filesystem.
BillyONeal Aug 10, 2023
af7748b
Deduplicate some error manipulation stuff LocalizedString <-> ParseCo…
BillyONeal Aug 10, 2023
62fb21f
Delete registries.private.h.
BillyONeal Aug 10, 2023
bfe1b6a
Plumb error reporting through the registries interfaces.
BillyONeal Aug 11, 2023
dc5a57a
Merge remote-tracking branch 'origin/main' into add-registry-errors
BillyONeal Aug 12, 2023
844588b
Merge remote-tracking branch 'origin/main' into add-registry-errors
BillyONeal Aug 17, 2023
1ef65c0
Merge remote-tracking branch 'origin/main' into add-registry-errors
BillyONeal Sep 12, 2023
3bb2cb4
Merge remote-tracking branch 'origin/main' into add-registry-errors
BillyONeal Sep 18, 2023
11c9a23
Fixed Robert CR comments.
BillyONeal Sep 18, 2023
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
7 changes: 7 additions & 0 deletions include/vcpkg/base/files.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,25 +140,32 @@ namespace vcpkg

virtual std::vector<Path> get_files_recursive(const Path& dir, std::error_code& ec) const = 0;
std::vector<Path> get_files_recursive(const Path& dir, LineInfo li) const;
ExpectedL<std::vector<Path>> try_get_files_recursive(const Path& dir) const;

virtual std::vector<Path> get_files_non_recursive(const Path& dir, std::error_code& ec) const = 0;
std::vector<Path> get_files_non_recursive(const Path& dir, LineInfo li) const;
ExpectedL<std::vector<Path>> try_get_files_non_recursive(const Path& dir) const;

virtual std::vector<Path> get_directories_recursive(const Path& dir, std::error_code& ec) const = 0;
std::vector<Path> get_directories_recursive(const Path& dir, LineInfo li) const;
ExpectedL<std::vector<Path>> try_get_directories_recursive(const Path& dir) const;

virtual std::vector<Path> get_directories_non_recursive(const Path& dir, std::error_code& ec) const = 0;
std::vector<Path> get_directories_non_recursive(const Path& dir, LineInfo li) const;
ExpectedL<std::vector<Path>> try_get_directories_non_recursive(const Path& dir) const;

virtual std::vector<Path> get_regular_files_recursive(const Path& dir, std::error_code& ec) const = 0;
std::vector<Path> get_regular_files_recursive(const Path& dir, LineInfo li) const;
ExpectedL<std::vector<Path>> try_get_regular_files_recursive(const Path& dir) const;

virtual std::vector<Path> get_regular_files_recursive_lexically_proximate(const Path& dir,
std::error_code& ec) const = 0;
std::vector<Path> get_regular_files_recursive_lexically_proximate(const Path& dir, LineInfo li) const;
ExpectedL<std::vector<Path>> try_get_regular_files_recursive_lexically_proximate(const Path& dir) const;

virtual std::vector<Path> get_regular_files_non_recursive(const Path& dir, std::error_code& ec) const = 0;
std::vector<Path> get_regular_files_non_recursive(const Path& dir, LineInfo li) const;
ExpectedL<std::vector<Path>> try_get_regular_files_non_recursive(const Path& dir) const;

bool exists(const Path& target, std::error_code& ec) const;
bool exists(const Path& target, LineInfo li) const;
Expand Down
26 changes: 10 additions & 16 deletions include/vcpkg/base/message-data.inc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,14 +668,10 @@ DECLARE_MESSAGE(CouldNotFindBaseline,
(msg::commit_sha, msg::path),
"",
"Could not find explicitly specified baseline `\"{commit_sha}\"` in baseline file {path}")
DECLARE_MESSAGE(CouldNotFindBaselineForRepo,
(msg::commit_sha, msg::package_name),
"",
"Couldn't find baseline `\"{commit_sha}\"` for repo {package_name}")
DECLARE_MESSAGE(CouldNotFindBaselineInCommit,
(msg::commit_sha, msg::package_name),
(msg::url, msg::commit_sha, msg::package_name),
"",
"Couldn't find baseline in commit `\"{commit_sha}\"` from repo {package_name}:")
"Couldn't find baseline in {url} at {commit_sha} for {package_name}.")
DECLARE_MESSAGE(CouldNotFindGitTreeAtCommit,
(msg::package_name, msg::commit_sha),
"",
Expand Down Expand Up @@ -978,10 +974,7 @@ DECLARE_MESSAGE(FailedToDetermineArchitecture,
DECLARE_MESSAGE(FailedToDetermineCurrentCommit, (), "", "Failed to determine the current commit:")
DECLARE_MESSAGE(FailedToDownloadFromMirrorSet, (), "", "Failed to download from mirror set")
DECLARE_MESSAGE(FailedToExtract, (msg::path), "", "Failed to extract \"{path}\":")
DECLARE_MESSAGE(FailedToFetchError,
vicroms marked this conversation as resolved.
Show resolved Hide resolved
(msg::error_msg, msg::package_name),
"",
"{error_msg}\nFailed to fetch {package_name}:")
DECLARE_MESSAGE(FailedToFetchRepo, (msg::url), "", "Failed to fetch {url}.")
DECLARE_MESSAGE(FailedToFindBaseline, (), "", "Failed to find baseline.json")
DECLARE_MESSAGE(FailedToFindPortFeature,
(msg::feature, msg::package_name),
Expand Down Expand Up @@ -1129,12 +1122,12 @@ DECLARE_MESSAGE(GitFailedToFetch,
"{value} is a git ref like 'origin/main'",
"failed to fetch ref {value} from repository {url}")
DECLARE_MESSAGE(GitFailedToInitializeLocalRepository, (msg::path), "", "failed to initialize local repository {path}")
DECLARE_MESSAGE(GitRegistryMustHaveBaseline,
(msg::package_name, msg::value),
"{value} is a commit sha",
"The git registry entry for \"{package_name}\" must have a \"baseline\" field that is a valid git "
"commit SHA (40 hexadecimal characters).\n"
"The current HEAD of that repo is \"{value}\".")
DECLARE_MESSAGE(
GitRegistryMustHaveBaseline,
(msg::url, msg::commit_sha),
"",
"The git registry \"{url}\" must have a \"baseline\" field that is a valid git commit SHA (40 hexadecimal "
"characters).\nTo use the current latest versions, set baseline to that repo's HEAD, \"{commit_sha}\".")
DECLARE_MESSAGE(GitStatusOutputExpectedFileName, (), "", "expected a file name")
DECLARE_MESSAGE(GitStatusOutputExpectedNewLine, (), "", "expected new line")
DECLARE_MESSAGE(GitStatusOutputExpectedRenameOrNewline, (), "", "expected renamed file or new lines")
Expand Down Expand Up @@ -2741,6 +2734,7 @@ DECLARE_MESSAGE(VersionBuiltinPortTreeEntryMissing,
"{expected} and {actual} are versions like 1.0.",
"no version database entry for {package_name} at {expected}; using the checked out ports tree "
"version ({actual}).")
DECLARE_MESSAGE(VersionDatabaseEntriesMissing, (msg::package_name), "", "no version entries for {package_name}.")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this have a {url} as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Turns out this message is not used; deleted

Copy link
Member Author

Choose a reason for hiding this comment

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

This being in this PR is either merge gore or fallout from my other branch from which this PR was extracted and it will come back :)

DECLARE_MESSAGE(VersionDatabaseEntryMissing,
(msg::package_name, msg::version),
"",
Expand Down
13 changes: 13 additions & 0 deletions include/vcpkg/paragraphparser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ namespace vcpkg
static std::string format_errors(View<std::unique_ptr<ParseControlErrorInfo>> errors);
void to_string(std::string& target) const;
std::string to_string() const;

static std::unique_ptr<ParseControlErrorInfo> from_error(StringView port_name, LocalizedString&& ls);
};
} // namespace vcpkg

Expand All @@ -46,6 +48,17 @@ namespace vcpkg
template<class P>
using ParseExpected = vcpkg::ExpectedT<std::unique_ptr<P>, std::unique_ptr<ParseControlErrorInfo>>;

template<class P>
ExpectedL<P> map_parse_expected_to_localized_string(ParseExpected<P>&& parse_expected)
{
if (auto value = parse_expected.get())
{
return std::move(**value);
}

return LocalizedString::from_raw(parse_expected.error()->to_string());
}

using Paragraph = std::map<std::string, std::pair<std::string, TextRowCol>, std::less<>>;

struct ParagraphParser
Expand Down
43 changes: 33 additions & 10 deletions include/vcpkg/registries.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#include <vcpkg/fwd/registries.h>
#include <vcpkg/fwd/vcpkgpaths.h>

#include <vcpkg/base/expected.h>
#include <vcpkg/base/optional.h>
#include <vcpkg/base/path.h>
#include <vcpkg/base/span.h>
#include <vcpkg/base/stringview.h>
Expand Down Expand Up @@ -43,10 +45,10 @@ namespace vcpkg
bool stale() const { return data->second.stale; }
const std::string& uri() const { return data->first; }

void ensure_up_to_date(const VcpkgPaths& paths) const;
ExpectedL<Unit> ensure_up_to_date(const VcpkgPaths& paths) const;
};

Entry get_or_fetch(const VcpkgPaths& paths, StringView repo, StringView reference);
ExpectedL<Entry> get_or_fetch(const VcpkgPaths& paths, StringView repo, StringView reference);

LockDataType lockdata;
bool modified = false;
Expand All @@ -63,7 +65,7 @@ namespace vcpkg

struct RegistryEntry
{
virtual View<Version> get_port_versions() const = 0;
virtual ExpectedL<View<Version>> get_port_versions() const = 0;

virtual ExpectedL<PathAndLocation> get_version(const Version& version) const = 0;

Expand All @@ -75,16 +77,16 @@ namespace vcpkg
virtual StringLiteral kind() const = 0;

// returns nullptr if the port doesn't exist
virtual std::unique_ptr<RegistryEntry> get_port_entry(StringView port_name) const = 0;
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
virtual void append_all_port_names(std::vector<std::string>& port_names) const = 0;
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
// network access.
// returns true if names were appended, otherwise returns false.
virtual bool try_append_all_port_names_no_network(std::vector<std::string>& port_names) const = 0;
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;

Expand Down Expand Up @@ -142,10 +144,10 @@ namespace vcpkg
bool has_modifications() const;

// Returns a sorted vector of all reachable port names in this set.
std::vector<std::string> get_all_reachable_port_names() const;
ExpectedL<std::vector<std::string>> get_all_reachable_port_names() const;

// Returns a sorted vector of all reachable port names we can provably determine without touching the network.
std::vector<std::string> get_all_known_reachable_port_names_no_network() const;
ExpectedL<std::vector<std::string>> get_all_known_reachable_port_names_no_network() const;

private:
std::unique_ptr<RegistryImplementation> default_registry_;
Expand All @@ -162,8 +164,8 @@ namespace vcpkg
Path path,
std::string baseline);

ExpectedL<std::vector<std::pair<SchemedVersion, std::string>>> get_builtin_versions(const VcpkgPaths& paths,
StringView port_name);
ExpectedL<Optional<std::vector<std::pair<SchemedVersion, std::string>>>> get_builtin_versions(
Copy link
Member

@vicroms vicroms Aug 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

With these return types I'm grateful that auto exists.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought the same thing when I looked at this lol

const VcpkgPaths& paths, StringView port_name);

ExpectedL<std::map<std::string, Version, std::less<>>> get_builtin_baseline(const VcpkgPaths& paths);

Expand All @@ -173,4 +175,25 @@ namespace vcpkg
// No match is 0, exact match is SIZE_MAX, wildcard match is the length of the pattern.
// Note that the * is included in the match size to distinguish from 0 == no match.
size_t package_pattern_match(StringView name, StringView pattern);

struct VersionDbEntry
{
Version version;
VersionScheme scheme = VersionScheme::String;

// only one of these may be non-empty
std::string git_tree;
Path p;
};

// VersionDbType::Git => VersionDbEntry.git_tree is filled
// VersionDbType::Filesystem => VersionDbEntry.path is filled
enum class VersionDbType
{
Git,
Filesystem,
};

std::unique_ptr<Json::IDeserializer<std::vector<VersionDbEntry>>> make_version_db_deserializer(VersionDbType type,
const Path& root);
}
35 changes: 0 additions & 35 deletions include/vcpkg/registries.private.h
View file
Open in desktop

This file was deleted.

16 changes: 8 additions & 8 deletions locales/messages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,10 +390,8 @@
"_CouldNotDeduceNugetIdAndVersion.comment": "An example of {path} is /foo/bar.",
"CouldNotFindBaseline": "Could not find explicitly specified baseline `\"{commit_sha}\"` in baseline file {path}",
"_CouldNotFindBaseline.comment": "An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949. An example of {path} is /foo/bar.",
"CouldNotFindBaselineForRepo": "Couldn't find baseline `\"{commit_sha}\"` for repo {package_name}",
"_CouldNotFindBaselineForRepo.comment": "An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949. An example of {package_name} is zlib.",
"CouldNotFindBaselineInCommit": "Couldn't find baseline in commit `\"{commit_sha}\"` from repo {package_name}:",
"_CouldNotFindBaselineInCommit.comment": "An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949. An example of {package_name} is zlib.",
"CouldNotFindBaselineInCommit": "Couldn't find baseline in {url} at {commit_sha} for {package_name}.",
"_CouldNotFindBaselineInCommit.comment": "An example of {url} is https://github.com/microsoft/vcpkg. An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949. An example of {package_name} is zlib.",
"CouldNotFindGitTreeAtCommit": "could not find the git tree for `versions` in repo {package_name} at commit {commit_sha}",
"_CouldNotFindGitTreeAtCommit.comment": "An example of {package_name} is zlib. An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949.",
"CouldNotFindToolVersion": "Could not find <tools version=\"{version}\"> in {path}",
Expand Down Expand Up @@ -595,8 +593,8 @@
"FailedToDownloadFromMirrorSet": "Failed to download from mirror set",
"FailedToExtract": "Failed to extract \"{path}\":",
"_FailedToExtract.comment": "An example of {path} is /foo/bar.",
"FailedToFetchError": "{error_msg}\nFailed to fetch {package_name}:",
"_FailedToFetchError.comment": "An example of {error_msg} is File Not Found. An example of {package_name} is zlib.",
"FailedToFetchRepo": "Failed to fetch {url}.",
"_FailedToFetchRepo.comment": "An example of {url} is https://github.com/microsoft/vcpkg.",
"FailedToFindBaseline": "Failed to find baseline.json",
"FailedToFindPortFeature": "{package_name} has no feature named {feature}.",
"_FailedToFindPortFeature.comment": "An example of {feature} is avisynthplus. An example of {package_name} is zlib.",
Expand Down Expand Up @@ -710,8 +708,8 @@
"_GitFailedToFetch.comment": "{value} is a git ref like 'origin/main' An example of {url} is https://github.com/microsoft/vcpkg.",
"GitFailedToInitializeLocalRepository": "failed to initialize local repository {path}",
"_GitFailedToInitializeLocalRepository.comment": "An example of {path} is /foo/bar.",
"GitRegistryMustHaveBaseline": "The git registry entry for \"{package_name}\" must have a \"baseline\" field that is a valid git commit SHA (40 hexadecimal characters).\nThe current HEAD of that repo is \"{value}\".",
"_GitRegistryMustHaveBaseline.comment": "{value} is a commit sha An example of {package_name} is zlib.",
"GitRegistryMustHaveBaseline": "The git registry \"{url}\" must have a \"baseline\" field that is a valid git commit SHA (40 hexadecimal characters).\nTo use the current latest versions, set baseline to that repo's HEAD, \"{commit_sha}\".",
"_GitRegistryMustHaveBaseline.comment": "An example of {url} is https://github.com/microsoft/vcpkg. An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949.",
"GitStatusOutputExpectedFileName": "expected a file name",
"GitStatusOutputExpectedNewLine": "expected new line",
"GitStatusOutputExpectedRenameOrNewline": "expected renamed file or new lines",
Expand Down Expand Up @@ -1505,6 +1503,8 @@
"_VersionConstraintUnresolvable.comment": "An example of {package_name} is zlib. An example of {spec} is zlib:x64-windows.",
"VersionConstraintViolated": "dependency {spec} was expected to be at least version {expected_version}, but is currently {actual_version}.",
"_VersionConstraintViolated.comment": "An example of {spec} is zlib:x64-windows. An example of {expected_version} is 1.3.8. An example of {actual_version} is 1.3.8.",
"VersionDatabaseEntriesMissing": "no version entries for {package_name}.",
"_VersionDatabaseEntriesMissing.comment": "An example of {package_name} is zlib.",
"VersionDatabaseEntryMissing": "no version entry for {package_name} at {version}.",
"_VersionDatabaseEntryMissing.comment": "An example of {package_name} is zlib. An example of {version} is 1.3.8.",
"VersionDatabaseFileMissing": "{package_name} is missing a version database file at {path}\nRun:\nvcpkg x-add-version {package_name}\nto create the versions file.",
Expand Down
16 changes: 8 additions & 8 deletions src/vcpkg-test/registries.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

#include <vcpkg/configuration.h>
#include <vcpkg/registries.h>
#include <vcpkg/registries.private.h>

using namespace vcpkg;

Expand All @@ -15,14 +14,15 @@ namespace
{
StringLiteral kind() const override { return "test"; }

std::unique_ptr<RegistryEntry> get_port_entry(StringView) const override { return nullptr; }
ExpectedL<std::unique_ptr<RegistryEntry>> get_port_entry(StringView) const override { return nullptr; }

void append_all_port_names(std::vector<std::string>& port_names) const override
ExpectedL<Unit> append_all_port_names(std::vector<std::string>& port_names) const override
{
port_names.insert(port_names.end(), all_port_names.begin(), all_port_names.end());
return Unit{};
}

bool try_append_all_port_names_no_network(std::vector<std::string>& port_names) const override
ExpectedL<bool> try_append_all_port_names_no_network(std::vector<std::string>& port_names) const override
{
port_names.insert(port_names.end(), no_network_port_names.begin(), no_network_port_names.end());
return !no_network_port_names.empty();
Expand Down Expand Up @@ -766,14 +766,14 @@ TEST_CASE ("get_all_port_names", "[registries]")
// All the known ports from the default registry
// hello, world, abcdefg, abc, abcde from the first registry
// twoRegistry from the second registry
CHECK(with_default_registry.get_all_reachable_port_names() ==
CHECK(with_default_registry.get_all_reachable_port_names().value_or_exit(VCPKG_LINE_INFO) ==
std::vector<std::string>{
"aDefault", "abc", "abcde", "abcdefg", "bDefault", "cDefault", "hello", "twoRegistry", "world"});

// All the old ports from the default registry
// hello, world, notpresent from the first registry (since network was unknown)
// twoOld from the second registry
CHECK(with_default_registry.get_all_known_reachable_port_names_no_network() ==
CHECK(with_default_registry.get_all_known_reachable_port_names_no_network().value_or_exit(VCPKG_LINE_INFO) ==
std::vector<std::string>{
"aDefaultOld", "bDefaultOld", "cDefaultOld", "hello", "notpresent", "twoOld", "world"});
}
Expand All @@ -784,12 +784,12 @@ TEST_CASE ("get_all_port_names", "[registries]")

// hello, world, abcdefg, abc, abcde from the first registry
// twoRegistry from the second registry
CHECK(without_default_registry.get_all_reachable_port_names() ==
CHECK(without_default_registry.get_all_reachable_port_names().value_or_exit(VCPKG_LINE_INFO) ==
std::vector<std::string>{"abc", "abcde", "abcdefg", "hello", "twoRegistry", "world"});

// hello, world, notpresent from the first registry
// twoOld from the second registry
CHECK(without_default_registry.get_all_known_reachable_port_names_no_network() ==
CHECK(without_default_registry.get_all_known_reachable_port_names_no_network().value_or_exit(VCPKG_LINE_INFO) ==
std::vector<std::string>{"hello", "notpresent", "twoOld", "world"});
}
}
Loading