From a0dae55a6970d5c1b89872b8ff827ed257398eca Mon Sep 17 00:00:00 2001 From: gabime Date: Sun, 7 Jun 2020 14:38:16 +0300 Subject: [PATCH] Revert 7f15fb2a21cfc7895846a22e5b0f5e4e88040e88 since it breaks the ABI --- include/spdlog/sinks/rotating_file_sink.h | 104 +++------------------- src/file_sinks.cpp | 4 + tests/CMakeLists.txt | 71 ++++++++------- tests/test_daily_logger.cpp | 22 +++++ tests/test_rotating_logger.cpp | 77 ---------------- 5 files changed, 73 insertions(+), 205 deletions(-) delete mode 100644 tests/test_rotating_logger.cpp diff --git a/include/spdlog/sinks/rotating_file_sink.h b/include/spdlog/sinks/rotating_file_sink.h index 4a016f44..e1e85a7d 100644 --- a/include/spdlog/sinks/rotating_file_sink.h +++ b/include/spdlog/sinks/rotating_file_sink.h @@ -7,7 +7,6 @@ #include #include #include -#include #include #include @@ -16,68 +15,20 @@ namespace spdlog { namespace sinks { -/* - * Generator of rotating log file names in format basename.index.extension - * e.g. calc_filename("logs/mylog.txt, 3) => "logs/mylog.3.txt". - */ -struct rotating_filename_calculator -{ - // Create filename for the form basename.index.extension - static filename_t calc_filename(const filename_t &filename, std::size_t index) - { - if (index == 0u) - { - return filename; - } - - filename_t basename, ext; - std::tie(basename, ext) = details::file_helper::split_by_extension(filename); - return fmt::format(SPDLOG_FILENAME_T("{}.{}{}"), basename, index, ext); - } -}; - // // Rotating file sink based on size // -template +template class rotating_file_sink final : public base_sink { public: - rotating_file_sink(filename_t base_filename, std::size_t max_size, std::size_t max_files, bool rotate_on_open = false) - : base_filename_(std::move(base_filename)) - , max_size_(max_size) - , max_files_(max_files) - { - file_helper_.open(FileNameCalc::calc_filename(base_filename_, 0)); - current_size_ = file_helper_.size(); // expensive. called only once - if (rotate_on_open && current_size_ > 0) - { - rotate_(); - } - } - filename_t filename() - { - std::lock_guard lock(base_sink::mutex_); - return file_helper_.filename(); - } + rotating_file_sink(filename_t base_filename, std::size_t max_size, std::size_t max_files, bool rotate_on_open = false); + static filename_t calc_filename(const filename_t &filename, std::size_t index); + filename_t filename(); protected: - void sink_it_(const details::log_msg &msg) override - { - memory_buf_t formatted; - base_sink::formatter_->format(msg, formatted); - current_size_ += formatted.size(); - if (current_size_ > max_size_) - { - rotate_(); - current_size_ = formatted.size(); - } - file_helper_.write(formatted); - } - void flush_() override - { - file_helper_.flush(); - } + void sink_it_(const details::log_msg &msg) override; + void flush_() override; private: // Rotate files: @@ -85,45 +36,11 @@ private: // log.1.txt -> log.2.txt // log.2.txt -> log.3.txt // log.3.txt -> delete - void rotate_() - { - using details::os::filename_to_str; - using details::os::path_exists; - file_helper_.close(); - for (auto i = max_files_; i > 0; --i) - { - filename_t src = FileNameCalc::calc_filename(base_filename_, i - 1); - if (!path_exists(src)) - { - continue; - } - filename_t target = FileNameCalc::calc_filename(base_filename_, i); - - if (!rename_file_(src, target)) - { - // if failed try again after a small delay. - // this is a workaround to a windows issue, where very high rotation - // rates can cause the rename to fail with permission denied (because of antivirus?). - details::os::sleep_for_millis(100); - if (!rename_file_(src, target)) - { - file_helper_.reopen(true); // truncate the log file anyway to prevent it to grow beyond its limit! - current_size_ = 0; - throw_spdlog_ex("rotating_file_sink: failed renaming " + filename_to_str(src) + " to " + filename_to_str(target), errno); - } - } - } - file_helper_.reopen(true); - } + void rotate_(); // delete the target if exists, and rename the src file to target // return true on success, false otherwise. - bool rename_file_(const filename_t &src_filename, const filename_t &target_filename) - { - // try to delete the target file in case it already exists. - (void)details::os::remove(target_filename); - return details::os::rename(src_filename, target_filename) == 0; - } + bool rename_file_(const filename_t &src_filename, const filename_t &target_filename); filename_t base_filename_; std::size_t max_size_; @@ -132,7 +49,6 @@ private: details::file_helper file_helper_; }; - using rotating_file_sink_mt = rotating_file_sink; using rotating_file_sink_st = rotating_file_sink; @@ -156,3 +72,7 @@ inline std::shared_ptr rotating_logger_st( return Factory::template create(logger_name, filename, max_file_size, max_files, rotate_on_open); } } // namespace spdlog + +#ifdef SPDLOG_HEADER_ONLY +#include "rotating_file_sink-inl.h" +#endif diff --git a/src/file_sinks.cpp b/src/file_sinks.cpp index 3d70e837..5fd7e98a 100644 --- a/src/file_sinks.cpp +++ b/src/file_sinks.cpp @@ -14,3 +14,7 @@ template class SPDLOG_API spdlog::sinks::basic_file_sink; template class SPDLOG_API spdlog::sinks::basic_file_sink; + +#include +template class SPDLOG_API spdlog::sinks::rotating_file_sink; +template class SPDLOG_API spdlog::sinks::rotating_file_sink; \ No newline at end of file diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index f57a98b8..790c8393 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,47 +2,46 @@ cmake_minimum_required(VERSION 3.2) project(spdlog_utests CXX) -if(NOT TARGET spdlog) +if (NOT TARGET spdlog) # Stand-alone build find_package(spdlog REQUIRED) -endif() +endif () include(../cmake/utils.cmake) find_package(PkgConfig) -if(PkgConfig_FOUND) +if (PkgConfig_FOUND) pkg_check_modules(systemd libsystemd) -endif() +endif () set(SPDLOG_UTESTS_SOURCES - test_file_helper.cpp - test_file_logging.cpp - test_daily_logger.cpp - test_rotating_logger.cpp - test_misc.cpp - test_eventlog.cpp - test_pattern_formatter.cpp - test_async.cpp - test_registry.cpp - test_macros.cpp - utils.cpp - main.cpp - test_mpmc_q.cpp - test_dup_filter.cpp - test_fmt_helper.cpp - test_stdout_api.cpp - test_backtrace.cpp - test_create_dir.cpp - test_cfg.cpp - test_time_point.cpp) + test_file_helper.cpp + test_file_logging.cpp + test_daily_logger.cpp + test_misc.cpp + test_eventlog.cpp + test_pattern_formatter.cpp + test_async.cpp + test_registry.cpp + test_macros.cpp + utils.cpp + main.cpp + test_mpmc_q.cpp + test_dup_filter.cpp + test_fmt_helper.cpp + test_stdout_api.cpp + test_backtrace.cpp + test_create_dir.cpp + test_cfg.cpp + test_time_point.cpp) -if(NOT SPDLOG_NO_EXCEPTIONS) +if (NOT SPDLOG_NO_EXCEPTIONS) list(APPEND SPDLOG_UTESTS_SOURCES test_errors.cpp) -endif() +endif () -if(systemd_FOUND) +if (systemd_FOUND) list(APPEND SPDLOG_UTESTS_SOURCES test_systemd.cpp) -endif() +endif () enable_testing() @@ -50,22 +49,22 @@ function(spdlog_prepare_test test_target spdlog_lib) add_executable(${test_target} ${SPDLOG_UTESTS_SOURCES}) spdlog_enable_warnings(${test_target}) target_link_libraries(${test_target} PRIVATE ${spdlog_lib}) - if(systemd_FOUND) + if (systemd_FOUND) target_link_libraries(${test_target} PRIVATE ${systemd_LIBRARIES}) - endif() - if(SPDLOG_SANITIZE_ADDRESS) + endif () + if (SPDLOG_SANITIZE_ADDRESS) spdlog_enable_sanitizer(${test_target}) - endif() + endif () add_test(NAME ${test_target} COMMAND ${test_target}) set_tests_properties(${test_target} PROPERTIES RUN_SERIAL ON) endfunction() # The compiled library tests -if(SPDLOG_BUILD_TESTS OR SPDLOG_BUILD_ALL) +if (SPDLOG_BUILD_TESTS OR SPDLOG_BUILD_ALL) spdlog_prepare_test(spdlog-utests spdlog::spdlog) -endif() +endif () # The header-only library version tests -if(SPDLOG_BUILD_TESTS_HO OR SPDLOG_BUILD_ALL) +if (SPDLOG_BUILD_TESTS_HO OR SPDLOG_BUILD_ALL) spdlog_prepare_test(spdlog-utests-ho spdlog::spdlog_header_only) -endif() +endif () diff --git a/tests/test_daily_logger.cpp b/tests/test_daily_logger.cpp index 3ccdfe65..a3e70e68 100644 --- a/tests/test_daily_logger.cpp +++ b/tests/test_daily_logger.cpp @@ -61,6 +61,28 @@ TEST_CASE("daily_logger with custom calculator", "[daily_logger]") require_message_count(filename, 10); } +/* + * File name calculations + */ + +TEST_CASE("rotating_file_sink::calc_filename1", "[rotating_file_sink]]") +{ + auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated.txt", 3); + REQUIRE(filename == "rotated.3.txt"); +} + +TEST_CASE("rotating_file_sink::calc_filename2", "[rotating_file_sink]]") +{ + auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated", 3); + REQUIRE(filename == "rotated.3"); +} + +TEST_CASE("rotating_file_sink::calc_filename3", "[rotating_file_sink]]") +{ + auto filename = spdlog::sinks::rotating_file_sink_st::calc_filename("rotated.txt", 0); + REQUIRE(filename == "rotated.txt"); +} + // regex supported only from gcc 4.9 and above #if defined(_MSC_VER) || !(__GNUC__ <= 4 && __GNUC_MINOR__ < 9) diff --git a/tests/test_rotating_logger.cpp b/tests/test_rotating_logger.cpp deleted file mode 100644 index f4c7ee0b..00000000 --- a/tests/test_rotating_logger.cpp +++ /dev/null @@ -1,77 +0,0 @@ -/* - * This content is released under the MIT License as specified in https://raw.githubusercontent.com/gabime/spdlog/master/LICENSE - */ -#include "includes.h" - -TEST_CASE("rotating_looger with default calculator", "[rotating_logger]") -{ - using sink_type = spdlog::sinks::rotating_file_sink; - - prepare_logdir(); - - std::string filename = "test_logs/rotating_default"; - - auto logger = spdlog::create("logger", filename, 5 * 1024, 1); - for (int i = 0; i < 10; ++i) - { - logger->info("Hello World!", i); - } - logger->flush(); - - require_message_count(filename, 10); -} - -struct custom_rotating_file_name_calculator -{ - static spdlog::filename_t calc_filename(const spdlog::filename_t &filename, std::size_t) - { - spdlog::filename_t basename, ext; - std::tie(basename, ext) = spdlog::details::file_helper::split_by_extension(filename); - return fmt::format(SPDLOG_FILENAME_T("{}_custom{}"), basename, ext); - } -}; - -TEST_CASE("rotating_logger with custom calculator", "[rotating_logger]") -{ - using sink_type = spdlog::sinks::rotating_file_sink; - - prepare_logdir(); - - // calculate filename - std::string basename = "test_logs/rotating"; - spdlog::memory_buf_t w; - fmt::format_to(w, "{}_custom", basename); - - auto logger = spdlog::create("logger", basename, 5 * 1024, 1); - for (int i = 0; i < 10; ++i) - { - logger->info("Hello World!"); - } - - logger->flush(); - - auto filename = fmt::to_string(w); - require_message_count(filename, 10); -} - -/* - * File name calculations - */ - -TEST_CASE("rotating_file_sink::calc_filename1", "[rotating_file_sink]]") -{ - auto filename = spdlog::sinks::rotating_filename_calculator::calc_filename("rotated.txt", 3); - REQUIRE(filename == "rotated.3.txt"); -} - -TEST_CASE("rotating_file_sink::calc_filename2", "[rotating_file_sink]]") -{ - auto filename = spdlog::sinks::rotating_filename_calculator::calc_filename("rotated", 3); - REQUIRE(filename == "rotated.3"); -} - -TEST_CASE("rotating_file_sink::calc_filename3", "[rotating_file_sink]]") -{ - auto filename = spdlog::sinks::rotating_filename_calculator::calc_filename("rotated.txt", 0); - REQUIRE(filename == "rotated.txt"); -}