Revert 7f15fb2a since it breaks the ABI

include/spdlog/sinks/rotating_file_sink.h

@@ -7,7 +7,6 @@
 #include <spdlog/details/file_helper.h>
 #include <spdlog/details/null_mutex.h>
 #include <spdlog/details/synchronous_factory.h>
-#include <spdlog/details/os.h>
 
 #include <chrono>
 #include <mutex>
@@ -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<typename Mutex, typename FileNameCalc = rotating_filename_calculator>
+template<typename Mutex>
 class rotating_file_sink final : public base_sink<Mutex>
 {
 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<Mutex> lock(base_sink<Mutex>::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<Mutex>::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<std::mutex>;
 using rotating_file_sink_st = rotating_file_sink<details::null_mutex>;
 
@@ -156,3 +72,7 @@ inline std::shared_ptr<logger> rotating_logger_st(
     return Factory::template create<sinks::rotating_file_sink_st>(logger_name, filename, max_file_size, max_files, rotate_on_open);
 }
 } // namespace spdlog
+
+#ifdef SPDLOG_HEADER_ONLY
+#include "rotating_file_sink-inl.h"
+#endif

src/file_sinks.cpp

@@ -14,3 +14,7 @@
 
 template class SPDLOG_API spdlog::sinks::basic_file_sink<std::mutex>;
 template class SPDLOG_API spdlog::sinks::basic_file_sink<spdlog::details::null_mutex>;
+
+#include <spdlog/sinks/rotating_file_sink-inl.h>
+template class SPDLOG_API spdlog::sinks::rotating_file_sink<std::mutex>;
+template class SPDLOG_API spdlog::sinks::rotating_file_sink<spdlog::details::null_mutex>;
\ No newline at end of file

tests/CMakeLists.txt

@@ -2,23 +2,22 @@ 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
@@ -36,13 +35,13 @@ set(SPDLOG_UTESTS_SOURCES
         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 ()

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)
 

tests/test_rotating_logger.cpp

-/*
- * 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<std::mutex, spdlog::sinks::rotating_filename_calculator>;
-
-    prepare_logdir();
-
-    std::string filename = "test_logs/rotating_default";
-    
-    auto logger = spdlog::create<sink_type>("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<std::mutex, custom_rotating_file_name_calculator>;
-
-    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<sink_type>("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");
-}