From 51cf1681e52bf6a8c53e8734fa819b83d601d0ab Mon Sep 17 00:00:00 2001 From: scawful Date: Sat, 18 Oct 2025 12:01:12 -0400 Subject: [PATCH] chore: update protobuf linking strategy for Windows - Adjusted CMake files to conditionally link protobuf using /WHOLEARCHIVE on Windows to include internal symbols while avoiding LNK1241 errors. - Updated linking logic for `yaze`, `yaze_editor`, `yaze_emu`, `z3ed`, and test suites to ensure compatibility across platforms. Benefits: - Improves build stability and compatibility for Windows users by ensuring proper symbol inclusion without resource duplication issues. --- .gitignore | 4 ++-- src/app/app.cmake | 12 +++++++----- src/app/editor/editor_library.cmake | 5 +++-- src/app/editor/overworld/overworld_editor.cc | 2 -- src/app/emu/emu.cmake | 8 ++++++-- src/app/net/net_library.cmake | 5 +++-- src/cli/agent.cmake | 5 +++-- src/cli/z3ed.cmake | 7 ++++--- test/CMakeLists.txt | 13 +++++++------ tools/test_helpers/CMakeLists.txt | 15 +++++++++------ 10 files changed, 44 insertions(+), 32 deletions(-) diff --git a/.gitignore b/.gitignore index 93c7b9b1..0fe59420 100644 --- a/.gitignore +++ b/.gitignore @@ -14,7 +14,6 @@ compile_commands.json CPackConfig.cmake CPackSourceConfig.cmake CTestTestfile.cmake -Testing/ # Build artifacts *.o @@ -92,4 +91,5 @@ recent_files.txt .vs/* .gitignore .genkit -.claude \ No newline at end of file +.claude +scripts/__pycache__/ diff --git a/src/app/app.cmake b/src/app/app.cmake index 0a93b98b..7588a1a0 100644 --- a/src/app/app.cmake +++ b/src/app/app.cmake @@ -127,9 +127,10 @@ if(YAZE_WITH_GRPC) grpc++ grpc++_reflection ) - if(YAZE_PROTOBUF_TARGETS) + # 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}) - # NOTE: Removed /WHOLEARCHIVE for protobuf - causes duplicate version.res in dependency chain endif() message(STATUS " - gRPC ROM service + canvas automation enabled") @@ -202,13 +203,14 @@ target_link_libraries(yaze PRIVATE absl::flags_parse ) if(YAZE_WITH_GRPC AND YAZE_PROTOBUF_TARGETS) - target_link_libraries(yaze PRIVATE ${YAZE_PROTOBUF_TARGETS}) - # Apply /WHOLEARCHIVE only at executable level to avoid duplicate version.res - # This ensures protobuf internal symbols are included while preventing resource duplication + # 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) target_link_options(yaze PRIVATE /WHOLEARCHIVE:$) endforeach() + else() + target_link_libraries(yaze PRIVATE ${YAZE_PROTOBUF_TARGETS}) endif() endif() diff --git a/src/app/editor/editor_library.cmake b/src/app/editor/editor_library.cmake index 899f36db..d3367592 100644 --- a/src/app/editor/editor_library.cmake +++ b/src/app/editor/editor_library.cmake @@ -167,9 +167,10 @@ if(YAZE_WITH_GRPC) grpc++ grpc++_reflection ) - if(YAZE_PROTOBUF_TARGETS) + # 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}) - # NOTE: Removed /WHOLEARCHIVE for protobuf - causes duplicate version.res in dependency chain endif() endif() diff --git a/src/app/editor/overworld/overworld_editor.cc b/src/app/editor/overworld/overworld_editor.cc index 4aef67d1..400337fb 100644 --- a/src/app/editor/overworld/overworld_editor.cc +++ b/src/app/editor/overworld/overworld_editor.cc @@ -154,9 +154,7 @@ void OverworldEditor::Initialize() { entity_renderer_ = std::make_unique( &overworld_, &ow_map_canvas_, &sprite_previews_); - // Setup Canvas Automation API callbacks (Phase 4) SetupCanvasAutomation(); - } absl::Status OverworldEditor::Load() { diff --git a/src/app/emu/emu.cmake b/src/app/emu/emu.cmake index ca6656da..8082be2e 100644 --- a/src/app/emu/emu.cmake +++ b/src/app/emu/emu.cmake @@ -34,11 +34,13 @@ if(YAZE_BUILD_EMU AND NOT YAZE_MINIMAL_BUILD) message(WARNING "yaze_emu needs yaze_test_support but TARGET not found") endif() - # Apply /WHOLEARCHIVE for protobuf at executable level (Windows) + # 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) target_link_options(yaze_emu PRIVATE /WHOLEARCHIVE:$) endforeach() + elseif(YAZE_WITH_GRPC AND YAZE_PROTOBUF_TARGETS) + target_link_libraries(yaze_emu PRIVATE ${YAZE_PROTOBUF_TARGETS}) endif() # Test engine is always available when tests are built @@ -61,11 +63,13 @@ if(YAZE_BUILD_EMU AND NOT YAZE_MINIMAL_BUILD) absl::str_format ) - # Apply /WHOLEARCHIVE for protobuf at executable level (Windows) + # 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) target_link_options(yaze_emu_test PRIVATE /WHOLEARCHIVE:$) endforeach() + elseif(YAZE_WITH_GRPC AND YAZE_PROTOBUF_TARGETS) + target_link_libraries(yaze_emu_test PRIVATE ${YAZE_PROTOBUF_TARGETS}) endif() message(STATUS "✓ yaze_emu_test: Headless emulator test harness configured") diff --git a/src/app/net/net_library.cmake b/src/app/net/net_library.cmake index 25faa0aa..49fabf67 100644 --- a/src/app/net/net_library.cmake +++ b/src/app/net/net_library.cmake @@ -84,9 +84,10 @@ if(YAZE_WITH_GRPC) grpc++ grpc++_reflection ) - if(YAZE_PROTOBUF_TARGETS) + # 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}) - # NOTE: Removed /WHOLEARCHIVE for protobuf - causes duplicate version.res in dependency chain endif() message(STATUS " - gRPC ROM service enabled") diff --git a/src/cli/agent.cmake b/src/cli/agent.cmake index 27b0bbaf..b3f0e649 100644 --- a/src/cli/agent.cmake +++ b/src/cli/agent.cmake @@ -161,9 +161,10 @@ if(YAZE_WITH_GRPC) grpc++ grpc++_reflection ) - if(YAZE_PROTOBUF_TARGETS) + # 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_agent PUBLIC ${YAZE_PROTOBUF_TARGETS}) - # NOTE: Removed /WHOLEARCHIVE for protobuf - causes duplicate version.res in dependency chain endif() # Note: YAZE_WITH_GRPC is defined globally via add_compile_definitions in root CMakeLists.txt diff --git a/src/cli/z3ed.cmake b/src/cli/z3ed.cmake index 631cd812..05925a81 100644 --- a/src/cli/z3ed.cmake +++ b/src/cli/z3ed.cmake @@ -41,13 +41,14 @@ if(YAZE_WITH_GRPC) message(STATUS "Adding gRPC support to z3ed CLI") target_link_libraries(z3ed PRIVATE grpc++ grpc++_reflection) if(YAZE_PROTOBUF_TARGETS) - target_link_libraries(z3ed PRIVATE ${YAZE_PROTOBUF_TARGETS}) - # Apply /WHOLEARCHIVE only at executable level to avoid duplicate version.res - # This ensures protobuf internal symbols are included while preventing resource duplication + # 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) target_link_options(z3ed PRIVATE /WHOLEARCHIVE:$) endforeach() + else() + target_link_libraries(z3ed PRIVATE ${YAZE_PROTOBUF_TARGETS}) endif() endif() endif() diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 510ca402..a5e2b2f9 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -43,16 +43,17 @@ if(YAZE_BUILD_TESTS) YAZE_GUI_TEST_TARGET=1) # Mark this as a GUI test target endif() + # Link protobuf with /WHOLEARCHIVE on Windows (instead of via libraries) + if(WIN32 AND MSVC AND YAZE_WITH_GRPC AND YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS) + foreach(_yaze_proto_target IN LISTS YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS) + target_link_options(${suite_name} PRIVATE /WHOLEARCHIVE:$) + endforeach() + endif() + if(WIN32) message(STATUS "Configuring Windows stack size for ${suite_name} to 16MB") if(MSVC) target_link_options(${suite_name} PRIVATE /STACK:16777216) - # Apply /WHOLEARCHIVE only at executable level to prevent duplicate version.res - if(YAZE_WITH_GRPC AND YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS) - foreach(_yaze_proto_target IN LISTS YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS) - target_link_options(${suite_name} PRIVATE /WHOLEARCHIVE:$) - endforeach() - endif() else() target_link_options(${suite_name} PRIVATE -Wl,--stack,16777216) endif() diff --git a/tools/test_helpers/CMakeLists.txt b/tools/test_helpers/CMakeLists.txt index 9531cd40..3368bdaf 100644 --- a/tools/test_helpers/CMakeLists.txt +++ b/tools/test_helpers/CMakeLists.txt @@ -47,15 +47,18 @@ set(HELPER_TOOLS ) foreach(TOOL ${HELPER_TOOLS}) + # Windows: Link protobuf with /WHOLEARCHIVE (not via yaze_core 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) + target_link_options(${TOOL} PRIVATE /WHOLEARCHIVE:$) + endforeach() + elseif(YAZE_WITH_GRPC AND YAZE_PROTOBUF_TARGETS) + target_link_libraries(${TOOL} PRIVATE ${YAZE_PROTOBUF_TARGETS}) + endif() + if(WIN32) if(MSVC) target_link_options(${TOOL} PRIVATE /STACK:16777216) - # Apply /WHOLEARCHIVE for protobuf at executable level - if(YAZE_WITH_GRPC AND YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS) - foreach(_yaze_proto_target IN LISTS YAZE_PROTOBUF_WHOLEARCHIVE_TARGETS) - target_link_options(${TOOL} PRIVATE /WHOLEARCHIVE:$) - endforeach() - endif() elseif(MINGW OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU") target_link_options(${TOOL} PRIVATE -Wl,--stack,16777216) else()