From 35813625eef842dd7f1968cbf4b912b76eb34f6a Mon Sep 17 00:00:00 2001
From: Brian Coutinho <bcoutinho@meta.com>
Date: Fri, 26 Apr 2024 11:39:40 -0700
Subject: [PATCH] Make ipcfabric have a single definition

Summary:
There is an ODR one definition rule violation that was causing a crash on sigrid
https://fb.workplace.com/groups/560979627394613/posts/2909061125919773/?comment_id=2909752389183980&reply_comment_id=2910102119149007
Sigrid includes both kineto and ipcfabric via dynolog, and on kineto the class is fused in internal. Also adding Logger.h caused a mess.

Tries to resolve this issue
Note ENABLE_IPC_FABRIC define is only used in open source kineto (and PyTorch) but not in internal fbcode

Differential Revision: D56577485
---
 dynolog/src/ipcfabric/CMakeLists.txt      |  1 -
 dynolog/src/ipcfabric/FabricManager.h     | 43 +++--------------------
 dynolog/src/tracing/IPCMonitor.h          |  4 +++
 dynolog/tests/ipcfabric/IPCFabricTest.cpp |  3 ++
 dynolog/tests/ipcfabric/IPCSender.cpp     |  1 +
 5 files changed, 13 insertions(+), 39 deletions(-)

diff --git a/dynolog/src/ipcfabric/CMakeLists.txt b/dynolog/src/ipcfabric/CMakeLists.txt
index 14b47bd2..09679b90 100644
--- a/dynolog/src/ipcfabric/CMakeLists.txt
+++ b/dynolog/src/ipcfabric/CMakeLists.txt
@@ -1,4 +1,3 @@
 # Copyright (c) Meta Platforms, Inc. and affiliates.
 
 add_library (dynolog_ipcfabric_lib INTERFACE)
-target_compile_options(dynolog_ipcfabric_lib INTERFACE "-DENABLE_IPC_FABRIC")
diff --git a/dynolog/src/ipcfabric/FabricManager.h b/dynolog/src/ipcfabric/FabricManager.h
index 2fddc4cd..0f328609 100644
--- a/dynolog/src/ipcfabric/FabricManager.h
+++ b/dynolog/src/ipcfabric/FabricManager.h
@@ -13,17 +13,13 @@
 #include "dynolog/src/ipcfabric/Utils.h"
 
 // If building inside Kineto, use its logger, otherwise use glog
-#if defined(KINETO_NAMESPACE) && defined(ENABLE_IPC_FABRIC)
-// We need to include the Logger header here for LOG() macros.
+#if defined USE_GOOGLE_LOG
+// We need to include the Logger header before here for LOG() macros.
 // However this can alias with other files that include this and
-// also use glog. TODO(T131440833). Thus, the user should also set
-#include "Logger.h" // @manual
-// add to namespace to get logger
-using namespace libkineto;
-
-#else // KINETO_NAMESPACE && ENABLE_IPC_FABRIC
+// also use glog. TODO(T131440833).
+// Whoever includes this needs to also include Logger.h for use in kineto
 #include <glog/logging.h>
-#endif // KINETO_NAMESPACE && ENABLE_IPC_FABRIC
+#endif // USE_GOOGLE_LOG
 
 namespace dynolog::ipcfabric {
 
@@ -90,7 +86,6 @@ struct Message {
   std::string src;
 };
 
-#if !defined(KINETO_NAMESPACE) || defined(ENABLE_IPC_FABRIC)
 class FabricManager {
  public:
   FabricManager(const FabricManager&) = delete;
@@ -222,32 +217,4 @@ class FabricManager {
   std::mutex dequeLock_;
 };
 
-#else
-
-// Adds an empty implementation so compilation works.
-class FabricManager {
- public:
-  FabricManager(const FabricManager&) = delete;
-  FabricManager& operator=(const FabricManager&) = delete;
-
-  static std::unique_ptr<FabricManager> factory(
-      std::string endpoint_name = "") {
-    return nullptr;
-  }
-
-  bool sync_send(
-      const Message& msg,
-      const std::string& dest_name,
-      int num_retries = 10,
-      int sleep_time_us = 10000) {
-    return false;
-  }
-
-  std::unique_ptr<Message> poll_recv(int max_retries, int sleep_time_us) {
-    return nullptr;
-  }
-};
-
-#endif // !KINETO_NAMESPACE || ENABLE_IPC_FABRIC
-
 } // namespace dynolog::ipcfabric
diff --git a/dynolog/src/tracing/IPCMonitor.h b/dynolog/src/tracing/IPCMonitor.h
index 2ce6b8a6..25d10e2e 100644
--- a/dynolog/src/tracing/IPCMonitor.h
+++ b/dynolog/src/tracing/IPCMonitor.h
@@ -6,6 +6,10 @@
 #pragma once
 
 #include <memory>
+
+// Use glog for FabricManager.h
+#define USE_GOOGLE_LOG
+
 #include "dynolog/src/ipcfabric/FabricManager.h"
 
 namespace dynolog {
diff --git a/dynolog/tests/ipcfabric/IPCFabricTest.cpp b/dynolog/tests/ipcfabric/IPCFabricTest.cpp
index 89ddf2ab..226bf8b3 100644
--- a/dynolog/tests/ipcfabric/IPCFabricTest.cpp
+++ b/dynolog/tests/ipcfabric/IPCFabricTest.cpp
@@ -3,6 +3,9 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the root directory of this source tree.
 
+// Use glog for FabricManager.h
+#define USE_GOOGLE_LOG
+
 #include <gtest/gtest.h>
 #include <linux/perf_event.h>
 #include <sys/ioctl.h>
diff --git a/dynolog/tests/ipcfabric/IPCSender.cpp b/dynolog/tests/ipcfabric/IPCSender.cpp
index 61046f6d..2d57cb60 100644
--- a/dynolog/tests/ipcfabric/IPCSender.cpp
+++ b/dynolog/tests/ipcfabric/IPCSender.cpp
@@ -3,6 +3,7 @@
 // This source code is licensed under the MIT license found in the
 // LICENSE file in the root directory of this source tree.
 
+#define USE_GOOGLE_LOG
 #include "dynolog/src/ipcfabric/FabricManager.h"
 
 int main() {