diff --git a/CMakeLists.txt b/CMakeLists.txt index e57d8c142..ca93d0b80 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -921,6 +921,44 @@ if (BUILD_TESTING) FIXTURES_REQUIRED cmake_package_config) set_tests_properties (cmake_package_config_build PROPERTIES FIXTURES_REQUIRED "cmake_package_config;cmake_package_config_working") + + add_executable (cleanup_immediately_unittest + src/cleanup_immediately_unittest.cc) + + target_link_libraries (cleanup_immediately_unittest PRIVATE ${_GLOG_TEST_LIBS}) + + add_executable (cleanup_with_prefix_unittest + src/cleanup_with_prefix_unittest.cc) + + target_link_libraries (cleanup_with_prefix_unittest PRIVATE ${_GLOG_TEST_LIBS}) + + set (CLEANUP_LOG_DIR ${CMAKE_CURRENT_BINARY_DIR}/cleanup_tests) + + add_test (NAME cleanup_init COMMAND + ${CMAKE_COMMAND} -E make_directory ${CLEANUP_LOG_DIR}) + add_test (NAME cleanup_logdir COMMAND + ${CMAKE_COMMAND} -E remove_directory ${CLEANUP_LOG_DIR}) + add_test (NAME cleanup_immediately COMMAND + ${CMAKE_COMMAND} + -DLOGCLEANUP=$ + # NOTE The trailing slash is important + -DTEST_DIR=${CLEANUP_LOG_DIR}/ + -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/RunCleanerTest1.cmake + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) + add_test (NAME cleanup_with_prefix COMMAND + ${CMAKE_COMMAND} + -DLOGCLEANUP=$ + -DTEST_DIR=${CMAKE_CURRENT_BINARY_DIR}/ + -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/RunCleanerTest2.cmake + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) + + # Fixtures setup + set_tests_properties (cleanup_init PROPERTIES FIXTURES_SETUP logcleanuptest) + ## Fixtures cleanup + set_tests_properties (cleanup_logdir PROPERTIES FIXTURES_CLEANUP logcleanuptest) + # Fixture requirements + set_tests_properties (cleanup_immediately PROPERTIES FIXTURES_REQUIRED logcleanuptest) + set_tests_properties (cleanup_with_prefix PROPERTIES FIXTURES_REQUIRED logcleanuptest) endif (BUILD_TESTING) install (TARGETS glog diff --git a/cmake/RunCleanerTest1.cmake b/cmake/RunCleanerTest1.cmake new file mode 100644 index 000000000..7fbf46336 --- /dev/null +++ b/cmake/RunCleanerTest1.cmake @@ -0,0 +1,22 @@ +set (RUNS 3) + +foreach (iter RANGE 1 ${RUNS}) + set (ENV{GOOGLE_LOG_DIR} ${TEST_DIR}) + execute_process (COMMAND ${LOGCLEANUP} RESULT_VARIABLE _RESULT) + + if (NOT _RESULT EQUAL 0) + message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})") + endif (NOT _RESULT EQUAL 0) + + # Ensure the log files to have different modification timestamps such that + # exactly one log file remains at the end. Otherwise all log files will be + # retained. + execute_process (COMMAND ${CMAKE_COMMAND} -E sleep 1) +endforeach (iter) + +file (GLOB LOG_FILES ${TEST_DIR}/*.foobar) +list (LENGTH LOG_FILES NUM_FILES) + +if (NOT NUM_FILES EQUAL 1) + message (SEND_ERROR "Expected 1 log file in log directory but found ${NUM_FILES}") +endif (NOT NUM_FILES EQUAL 1) diff --git a/cmake/RunCleanerTest2.cmake b/cmake/RunCleanerTest2.cmake new file mode 100644 index 000000000..d3b51e355 --- /dev/null +++ b/cmake/RunCleanerTest2.cmake @@ -0,0 +1,22 @@ +set (RUNS 3) + +foreach (iter RANGE 1 ${RUNS}) + execute_process (COMMAND ${LOGCLEANUP} -log_dir=${TEST_DIR} + RESULT_VARIABLE _RESULT) + + if (NOT _RESULT EQUAL 0) + message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})") + endif (NOT _RESULT EQUAL 0) + + # Ensure the log files to have different modification timestamps such that + # exactly one log file remains at the end. Otherwise all log files will be + # retained. + execute_process (COMMAND ${CMAKE_COMMAND} -E sleep 1) +endforeach (iter) + +file (GLOB LOG_FILES ${TEST_DIR}/test_cleanup_*.barfoo) +list (LENGTH LOG_FILES NUM_FILES) + +if (NOT NUM_FILES EQUAL 1) + message (SEND_ERROR "Expected 1 log file in build directory ${TEST_DIR} but found ${NUM_FILES}") +endif (NOT NUM_FILES EQUAL 1) diff --git a/src/cleanup_immediately_unittest.cc b/src/cleanup_immediately_unittest.cc new file mode 100644 index 000000000..89d008eb9 --- /dev/null +++ b/src/cleanup_immediately_unittest.cc @@ -0,0 +1,96 @@ +// Copyright (c) 2021, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include +#include + +#include "base/commandlineflags.h" +#include "googletest.h" + +#ifdef HAVE_LIB_GFLAGS +#include +using namespace GFLAGS_NAMESPACE; +#endif + +#ifdef HAVE_LIB_GMOCK +#include + +#include "mock-log.h" +// Introduce several symbols from gmock. +using GOOGLE_NAMESPACE::glog_testing::ScopedMockLog; +using testing::_; +using testing::AllOf; +using testing::AnyNumber; +using testing::HasSubstr; +using testing::InitGoogleMock; +using testing::StrictMock; +using testing::StrNe; +#endif + +using namespace GOOGLE_NAMESPACE; + +TEST(CleanImmediately, logging) { + google::SetLogFilenameExtension(".foobar"); + google::EnableLogCleaner(0); + + for (unsigned i = 0; i < 1000; ++i) { + LOG(INFO) << "cleanup test"; + } + + google::DisableLogCleaner(); +} + +int main(int argc, char **argv) { + FLAGS_colorlogtostderr = false; + FLAGS_timestamp_in_logfile_name = true; +#ifdef HAVE_LIB_GFLAGS + ParseCommandLineFlags(&argc, &argv, true); +#endif + // Make sure stderr is not buffered as stderr seems to be buffered + // on recent windows. + setbuf(stderr, NULL); + + // Test some basics before InitGoogleLogging: + CaptureTestStderr(); + const string early_stderr = GetCapturedTestStderr(); + + EXPECT_FALSE(IsGoogleLoggingInitialized()); + + InitGoogleLogging(argv[0]); + + EXPECT_TRUE(IsGoogleLoggingInitialized()); + + InitGoogleTest(&argc, argv); +#ifdef HAVE_LIB_GMOCK + InitGoogleMock(&argc, argv); +#endif + + // so that death tests run before we use threads + CHECK_EQ(RUN_ALL_TESTS(), 0); +} diff --git a/src/cleanup_with_prefix_unittest.cc b/src/cleanup_with_prefix_unittest.cc new file mode 100644 index 000000000..01e5fa8fe --- /dev/null +++ b/src/cleanup_with_prefix_unittest.cc @@ -0,0 +1,97 @@ +// Copyright (c) 2021, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#include +#include + +#include "base/commandlineflags.h" +#include "googletest.h" + +#ifdef HAVE_LIB_GFLAGS +#include +using namespace GFLAGS_NAMESPACE; +#endif + +#ifdef HAVE_LIB_GMOCK +#include + +#include "mock-log.h" +// Introduce several symbols from gmock. +using GOOGLE_NAMESPACE::glog_testing::ScopedMockLog; +using testing::_; +using testing::AllOf; +using testing::AnyNumber; +using testing::HasSubstr; +using testing::InitGoogleMock; +using testing::StrictMock; +using testing::StrNe; +#endif + +using namespace GOOGLE_NAMESPACE; + +TEST(CleanImmediatelyWithPrefix, logging) { + google::EnableLogCleaner(0); + google::SetLogFilenameExtension(".barfoo"); + google::SetLogDestination(GLOG_INFO, "test_cleanup_"); + + for (unsigned i = 0; i < 1000; ++i) { + LOG(INFO) << "cleanup test"; + } + + google::DisableLogCleaner(); +} + +int main(int argc, char **argv) { + FLAGS_colorlogtostderr = false; + FLAGS_timestamp_in_logfile_name = true; +#ifdef HAVE_LIB_GFLAGS + ParseCommandLineFlags(&argc, &argv, true); +#endif + // Make sure stderr is not buffered as stderr seems to be buffered + // on recent windows. + setbuf(stderr, NULL); + + // Test some basics before InitGoogleLogging: + CaptureTestStderr(); + const string early_stderr = GetCapturedTestStderr(); + + EXPECT_FALSE(IsGoogleLoggingInitialized()); + + InitGoogleLogging(argv[0]); + + EXPECT_TRUE(IsGoogleLoggingInitialized()); + + InitGoogleTest(&argc, argv); +#ifdef HAVE_LIB_GMOCK + InitGoogleMock(&argc, argv); +#endif + + // so that death tests run before we use threads + CHECK_EQ(RUN_ALL_TESTS(), 0); +} diff --git a/src/logging.cc b/src/logging.cc index 064acbd97..b6625616b 100644 --- a/src/logging.cc +++ b/src/logging.cc @@ -502,7 +502,6 @@ class LogCleaner { bool enabled_; int overdue_days_; - char dir_delim_; // filepath delimiter ('/' or '\\') }; LogCleaner log_cleaner; @@ -916,6 +915,13 @@ void SetApplicationFingerprint(const std::string& fingerprint) { namespace { +// Directory delimiter; Windows supports both forward slashes and backslashes +#ifdef GLOG_OS_WINDOWS +const char possible_dir_delim[] = {'\\', '/'}; +#else +const char possible_dir_delim[] = {'/'}; +#endif + string PrettyDuration(int secs) { std::stringstream result; int mins = secs / 60; @@ -1288,7 +1294,7 @@ void LogFileObject::Write(bool force_flush, } #endif - // Perform clean up for old logs + // Remove old logs if (log_cleaner.enabled()) { if (base_filename_selected_ && base_filename_.empty()) { return; @@ -1300,17 +1306,11 @@ void LogFileObject::Write(bool force_flush, } } - -LogCleaner::LogCleaner() : enabled_(false), overdue_days_(7), dir_delim_('/') { -#ifdef GLOG_OS_WINDOWS - dir_delim_ = '\\'; -#endif -} +LogCleaner::LogCleaner() : enabled_(false), overdue_days_(7) {} void LogCleaner::Enable(int overdue_days) { - // Setting overdue_days to 0 day should not be allowed! - // Since all logs will be deleted immediately, which will cause troubles. - assert(overdue_days > 0); + // Setting overdue_days to 0 days will delete all logs. + assert(overdue_days >= 0); enabled_ = true; overdue_days_ = overdue_days; @@ -1323,15 +1323,21 @@ void LogCleaner::Disable() { void LogCleaner::Run(bool base_filename_selected, const string& base_filename, const string& filename_extension) const { - assert(enabled_ && overdue_days_ > 0); + assert(enabled_ && overdue_days_ >= 0); vector dirs; - if (base_filename_selected) { - string dir = base_filename.substr(0, base_filename.find_last_of(dir_delim_) + 1); - dirs.push_back(dir); - } else { + if (!base_filename_selected) { dirs = GetLoggingDirectories(); + } else { + size_t pos = base_filename.find_last_of(possible_dir_delim, 0, + sizeof(possible_dir_delim)); + if (pos != string::npos) { + string dir = base_filename.substr(0, pos + 1); + dirs.push_back(dir); + } else { + dirs.push_back("."); + } } for (size_t i = 0; i < dirs.size(); i++) { @@ -1356,17 +1362,23 @@ vector LogCleaner::GetOverdueLogNames(string log_directory, DIR *dir; struct dirent *ent; - // If log_directory doesn't end with a slash, append a slash to it. - if (log_directory.at(log_directory.size() - 1) != dir_delim_) { - log_directory += dir_delim_; - } - if ((dir=opendir(log_directory.c_str()))) { - while ((ent=readdir(dir))) { - if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) { + if ((dir = opendir(log_directory.c_str()))) { + while ((ent = readdir(dir))) { + if (strcmp(ent->d_name, ".") == 0 || strcmp(ent->d_name, "..") == 0) { continue; } - string filepath = log_directory + ent->d_name; + + string filepath = ent->d_name; + const char* const dir_delim_end = + possible_dir_delim + sizeof(possible_dir_delim); + + if (!log_directory.empty() && + std::find(possible_dir_delim, dir_delim_end, + log_directory[log_directory.size() - 1]) != dir_delim_end) { + filepath = log_directory + filepath; + } + if (IsLogFromCurrentProject(filepath, base_filename, filename_extension) && IsLogLastModifiedOver(filepath, days)) { overdue_log_names.push_back(filepath); @@ -1386,14 +1398,19 @@ bool LogCleaner::IsLogFromCurrentProject(const string& filepath, // after: "/tmp/.." string cleaned_base_filename; + const char* const dir_delim_end = + possible_dir_delim + sizeof(possible_dir_delim); + size_t real_filepath_size = filepath.size(); for (size_t i = 0; i < base_filename.size(); ++i) { const char& c = base_filename[i]; if (cleaned_base_filename.empty()) { cleaned_base_filename += c; - } else if (c != dir_delim_ || - c != cleaned_base_filename.at(cleaned_base_filename.size() - 1)) { + } else if (std::find(possible_dir_delim, dir_delim_end, c) == + dir_delim_end || + (!cleaned_base_filename.empty() && + c != cleaned_base_filename[cleaned_base_filename.size() - 1])) { cleaned_base_filename += c; } } @@ -1457,10 +1474,10 @@ bool LogCleaner::IsLogLastModifiedOver(const string& filepath, int days) const { struct stat file_stat; if (stat(filepath.c_str(), &file_stat) == 0) { - // A day is 86400 seconds, so 7 days is 86400 * 7 = 604800 seconds. + const time_t seconds_in_a_day = 60 * 60 * 24; time_t last_modified_time = file_stat.st_mtime; time_t current_time = time(NULL); - return difftime(current_time, last_modified_time) > days * 86400; + return difftime(current_time, last_modified_time) > days * seconds_in_a_day; } // If failed to get file stat, don't return true!