Switching from usage of libprotobuf internals to loguru, which has a similar interface.

John-LittleBearLabs · John-LittleBearLabs · commit 664e7a1fed05 · 2025-03-03T16:52:24.000-05:00
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -13,6 +13,7 @@ project(ipfs_client
 )
 
 set(TESTING ON CACHE BOOL "Do we want to run tests?")
+set(TEST_BY_DEFAULT OFF CACHE BOOL "If TESTING, do we want to run tests as a part of the default 'all' target.")
 set(CXX_VERSION 20 CACHE STRING "Should be no less than your conan profile.")
 
 file(READ bld/protobuf_version.txt pb_ver)
@@ -24,7 +25,7 @@ dependency(protobuf VERSION ${pb_ver})
 dependency(absl)
 dependency(Boost)
 dependency(c-ares)
-dependency(glog)
+dependency(loguru)
 dependency(nlohmann_json)
 dependency(OpenSSL)
 if(TESTING)
@@ -138,7 +139,7 @@ foreach(libname ipfs_client ipfs_client_covered)
         absl::strings
         Boost::headers
         c-ares::cares
-        glog::glog
+        loguru::loguru
         nlohmann_json::nlohmann_json
     )
 endforeach()
diff --git a/conanfile.py b/conanfile.py
@@ -40,7 +40,6 @@ class IpfsChromium(ConanFile):
         'boost/1.81.0',
         'bzip2/1.0.8',
         'c-ares/1.22.1',
-        'glog/0.7.1',
         'nlohmann_json/3.11.2',
         'openssl/1.1.1w',
         f'protobuf/{protobuf_version()}',
@@ -51,7 +50,7 @@ class IpfsChromium(ConanFile):
     default_options = {
         "testing": False,
         "boost/*:bzip2": True,
-        "boost/*:with_stacktrace_backtrace": True
+        "boost/*:with_stacktrace_backtrace": True,
     }
     tool_requires = [
         'cmake/3.22.6',
@@ -99,6 +98,7 @@ def requirements(self):
             self.requires(lib, transitive_headers=True)
         if self.options.testing:
             self.requires('gtest/1.15.0')
+        self.requires('loguru/cci.20230406', options={'enable_streams': 'True'})
 
     @property
     def _min_cppstd(self):
diff --git a/include/ipfs_client/logger.h b/include/ipfs_client/logger.h
@@ -1,6 +1,7 @@
 #ifndef IPFS_LOGGER_H_
 #define IPFS_LOGGER_H_
 
+#include <functional>
 #include <string>
 
 namespace ipfs::log {
@@ -15,19 +16,14 @@ enum class Level {
   Off
 };
 
+Level GetLevel();
 void SetLevel(Level);
 
-using Handler = void (*)(std::string const&, char const*, int, Level);
-void SetHandler(Handler) noexcept;
-
-void DefaultHandler(std::string const& message,
-                    char const* source_file,
-                    int source_line,
-                    Level for_prefix);
-
 std::string_view LevelDescriptor(Level);
 
-bool IsInitialized() noexcept;
+using Hook = std::function<void(std::string_view, std::string_view, int, Level)>;
+void AddHook(std::string id, Hook);
+void Unhook(std::string id);
 
 }  // namespace ipfs::log
 
diff --git a/include/ipfs_client/opinionated_context.h b/include/ipfs_client/opinionated_context.h
@@ -15,27 +15,14 @@
 
 #define HAS_OPINIONATED_CONTEXT 1
 
-#include <google/protobuf/stubs/logging.h>
-
 namespace boost::asio {
 class io_context;
 }
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-variable"
-namespace google::protobuf {
-  namespace {
-    LogLevel LOGLEVEL_DEBUG = static_cast<LogLevel>(-1);
-    LogLevel LOGLEVEL_TRACE = static_cast<LogLevel>(-2);
-  }
-}  // namespace google::protobuf
-#pragma GCC diagnostic pop
-
 class HttpSession;
 
 namespace ipfs {
 
-// std::pair<std::shared_ptr<ContextApi>, std::shared_ptr<Partition>>
 std::shared_ptr<Client> start_default(boost::asio::io_context& io);
 
 }  // namespace ipfs
diff --git a/src/ipfs_client/client.cc b/src/ipfs_client/client.cc
@@ -130,7 +130,7 @@ auto Self::json() -> ctx::JsonParser& {
     LOG(FATAL) << "A JSON parser must be provided.";
 #endif
   }
-  DCHECK(json_parser_);
+  DCHECK(!!json_parser_);
   return *json_parser_;
 }
 auto Self::cbor() -> ctx::CborParser& {
@@ -142,7 +142,7 @@ auto Self::cbor() -> ctx::CborParser& {
     cbor_parser_ = std::make_unique<ctx::NullCborParser>();
 #endif
   }
-  DCHECK(cbor_parser_);
+  DCHECK(!!cbor_parser_);
   return *cbor_parser_;
 }
 auto Self::requestor() -> std::shared_ptr<gw::Requestor> {
diff --git a/src/ipfs_client/ctx/null_dns_txt_lookup.cc b/src/ipfs_client/ctx/null_dns_txt_lookup.cc
@@ -8,7 +8,6 @@ using Self = ipfs::ctx::NullDnsTxtLookup;
 void Self::SendDnsTextRequest(std::string /*hostname*/,
                               DnsTextResultsCallback /*unused*/,
                               DnsTextCompleteCallback callback) {
-  LOG(ERROR)
-      << "DNS TXT lookup provider not instantiated. DNSLink will not work.";
+  LOG(ERROR) << "DNS TXT lookup provider not instantiated. DNSLink will not work.";
   callback();
 }
diff --git a/src/ipfs_client/gw/gateway_request.cc b/src/ipfs_client/gw/gateway_request.cc
@@ -278,7 +278,7 @@ auto Self::RespondSuccessfully(std::string_view bytes,
       }
     } break;
     case GatewayRequestType::Car: {
-      DCHECK(api);
+      DCHECK(!!api);
       Car car(as_bytes(bytes), api->cbor());
       AddBlocks(car, api, success, valid, src);
       break;
@@ -369,7 +369,7 @@ void Self::AddBlock(std::string_view bytes,
         success = false;
         return;
     }
-    DCHECK(api);
+    DCHECK(!!api);
     auto node = ipld::DagNode::fromBytes(api, cid().value(), bytes);
     if (!node) {
         success = false;
@@ -405,7 +405,7 @@ void Self::AddBlocks(Car& car, const std::shared_ptr<Client>& api, bool& success
 }
 auto Self::IpnsResponse(ByteView bytes, std::shared_ptr<Client> const& api, bool& success, bool* valid, ipld::BlockSource src) -> bool {
   if (cid().has_value()) {
-    DCHECK(api);
+    DCHECK(!!api);
     auto rec = ipfs::ValidateIpnsRecord(bytes, cid().value(), *api);
     if (rec.has_value()) {
       ValidatedIpns const validated{rec.value()};
diff --git a/src/ipfs_client/gw/gateway_state.cc b/src/ipfs_client/gw/gateway_state.cc
@@ -106,10 +106,10 @@ void Self::timed_out() {
 }
 // NOLINTEND(readability-magic-numbers)
 auto Self::cfg() -> ctx::GatewayConfig& {
-  DCHECK(api_);
+  DCHECK(!!api_);
   return api_->gw_cfg();
 }
 auto Self::cfg() const -> ctx::GatewayConfig const& {
-  DCHECK(api_);
+  DCHECK(!!api_);
   return api_->gw_cfg();
 }
diff --git a/src/ipfs_client/ipns_names_unittest.cc b/src/ipfs_client/ipns_names_unittest.cc
@@ -1,13 +1,12 @@
 #include <ipfs_client/ipns_names.h>
 
-#include <google/protobuf/stubs/logging.h>
 #include <gtest/gtest.h>
 
 namespace {
 struct IpnsNamesTest : public ::testing::Test {
   IpnsNamesTest();
   ipfs::IpnsNames tested;
-  google::protobuf::LogSilencer no_log;
+  // google::protobuf::LogSilencer no_log;
   void fake(std::string n, std::string t);
 };
 }  // namespace
diff --git a/src/ipfs_client/ipns_record_unittest.cc b/src/ipfs_client/ipns_record_unittest.cc
@@ -74,12 +74,6 @@ struct Api final : public i::Client {
   void AddGateway(std::string_view) {}
   virtual ~Api() noexcept {}
 };
-  std::vector<std::string> log_lines;
-  std::string all_log;
-  void save_log(std::string const& msg, char const*, int, il::Level) {
-    log_lines.push_back(msg);
-    all_log.append(msg).push_back(';');
-  }
 }  // namespace
 
 TEST(IpnsRecordTest, AKnownKuboRecord) {
@@ -432,13 +426,17 @@ TEST(IpnsRecordTest, ValueMismatch) {
   auto real_validates = ipfs::ValidateIpnsRecord(i::as_bytes(xxd), cid, api);
   EXPECT_TRUE(real_validates);
   xxd[12]++;//cause a mismatch
-  il::SetLevel(il::Level::Error);
-  il::SetHandler(save_log);
+  //il::SetLevel(il::Level::Error);
+  //il::SetHandler(save_log);
+  LogRecorder rec;
   auto modified_validates = ipfs::ValidateIpnsRecord(i::as_bytes(xxd), cid, api);
-  EXPECT_FALSE(modified_validates) << all_log;
-  il::SetHandler(il::DefaultHandler);
-  EXPECT_EQ(log_lines.size(),1UL) << all_log;
-  auto& log_line = log_lines.at(0);
-  EXPECT_LT(log_line.find("Mismatch on Value"),log_line.size()) << all_log;
+  EXPECT_FALSE(modified_validates);
+  //il::SetHandler(il::DefaultHandler);
+  //EXPECT_EQ(log_lines.size(),1UL);
+  //auto& log_line = log_lines.at(0);
+  //EXPECT_LT(log_line.find("Mismatch on Value"),log_line.size()) << all_log;
+  auto is_expected = [](auto&m){return m.message.find("Mismatch on Value") < m.message.size();};
+  auto expected_log = std::count_if(rec.messages.begin(), rec.messages.end(), is_expected);
+  EXPECT_EQ(expected_log, 1);
 }
 #endif
diff --git a/src/ipfs_client/logger.cc b/src/ipfs_client/logger.cc
@@ -1,51 +1,29 @@
 #include <ipfs_client/logger.h>
 
-#include <google/protobuf/stubs/logging.h>
+#undef LOGURU_WITH_STREAMS
+#define LOGURU_WITH_STREAMS 1
+#include <loguru.hpp>
 
 #include <iostream>
+#include <map>
 #include <string>
 #include <string_view>
 
 namespace lg = ipfs::log;
 
 namespace {
-lg::Level current_level = lg::Level::Warn;
-lg::Handler current_handler = nullptr;
-
-void CheckLevel(google::protobuf::LogLevel lv,
-                char const* f,
-                int l,
-                std::string const& m) {
-  auto lev = static_cast<int>(lv);
-  if (lev < static_cast<int>(current_level)) {
-    return;
-  }
-  if (current_handler == nullptr) {
-    return;
-  }
-  current_handler(m, f, l, static_cast<lg::Level>(lev));
+auto to_lev(int i) {
+  i = std::max(-i, -2);
+  i = std::min( i,  4);
+  return static_cast<lg::Level>(i);
 }
-}  // namespace
-
-void lg::SetLevel(Level lev) {
-  IsInitialized();
-  current_level = lev;
 }
 
-void lg::SetHandler(Handler h) noexcept {
-  current_handler = h;
-  google::protobuf::SetLogHandler(&CheckLevel);
+auto lg::GetLevel() -> Level {
+  return to_lev(loguru::g_stderr_verbosity);
 }
-
-void lg::DefaultHandler(std::string const& message,
-                        char const* source_file,
-                        int source_line,
-                        Level lev) {
-  std::clog << source_file << ':' << source_line << ": " << LevelDescriptor(lev)
-            << ": " << message << '\n';
-  if (lev == Level::Fatal) {
-    std::abort();
-  }
+void lg::SetLevel(Level lev) {
+  loguru::g_stderr_verbosity = -static_cast<int>(lev);
 }
 
 auto lg::LevelDescriptor(Level l) -> std::string_view {
@@ -68,11 +46,20 @@ auto lg::LevelDescriptor(Level l) -> std::string_view {
       return "Unknown log level used: possible corruption?";
   }
 }
-
-auto lg::IsInitialized() noexcept -> bool {
-  if (current_handler != nullptr) {
-    return true;
-  }
-  SetHandler(&DefaultHandler);
-  return false;
+namespace {
+void adapt_hook(void* user_data, loguru::Message const& mes) {
+  auto* h = reinterpret_cast<lg::Hook*>(user_data);
+  auto lev = to_lev(mes.verbosity);
+  (*h)(mes.message, mes.filename, mes.line, lev);
+}
+std::map<std::string,lg::Hook> hook_storage;
+}
+void lg::AddHook(std::string id, Hook hook) {
+  auto& h = hook_storage[id];
+  h = hook;
+  loguru::add_callback(id.c_str(), adapt_hook, &h, loguru::NamedVerbosity::Verbosity_MAX);
+}
+void lg::Unhook(std::string id) {
+  hook_storage.erase(id);
+  loguru::remove_callback(id.c_str());
 }
diff --git a/src/ipfs_client/multicodec.cc b/src/ipfs_client/multicodec.cc
@@ -1,6 +1,8 @@
 #include <ipfs_client/multicodec.h>
 #include <string_view>
 
+#include <iostream>
+
 using Cdc = ipfs::MultiCodec;
 
 namespace {
@@ -32,3 +34,9 @@ auto ipfs::Validate(Cdc c) -> Cdc {
   }
   return c;
 }
+
+namespace std {
+auto operator<<(ostream& s, ipfs::MultiCodec c) -> ostream& {
+  return s << GetName(c);
+}
+}
diff --git a/src/ipfs_client/partition.cc b/src/ipfs_client/partition.cc
@@ -24,7 +24,7 @@ using Self = ipfs::Partition;
 Self::Partition(std::shared_ptr<gw::Requestor> requestor,
                 std::shared_ptr<Client> api)
     : api_{std::move(api)}, requestor_{requestor} {
-  DCHECK(requestor);
+  DCHECK(!!requestor);
 }
 
 void Self::build_response(std::shared_ptr<IpfsRequest> req) {
diff --git a/src/log_macros.h b/src/log_macros.h
@@ -8,34 +8,20 @@
 #include "base/logging.h"
 #include "base/check_op.h"
 
-#elif __has_include(<glog/logging.h>)
+#elif __has_include(<loguru.hpp>)
 
-#include <glog/logging.h>
+#undef LOGURU_WITH_STREAMS
+#define LOGURU_WITH_STREAMS 1
+#include <loguru.hpp>
 
-#else
-
-#include <ipfs_client/logger.h>
-
-#include <google/protobuf/stubs/logging.h>
+#define DCHECK(X) DCHECK_S( X )
+#define DCHECK_EQ(X, Y) DCHECK_EQ_S( X , Y )
+#define LOG(X) LOG_S( X )
+#define VLOG(X) VLOG_S( X )
 
-#define DCHECK_EQ GOOGLE_DCHECK_EQ
-#define DCHECK_GT GOOGLE_DCHECK_GT
-#define DCHECK GOOGLE_DCHECK
-#define LOG GOOGLE_LOG
-
-#define VLOG(X)                                       \
-  ::google::protobuf::internal::LogFinisher() =      \
-      ::google::protobuf::internal::LogMessage(       \
-          static_cast<::google::protobuf::LogLevel>(  \
-              ::google::protobuf::LOGLEVEL_INFO - ( X ) ), \
-          __FILE__, __LINE__)
+#else
 
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-variable"
-namespace {
-static bool const is_logging_initialized = ::ipfs::log::IsInitialized();
-} // namespace
-#pragma GCC diagnostic pop
+#error "Provide logging."
 
 #endif //Chromium in-tree check
 
diff --git a/src/vocab/slash_delimited.cc b/src/vocab/slash_delimited.cc
diff --git a/test_data/include/log_recorder.h b/test_data/include/log_recorder.h

Original file line number	Diff line number	Diff line change
`@@ -130,7 +130,7 @@ auto Self::json() -> ctx::JsonParser& {`
`130`	`130`	`LOG(FATAL) << "A JSON parser must be provided.";`
`131`	`131`	`#endif`
`132`	`132`	`}`
`133`		`- DCHECK(json_parser_);`
	`133`	`+ DCHECK(!!json_parser_);`
`134`	`134`	`return *json_parser_;`
`135`	`135`	`}`
`136`	`136`	`auto Self::cbor() -> ctx::CborParser& {`
`@@ -142,7 +142,7 @@ auto Self::cbor() -> ctx::CborParser& {`
`142`	`142`	`cbor_parser_ = std::make_unique<ctx::NullCborParser>();`
`143`	`143`	`#endif`
`144`	`144`	`}`
`145`		`- DCHECK(cbor_parser_);`
	`145`	`+ DCHECK(!!cbor_parser_);`
`146`	`146`	`return *cbor_parser_;`
`147`	`147`	`}`
`148`	`148`	`auto Self::requestor() -> std::shared_ptr<gw::Requestor> {`
Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,6 @@ using Self = ipfs::ctx::NullDnsTxtLookup;`
`8`	`8`	`void Self::SendDnsTextRequest(std::string /hostname/,`
`9`	`9`	`DnsTextResultsCallback /unused/,`
`10`	`10`	`DnsTextCompleteCallback callback) {`
`11`		`- LOG(ERROR)`
`12`		`- << "DNS TXT lookup provider not instantiated. DNSLink will not work.";`
	`11`	`+ LOG(ERROR) << "DNS TXT lookup provider not instantiated. DNSLink will not work.";`
`13`	`12`	`callback();`
`14`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -106,10 +106,10 @@ void Self::timed_out() {`
`106`	`106`	`}`
`107`	`107`	`// NOLINTEND(readability-magic-numbers)`
`108`	`108`	`auto Self::cfg() -> ctx::GatewayConfig& {`
`109`		`- DCHECK(api_);`
	`109`	`+ DCHECK(!!api_);`
`110`	`110`	`return api_->gw_cfg();`
`111`	`111`	`}`
`112`	`112`	`auto Self::cfg() const -> ctx::GatewayConfig const& {`
`113`		`- DCHECK(api_);`
	`113`	`+ DCHECK(!!api_);`
`114`	`114`	`return api_->gw_cfg();`
`115`	`115`	`}`