diff --git a/include/vcpkg/base/message-data.inc.h b/include/vcpkg/base/message-data.inc.h index cb217aee92..156583dd10 100644 --- a/include/vcpkg/base/message-data.inc.h +++ b/include/vcpkg/base/message-data.inc.h @@ -341,6 +341,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", diff --git a/include/vcpkg/binarycaching.h b/include/vcpkg/binarycaching.h index 49bfb2b329..ee5ecb90ac 100644 --- a/include/vcpkg/binarycaching.h +++ b/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 { diff --git a/locales/messages.json b/locales/messages.json index 9002acda53..a094626d9b 100644 --- a/locales/messages.json +++ b/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}", diff --git a/src/vcpkg-test/binarycaching.cpp b/src/vcpkg-test/binarycaching.cpp index 8fccf52a38..d118ff338d 100644 --- a/src/vcpkg-test/binarycaching.cpp +++ b/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 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&& 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 +)", + ""); + 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(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(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; diff --git a/src/vcpkg/binarycaching.cpp b/src/vcpkg/binarycaching.cpp index 09ffbf109a..15ff8e7923 100644 --- a/src/vcpkg/binarycaching.cpp +++ b/src/vcpkg/binarycaching.cpp @@ -213,6 +213,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) { @@ -2899,8 +2924,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(); } @@ -2980,6 +3006,7 @@ namespace vcpkg std::vector 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) @@ -2999,7 +3026,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); } }