Add LogSink & LogMessage classes for a more robust sink solution.

Signed-off-by: Sean Luchen <[email protected]>

Author: Logikable

.github/actions/setup-build-env/action.yml

@@ -8,7 +8,7 @@ runs:
       shell: bash
       run: |
         sudo apt-get update
-        sudo apt-get install gperf build-essential bison flex libfl-dev libreadline-dev gawk tcl-dev libffi-dev git graphviz xdot pkg-config python3 libboost-system-dev libboost-python-dev libboost-filesystem-dev zlib1g-dev libbz2-dev libgtest-dev
+        sudo apt-get install gperf build-essential bison flex libfl-dev libreadline-dev gawk tcl-dev libffi-dev git graphviz xdot pkg-config python3 libboost-system-dev libboost-python-dev libboost-filesystem-dev zlib1g-dev libbz2-dev libgtest-dev libgmock-dev
 
     - name: Install macOS Dependencies
       if: runner.os == 'macOS'

kernel/log.cc

@@ -39,7 +39,8 @@
 YOSYS_NAMESPACE_BEGIN
 
 std::vector<FILE*> log_files;
-std::vector<std::ostream*> log_streams, log_warning_streams;
+std::vector<std::ostream*> log_streams;
+std::vector<LogSink*> log_sinks;
 std::vector<std::string> log_scratchpads;
 std::map<std::string, std::set<std::string>> log_hdump;
 std::vector<std::regex> log_warn_regexes, log_nowarn_regexes, log_werror_regexes;
@@ -156,10 +157,6 @@ static void logv_string(std::string_view format, std::string str, LogSeverity se
 
 		for (auto f : log_streams)
 			*f << time_str;
-
-		if (severity >= LogSeverity::LOG_WARNING)
-			for (auto f : log_warning_streams)
-				*f << time_str;
 	}
 
 	for (auto f : log_files)
@@ -168,11 +165,9 @@ static void logv_string(std::string_view format, std::string str, LogSeverity se
 	for (auto f : log_streams)
 		*f << str;
 
-	if (severity >= LogSeverity::LOG_WARNING)
-		for (auto f : log_warning_streams) {
-			*f << str;
-			f->flush();
-		}
+	LogMessage log_msg(severity, str);
+	for (LogSink* sink : log_sinks)
+		sink->log(log_msg);
 
 	RTLIL::Design *design = yosys_get_design();
 	if (design != nullptr)

kernel/log.h

@@ -100,9 +100,22 @@ enum LogSeverity {
 	LOG_ERROR
 };
 
+struct LogMessage {
+	LogMessage(LogSeverity severity, std::string_view message) :
+		severity(severity), timestamp(std::time(nullptr)), message(message) {}
+	LogSeverity severity;
+	std::time_t timestamp;
+	std::string message;
+};
+
+class LogSink {
+ public:
+	virtual void log(const LogMessage& message) = 0;
+};
+
 extern std::vector<FILE*> log_files;
 extern std::vector<std::ostream*> log_streams;
-extern std::vector<std::ostream*> log_warning_streams;
+extern std::vector<LogSink*> log_sinks;
 extern std::vector<std::string> log_scratchpads;
 extern std::map<std::string, std::set<std::string>> log_hdump;
 extern std::vector<std::regex> log_warn_regexes, log_nowarn_regexes, log_werror_regexes;

tests/unit/Makefile

@@ -4,10 +4,10 @@ UNAME_S := $(shell uname -s)
 GTEST_PREFIX := $(shell brew --prefix googletest 2>/dev/null)
 ifeq ($(GTEST_PREFIX),)
   GTEST_CXXFLAGS :=
-  GTEST_LDFLAGS := -lgtest -lgtest_main
+  GTEST_LDFLAGS := -lgtest -lgtest_main -lgmock
 else
   GTEST_CXXFLAGS := -I$(GTEST_PREFIX)/include
-  GTEST_LDFLAGS := -L$(GTEST_PREFIX)/lib -lgtest -lgtest_main
+  GTEST_LDFLAGS := -L$(GTEST_PREFIX)/lib -lgtest -lgtest_main -lgmock
 endif
 
 ifeq ($(UNAME_S),Darwin)

tests/unit/kernel/logTest.cc

@@ -1,3 +1,4 @@
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 #include "kernel/yosys.h"
@@ -11,4 +12,38 @@ TEST(KernelLogTest, logvValidValues)
 	EXPECT_EQ(7, 7);
 }
 
+class TestSink : public LogSink {
+ public:
+	void log(const LogMessage& message) override {
+		messages_.push_back(message);
+	}
+	std::vector<LogMessage> messages_;
+};
+
+TEST(KernelLogTest, logToSink)
+{
+	TestSink sink;
+	log_sinks.push_back(&sink);
+	log("test info log");
+	log_warning("test warning log");
+
+	std::vector<LogMessage> expected{
+		LogMessage(LogSeverity::LOG_INFO, "test info log"),
+		LogMessage(LogSeverity::LOG_WARNING, "test warning log"),
+	};
+	// Certain calls to the log.h interface may prepend a string to
+	// the provided string. We should ensure that the expected string
+	// is a subset of the actual string. Additionally, we don't want to
+	// compare timestamps. So, we use a custom comparator.
+	for (const LogMessage& expected_msg : expected) {
+		EXPECT_THAT(sink.messages_, ::testing::Contains(::testing::Truly(
+			[&](const LogMessage& actual) {
+				return actual.severity == expected_msg.severity &&
+					actual.message.find(expected_msg.message) != std::string::npos;
+			}
+		)));
+	}
+	EXPECT_NE(sink.messages_[0].timestamp, 0);
+}
+
 YOSYS_NAMESPACE_END