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

"<unknown>" and "<command>" are not origins #1346

Merged
merged 13 commits into from
Mar 9, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions azure-pipelines/end-to-end-tests-dir/cli.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,36 @@ if (-Not ($out.StartsWith('error: unexpected switch: --not-a-switch')))
throw 'Bad install --not-a-switch output'
}

$out = Run-VcpkgAndCaptureOutput -TestArgs @('install', 'this-is-super-not-a-#port')
Throw-IfNotFailed

[string]$expected = @"
error: expected eof
BillyONeal marked this conversation as resolved.
Show resolved Hide resolved
on expression: this-is-super-not-a-#port
^

"@

if (-Not ($out.Replace("`r`n", "`n").EndsWith($expected)))
{
throw 'Bad malformed port name output; it was: ' + $out
}

$out = Run-VcpkgAndCaptureOutput -TestArgs @('install', 'zlib', '--binarysource=clear;not-a-backend')
Throw-IfNotFailed

[string]$expected = @"
error: unknown binary provider type: valid providers are 'clear', 'default', 'nuget', 'nugetconfig', 'nugettimeout', 'interactive', 'x-azblob', 'x-gcs', 'x-aws', 'x-aws-config', 'http', and 'files'
on expression: clear;not-a-backend
^

"@

if (-Not ($out.Replace("`r`n", "`n").EndsWith($expected)))
{
throw 'Bad malformed --binarysource output; it was: ' + $out
}

if ($IsWindows) {
$warningText = 'In the September 2023 release'

Expand Down
1 change: 1 addition & 0 deletions include/vcpkg/base/jsonreader.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ namespace vcpkg::Json
const std::vector<LocalizedString>& warnings() const { return m_warnings; }

std::string path() const noexcept;
StringView origin() const noexcept;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is origin for ParserBase an Optional<StringView> but for JsonReader a StringView?

Copy link
Member Author

Choose a reason for hiding this comment

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

ParserBase is reasonably used for things that don't have origins, like content from environment variables or command lines. At least at present, JSON only comes from honest to goodness files which always have origins, so making the parameter nullable is unnecessary.


private:
template<class Type>
Expand Down
2 changes: 1 addition & 1 deletion include/vcpkg/base/message-data.inc.h
Original file line number Diff line number Diff line change
Expand Up @@ -2705,7 +2705,7 @@ DECLARE_MESSAGE(UnknownBinaryProviderType,
(),
"",
"unknown binary provider type: valid providers are 'clear', 'default', 'nuget', "
"'nugetconfig','nugettimeout', 'interactive', 'x-azblob', 'x-gcs', 'x-aws', "
"'nugetconfig', 'nugettimeout', 'interactive', 'x-azblob', 'x-gcs', 'x-aws', "
"'x-aws-config', 'http', and 'files'")
DECLARE_MESSAGE(UnknownBooleanSetting,
(msg::option, msg::value),
Expand Down
4 changes: 2 additions & 2 deletions include/vcpkg/base/parse.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ namespace vcpkg

struct ParserBase
{
ParserBase(StringView text, StringView origin, TextRowCol init_rowcol = {});
ParserBase(StringView text, Optional<StringView> origin, TextRowCol init_rowcol = {});

static constexpr bool is_whitespace(char32_t ch) { return ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n'; }
static constexpr bool is_lower_alpha(char32_t ch) { return ch >= 'a' && ch <= 'z'; }
Expand Down Expand Up @@ -119,7 +119,7 @@ namespace vcpkg
int m_column;

StringView m_text;
StringView m_origin;
Optional<StringView> m_origin;

ParseMessages m_messages;
};
Expand Down
8 changes: 4 additions & 4 deletions include/vcpkg/paragraphparser.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ namespace vcpkg
};

ExpectedL<std::vector<std::string>> parse_default_features_list(const std::string& str,
StringView origin = "<unknown>",
TextRowCol textrowcol = {});
Optional<StringView> origin,
TextRowCol textrowcol);
ExpectedL<std::vector<ParsedQualifiedSpecifier>> parse_qualified_specifier_list(const std::string& str,
StringView origin = "<unknown>",
TextRowCol textrowcol = {});
Optional<StringView> origin,
TextRowCol textrowcol);
}
2 changes: 1 addition & 1 deletion locales/messages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1528,7 +1528,7 @@
"UnexpectedWindowsArchitecture": "unexpected Windows host architecture: {actual}",
"_UnexpectedWindowsArchitecture.comment": "{actual} is the CPU kind we observed like ARM or MIPS",
"UnknownBaselineFileContent": "unrecognizable baseline entry; expected 'port:triplet=(fail|skip|pass)'",
"UnknownBinaryProviderType": "unknown binary provider type: valid providers are 'clear', 'default', 'nuget', 'nugetconfig','nugettimeout', 'interactive', 'x-azblob', 'x-gcs', 'x-aws', 'x-aws-config', 'http', and 'files'",
"UnknownBinaryProviderType": "unknown binary provider type: valid providers are 'clear', 'default', 'nuget', 'nugetconfig', 'nugettimeout', 'interactive', 'x-azblob', 'x-gcs', 'x-aws', 'x-aws-config', 'http', and 'files'",
"UnknownBooleanSetting": "unknown boolean setting for {option}: \"{value}\". Valid values are '', '1', '0', 'ON', 'OFF', 'TRUE', and 'FALSE'.",
"_UnknownBooleanSetting.comment": "{value} is what {option} is set to An example of {option} is editable.",
"UnknownOptions": "Unknown option(s) for command '{command_name}':",
Expand Down
8 changes: 4 additions & 4 deletions src/vcpkg-test/binarycaching.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ Build-Depends: bzip
)",
"<testdata>");
REQUIRE(pghs.has_value());
auto maybe_scf = SourceControlFile::parse_control_file("", std::move(*pghs.get()));
auto maybe_scf = SourceControlFile::parse_control_file("test-origin", std::move(*pghs.get()));
REQUIRE(maybe_scf.has_value());
SourceControlFileAndLocation scfl{std::move(*maybe_scf.get()), Path()};

Expand Down Expand Up @@ -338,7 +338,7 @@ Version: 1.5
)",
"<testdata>");
REQUIRE(pghs.has_value());
auto maybe_scf = SourceControlFile::parse_control_file("", std::move(*pghs.get()));
auto maybe_scf = SourceControlFile::parse_control_file("test-origin", std::move(*pghs.get()));
REQUIRE(maybe_scf.has_value());
SourceControlFileAndLocation scfl{std::move(*maybe_scf.get()), Path()};
std::vector<InstallPlanAction> install_plan;
Expand Down Expand Up @@ -416,7 +416,7 @@ Description: a spiffy compression library wrapper
)",
"<testdata>");
REQUIRE(pghs.has_value());
auto maybe_scf = SourceControlFile::parse_control_file("", std::move(*pghs.get()));
auto maybe_scf = SourceControlFile::parse_control_file("test-origin", std::move(*pghs.get()));
REQUIRE(maybe_scf.has_value());
SourceControlFileAndLocation scfl{std::move(*maybe_scf.get()), Path()};
plan.install_actions.emplace_back(PackageSpec("zlib", Test::X64_ANDROID),
Expand Down Expand Up @@ -444,7 +444,7 @@ Description: a spiffy compression library wrapper
)",
"<testdata>");
REQUIRE(pghs2.has_value());
auto maybe_scf2 = SourceControlFile::parse_control_file("", std::move(*pghs2.get()));
auto maybe_scf2 = SourceControlFile::parse_control_file("test-origin", std::move(*pghs2.get()));
REQUIRE(maybe_scf2.has_value());
SourceControlFileAndLocation scfl2{std::move(*maybe_scf2.get()), Path()};
plan.install_actions.emplace_back(PackageSpec("zlib2", Test::X64_ANDROID),
Expand Down
34 changes: 17 additions & 17 deletions src/vcpkg-test/paragraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace
for (auto&& kv : p)
pghs.back().emplace(kv.first, std::make_pair(kv.second, vcpkg::TextRowCol{}));
}
return vcpkg::SourceControlFile::parse_control_file("", std::move(pghs));
return vcpkg::SourceControlFile::parse_control_file("test-origin", std::move(pghs));
}

auto test_make_binary_paragraph(const std::unordered_map<std::string, std::string>& v)
Expand Down Expand Up @@ -332,14 +332,14 @@ TEST_CASE ("BinaryParagraph default features", "[paragraph]")
TEST_CASE ("parse paragraphs empty", "[paragraph]")
{
const char* str = "";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);
REQUIRE(pghs.empty());
}

TEST_CASE ("parse paragraphs one field", "[paragraph]")
{
const char* str = "f1: v1";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);
REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0].size() == 1);
REQUIRE(pghs[0]["f1"].first == "v1");
Expand All @@ -349,7 +349,7 @@ TEST_CASE ("parse paragraphs one pgh", "[paragraph]")
{
const char* str = "f1: v1\n"
"f2: v2";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);
REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0].size() == 2);
REQUIRE(pghs[0]["f1"].first == "v1");
Expand All @@ -363,7 +363,7 @@ TEST_CASE ("parse paragraphs two pgh", "[paragraph]")
"\n"
"f3: v3\n"
"f4: v4";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 2);
REQUIRE(pghs[0].size() == 2);
Expand All @@ -381,7 +381,7 @@ TEST_CASE ("parse paragraphs field names", "[paragraph]")
"F:\n"
"0:\n"
"F-2:\n";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0].size() == 5);
Expand All @@ -395,7 +395,7 @@ TEST_CASE ("parse paragraphs multiple blank lines", "[paragraph]")
"\n"
"f3: v3\n"
"f4: v4";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 2);
}
Expand All @@ -404,7 +404,7 @@ TEST_CASE ("parse paragraphs empty fields", "[paragraph]")
{
const char* str = "f1:\n"
"f2: ";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0].size() == 2);
Expand All @@ -420,7 +420,7 @@ TEST_CASE ("parse paragraphs multiline fields", "[paragraph]")
"f2:\r\n"
" f2\r\n"
" continue\r\n";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0]["f1"].first == "simple\n f1");
Expand All @@ -434,7 +434,7 @@ TEST_CASE ("parse paragraphs crlfs", "[paragraph]")
"\r\n"
"f3: v3\r\n"
"f4: v4";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 2);
REQUIRE(pghs[0].size() == 2);
Expand All @@ -456,7 +456,7 @@ TEST_CASE ("parse paragraphs comment", "[paragraph]")
"f3: v3\r\n"
"#comment\r\n"
"f4: v4";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 2);
REQUIRE(pghs[0].size() == 2);
Expand All @@ -471,7 +471,7 @@ TEST_CASE ("parse comment before single line feed", "[paragraph]")
{
const char* str = "f1: v1\r\n"
"#comment\n";
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(str, "test-origin").value_or_exit(VCPKG_LINE_INFO);
REQUIRE(pghs[0].size() == 1);
REQUIRE(pghs[0]["f1"].first == "v1");
}
Expand All @@ -485,7 +485,7 @@ TEST_CASE ("BinaryParagraph serialize min", "[paragraph]")
{"Multi-Arch", "same"},
});
std::string ss = Strings::serialize(pgh);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0].size() == 4);
Expand All @@ -507,7 +507,7 @@ TEST_CASE ("BinaryParagraph serialize max", "[paragraph]")
{"Multi-Arch", "same"},
});
std::string ss = Strings::serialize(pgh);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0].size() == 7);
Expand All @@ -531,7 +531,7 @@ TEST_CASE ("BinaryParagraph serialize multiple deps", "[paragraph]")
{"Depends", "a, b, c"},
});
std::string ss = Strings::serialize(pgh);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0]["Depends"].first == "a, b, c");
Expand All @@ -546,7 +546,7 @@ TEST_CASE ("BinaryParagraph serialize multiple deps", "[paragraph]")
{"Depends", "a:x64-windows, b, c:arm-uwp"},
});
std::string ss = Strings::serialize(pgh);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0]["Depends"].first == "a:x64-windows, b, c:arm-uwp");
Expand All @@ -564,7 +564,7 @@ TEST_CASE ("BinaryParagraph serialize abi", "[paragraph]")
{"Abi", "123abc"},
});
std::string ss = Strings::serialize(pgh);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "").value_or_exit(VCPKG_LINE_INFO);
auto pghs = vcpkg::Paragraphs::parse_paragraphs(ss, "test-origin").value_or_exit(VCPKG_LINE_INFO);

REQUIRE(pghs.size() == 1);
REQUIRE(pghs[0]["Abi"].first == "123abc");
Expand Down
8 changes: 4 additions & 4 deletions src/vcpkg-test/statusparagraphs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ Multi-Arch: same
Description:
Status: install ok installed
)",
"");
"test-origin");

REQUIRE(pghs);

Expand All @@ -41,7 +41,7 @@ Multi-Arch: same
Description:
Status: purge ok not-installed
)",
"");
"test-origin");

REQUIRE(pghs);

Expand Down Expand Up @@ -71,7 +71,7 @@ Multi-Arch: same
Description:
Status: purge ok not-installed
)",
"");
"test-origin");

REQUIRE(pghs);

Expand Down Expand Up @@ -105,7 +105,7 @@ Multi-Arch: same
Description:
Status: install ok installed
)",
"");
"test-origin");
REQUIRE(pghs);

StatusParagraphs status_db(Util::fmap(*pghs.get(), [](Paragraph& rpgh) {
Expand Down
2 changes: 1 addition & 1 deletion src/vcpkg-test/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ namespace vcpkg::Test
pghs.back().emplace(kv.first, std::make_pair(kv.second, vcpkg::TextRowCol{}));
}
}
return vcpkg::SourceControlFile::parse_control_file("", std::move(pghs));
return vcpkg::SourceControlFile::parse_control_file("test-origin", std::move(pghs));
}

std::unique_ptr<vcpkg::StatusParagraph> make_status_pgh(const char* name,
Expand Down
2 changes: 2 additions & 0 deletions src/vcpkg/base/json.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,8 @@ namespace vcpkg::Json
return p;
}

StringView Reader::origin() const noexcept { return m_origin; }

LocalizedString ParagraphDeserializer::type_name() const { return msg::format(msgAStringOrArrayOfStrings); }

Optional<std::vector<std::string>> ParagraphDeserializer::visit_string(Reader&, StringView sv) const
Expand Down
15 changes: 12 additions & 3 deletions src/vcpkg/base/parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -90,14 +90,23 @@ namespace vcpkg
}
}

ParserBase::ParserBase(StringView text, StringView origin, TextRowCol init_rowcol)
ParserBase::ParserBase(StringView text, Optional<StringView> origin, TextRowCol init_rowcol)
: m_it(text.begin(), text.end())
, m_start_of_line(m_it)
, m_row(init_rowcol.row_or(1))
, m_column(init_rowcol.column_or(1))
, m_text(text)
, m_origin(origin)
{
#ifndef NDEBUG
if (auto check_origin = origin.get())
{
if (check_origin->empty())
{
Checks::unreachable(VCPKG_LINE_INFO, "origin should not be empty");
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this change I see this error:

$ /vcpkg format-manifest scripts/test_ports/vcpkg-ci-skia/vcpkg.json
/home/<me>/vcpkg-tool/src/vcpkg/base/parse.cpp(106): origin should not be empty
Aborted (core dumped)

}
}
#endif
}

StringView ParserBase::skip_whitespace() { return match_while(is_whitespace); }
Expand Down Expand Up @@ -187,9 +196,9 @@ namespace vcpkg
if (!m_messages.error)
{
auto& res = m_messages.error.emplace();
if (!m_origin.empty())
if (auto origin = m_origin.get())
{
res.append_raw(fmt::format("{}:{}:{}: ", m_origin, loc.row, loc.column));
res.append_raw(fmt::format("{}:{}:{}: ", *origin, loc.row, loc.column));
}

res.append_raw(ErrorPrefix);
Expand Down
4 changes: 2 additions & 2 deletions src/vcpkg/binarycaching.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1229,7 +1229,7 @@ namespace

struct BinaryConfigParser : ConfigSegmentsParser
{
BinaryConfigParser(StringView text, StringView origin, BinaryConfigParserState* state)
BinaryConfigParser(StringView text, Optional<StringView> origin, BinaryConfigParserState* state)
: ConfigSegmentsParser(text, origin), state(state)
{
}
Expand Down Expand Up @@ -2365,7 +2365,7 @@ ExpectedL<BinaryConfigParserState> vcpkg::parse_binary_provider_configs(const st

for (auto&& arg : args)
{
BinaryConfigParser arg_parser(arg, "<command>", &s);
BinaryConfigParser arg_parser(arg, nullopt, &s);
arg_parser.parse();
if (auto err = arg_parser.get_error())
{
Expand Down