Merge bitcoin/bitcoin#35597: logging: More fully remove libevent log category

3765b42 logging: More fully remove libevent log category (Ryan Ofsky)

Pull request description:

  Libevent log category was partially removed in 39e9099, and this commit extends that with the following changes:

  - Stops showing libevent in the list of supported log categories in `bitcoind -help` and `bitcoin-cli help logging` output.
  - Stops returning `"libevent": false` in `logging` RPC output.

  It's not good to treat libevent as a supported log category when it can't be enabled and trying to enable it results in warnings.

  There's also no need to define an unused LIBEVENT constant value and keep more complicated logic for dealing with deprecated log categories, so this change also simplifies code internally.

ACKs for top commit:
  l0rinc:
    code review ACK 3765b42
  pinheadmz:
    ACK 3765b42
  sedited:
    ACK 3765b42

Tree-SHA512: 09e9514f905bb0a79d870689af491886baaa31fa19f2ad6aef4283fa20c2fa6ce8d384178139227aeeabffabff6e83d254114daaeefbbfe2c6172b9da8871298

Author: sedited

doc/release-notes-35182.md

@@ -3,9 +3,9 @@ HTTP: RPC / REST
 
 The HTTP server has been rewritten from scratch to replace libevent. (#35182)
 
-`libevent` has been deprecated as a logging category and will be removed in
-a future release. At that time configurations like `debugexclude=libevent` will
-be invalid.
+The `libevent` logging category has been removed. Configurations like
+`-debug=libevent` or `-debugexclude=libevent` will log a deprecation warning
+and be ignored. These configurations will result in an error in a future release.
 
 Certain HTTP edge cases will observe different behavior to be more RFC-compliant:
 

src/logging.cpp

@@ -133,11 +133,6 @@ void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
 bool BCLog::Logger::EnableCategory(std::string_view str)
 {
     if (const auto flag{GetLogCategory(str)}) {
-        if (*flag & DEPRECATED){
-            LogWarning("The logging category `%s` is deprecated, can not be enabled, and will be removed in a future version", str);
-            // Deprecated does not mean unsupported, which may prevent startup
-            return true;
-        }
         EnableCategory(*flag);
         return true;
     }
@@ -152,11 +147,6 @@ void BCLog::Logger::DisableCategory(BCLog::LogFlags flag)
 bool BCLog::Logger::DisableCategory(std::string_view str)
 {
     if (const auto flag{GetLogCategory(str)}) {
-        if (*flag & DEPRECATED){
-            LogWarning("The logging category `%s` is deprecated and will be removed in a future version", str);
-            // Deprecated does not mean unsupported, which may prevent startup
-            return true;
-        }
         DisableCategory(*flag);
         return true;
     }
@@ -204,7 +194,6 @@ static const std::map<std::string, BCLog::LogFlags, std::less<>> LOG_CATEGORIES_
     {"prune", BCLog::PRUNE},
     {"proxy", BCLog::PROXY},
     {"mempoolrej", BCLog::MEMPOOLREJ},
-    {"libevent", BCLog::LIBEVENT},
     {"coindb", BCLog::COINDB},
     {"qt", BCLog::QT},
     {"leveldb", BCLog::LEVELDB},
@@ -243,6 +232,10 @@ std::optional<BCLog::LogFlags> BCLog::Logger::GetLogCategory(std::string_view st
     if (it != LOG_CATEGORIES_BY_STR.end()) {
         return it->second;
     }
+    if (str == "libevent") {
+       LogWarning("The logging category `%s` is deprecated, does nothing, and will be removed in a future version", str);
+       return BCLog::NONE;
+    }
     return std::nullopt;
 }
 
@@ -616,6 +609,7 @@ bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::stri
 
     const auto level = GetLogLevel(level_str);
     if (!level.has_value() || level.value() > MAX_USER_SETABLE_SEVERITY_LEVEL) return false;
+    if (*flag == BCLog::NONE) return true;
 
     STDLOCK(m_cs);
     m_category_log_levels[*flag] = level.value();

src/logging/categories.h

@@ -30,26 +30,24 @@ enum LogFlags : CategoryMask {
     PRUNE = (CategoryMask{1} << 14),
     PROXY = (CategoryMask{1} << 15),
     MEMPOOLREJ = (CategoryMask{1} << 16),
-    LIBEVENT = (CategoryMask{1} << 17),
-    COINDB = (CategoryMask{1} << 18),
-    QT = (CategoryMask{1} << 19),
-    LEVELDB = (CategoryMask{1} << 20),
-    VALIDATION = (CategoryMask{1} << 21),
-    I2P = (CategoryMask{1} << 22),
-    IPC = (CategoryMask{1} << 23),
+    COINDB = (CategoryMask{1} << 17),
+    QT = (CategoryMask{1} << 18),
+    LEVELDB = (CategoryMask{1} << 19),
+    VALIDATION = (CategoryMask{1} << 20),
+    I2P = (CategoryMask{1} << 21),
+    IPC = (CategoryMask{1} << 22),
 #ifdef DEBUG_LOCKCONTENTION
-    LOCK = (CategoryMask{1} << 24),
+    LOCK = (CategoryMask{1} << 23),
 #endif
-    BLOCKSTORAGE = (CategoryMask{1} << 25),
-    TXRECONCILIATION = (CategoryMask{1} << 26),
-    SCAN = (CategoryMask{1} << 27),
-    TXPACKAGES = (CategoryMask{1} << 28),
-    KERNEL = (CategoryMask{1} << 29),
-    PRIVBROADCAST = (CategoryMask{1} << 30),
-    DEPRECATED = LIBEVENT,
-    // Remove deprecated categories from ALL
-    ALL = ~DEPRECATED,
+    BLOCKSTORAGE = (CategoryMask{1} << 24),
+    TXRECONCILIATION = (CategoryMask{1} << 25),
+    SCAN = (CategoryMask{1} << 26),
+    TXPACKAGES = (CategoryMask{1} << 27),
+    KERNEL = (CategoryMask{1} << 28),
+    PRIVBROADCAST = (CategoryMask{1} << 29),
+    ALL = ~NONE,
 };
+
 } // namespace BCLog
 
 #endif // BITCOIN_LOGGING_CATEGORIES_H

src/test/logging_tests.cpp

@@ -167,11 +167,7 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros_CategoryName, LogSetup)
     for (const auto& category_name : category_names) {
         const auto trimmed_category_name = TrimString(category_name);
         const auto category{*Assert(BCLog::Logger::GetLogCategory(trimmed_category_name))};
-        if (category & BCLog::LogFlags::ALL) {
-            expected_category_names.emplace_back(category, trimmed_category_name);
-        } else {
-            BOOST_CHECK(category & BCLog::LogFlags::DEPRECATED);
-        }
+        expected_category_names.emplace_back(category, trimmed_category_name);
     }
 
     std::vector<std::string> expected;
@@ -272,6 +268,13 @@ BOOST_FIXTURE_TEST_CASE(logging_Conf, LogSetup)
         BOOST_CHECK(http_it != category_levels.end());
         BOOST_CHECK_EQUAL(http_it->second, BCLog::Level::Info);
     }
+
+    // Removed categories (like "libevent") should not store a category-specific level
+    {
+        ResetLogger();
+        BOOST_CHECK(LogInstance().SetCategoryLogLevel(/*category_str=*/"libevent", /*level_str=*/"trace"));
+        BOOST_CHECK(LogInstance().CategoryLevels().empty());
+    }
 }
 
 struct ScopedScheduler {

test/functional/rpc_misc.py

@@ -121,16 +121,17 @@ def run_test(self):
         assert_equal(node.getindexinfo("foo"), {})
 
         # Test a deprecated category
-        node.logging(include=['all'])
-        for category, value in node.logging().items():
-            # Everything True except one...
-            assert_equal(value, category != "libevent")
-        with self.nodes[0].assert_debug_log(["The logging category `libevent` is deprecated"]):
-            node.logging(include=['libevent'])
-        assert_equal(node.logging()['libevent'], False)
-        with self.nodes[0].assert_debug_log(["The logging category `libevent` is deprecated"]):
-            node.logging(exclude=['libevent'])
-        assert_equal(node.logging()['libevent'], False)
+        all_result = node.logging(include=['all'])
+        assert_equal(True, all(enabled is True for category, enabled in all_result.items()))
+        assert_equal(True, 'libevent' not in all_result)
+        assert_equal(all_result, node.logging())
+        libevent_warning = "The logging category `libevent` is deprecated"
+        with self.nodes[0].assert_debug_log([libevent_warning]):
+            assert_equal(all_result, node.logging(include=['libevent']))
+        assert_equal(all_result, node.logging())
+        with self.nodes[0].assert_debug_log([libevent_warning]):
+            assert_equal(all_result, node.logging(exclude=['libevent']))
+        assert_equal(all_result, node.logging())
 
 
 if __name__ == '__main__':