chore: consolidate gRPC and protobuf linking into a dedicated support library

- Introduced a new `yaze_grpc_support` library to centralize all gRPC and protobuf usage, addressing Windows linker errors and improving build stability. - Updated CMake configurations across various components to link against the new support library instead of individual protobuf targets, simplifying the linking process. - Removed legacy whole-archive linking logic, ensuring a cleaner and more maintainable build setup. Benefits: - Reduces complexity in CMake files and enhances compatibility across platforms. - Prevents potential linker errors by consolidating gRPC and protobuf dependencies into a single library.
2025-10-18 15:58:58 -04:00
parent 8e86c1bbdf
commit 6db7ba4782
15 changed files with 152 additions and 170 deletions
--- a/src/app/app.cmake
+++ b/src/app/app.cmake
@@ -108,30 +108,8 @@ if(YAZE_WITH_GRPC)
    ${CMAKE_SOURCE_DIR}/third_party/json/include)
  target_compile_definitions(yaze_app_core_lib PRIVATE YAZE_WITH_JSON)

-  # Add proto definitions for ROM service, canvas automation, and test harness
-  # Test harness proto is needed because widget_discovery_service.h includes it
-  target_add_protobuf(yaze_app_core_lib
-    ${PROJECT_SOURCE_DIR}/src/protos/rom_service.proto)
-  target_add_protobuf(yaze_app_core_lib
-    ${PROJECT_SOURCE_DIR}/src/protos/canvas_automation.proto)
-  target_add_protobuf(yaze_app_core_lib
-    ${PROJECT_SOURCE_DIR}/src/protos/imgui_test_harness.proto)
-
-  # Add unified gRPC server (non-test services only)
-  target_sources(yaze_app_core_lib PRIVATE
-    ${CMAKE_SOURCE_DIR}/src/app/service/unified_grpc_server.cc
-    ${CMAKE_SOURCE_DIR}/src/app/service/unified_grpc_server.h
-  )
-
-  target_link_libraries(yaze_app_core_lib PUBLIC
-    grpc++
-    grpc++_reflection
-  )
-  # NOTE: Do NOT link protobuf at library level on Windows - causes LNK1241
-  # Executables will link it with /WHOLEARCHIVE to include internal symbols
-  if(NOT WIN32 AND YAZE_PROTOBUF_TARGETS)
-    target_link_libraries(yaze_app_core_lib PUBLIC ${YAZE_PROTOBUF_TARGETS})
-  endif()
+  # Link to consolidated gRPC support library
+  target_link_libraries(yaze_app_core_lib PUBLIC yaze_grpc_support)
  
  message(STATUS "  - gRPC ROM service + canvas automation enabled")
 endif()
@@ -202,22 +180,7 @@ target_link_libraries(yaze PRIVATE
  absl::flags
  absl::flags_parse
 )
-if(YAZE_WITH_GRPC AND YAZE_PROTOBUF_TARGETS)
-  # On Windows: Use /WHOLEARCHIVE instead of normal linking to include internal symbols
-  # On Unix: Use normal linking (symbols resolve correctly without whole-archive)
-  if(MSVC AND YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS)
-    foreach(_yaze_proto_target IN LISTS YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS)
-      if(TARGET ${_yaze_proto_target})
-        get_target_property(_target_type ${_yaze_proto_target} TYPE)
-        if(_target_type MATCHES ".*_LIBRARY")
-          target_link_options(yaze PRIVATE /WHOLEARCHIVE:$<TARGET_FILE:${_yaze_proto_target}>)
-        endif()
-      endif()
-    endforeach()
-  else()
-    target_link_libraries(yaze PRIVATE ${YAZE_PROTOBUF_TARGETS})
-  endif()
-endif()
+# gRPC/protobuf linking is now handled by yaze_grpc_support library

 # Link test support library (yaze_editor needs TestManager)
 if(TARGET yaze_test_support)
--- a/src/app/editor/editor_library.cmake
+++ b/src/app/editor/editor_library.cmake
@@ -163,15 +163,7 @@ endif()

 # Conditionally link gRPC if enabled
 if(YAZE_WITH_GRPC)
-  target_link_libraries(yaze_editor PRIVATE
-    grpc++
-    grpc++_reflection
-  )
-  # NOTE: Do NOT link protobuf at library level on Windows - causes LNK1241
-  # Executables will link it with /WHOLEARCHIVE to include internal symbols
-  if(NOT WIN32 AND YAZE_PROTOBUF_TARGETS)
-    target_link_libraries(yaze_editor PRIVATE ${YAZE_PROTOBUF_TARGETS})
-  endif()
+  target_link_libraries(yaze_editor PUBLIC yaze_grpc_support)
 endif()

 set_target_properties(yaze_editor PROPERTIES
--- a/src/app/emu/emu.cmake
+++ b/src/app/emu/emu.cmake
@@ -34,19 +34,7 @@ if(YAZE_BUILD_EMU AND NOT YAZE_MINIMAL_BUILD)
    message(WARNING "yaze_emu needs yaze_test_support but TARGET not found")
  endif()

-  # Windows: Link protobuf with /WHOLEARCHIVE (not via libraries to avoid LNK1241)
-  if(WIN32 AND MSVC AND YAZE_WITH_GRPC AND YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS)
-    foreach(_yaze_proto_target IN LISTS YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS)
-      if(TARGET ${_yaze_proto_target})
-        get_target_property(_target_type ${_yaze_proto_target} TYPE)
-        if(_target_type MATCHES ".*_LIBRARY")
-          target_link_options(yaze_emu PRIVATE /WHOLEARCHIVE:$<TARGET_FILE:${_yaze_proto_target}>)
-        endif()
-      endif()
-    endforeach()
-  elseif(YAZE_WITH_GRPC AND YAZE_PROTOBUF_TARGETS)
-    target_link_libraries(yaze_emu PRIVATE ${YAZE_PROTOBUF_TARGETS})
-  endif()
+  # gRPC/protobuf linking is now handled by yaze_grpc_support library

  # Test engine is always available when tests are built
  # No need for conditional definitions
@@ -68,19 +56,7 @@ if(YAZE_BUILD_EMU AND NOT YAZE_MINIMAL_BUILD)
    absl::str_format
  )
  
-  # Windows: Link protobuf with /WHOLEARCHIVE (not via libraries to avoid LNK1241)
-  if(WIN32 AND MSVC AND YAZE_WITH_GRPC AND YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS)
-    foreach(_yaze_proto_target IN LISTS YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS)
-      if(TARGET ${_yaze_proto_target})
-        get_target_property(_target_type ${_yaze_proto_target} TYPE)
-        if(_target_type MATCHES ".*_LIBRARY")
-          target_link_options(yaze_emu_test PRIVATE /WHOLEARCHIVE:$<TARGET_FILE:${_yaze_proto_target}>)
-        endif()
-      endif()
-    endforeach()
-  elseif(YAZE_WITH_GRPC AND YAZE_PROTOBUF_TARGETS)
-    target_link_libraries(yaze_emu_test PRIVATE ${YAZE_PROTOBUF_TARGETS})
-  endif()
+  # gRPC/protobuf linking is now handled by yaze_grpc_support library
  
  message(STATUS "✓ yaze_emu_test: Headless emulator test harness configured")
  message(STATUS "✓ yaze_emu: Standalone emulator executable configured")
--- a/src/app/net/net_library.cmake
+++ b/src/app/net/net_library.cmake
@@ -78,18 +78,7 @@ endif()

 # Add gRPC support for ROM service
 if(YAZE_WITH_GRPC)
-  target_add_protobuf(yaze_net ${PROJECT_SOURCE_DIR}/src/protos/rom_service.proto)
-  
-  target_link_libraries(yaze_net PUBLIC
-    grpc++
-    grpc++_reflection
-  )
-  # NOTE: Do NOT link protobuf at library level on Windows - causes LNK1241
-  # Executables will link it with /WHOLEARCHIVE to include internal symbols
-  if(NOT WIN32 AND YAZE_PROTOBUF_TARGETS)
-    target_link_libraries(yaze_net PUBLIC ${YAZE_PROTOBUF_TARGETS})
-  endif()
-  
+  target_link_libraries(yaze_net PUBLIC yaze_grpc_support)
  message(STATUS "  - gRPC ROM service enabled")
 endif()

--- a/src/app/service/grpc_support.cmake
+++ b/src/app/service/grpc_support.cmake
@@ -0,0 +1,97 @@
+# ==============================================================================
+# Yaze gRPC Support Library
+# ==============================================================================
+# This library consolidates ALL gRPC/protobuf usage to eliminate Windows
+# linker errors (LNK1241, LNK2005). All protobuf definitions and gRPC
+# service implementations are contained here, with other libraries linking
+# to this single source of truth.
+#
+# Dependencies: yaze_util, yaze_common, yaze_app_core_lib, yaze_zelda3, yaze_gfx, yaze_gui
+# ==============================================================================
+
+set(
+  YAZE_GRPC_SOURCES
+  # Core gRPC services
+  app/service/unified_grpc_server.cc
+  app/service/canvas_automation_service.cc
+  app/service/imgui_test_harness_service.cc
+  app/service/widget_discovery_service.cc
+  app/service/screenshot_utils.cc
+  
+  # Test infrastructure
+  app/test/test_recorder.cc
+  app/test/test_script_parser.cc
+  
+  # CLI agent gRPC client code (only files that actually exist)
+  cli/service/planning/tile16_proposal_generator.cc
+  cli/service/gui/gui_automation_client.cc
+)
+
+add_library(yaze_grpc_support STATIC ${YAZE_GRPC_SOURCES})
+
+target_precompile_headers(yaze_grpc_support PRIVATE
+  "$<$<COMPILE_LANGUAGE:CXX>:${CMAKE_SOURCE_DIR}/src/yaze_pch.h>"
+)
+
+target_include_directories(yaze_grpc_support PUBLIC
+  ${CMAKE_SOURCE_DIR}/src
+  ${CMAKE_SOURCE_DIR}/src/app
+  ${CMAKE_SOURCE_DIR}/src/lib
+  ${CMAKE_SOURCE_DIR}/src/lib/imgui
+  ${CMAKE_SOURCE_DIR}/src/lib/imgui_test_engine
+  ${CMAKE_SOURCE_DIR}/incl
+  ${SDL2_INCLUDE_DIR}
+  ${PROJECT_BINARY_DIR}
+)
+
+# Link all required yaze libraries
+target_link_libraries(yaze_grpc_support PUBLIC
+  yaze_app_core_lib
+  yaze_util
+  yaze_common
+  yaze_zelda3
+  yaze_gfx
+  yaze_gui
+  yaze_emulator
+  ${ABSL_TARGETS}
+  ${SDL_TARGETS}
+)
+
+# Add JSON support
+if(YAZE_WITH_JSON)
+  target_include_directories(yaze_grpc_support PUBLIC
+    ${CMAKE_SOURCE_DIR}/third_party/json/include)
+  target_compile_definitions(yaze_grpc_support PUBLIC YAZE_WITH_JSON)
+endif()
+
+# Add ALL protobuf definitions (consolidated from multiple libraries)
+target_add_protobuf(yaze_grpc_support
+  ${PROJECT_SOURCE_DIR}/src/protos/rom_service.proto
+  ${PROJECT_SOURCE_DIR}/src/protos/canvas_automation.proto
+  ${PROJECT_SOURCE_DIR}/src/protos/imgui_test_harness.proto
+  ${PROJECT_SOURCE_DIR}/src/protos/emulator_service.proto
+)
+
+# Link gRPC and protobuf libraries (single point of linking)
+target_link_libraries(yaze_grpc_support PUBLIC
+  grpc++
+  grpc++_reflection
+  ${YAZE_PROTOBUF_TARGETS}
+)
+
+set_target_properties(yaze_grpc_support PROPERTIES
+  POSITION_INDEPENDENT_CODE ON
+  ARCHIVE_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib"
+  LIBRARY_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/lib"
+)
+
+# Platform-specific compile definitions
+if(UNIX AND NOT APPLE)
+  target_compile_definitions(yaze_grpc_support PRIVATE linux stricmp=strcasecmp)
+elseif(APPLE)
+  target_compile_definitions(yaze_grpc_support PRIVATE MACOS)
+elseif(WIN32)
+  target_compile_definitions(yaze_grpc_support PRIVATE WINDOWS)
+endif()
+
+message(STATUS "✓ yaze_grpc_support library configured (consolidated gRPC/protobuf)")
--- a/src/app/test/test.cmake
+++ b/src/app/test/test.cmake
@@ -15,16 +15,7 @@ set(YAZE_TEST_SOURCES
  app/test/z3ed_test_suite.cc
 )

-# Add gRPC test harness services if enabled (depend on TestManager)
-if(YAZE_WITH_GRPC)
-  list(APPEND YAZE_TEST_SOURCES
-    app/service/imgui_test_harness_service.cc
-    app/service/screenshot_utils.cc
-    app/service/widget_discovery_service.cc
-    app/test/test_recorder.cc
-    app/test/test_script_parser.cc
-  )
-endif()
+# gRPC test harness services are now in yaze_grpc_support library

 add_library(yaze_test_support STATIC ${YAZE_TEST_SOURCES})

@@ -55,14 +46,8 @@ if(YAZE_WITH_GRPC)
    ${CMAKE_SOURCE_DIR}/third_party/json/include)
  target_compile_definitions(yaze_test_support PRIVATE YAZE_WITH_JSON)
  
-  # Add test harness proto definition
-  target_add_protobuf(yaze_test_support
-    ${PROJECT_SOURCE_DIR}/src/protos/imgui_test_harness.proto)
-  
-  target_link_libraries(yaze_test_support PUBLIC
-    grpc++
-    grpc++_reflection
-  )
+  # Link to consolidated gRPC support library
+  target_link_libraries(yaze_test_support PUBLIC yaze_grpc_support)
  
  message(STATUS "  - gRPC test harness service enabled in yaze_test_support")
 endif()