Standardize binary cache upload failure diagnostics

include/vcpkg/base/message-data.inc.h

@@ -345,6 +345,10 @@ DECLARE_MESSAGE(BaselineOnlyPlatformExpressionOrTriplet,
                 (),
                 "",
                 "You can not specify a platform expression and a triplet")
+DECLARE_MESSAGE(BinaryCacheUploadFailed,
+                (),
+                "This is a standardized prefix prepended to binary cache upload failure diagnostics.",
+                "Binary cache upload failed")
 DECLARE_MESSAGE(BinarySourcesArg,
                 (),
                 "'vcpkg help binarycaching' is a command line and should not be localized",

include/vcpkg/binarycaching.h

@@ -302,6 +302,9 @@ namespace vcpkg
         void print_updates();
         void wait_for_async_complete_and_join();
 
+    protected:
+        BinaryCache(const Filesystem& fs, MessageSink& message_sink);
+
     private:
         struct ActionToPush
         {

locales/messages.json

@@ -216,6 +216,8 @@
   "_BaselineMissing.comment": "An example of {package_name} is zlib.",
   "BaselineOnlyPlatformExpressionOrTriplet": "You can not specify a platform expression and a triplet",
   "BinariesRelativeToThePackageDirectoryHere": "the binaries are relative to ${{CURRENT_PACKAGES_DIR}} here",
+  "BinaryCacheUploadFailed": "Binary cache upload failed",
+  "_BinaryCacheUploadFailed.comment": "This is a standardized prefix prepended to binary cache upload failure diagnostics.",
   "BinarySourcesArg": "Binary caching sources. See 'vcpkg help binarycaching'",
   "_BinarySourcesArg.comment": "'vcpkg help binarycaching' is a command line and should not be localized",
   "BinaryWithInvalidArchitecture": "{path} is built for {arch}",

src/vcpkg-test/binarycaching.cpp

@@ -46,6 +46,55 @@ struct KnowNothingBinaryProvider : IReadBinaryProvider
     }
 };
 
+struct TestMessageSink : MessageSink
+{
+    void println(const MessageLine& line) override { lines.push_back(line.to_string()); }
+    void println(MessageLine&& line) override { lines.push_back(line.to_string()); }
+
+    bool contains_line(const std::string& expected) const
+    {
+        for (const auto& line : lines)
+        {
+            if (line == expected) return true;
+        }
+
+        return false;
+    }
+
+    std::vector<std::string> lines;
+};
+
+struct TestWriteBinaryProvider : IWriteBinaryProvider
+{
+    TestWriteBinaryProvider(bool upload_fails) : upload_fails(upload_fails) { }
+
+    size_t push_success(DiagnosticContext& context, const Filesystem&, const BinaryPackageWriteInfo&) override
+    {
+        if (upload_fails)
+        {
+            context.report(DiagnosticLine{DiagKind::Warning, LocalizedString::from_raw("provider-specific failure")});
+            return 0;
+        }
+
+        return 1;
+    }
+
+    bool needs_nuspec_data() const override { return false; }
+    bool needs_zip_file() const override { return false; }
+
+    bool upload_fails;
+};
+
+struct TestBinaryCache : BinaryCache
+{
+    TestBinaryCache(const Filesystem& fs, MessageSink& message_sink) : BinaryCache(fs, message_sink) { }
+
+    void install_write_provider(std::unique_ptr<IWriteBinaryProvider>&& provider)
+    {
+        m_config.write.push_back(std::move(provider));
+    }
+};
+
 TEST_CASE ("CacheStatus operations", "[BinaryCache]")
 {
     KnowNothingBinaryProvider know_nothing;
@@ -540,6 +589,54 @@ TEST_CASE ("Synchronizer operations", "[BinaryCache]")
     }
 }
 
+TEST_CASE ("Binary cache upload failure logging", "[BinaryCache]")
+{
+    auto pghs = Paragraphs::parse_paragraphs(R"(
+Source: test-port
+Version: 1
+Description: test port
+)",
+                                             "<testdata>");
+    REQUIRE(pghs.has_value());
+    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()};
+    PackagesDirAssigner packages_dir_assigner{"test_packages_root"};
+    InstallPlanAction action(PackageSpec{"test-port", Test::X64_WINDOWS},
+                             scfl,
+                             packages_dir_assigner,
+                             RequestType::USER_REQUESTED,
+                             UseHeadVersion::No,
+                             Editable::No,
+                             {},
+                             {},
+                             {});
+    action.abi_info = AbiInfo{};
+    action.abi_info.get()->package_abi = "packageabi";
+
+    const std::string expected_warning = "warning: Binary cache upload failed: provider-specific failure";
+
+    SECTION ("all uploads succeed")
+    {
+        TestMessageSink message_sink;
+        TestBinaryCache binary_cache{always_failing_filesystem, message_sink};
+        binary_cache.install_write_provider(std::make_unique<TestWriteBinaryProvider>(false));
+        binary_cache.push_success(CleanPackages::No, action);
+        binary_cache.wait_for_async_complete_and_join();
+        CHECK_FALSE(message_sink.contains_line(expected_warning));
+    }
+
+    SECTION ("upload fails")
+    {
+        TestMessageSink message_sink;
+        TestBinaryCache binary_cache{always_failing_filesystem, message_sink};
+        binary_cache.install_write_provider(std::make_unique<TestWriteBinaryProvider>(true));
+        binary_cache.push_success(CleanPackages::No, action);
+        binary_cache.wait_for_async_complete_and_join();
+        CHECK(message_sink.contains_line(expected_warning));
+    }
+}
+
 TEST_CASE ("Test batch_command_arguments_with_fixed_length", "[batch-arguments]")
 {
     static constexpr std::size_t MAX_LEN = 100;

src/vcpkg/binarycaching.cpp

@@ -212,6 +212,31 @@ namespace
         return false;
     }
 
+    struct BinaryCacheUploadDiagnosticContext final : DiagnosticContext
+    {
+        explicit BinaryCacheUploadDiagnosticContext(DiagnosticContext& inner_context) : inner_context(inner_context) { }
+
+        void report(const DiagnosticLine& line) override
+        {
+            if (line.kind() == DiagKind::Error || line.kind() == DiagKind::Warning)
+            {
+                inner_context.report(DiagnosticLine{
+                    line.kind(), msg::format(msgBinaryCacheUploadFailed).append_raw(": ").append(line.message_text())});
+            }
+            else
+            {
+                inner_context.report(line);
+            }
+        }
+
+        void statusln(const LocalizedString& message) override { inner_context.statusln(message); }
+        void statusln(LocalizedString&& message) override { inner_context.statusln(std::move(message)); }
+        void statusln(const MessageLine& message) override { inner_context.statusln(message); }
+        void statusln(MessageLine&& message) override { inner_context.statusln(std::move(message)); }
+
+        DiagnosticContext& inner_context;
+    };
+
 #ifdef _WIN32
     bool directory_last_write_time(DiagnosticContext& context, const Filesystem& fs, const Path& dir)
     {
@@ -2893,8 +2918,9 @@ namespace vcpkg
 
         return true;
     }
-    BinaryCache::BinaryCache(const Filesystem& fs)
-        : m_fs(fs), m_bg_msg_sink(stdout_sink), m_push_thread(&BinaryCache::push_thread_main, this)
+    BinaryCache::BinaryCache(const Filesystem& fs) : BinaryCache(fs, stdout_sink) { }
+    BinaryCache::BinaryCache(const Filesystem& fs, MessageSink& message_sink)
+        : m_fs(fs), m_bg_msg_sink(message_sink), m_push_thread(&BinaryCache::push_thread_main, this)
     {
     }
     BinaryCache::~BinaryCache() { wait_for_async_complete_and_join(); }
@@ -2974,6 +3000,7 @@ namespace vcpkg
         std::vector<ActionToPush> my_tasks;
         PrintingDiagnosticContext pdc{m_bg_msg_sink};
         WarningDiagnosticContext wdc{pdc};
+        BinaryCacheUploadDiagnosticContext upload_context{pdc};
         while (m_actions_to_push.get_work(my_tasks))
         {
             for (auto& action_to_push : my_tasks)
@@ -2993,7 +3020,7 @@ namespace vcpkg
                 {
                     if (!provider->needs_zip_file() || action_to_push.request.zip_path.has_value())
                     {
-                        num_destinations += provider->push_success(pdc, m_fs, action_to_push.request);
+                        num_destinations += provider->push_success(upload_context, m_fs, action_to_push.request);
                     }
                 }