Retain detailed error information when finding the baseline fails

microsoft · May 20, 2022 · 3ff332c · 3ff332c
1 parent 44b6add
commit 3ff332c
Show file tree

Hide file tree

Showing 12 changed files with 135 additions and 59 deletions.
diff --git a/azure-pipelines/e2e_ports/vcpkg-internal-e2e-test-port3/portfile.cmake b/azure-pipelines/e2e_ports/vcpkg-internal-e2e-test-port3/portfile.cmake
@@ -0,0 +1 @@
+set(VCPKG_POLICY_EMPTY_PACKAGE enabled)
diff --git a/azure-pipelines/e2e_ports/vcpkg-internal-e2e-test-port3/vcpkg.json b/azure-pipelines/e2e_ports/vcpkg-internal-e2e-test-port3/vcpkg.json
@@ -0,0 +1,5 @@
+{
+	"name": "vcpkg-internal-e2e-test-port3",
+	"version-string": "1.0.0",
+	"dependencies": ["vcpkg-internal-e2e-test-port2"]
+}
diff --git a/azure-pipelines/end-to-end-tests-dir/build-test-ports.ps1 b/azure-pipelines/end-to-end-tests-dir/build-test-ports.ps1
@@ -2,9 +2,23 @@
 
 $CurrentTest = "Build Test Ports"
 
-Run-Vcpkg --overlay-ports="$PSScriptRoot/../e2e_ports/overlays" install vcpkg-empty-port
-Run-Vcpkg --overlay-ports="$PSScriptRoot/../e2e_ports" install vcpkg-internal-e2e-test-port
-if ($IsWindows) {
-    Run-Vcpkg --overlay-ports="$PSScriptRoot/../e2e_ports" install vcpkg-find-acquire-program
+Run-Vcpkg @commonArgs --overlay-ports="$PSScriptRoot/../e2e_ports" install vcpkg-internal-e2e-test-port3
+Throw-IfFailed
+
+$output = Run-Vcpkg @commonArgs --overlay-ports="$PSScriptRoot/../e2e_ports/vcpkg-internal-e2e-test-port2" install vcpkg-internal-e2e-test-port2
+$output
+Throw-IfFailed
+if ($output -match "vcpkg-internal-e2e-test-port3") {
+    throw "Should not emit messages about -port3 while checking -port2"
 }
+
+return
+
+Run-Vcpkg @commonArgs --overlay-ports="$PSScriptRoot/../e2e_ports/overlays" install vcpkg-empty-port
+Throw-IfFailed
+Run-Vcpkg @commonArgs --overlay-ports="$PSScriptRoot/../e2e_ports" install vcpkg-internal-e2e-test-port
 Throw-IfFailed
+if ($IsWindows) {
+    Run-Vcpkg @commonArgs --overlay-ports="$PSScriptRoot/../e2e_ports" install vcpkg-find-acquire-program
+    Throw-IfFailed
+}
diff --git a/include/vcpkg/paragraphparser.h b/include/vcpkg/paragraphparser.h
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <vcpkg/base/fwd/messages.h>
+
 #include <vcpkg/fwd/paragraphparser.h>
 
 #include <vcpkg/base/expected.h>
@@ -30,6 +32,8 @@ namespace vcpkg
             return !missing_fields.empty() || !extra_fields.empty() || !expected_types.empty() ||
                    !other_errors.empty() || !error.empty();
         }
+
+        static std::string format_errors(View<std::unique_ptr<ParseControlErrorInfo>> errors);
     };
 
     template<class P>

diff --git a/include/vcpkg/portfileprovider.h b/include/vcpkg/portfileprovider.h
@@ -42,7 +42,7 @@ namespace vcpkg::PortFileProvider
 
     struct IBaselineProvider
     {
-        virtual Optional<Version> get_baseline_version(StringView port_name) const = 0;
+        virtual ExpectedL<Version> get_baseline_version(StringView port_name) const = 0;
         virtual ~IBaselineProvider() = default;
     };
 

diff --git a/include/vcpkg/registries.h b/include/vcpkg/registries.h
@@ -80,7 +80,7 @@ namespace vcpkg
         // may result in duplicated port names; make sure to Util::sort_unique_erase at the end
         virtual void get_all_port_names(std::vector<std::string>& port_names) const = 0;
 
-        virtual Optional<Version> get_baseline_version(StringView port_name) const = 0;
+        virtual ExpectedL<Version> get_baseline_version(StringView port_name) const = 0;
 
         virtual ~RegistryImplementation() = default;
     };
@@ -119,7 +119,7 @@ namespace vcpkg
         // finds the correct registry for the port name
         // Returns the null pointer if there is no registry set up for that name
         const RegistryImplementation* registry_for_port(StringView port_name) const;
-        Optional<Version> baseline_for_port(StringView port_name) const;
+        ExpectedL<Version> baseline_for_port(StringView port_name) const;
 
         View<Registry> registries() const { return registries_; }
 

diff --git a/src/vcpkg-test/dependencies.cpp b/src/vcpkg-test/dependencies.cpp
@@ -26,10 +26,10 @@ struct MockBaselineProvider : PortFileProvider::IBaselineProvider
 {
     mutable std::map<std::string, Version, std::less<>> v;
 
-    Optional<Version> get_baseline_version(StringView name) const override
+    ExpectedL<Version> get_baseline_version(StringView name) const override
     {
         auto it = v.find(name);
-        if (it == v.end()) return nullopt;
+        if (it == v.end()) return LocalizedString::from_raw("error");
         return it->second;
     }
 };

diff --git a/src/vcpkg-test/registries.cpp b/src/vcpkg-test/registries.cpp
@@ -17,7 +17,10 @@ namespace
 
         void get_all_port_names(std::vector<std::string>&) const override { }
 
-        Optional<Version> get_baseline_version(StringView) const override { return nullopt; }
+        ExpectedL<Version> get_baseline_version(StringView) const override
+        {
+            return LocalizedString::from_raw("error");
+        }
 
         int number;
 

diff --git a/src/vcpkg/dependencies.cpp b/src/vcpkg/dependencies.cpp
@@ -241,7 +241,7 @@ namespace vcpkg::Dependencies
                 {
                     Checks::exit_maybe_upgrade(
                         VCPKG_LINE_INFO,
-                        "Error: while loading control file for %s: %s.\nPlease run \"%s remove %s\" and re-attempt.",
+                        "Error: while loading control file for %s:\n%s\nPlease run \"%s remove %s\" and re-attempt.",
                         m_spec,
                         m_scfl.error(),
                         vcpkg_remove_cmd,
@@ -1377,7 +1377,7 @@ namespace vcpkg::Dependencies
                                 VersionSchemeInfo& vsi,
                                 const std::string& feature);
 
-            Optional<Version> dep_to_version(const std::string& name, const DependencyConstraint& dc);
+            ExpectedL<Version> dep_to_version(const std::string& name, const DependencyConstraint& dc);
 
             static std::string format_incomparable_versions_message(const PackageSpec& on,
                                                                     StringView from,
@@ -1667,7 +1667,8 @@ namespace vcpkg::Dependencies
             return *m_graph.emplace(spec, PackageNode{}).first;
         }
 
-        Optional<Version> VersionedPackageGraph::dep_to_version(const std::string& name, const DependencyConstraint& dc)
+        ExpectedL<Version> VersionedPackageGraph::dep_to_version(const std::string& name,
+                                                                 const DependencyConstraint& dc)
         {
             auto maybe_overlay = m_o_provider.get_control_file(name);
             if (auto p_overlay = maybe_overlay.get())
@@ -1681,10 +1682,10 @@ namespace vcpkg::Dependencies
                 return over_it->second;
             }
 
-            const auto maybe_cons = dc.try_get_minimum_version();
-            if (maybe_cons)
+            auto maybe_cons = dc.try_get_minimum_version();
+            if (auto p = maybe_cons.get())
             {
-                return maybe_cons;
+                return std::move(*p);
             }
 
             return m_base_provider.get_baseline_version(name);

diff --git a/src/vcpkg/portfileprovider.cpp b/src/vcpkg/portfileprovider.cpp
@@ -76,7 +76,7 @@ namespace vcpkg::PortFileProvider
         }
         else
         {
-            return Strings::concat("Error: unable to get baseline for port ", spec);
+            return std::move(maybe_baseline).error().extract_data();
         }
     }
 
@@ -102,7 +102,7 @@ namespace vcpkg::PortFileProvider
             BaselineProviderImpl(const BaselineProviderImpl&) = delete;
             BaselineProviderImpl& operator=(const BaselineProviderImpl&) = delete;
 
-            virtual Optional<Version> get_baseline_version(StringView port_name) const override
+            virtual ExpectedL<Version> get_baseline_version(StringView port_name) const override
             {
                 auto it = m_baseline_cache.find(port_name);
                 if (it != m_baseline_cache.end())
@@ -119,7 +119,7 @@ namespace vcpkg::PortFileProvider
 
         private:
             const VcpkgPaths& paths;
-            mutable std::map<std::string, Optional<Version>, std::less<>> m_baseline_cache;
+            mutable std::map<std::string, ExpectedL<Version>, std::less<>> m_baseline_cache;
         };
 
         struct VersionedPortfileProviderImpl : IVersionedPortfileProvider

diff --git a/src/vcpkg/registries.cpp b/src/vcpkg/registries.cpp
@@ -24,6 +24,11 @@ namespace
 
     static constexpr StringLiteral registry_versions_dir_name = "versions";
 
+    DECLARE_AND_REGISTER_MESSAGE(PortNotInBaseline,
+                                 (msg::package_name),
+                                 "",
+                                 "the baseline does not contain an entry for port {package_name}");
+
     struct GitRegistry;
 
     struct GitRegistryEntry final : RegistryEntry
@@ -65,7 +70,7 @@ namespace
 
         void get_all_port_names(std::vector<std::string>&) const override;
 
-        Optional<Version> get_baseline_version(StringView) const override;
+        ExpectedL<Version> get_baseline_version(StringView) const override;
 
     private:
         friend struct GitRegistryEntry;
@@ -234,7 +239,7 @@ namespace
 
         void get_all_port_names(std::vector<std::string>&) const override;
 
-        Optional<Version> get_baseline_version(StringView port_name) const override;
+        ExpectedL<Version> get_baseline_version(StringView port_name) const override;
 
         ~BuiltinFilesRegistry() = default;
 
@@ -277,7 +282,7 @@ namespace
 
         void get_all_port_names(std::vector<std::string>&) const override;
 
-        Optional<Version> get_baseline_version(StringView port_name) const override;
+        ExpectedL<Version> get_baseline_version(StringView port_name) const override;
 
         ~BuiltinGitRegistry() = default;
 
@@ -306,7 +311,7 @@ namespace
 
         void get_all_port_names(std::vector<std::string>&) const override { fail_require_baseline(VCPKG_LINE_INFO); }
 
-        Optional<Version> get_baseline_version(StringView) const override { fail_require_baseline(VCPKG_LINE_INFO); }
+        ExpectedL<Version> get_baseline_version(StringView) const override { fail_require_baseline(VCPKG_LINE_INFO); }
 
         ~BuiltinErrorRegistry() = default;
 
@@ -332,7 +337,7 @@ namespace
 
         void get_all_port_names(std::vector<std::string>&) const override;
 
-        Optional<Version> get_baseline_version(StringView) const override;
+        ExpectedL<Version> get_baseline_version(StringView) const override;
 
     private:
         const Filesystem& m_fs;
@@ -471,7 +476,7 @@ namespace
         return nullptr;
     }
 
-    Optional<Version> BuiltinFilesRegistry::get_baseline_version(StringView port_name) const
+    ExpectedL<Version> BuiltinFilesRegistry::get_baseline_version(StringView port_name) const
     {
         // if a baseline is not specified, use the ports directory version
         auto port_path = m_builtin_ports_directory / port_name;
@@ -480,8 +485,7 @@ namespace
         {
             return (*pscf)->to_version();
         }
-        print_error_message(maybe_scf.error());
-        return nullopt;
+        return LocalizedString::from_raw(ParseControlErrorInfo::format_errors({&maybe_scf.error(), 1}));
     }
 
     void BuiltinFilesRegistry::get_all_port_names(std::vector<std::string>& out) const
@@ -529,7 +533,7 @@ namespace
         return m_files_impl->get_port_entry(port_name);
     }
 
-    Optional<Version> BuiltinGitRegistry::get_baseline_version(StringView port_name) const
+    ExpectedL<Version> BuiltinGitRegistry::get_baseline_version(StringView port_name) const
     {
         const auto& baseline = m_baseline.get([this]() -> Baseline {
             auto maybe_path = git_checkout_baseline(m_paths, m_baseline_identifier);
@@ -553,7 +557,7 @@ namespace
         {
             return it->second;
         }
-        return nullopt;
+        return msg::format(msg::msgErrorMessage).append(msgPortNotInBaseline, msg::package_name = port_name);
     }
 
     void BuiltinGitRegistry::get_all_port_names(std::vector<std::string>& out) const
@@ -570,7 +574,7 @@ namespace
     // } BuiltinGitRegistry::RegistryImplementation
 
     // { FilesystemRegistry::RegistryImplementation
-    Optional<Version> FilesystemRegistry::get_baseline_version(StringView port_name) const
+    ExpectedL<Version> FilesystemRegistry::get_baseline_version(StringView port_name) const
     {
         const auto& baseline = m_baseline.get([this]() -> Baseline {
             auto path_to_baseline = m_path / registry_versions_dir_name / "baseline.json";
@@ -604,7 +608,7 @@ namespace
         }
         else
         {
-            return nullopt;
+            return msg::format(msg::msgErrorMessage).append(msgPortNotInBaseline, msg::package_name = port_name);
         }
     }
 
@@ -645,7 +649,7 @@ namespace
         fill_data_from_path(parent.m_paths.get_filesystem(), vtp.p);
     }
 
-    Optional<Version> GitRegistry::get_baseline_version(StringView port_name) const
+    ExpectedL<Version> GitRegistry::get_baseline_version(StringView port_name) const
     {
         const auto& baseline = m_baseline.get([this]() -> Baseline {
             // We delay baseline validation until here to give better error messages and suggestions
@@ -733,7 +737,7 @@ namespace
             return it->second;
         }
 
-        return nullopt;
+        return msg::format(msg::msgErrorMessage).append(msgPortNotInBaseline, msg::package_name = port_name);
     }
 
     void GitRegistry::get_all_port_names(std::vector<std::string>& out) const
@@ -1178,10 +1182,15 @@ namespace vcpkg
         return default_registry();
     }
 
-    Optional<Version> RegistrySet::baseline_for_port(StringView port_name) const
+    DECLARE_AND_REGISTER_MESSAGE(NoRegistryForPort,
+                                 (msg::package_name),
+                                 "",
+                                 "no registry configured for port {package_name}");
+
+    ExpectedL<Version> RegistrySet::baseline_for_port(StringView port_name) const
     {
         auto impl = registry_for_port(port_name);
-        if (!impl) return nullopt;
+        if (!impl) return msg::format(msg::msgErrorMessage).append(msgNoRegistryForPort, msg::package_name = port_name);
         return impl->get_baseline_version(port_name);
     }