Add a "status" API to DiagnosticContext, overhaul Downloads console output (microsoft#1565)

Extensive overhaul of our downloads handling and console output; @JavierMatosD and I have gone back and forth several times and yet kept introducing unintended bugs in other places, which led me to believe targeted fixes would no longer cut it.

Fixes many longstanding bugs and hopefully makes our console output for this more understandable:
* We no longer print 'error' when an asset cache misses but the authoritative download succeeds. This partially undoes microsoft#1541. It is good to print errors immediately when they happen, but if a subsequent authoritative download succeeds we need to not print those errors.
* We now always and consistently print output from x-script s at the time that actually happens. Resolves https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2300063
* We don't tell the user that proxy settings might fix a hash mismatch problem.
* We do tell the user that proxy settings might fix a download from asset cache problem.
* We now always tell the user the full command line we tried when invoking an x-script that fails.
* We don't crash if an x-script doesn't create the file we expect, or creates a file with the wrong hash.
* We now always print what we are doing *before* touching the network, so if we hang the user knows which server is being problematic. Note that this includes storing back to asset caches which we were previously entirely silent about except in case of failure.

Other changes:
* Removed debug output about asset cache configuration. The output was misleading / wrong depending on readwrite settings, and echoing to the user exactly what they said before we've interpreted it is not useful debug output. (Contrast with other `VcpkgPaths` debug output which tend to be paths we have likely changed from something a user said) 

Other notes:
* This makes all dependencies of microsoft#908 speak `DiagnosticContext` so it will be easy to audit that the foreground/background thread behavior is correct after this.
* I did test the curl status parsing on old Ubuntu again.

Special thanks to @JavierMatosD for his help in review of the first console output attempts and for blowing the dust off this area in the first place.

Author: BillyONeal

azure-pipelines/e2e-assets/asset-caching/bad-hash-script.ps1

@@ -0,0 +1,3 @@
+Param([string]$File)
+Write-Host "Creating file with the wrong hash"
+Set-Content -Path $File -Value "This is a file with the wrong hash" -Encoding Ascii -NoNewline

azure-pipelines/e2e-assets/asset-caching/failing-script.ps1

@@ -1 +1,2 @@
-throw "Script download error"
+Write-Host "Script download error"
+exit 1

azure-pipelines/e2e-assets/asset-caching/no-file-script.ps1

@@ -0,0 +1 @@
+Write-Host "Not creating a file"

azure-pipelines/end-to-end-tests-dir/asset-caching.ps1

@@ -1,6 +1,7 @@
 #pragma once
 
-#include <vcpkg/base/expected.h>
+#include <vcpkg/base/diagnostics.h>
+#include <vcpkg/base/optional.h>
 #include <vcpkg/base/stringview.h>
 
 #include <string>
@@ -10,21 +11,22 @@ namespace vcpkg
     namespace details
     {
         template<class F>
-        void api_stable_format_cb(void* f, std::string& s, StringView sv)
+        bool api_stable_format_cb(void* f, std::string& s, StringView sv)
         {
-            (*(F*)(f))(s, sv);
+            return (*(F*)(f))(s, sv);
         }
 
-        ExpectedL<std::string> api_stable_format_impl(StringView fmtstr,
-                                                      void (*cb)(void*, std::string&, StringView),
-                                                      void* data);
+        Optional<std::string> api_stable_format_impl(DiagnosticContext& context,
+                                                     StringView fmtstr,
+                                                     bool (*cb)(void*, std::string&, StringView),
+                                                     void* data);
     }
 
     // This function exists in order to provide an API-stable formatting function similar to `std::format()` that does
     // not depend on the feature set of fmt or the C++ standard library and thus can be contractual for user interfaces.
     template<class F>
-    ExpectedL<std::string> api_stable_format(StringView fmtstr, F&& handler)
+    Optional<std::string> api_stable_format(DiagnosticContext& context, StringView fmtstr, F&& handler)
     {
-        return details::api_stable_format_impl(fmtstr, &details::api_stable_format_cb<F>, &handler);
+        return details::api_stable_format_impl(context, fmtstr, &details::api_stable_format_cb<F>, &handler);
     }
 }

include/vcpkg/base/api-stable-format.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <vcpkg/base/expected.h>
+#include <vcpkg/base/message_sinks.h>
 #include <vcpkg/base/messages.h>
 #include <vcpkg/base/optional.h>
 
@@ -66,11 +67,22 @@ namespace vcpkg
         std::string to_string() const;
         void to_string(std::string& target) const;
 
+        MessageLine to_message_line() const;
+
         LocalizedString to_json_reader_string(const std::string& path, const LocalizedString& type) const;
 
         DiagKind kind() const noexcept { return m_kind; }
+        // Returns this DiagnosticLine with kind == Error reduced to Warning.
+        DiagnosticLine reduce_to_warning() const&;
+        DiagnosticLine reduce_to_warning() &&;
 
     private:
+        DiagnosticLine(DiagKind kind,
+                       const Optional<std::string>& origin,
+                       TextRowCol position,
+                       const LocalizedString& message);
+        DiagnosticLine(DiagKind kind, Optional<std::string>&& origin, TextRowCol position, LocalizedString&& message);
+
         DiagKind m_kind;
         Optional<std::string> m_origin;
         TextRowCol m_position;
@@ -79,8 +91,11 @@ namespace vcpkg
 
     struct DiagnosticContext
     {
+        // The `report` family are used to report errors or warnings that may result in a function failing
+        // to do what it is intended to do. Data sent to the `report` family is expected to not be printed
+        // to the console if a caller decides to handle an error.
         virtual void report(const DiagnosticLine& line) = 0;
-        virtual void report(DiagnosticLine&& line) { report(line); }
+        virtual void report(DiagnosticLine&& line);
 
         void report_error(const LocalizedString& message) { report(DiagnosticLine{DiagKind::Error, message}); }
         void report_error(LocalizedString&& message) { report(DiagnosticLine{DiagKind::Error, std::move(message)}); }
@@ -92,15 +107,64 @@ namespace vcpkg
             this->report_error(std::move(message));
         }
 
+        template<VCPKG_DECL_MSG_TEMPLATE>
+        void report_error_with_log(StringView log_content, VCPKG_DECL_MSG_ARGS)
+        {
+            LocalizedString message;
+            msg::format_to(message, VCPKG_EXPAND_MSG_ARGS);
+            message.append_raw('\n');
+            message.append_raw(log_content);
+            this->report_error(std::move(message));
+        }
+
+        void report_system_error(StringLiteral system_api_name, int error_value);
+
+        // The `status` family are used to report status or progress information that callers are expected
+        // to show on the console, even if it would decide to handle errors or warnings itself.
+        // Examples:
+        //  * "Downloading file..."
+        //  * "Building package 1 of 47..."
+        //
+        // Some implementations of DiagnosticContext may buffer these messages *anyway* if that makes sense,
+        // for example, if the work is happening on a background thread.
+        virtual void statusln(const LocalizedString& message) = 0;
+        virtual void statusln(LocalizedString&& message) = 0;
+        virtual void statusln(const MessageLine& message) = 0;
+        virtual void statusln(MessageLine&& message) = 0;
+
     protected:
         ~DiagnosticContext() = default;
     };
 
+    struct PrintingDiagnosticContext final : DiagnosticContext
+    {
+        PrintingDiagnosticContext(MessageSink& sink) : sink(sink) { }
+
+        virtual void report(const DiagnosticLine& line) override;
+
+        virtual void statusln(const LocalizedString& message) override;
+        virtual void statusln(LocalizedString&& message) override;
+        virtual void statusln(const MessageLine& message) override;
+        virtual void statusln(MessageLine&& message) override;
+
+    private:
+        MessageSink& sink;
+    };
+
+    // Stores all diagnostics into a vector, while passing through status lines to an underlying MessageSink.
     struct BufferedDiagnosticContext final : DiagnosticContext
     {
+        BufferedDiagnosticContext(MessageSink& status_sink) : status_sink(status_sink) { }
+
         virtual void report(const DiagnosticLine& line) override;
         virtual void report(DiagnosticLine&& line) override;
 
+        virtual void statusln(const LocalizedString& message) override;
+        virtual void statusln(LocalizedString&& message) override;
+        virtual void statusln(const MessageLine& message) override;
+        virtual void statusln(MessageLine&& message) override;
+
+        MessageSink& status_sink;
         std::vector<DiagnosticLine> lines;
 
         // Prints all diagnostics to the supplied sink.
@@ -111,9 +175,75 @@ namespace vcpkg
         void to_string(std::string& target) const;
 
         bool any_errors() const noexcept;
+        bool empty() const noexcept;
+    };
+
+    // Stores all diagnostics and status messages into a vector. This is generally used for background thread or similar
+    // scenarios where even status messages can't be immediately printed.
+    struct FullyBufferedDiagnosticContext final : DiagnosticContext
+    {
+        virtual void report(const DiagnosticLine& line) override;
+        virtual void report(DiagnosticLine&& line) override;
+
+        virtual void statusln(const LocalizedString& message) override;
+        virtual void statusln(LocalizedString&& message) override;
+        virtual void statusln(const MessageLine& message) override;
+        virtual void statusln(MessageLine&& message) override;
+
+        std::vector<MessageLine> lines;
+
+        // Prints all diagnostics to the supplied sink.
+        void print_to(MessageSink& sink) const;
+        // Converts this message into a string
+        // Prefer print() if possible because it applies color
+        std::string to_string() const;
+        void to_string(std::string& target) const;
+
+        bool empty() const noexcept;
+    };
+
+    // DiagnosticContext for attempted operations that may be recovered.
+    // Stores all diagnostics and passes through all status messages. Afterwards, call commit() to report all
+    // diagnostics to the outer DiagnosticContext, or handle() to forget them.
+    struct AttemptDiagnosticContext final : DiagnosticContext
+    {
+        AttemptDiagnosticContext(DiagnosticContext& inner_context) : inner_context(inner_context) { }
+
+        virtual void report(const DiagnosticLine& line) override;
+        virtual void report(DiagnosticLine&& line) override;
+
+        virtual void statusln(const LocalizedString& message) override;
+        virtual void statusln(LocalizedString&& message) override;
+        virtual void statusln(const MessageLine& message) override;
+        virtual void statusln(MessageLine&& message) override;
+
+        void commit();
+        void handle();
+
+        ~AttemptDiagnosticContext();
+
+        DiagnosticContext& inner_context;
+        std::vector<DiagnosticLine> lines;
+    };
+
+    // Wraps another DiagnosticContext and reduces the severity of any reported diagnostics to warning from error.
+    struct WarningDiagnosticContext final : DiagnosticContext
+    {
+        WarningDiagnosticContext(DiagnosticContext& inner_context) : inner_context(inner_context) { }
+
+        virtual void report(const DiagnosticLine& line) override;
+        virtual void report(DiagnosticLine&& line) override;
+
+        virtual void statusln(const LocalizedString& message) override;
+        virtual void statusln(LocalizedString&& message) override;
+        virtual void statusln(const MessageLine& message) override;
+        virtual void statusln(MessageLine&& message) override;
+
+        DiagnosticContext& inner_context;
     };
 
     extern DiagnosticContext& console_diagnostic_context;
+    extern DiagnosticContext& status_only_diagnostic_context;
     extern DiagnosticContext& null_diagnostic_context;
 
     // The following overloads are implementing
@@ -191,7 +321,7 @@ namespace vcpkg
     {
         using Unwrapper = AdaptContextUnwrapOptional<std::invoke_result_t<Fn, BufferedDiagnosticContext&, Args...>>;
         using ReturnType = ExpectedL<typename Unwrapper::type>;
-        BufferedDiagnosticContext bdc;
+        BufferedDiagnosticContext bdc{out_sink};
         decltype(auto) maybe_result = functor(bdc, std::forward<Args>(args)...);
         if (auto result = maybe_result.get())
         {
@@ -261,7 +391,7 @@ namespace vcpkg
     {
         using ReturnType = ExpectedL<
             typename AdaptContextDetectUniquePtr<std::invoke_result_t<Fn, BufferedDiagnosticContext&, Args...>>::type>;
-        BufferedDiagnosticContext bdc;
+        BufferedDiagnosticContext bdc{out_sink};
         decltype(auto) maybe_result = functor(bdc, std::forward<Args>(args)...);
         if (maybe_result)
         {