Make status line parsing testable. (#1528)

BillyONeal · web-flow · commit 84e2dcfb2e7b · 2024-11-04T22:34:17.000Z
Removes Want::UNKNOWN as this was only ever referenced in to_string, and never otherwise used in the product.
diff --git a/include/vcpkg/base/contractual-constants.h b/include/vcpkg/base/contractual-constants.h
@@ -581,5 +581,4 @@ namespace vcpkg
     inline constexpr StringLiteral StatusInstalled = "installed";
     inline constexpr StringLiteral StatusNotInstalled = "not-installed";
     inline constexpr StringLiteral StatusPurge = "purge";
-    inline constexpr StringLiteral StatusUnknown = "unknown";
 }
diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h
@@ -1172,6 +1172,10 @@ DECLARE_MESSAGE(ExpectedFailOrSkip, (), "", "expected 'fail', 'skip', or 'pass'
 DECLARE_MESSAGE(ExpectedFeatureListTerminal, (), "", "expected ',' or ']' in feature list")
 DECLARE_MESSAGE(ExpectedFeatureName, (), "", "expected feature name (must be lowercase, digits, '-')")
 DECLARE_MESSAGE(ExpectedExplicitTriplet, (), "", "expected an explicit triplet")
+DECLARE_MESSAGE(ExpectedInstallStateField,
+                (),
+                "The values in ''s are locale-invariant",
+                "expected one of 'not-installed', 'half-installed', or 'installed'")
 DECLARE_MESSAGE(ExpectedOneSetOfTags,
                 (msg::count, msg::old_value, msg::new_value, msg::value),
                 "{old_value} is a left tag and {new_value} is the right tag. {value} is the input.",
@@ -1181,7 +1185,15 @@ DECLARE_MESSAGE(ExpectedPathToExist, (msg::path), "", "Expected {path} to exist
 DECLARE_MESSAGE(ExpectedPortName, (), "", "expected a port name here (must be lowercase, digits, '-')")
 DECLARE_MESSAGE(ExpectedReadWriteReadWrite, (), "", "unexpected argument: expected 'read', readwrite', or 'write'")
 DECLARE_MESSAGE(ExpectedStatusField, (), "", "Expected 'status' field in status paragraph")
+DECLARE_MESSAGE(ExpectedTextHere,
+                (msg::expected),
+                "{expected} is a locale-invariant string a parser was searching for",
+                "expected '{expected}' here")
 DECLARE_MESSAGE(ExpectedTripletName, (), "", "expected a triplet name here (must be lowercase, digits, '-')")
+DECLARE_MESSAGE(ExpectedWantField,
+                (),
+                "The values in ''s are locale-invariant",
+                "expected one of 'install', 'hold', 'deinstall', or 'purge' here")
 DECLARE_MESSAGE(ExportArchitectureReq,
                 (),
                 "",
diff --git a/include/vcpkg/base/parse.h b/include/vcpkg/base/parse.h
@@ -90,6 +90,7 @@ namespace vcpkg
         }
 
         bool require_character(char ch);
+        bool require_text(StringLiteral keyword);
 
         bool try_match_keyword(StringView keyword_content);
 
diff --git a/include/vcpkg/fwd/statusparagraph.h b/include/vcpkg/fwd/statusparagraph.h
@@ -13,13 +13,13 @@ namespace vcpkg
     enum class Want
     {
         ERROR_STATE,
-        UNKNOWN,
         INSTALL,
         HOLD,
         DEINSTALL,
         PURGE
     };
 
+    struct StatusLine;
     struct StatusParagraph;
     struct InstalledPackageView;
 }
diff --git a/include/vcpkg/paragraphs.h b/include/vcpkg/paragraphs.h
@@ -9,6 +9,7 @@
 
 #include <vcpkg/sourceparagraph.h>
 
+#include <string>
 #include <utility>
 #include <vector>
 
@@ -24,6 +25,8 @@ namespace vcpkg::Paragraphs
 
     ExpectedL<std::vector<Paragraph>> parse_paragraphs(StringView str, StringView origin);
 
+    void append_paragraph_field(StringView name, StringView field, std::string& out_str);
+
     bool is_port_directory(const ReadOnlyFilesystem& fs, const Path& maybe_directory);
 
     struct PortLoadResult
diff --git a/include/vcpkg/statusparagraph.h b/include/vcpkg/statusparagraph.h
@@ -5,6 +5,8 @@
 #include <vcpkg/fwd/installedpaths.h>
 #include <vcpkg/fwd/statusparagraph.h>
 
+#include <vcpkg/base/fmt.h>
+
 #include <vcpkg/binaryparagraph.h>
 
 #include <map>
@@ -13,18 +15,35 @@
 
 namespace vcpkg
 {
+    struct StatusLine
+    {
+        Want want = Want::ERROR_STATE;
+        InstallState state = InstallState::ERROR_STATE;
+
+        bool is_installed() const noexcept { return want == Want::INSTALL && state == InstallState::INSTALLED; }
+        void to_string(std::string& out) const;
+        std::string to_string() const;
+
+        friend bool operator==(const StatusLine& lhs, const StatusLine& rhs)
+        {
+            return lhs.want == rhs.want && lhs.state == rhs.state;
+        }
+
+        friend bool operator!=(const StatusLine& lhs, const StatusLine& rhs) { return !(lhs == rhs); }
+    };
+
+    ExpectedL<StatusLine> parse_status_line(StringView text, Optional<StringView> origin, TextRowCol init_rowcol);
 
     // metadata for a package's representation in the 'installed' tree
     struct StatusParagraph
     {
-        StatusParagraph() noexcept;
+        StatusParagraph() = default;
         StatusParagraph(StringView origin, Paragraph&& fields);
 
-        bool is_installed() const { return want == Want::INSTALL && state == InstallState::INSTALLED; }
+        bool is_installed() const noexcept { return status.is_installed(); }
 
         BinaryParagraph package;
-        Want want;
-        InstallState state;
+        StatusLine status;
     };
 
     void serialize(const StatusParagraph& pgh, std::string& out_str);
@@ -59,3 +78,4 @@ namespace vcpkg
 
 VCPKG_FORMAT_WITH_TO_STRING_LITERAL_NONMEMBER(vcpkg::InstallState);
 VCPKG_FORMAT_WITH_TO_STRING_LITERAL_NONMEMBER(vcpkg::Want);
+VCPKG_FORMAT_WITH_TO_STRING(vcpkg::StatusLine);
diff --git a/locales/messages.json b/locales/messages.json
@@ -691,6 +691,8 @@
   "ExpectedFailOrSkip": "expected 'fail', 'skip', or 'pass' here",
   "ExpectedFeatureListTerminal": "expected ',' or ']' in feature list",
   "ExpectedFeatureName": "expected feature name (must be lowercase, digits, '-')",
+  "ExpectedInstallStateField": "expected one of 'not-installed', 'half-installed', or 'installed'",
+  "_ExpectedInstallStateField.comment": "The values in ''s are locale-invariant",
   "ExpectedOneSetOfTags": "Found {count} sets of {old_value}.*{new_value} but expected exactly 1, in block:\n{value}",
   "_ExpectedOneSetOfTags.comment": "{old_value} is a left tag and {new_value} is the right tag. {value} is the input. An example of {count} is 42.",
   "ExpectedOneVersioningField": "expected only one versioning field",
@@ -699,7 +701,11 @@
   "ExpectedPortName": "expected a port name here (must be lowercase, digits, '-')",
   "ExpectedReadWriteReadWrite": "unexpected argument: expected 'read', readwrite', or 'write'",
   "ExpectedStatusField": "Expected 'status' field in status paragraph",
+  "ExpectedTextHere": "expected '{expected}' here",
+  "_ExpectedTextHere.comment": "{expected} is a locale-invariant string a parser was searching for",
   "ExpectedTripletName": "expected a triplet name here (must be lowercase, digits, '-')",
+  "ExpectedWantField": "expected one of 'install', 'hold', 'deinstall', or 'purge' here",
+  "_ExpectedWantField.comment": "The values in ''s are locale-invariant",
   "ExportArchitectureReq": "Export prefab requires targeting at least one of the following architectures arm64-v8a, armeabi-v7a, x86_64, x86 to be present.",
   "ExportPrefabRequiresAndroidTriplet": "export prefab requires an Android triplet.",
   "Exported7zipArchive": "7zip archive exported at: {path}",
diff --git a/src/vcpkg-test/statusparagraphs.cpp b/src/vcpkg-test/statusparagraphs.cpp
@@ -9,6 +9,38 @@ using namespace vcpkg;
 using namespace vcpkg::Paragraphs;
 using namespace vcpkg::Test;
 
+static constexpr StringLiteral test_origin = "test";
+static constexpr TextRowCol test_textrowcol = {42, 34};
+
+TEST_CASE ("parse status lines", "[statusparagraphs]")
+{
+    REQUIRE(parse_status_line("install ok installed", test_origin, test_textrowcol).value_or_exit(VCPKG_LINE_INFO) ==
+            StatusLine{Want::INSTALL, InstallState::INSTALLED});
+    REQUIRE(parse_status_line("hold ok installed", test_origin, test_textrowcol).value_or_exit(VCPKG_LINE_INFO) ==
+            StatusLine{Want::HOLD, InstallState::INSTALLED});
+    REQUIRE(parse_status_line("deinstall ok installed", test_origin, test_textrowcol).value_or_exit(VCPKG_LINE_INFO) ==
+            StatusLine{Want::DEINSTALL, InstallState::INSTALLED});
+    REQUIRE(parse_status_line("purge ok installed", test_origin, test_textrowcol).value_or_exit(VCPKG_LINE_INFO) ==
+            StatusLine{Want::PURGE, InstallState::INSTALLED});
+
+    REQUIRE(
+        parse_status_line("install ok not-installed", test_origin, test_textrowcol).value_or_exit(VCPKG_LINE_INFO) ==
+        StatusLine{Want::INSTALL, InstallState::NOT_INSTALLED});
+    REQUIRE(
+        parse_status_line("install ok half-installed", test_origin, test_textrowcol).value_or_exit(VCPKG_LINE_INFO) ==
+        StatusLine{Want::INSTALL, InstallState::HALF_INSTALLED});
+
+    REQUIRE(parse_status_line("meow ok installed", test_origin, test_textrowcol).error() ==
+            LocalizedString::from_raw("test:42:34: error: expected one of 'install', 'hold', 'deinstall', or 'purge' "
+                                      "here\n  on expression: meow ok installed\n                 ^"));
+    REQUIRE(parse_status_line("install ko half-installed", test_origin, test_textrowcol).error() ==
+            LocalizedString::from_raw("test:42:41: error: expected ' ok ' here\n  on expression: install ko "
+                                      "half-installed\n                        ^"));
+    REQUIRE(parse_status_line("install ok meow", test_origin, test_textrowcol).error() ==
+            LocalizedString::from_raw("test:42:45: error: expected one of 'not-installed', 'half-installed', or "
+                                      "'installed'\n  on expression: install ok meow\n                            ^"));
+}
+
 TEST_CASE ("find installed", "[statusparagraphs]")
 {
     auto pghs = parse_paragraphs(R"(
@@ -23,9 +55,8 @@ Status: install ok installed
 
     REQUIRE(pghs);
 
-    StatusParagraphs status_db(Util::fmap(*pghs.get(), [](Paragraph& rpgh) {
-        return std::make_unique<StatusParagraph>(StringLiteral{"test"}, std::move(rpgh));
-    }));
+    StatusParagraphs status_db(Util::fmap(
+        *pghs.get(), [](Paragraph& rpgh) { return std::make_unique<StatusParagraph>(test_origin, std::move(rpgh)); }));
 
     auto it = status_db.find_installed({"ffmpeg", Test::X64_WINDOWS});
     REQUIRE(it != status_db.end());
@@ -45,9 +76,8 @@ Status: purge ok not-installed
 
     REQUIRE(pghs);
 
-    StatusParagraphs status_db(Util::fmap(*pghs.get(), [](Paragraph& rpgh) {
-        return std::make_unique<StatusParagraph>(StringLiteral{"test"}, std::move(rpgh));
-    }));
+    StatusParagraphs status_db(Util::fmap(
+        *pghs.get(), [](Paragraph& rpgh) { return std::make_unique<StatusParagraph>(test_origin, std::move(rpgh)); }));
 
     auto it = status_db.find_installed({"ffmpeg", Test::X64_WINDOWS});
     REQUIRE(it == status_db.end());
@@ -75,9 +105,8 @@ Status: purge ok not-installed
 
     REQUIRE(pghs);
 
-    StatusParagraphs status_db(Util::fmap(*pghs.get(), [](Paragraph& rpgh) {
-        return std::make_unique<StatusParagraph>(StringLiteral{"test"}, std::move(rpgh));
-    }));
+    StatusParagraphs status_db(Util::fmap(
+        *pghs.get(), [](Paragraph& rpgh) { return std::make_unique<StatusParagraph>(test_origin, std::move(rpgh)); }));
 
     auto it = status_db.find_installed({"ffmpeg", Test::X64_WINDOWS});
     REQUIRE(it != status_db.end());
@@ -108,9 +137,8 @@ Status: install ok installed
                                  "test-origin");
     REQUIRE(pghs);
 
-    StatusParagraphs status_db(Util::fmap(*pghs.get(), [](Paragraph& rpgh) {
-        return std::make_unique<StatusParagraph>(StringLiteral{"test"}, std::move(rpgh));
-    }));
+    StatusParagraphs status_db(Util::fmap(
+        *pghs.get(), [](Paragraph& rpgh) { return std::make_unique<StatusParagraph>(test_origin, std::move(rpgh)); }));
 
     // Feature "openssl" is installed and should therefore be found
     auto it = status_db.find_installed({{"ffmpeg", Test::X64_WINDOWS}, "openssl"});
diff --git a/src/vcpkg-test/update.cpp b/src/vcpkg-test/update.cpp
@@ -64,8 +64,7 @@ TEST_CASE ("find outdated packages features 2", "[update]")
 
     status_paragraphs.push_back(make_status_feature_pgh("a", "b"));
     status_paragraphs.back()->package.version = Version{"0", 0};
-    status_paragraphs.back()->state = InstallState::NOT_INSTALLED;
-    status_paragraphs.back()->want = Want::PURGE;
+    status_paragraphs.back()->status = {Want::PURGE, InstallState::NOT_INSTALLED};
 
     StatusParagraphs status_db(std::move(status_paragraphs));
 
diff --git a/src/vcpkg/base/parse.cpp b/src/vcpkg/base/parse.cpp
@@ -146,6 +146,27 @@ namespace vcpkg
         return true;
     }
 
+    bool ParserBase::require_text(StringLiteral text)
+    {
+        auto encoded = m_it;
+        // check that the encoded stream matches the keyword:
+        for (const char ch : text)
+        {
+            if (encoded.is_eof() || *encoded != static_cast<char32_t>(ch))
+            {
+                add_error(msg::format(msgExpectedTextHere, msg::expected = text));
+                return false;
+            }
+
+            ++encoded;
+        }
+
+        // success
+        m_it = encoded;
+        m_column += static_cast<int>(text.size());
+        return true;
+    }
+
     bool ParserBase::try_match_keyword(StringView keyword_content)
     {
         auto encoded = m_it;
diff --git a/src/vcpkg/binaryparagraph.cpp b/src/vcpkg/binaryparagraph.cpp
@@ -7,9 +7,12 @@
 #include <vcpkg/paragraphparser.h>
 #include <vcpkg/paragraphs.h>
 
+using namespace vcpkg::Paragraphs;
+
 namespace vcpkg
 {
     BinaryParagraph::BinaryParagraph(StringView origin, Paragraph&& fields)
+        : spec(), version(), description(), maintainers(), feature(), default_features(), dependencies(), abi()
     {
         ParagraphParser parser(origin, std::move(fields));
         this->spec = PackageSpec(parser.required_field(ParagraphIdPackage),
@@ -177,15 +180,6 @@ namespace vcpkg
 
     bool operator!=(const BinaryParagraph& lhs, const BinaryParagraph& rhs) { return !(lhs == rhs); }
 
-    static void serialize_string(StringView name, const std::string& field, std::string& out_str)
-    {
-        if (field.empty())
-        {
-            return;
-        }
-
-        out_str.append(name.data(), name.size()).append(": ").append(field).push_back('\n');
-    }
     static void serialize_array(StringView name,
                                 const std::vector<std::string>& array,
                                 std::string& out_str,
@@ -223,33 +217,29 @@ namespace vcpkg
     {
         const size_t initial_end = out_str.size();
 
-        serialize_string(ParagraphIdPackage, pgh.spec.name(), out_str);
-
-        serialize_string(ParagraphIdVersion, pgh.version.text, out_str);
+        append_paragraph_field(ParagraphIdPackage, pgh.spec.name(), out_str);
+        append_paragraph_field(ParagraphIdVersion, pgh.version.text, out_str);
         if (pgh.version.port_version != 0)
         {
             fmt::format_to(std::back_inserter(out_str), "{}: {}\n", ParagraphIdPortVersion, pgh.version.port_version);
         }
 
         if (pgh.is_feature())
         {
-            serialize_string(ParagraphIdFeature, pgh.feature, out_str);
+            append_paragraph_field(ParagraphIdFeature, pgh.feature, out_str);
         }
 
         if (!pgh.dependencies.empty())
         {
-            serialize_string(ParagraphIdDepends, serialize_deps_list(pgh.dependencies, pgh.spec.triplet()), out_str);
+            append_paragraph_field(
+                ParagraphIdDepends, serialize_deps_list(pgh.dependencies, pgh.spec.triplet()), out_str);
         }
 
-        serialize_string(ParagraphIdArchitecture, pgh.spec.triplet().to_string(), out_str);
-        serialize_string(ParagraphIdMultiArch, "same", out_str);
-
+        append_paragraph_field(ParagraphIdArchitecture, pgh.spec.triplet().to_string(), out_str);
+        append_paragraph_field(ParagraphIdMultiArch, "same", out_str);
         serialize_paragraph(ParagraphIdMaintainer, pgh.maintainers, out_str);
-
-        serialize_string(ParagraphIdAbi, pgh.abi, out_str);
-
+        append_paragraph_field(ParagraphIdAbi, pgh.abi, out_str);
         serialize_paragraph(ParagraphIdDescription, pgh.description, out_str);
-
         serialize_array(ParagraphIdDefaultFeatures, pgh.default_features, out_str);
 
         // sanity check the serialized data
diff --git a/src/vcpkg/commands.install.cpp b/src/vcpkg/commands.install.cpp
@@ -280,8 +280,7 @@ namespace vcpkg
 
         StatusParagraph source_paragraph;
         source_paragraph.package = bcf.core_paragraph;
-        source_paragraph.want = Want::INSTALL;
-        source_paragraph.state = InstallState::HALF_INSTALLED;
+        source_paragraph.status = StatusLine{Want::INSTALL, InstallState::HALF_INSTALLED};
 
         write_update(fs, installed, source_paragraph);
         status_db->insert(std::make_unique<StatusParagraph>(source_paragraph));
@@ -291,8 +290,7 @@ namespace vcpkg
         {
             StatusParagraph& feature_paragraph = features_spghs.emplace_back();
             feature_paragraph.package = feature;
-            feature_paragraph.want = Want::INSTALL;
-            feature_paragraph.state = InstallState::HALF_INSTALLED;
+            feature_paragraph.status = StatusLine{Want::INSTALL, InstallState::HALF_INSTALLED};
 
             write_update(fs, installed, feature_paragraph);
             status_db->insert(std::make_unique<StatusParagraph>(feature_paragraph));
@@ -303,13 +301,13 @@ namespace vcpkg
 
         install_package_and_write_listfile(fs, package_dir, install_dir);
 
-        source_paragraph.state = InstallState::INSTALLED;
+        source_paragraph.status.state = InstallState::INSTALLED;
         write_update(fs, installed, source_paragraph);
         status_db->insert(std::make_unique<StatusParagraph>(source_paragraph));
 
         for (auto&& feature_paragraph : features_spghs)
         {
-            feature_paragraph.state = InstallState::INSTALLED;
+            feature_paragraph.status.state = InstallState::INSTALLED;
             write_update(fs, installed, feature_paragraph);
             status_db->insert(std::make_unique<StatusParagraph>(feature_paragraph));
         }
diff --git a/src/vcpkg/commands.remove.cpp b/src/vcpkg/commands.remove.cpp
diff --git a/src/vcpkg/paragraphs.cpp b/src/vcpkg/paragraphs.cpp
diff --git a/src/vcpkg/statusparagraph.cpp b/src/vcpkg/statusparagraph.cpp
diff --git a/src/vcpkg/statusparagraphs.cpp b/src/vcpkg/statusparagraphs.cpp

Original file line number	Diff line number	Diff line change
`@@ -581,5 +581,4 @@ namespace vcpkg`
`581`	`581`	`inline constexpr StringLiteral StatusInstalled = "installed";`
`582`	`582`	`inline constexpr StringLiteral StatusNotInstalled = "not-installed";`
`583`	`583`	`inline constexpr StringLiteral StatusPurge = "purge";`
`584`		`- inline constexpr StringLiteral StatusUnknown = "unknown";`
`585`	`584`	`}`
Original file line number	Diff line number	Diff line change
`@@ -90,6 +90,7 @@ namespace vcpkg`
`90`	`90`	`}`
`91`	`91`
`92`	`92`	`bool require_character(char ch);`
	`93`	`+ bool require_text(StringLiteral keyword);`
`93`	`94`
`94`	`95`	`bool try_match_keyword(StringView keyword_content);`
`95`	`96`
Original file line number	Diff line number	Diff line change
`@@ -13,13 +13,13 @@ namespace vcpkg`
`13`	`13`	`enum class Want`
`14`	`14`	`{`
`15`	`15`	`ERROR_STATE,`
`16`		`- UNKNOWN,`
`17`	`16`	`INSTALL,`
`18`	`17`	`HOLD,`
`19`	`18`	`DEINSTALL,`
`20`	`19`	`PURGE`
`21`	`20`	`};`
`22`	`21`
	`22`	`+ struct StatusLine;`
`23`	`23`	`struct StatusParagraph;`
`24`	`24`	`struct InstalledPackageView;`
`25`	`25`	`}`