From 5a118a4e2bc1a04abcdec5f6cfb2c1fa8530696c Mon Sep 17 00:00:00 2001 From: Niq Dudfield Date: Wed, 17 Dec 2025 06:45:41 +0700 Subject: [PATCH] fix(logs): formatting fixes, color handling, and debug build defaults (#607) --- Builds/CMake/RippledCore.cmake | 24 +++++------ CMakeLists.txt | 19 +++----- src/ripple/basics/impl/Log.cpp | 6 ++- src/ripple/beast/utility/EnhancedLogging.h | 8 ++++ .../utility/src/beast_EnhancedLogging.cpp | 4 ++ .../beast/utility/src/beast_Journal.cpp | 43 ++++++++++++++++--- 6 files changed, 69 insertions(+), 35 deletions(-) diff --git a/Builds/CMake/RippledCore.cmake b/Builds/CMake/RippledCore.cmake index 683032366..74ff58f60 100644 --- a/Builds/CMake/RippledCore.cmake +++ b/Builds/CMake/RippledCore.cmake @@ -48,13 +48,9 @@ target_sources (xrpl_core PRIVATE src/ripple/beast/net/impl/IPAddressV6.cpp src/ripple/beast/net/impl/IPEndpoint.cpp src/ripple/beast/utility/src/beast_Journal.cpp - src/ripple/beast/utility/src/beast_PropertyStream.cpp) - -# Conditionally add enhanced logging source when BEAST_ENHANCED_LOGGING is enabled -if(DEFINED BEAST_ENHANCED_LOGGING AND BEAST_ENHANCED_LOGGING) - target_sources(xrpl_core PRIVATE - src/ripple/beast/utility/src/beast_EnhancedLogging.cpp) -endif() + src/ripple/beast/utility/src/beast_PropertyStream.cpp + # Enhanced logging - compiles to empty when BEAST_ENHANCED_LOGGING is not defined + src/ripple/beast/utility/src/beast_EnhancedLogging.cpp) #[===============================[ core sources @@ -162,12 +158,16 @@ target_link_libraries (xrpl_core date::date Ripple::opts) -# Link date-tz library when enhanced logging is enabled -if(DEFINED BEAST_ENHANCED_LOGGING AND BEAST_ENHANCED_LOGGING) - if(TARGET date::date-tz) - target_link_libraries(xrpl_core PUBLIC date::date-tz) - endif() +# date-tz for enhanced logging (always linked, code is #ifdef guarded) +if(TARGET date::date-tz) + target_link_libraries(xrpl_core PUBLIC date::date-tz) endif() + +# BEAST_ENHANCED_LOGGING: enable for Debug builds OR when explicitly requested +# Uses generator expression so it works with multi-config generators (Xcode, VS, Ninja Multi-Config) +target_compile_definitions(xrpl_core PUBLIC + $<$,$>:BEAST_ENHANCED_LOGGING=1> +) #[=================================[ main/core headers installation #]=================================] diff --git a/CMakeLists.txt b/CMakeLists.txt index 9a31d4931..d9d42149e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -37,20 +37,11 @@ endif() #git set(SOURCE_ROOT_PATH "${CMAKE_CURRENT_SOURCE_DIR}/src/") add_definitions(-DSOURCE_ROOT_PATH="${SOURCE_ROOT_PATH}") -# BEAST_ENHANCED_LOGGING option - adds file:line numbers and formatting to logs -# Default to ON for Debug builds, OFF for Release -if(CMAKE_BUILD_TYPE STREQUAL "Debug") - option(BEAST_ENHANCED_LOGGING "Include file and line numbers in log messages" ON) -else() - option(BEAST_ENHANCED_LOGGING "Include file and line numbers in log messages" OFF) -endif() - -if(BEAST_ENHANCED_LOGGING) - add_definitions(-DBEAST_ENHANCED_LOGGING=1) - message(STATUS "Log line numbers enabled") -else() - message(STATUS "Log line numbers disabled") -endif() +# BEAST_ENHANCED_LOGGING - adds file:line numbers and formatting to logs +# Automatically enabled for Debug builds via generator expression +# Can be explicitly controlled with -DBEAST_ENHANCED_LOGGING=ON/OFF +option(BEAST_ENHANCED_LOGGING "Include file and line numbers in log messages (auto: Debug=ON, Release=OFF)" OFF) +message(STATUS "BEAST_ENHANCED_LOGGING option: ${BEAST_ENHANCED_LOGGING}") if(thread_safety_analysis) add_compile_options(-Wthread-safety -D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS -DRIPPLE_ENABLE_THREAD_SAFETY_ANNOTATIONS) diff --git a/src/ripple/basics/impl/Log.cpp b/src/ripple/basics/impl/Log.cpp index f4f52cdc5..ddde4584a 100644 --- a/src/ripple/basics/impl/Log.cpp +++ b/src/ripple/basics/impl/Log.cpp @@ -360,7 +360,8 @@ Logs::format( if (!partition.empty()) { #ifdef BEAST_ENHANCED_LOGGING - output += beast::detail::get_log_highlight_color(); + if (beast::detail::should_log_use_colors()) + output += beast::detail::get_log_highlight_color(); #endif output += partition + ":"; } @@ -392,7 +393,8 @@ Logs::format( } #ifdef BEAST_ENHANCED_LOGGING - output += "\033[0m"; + if (beast::detail::should_log_use_colors()) + output += "\033[0m"; #endif output += message; diff --git a/src/ripple/beast/utility/EnhancedLogging.h b/src/ripple/beast/utility/EnhancedLogging.h index 198e0b52f..626562f67 100644 --- a/src/ripple/beast/utility/EnhancedLogging.h +++ b/src/ripple/beast/utility/EnhancedLogging.h @@ -41,6 +41,14 @@ get_log_highlight_color(); constexpr const char* strip_source_root(const char* file) { + // Handle relative paths from build/ directory (common with ccache) + // e.g., "../src/ripple/..." -> "ripple/..." + if (file && file[0] == '.' && file[1] == '.' && file[2] == '/' && + file[3] == 's' && file[4] == 'r' && file[5] == 'c' && file[6] == '/') + { + return file + 7; // skip "../src/" + } + #ifdef SOURCE_ROOT_PATH constexpr const char* sourceRoot = SOURCE_ROOT_PATH; constexpr auto strlen_constexpr = [](const char* s) constexpr diff --git a/src/ripple/beast/utility/src/beast_EnhancedLogging.cpp b/src/ripple/beast/utility/src/beast_EnhancedLogging.cpp index c5f34bd88..f5a608dd0 100644 --- a/src/ripple/beast/utility/src/beast_EnhancedLogging.cpp +++ b/src/ripple/beast/utility/src/beast_EnhancedLogging.cpp @@ -17,6 +17,8 @@ */ //============================================================================== +#ifdef BEAST_ENHANCED_LOGGING + #include #include #include @@ -112,3 +114,5 @@ log_write_location_string(std::ostream& os, const char* file, int line) } // namespace detail } // namespace beast + +#endif // BEAST_ENHANCED_LOGGING diff --git a/src/ripple/beast/utility/src/beast_Journal.cpp b/src/ripple/beast/utility/src/beast_Journal.cpp index 037da0b76..1a86e5bbf 100644 --- a/src/ripple/beast/utility/src/beast_Journal.cpp +++ b/src/ripple/beast/utility/src/beast_Journal.cpp @@ -155,14 +155,43 @@ Journal::ScopedStream::~ScopedStream() #ifdef BEAST_ENHANCED_LOGGING // Add suffix if location is enabled - if (file_ && detail::should_show_location() && !s.empty() && s != "\n") + if (file_ && detail::should_show_location() && !s.empty()) { - std::ostringstream combined; - combined << s; - if (!s.empty() && s.back() != ' ') - combined << " "; - detail::log_write_location_string(combined, file_, line_); - s = combined.str(); + // Single optimized scan from the end + size_t const lastNonWhitespace = s.find_last_not_of(" \n\r\t"); + + // Skip if message is only whitespace (e.g., just "\n" or " \n\n") + if (lastNonWhitespace != std::string::npos) + { + // Count only the trailing newlines (tiny range) + size_t trailingNewlines = 0; + for (size_t i = lastNonWhitespace + 1; i < s.length(); ++i) + { + if (s[i] == '\n') + ++trailingNewlines; + } + + // Build location string once + std::ostringstream locStream; + detail::log_write_location_string(locStream, file_, line_); + std::string const location = locStream.str(); + + // Pre-allocate exact size → zero reallocations + size_t const finalSize = lastNonWhitespace + 1 + 1 + + location.length() + trailingNewlines; + + std::string result; + result.reserve(finalSize); + + // Direct string ops (no ostringstream overhead) + result.append(s, 0, lastNonWhitespace + 1); + result.push_back(' '); + result += location; + if (trailingNewlines > 0) + result.append(trailingNewlines, '\n'); + + s = std::move(result); // Move, no copy + } } #endif