Fix static builds and optimize test compilation (#1343)

Static builds of sdformat failbecause we are creating the using_parser_urdf target and linking it against the core library target, which is then exported. When the core library is built as a static library, CMake requires that other targets linked against it are exported as well (see https://stackoverflow.com/a/71080574/283225 case (2)). The solution in this PR is to compile the URDF sources directly into the core library instead of creating an extra target. While working on this, I realized we could create a static library that includes all the extra sources needed for tests instead of building each test with its own extra sources. I believe this will marginally improve compilation time, but more importantly, I think it cleans up the CMake file and will make adding tests easier. --------- Signed-off-by: Addisu Z. Taddese <[email protected]>
gazebosim · Nov 27, 2023 · ffd7e0d · ffd7e0d
1 parent f5b5333
commit ffd7e0d
Showing 1 changed file with 64 additions and 84 deletions.
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
@@ -5,10 +5,9 @@ ign_get_libsources_and_unittests(sources gtest_sources)
 # Add the source file auto-generated into the build folder from sdf/CMakeLists.txt
 list(APPEND sources EmbeddedSdf.cc)
 
-# Use interface library to deduplicate cmake logic for URDF linking
-add_library(using_parser_urdf INTERFACE)
+# When using the internal URDF parser, we build its sources with the core library
 if (USE_INTERNAL_URDF)
-  set(sources ${sources}
+  set(urdf_internal_sources
     urdf/urdf_parser/model.cpp
     urdf/urdf_parser/link.cpp
     urdf/urdf_parser/joint.cpp
@@ -17,59 +16,48 @@ if (USE_INTERNAL_URDF)
     urdf/urdf_parser/urdf_model_state.cpp
     urdf/urdf_parser/urdf_sensor.cpp
     urdf/urdf_parser/world.cpp)
-  target_include_directories(using_parser_urdf INTERFACE
-    ${CMAKE_CURRENT_SOURCE_DIR}/urdf)
-  if (WIN32)
-    target_compile_definitions(using_parser_urdf INTERFACE -D_USE_MATH_DEFINES)
-  endif()
-else()
-  target_link_libraries(using_parser_urdf INTERFACE
-    IgnURDFDOM::IgnURDFDOM)
+  set(sources ${sources} ${urdf_internal_sources})
 endif()
 
-if (BUILD_TESTING)
-  # Build this test file only if Ignition Tools is installed.
-  if (NOT IGN_PROGRAM)
-    list(REMOVE_ITEM gtest_sources ign_TEST.cc)
-  endif()
+ign_create_core_library(SOURCES ${sources}
+  CXX_STANDARD 17
+  LEGACY_PROJECT_PREFIX SDFormat
+  )
 
-  # Skip tests that don't work on Windows
-  if (WIN32)
-    list(REMOVE_ITEM gtest_sources Converter_TEST.cc)
-    list(REMOVE_ITEM gtest_sources FrameSemantics_TEST.cc)
-    list(REMOVE_ITEM gtest_sources ParamPassing_TEST.cc)
-    list(REMOVE_ITEM gtest_sources Utils_TEST.cc)
-    list(REMOVE_ITEM gtest_sources XmlUtils_TEST.cc)
-    list(REMOVE_ITEM gtest_sources parser_urdf_TEST.cc)
+target_link_libraries(${PROJECT_LIBRARY_TARGET_NAME}
+  PUBLIC
+    ignition-math${IGN_MATH_VER}::ignition-math${IGN_MATH_VER}
+    ignition-utils${IGN_UTILS_VER}::ignition-utils${IGN_UTILS_VER}
+  PRIVATE
+    TINYXML2::TINYXML2)
+
+  if (USE_INTERNAL_URDF)
+    target_include_directories(${PROJECT_LIBRARY_TARGET_NAME} PRIVATE
+      ${CMAKE_CURRENT_SOURCE_DIR}/urdf)
+    if (WIN32)
+      target_compile_definitions(${PROJECT_LIBRARY_TARGET_NAME} PRIVATE -D_USE_MATH_DEFINES)
+    endif()
+  else()
+    target_link_libraries(${PROJECT_LIBRARY_TARGET_NAME} PRIVATE
+      IgnURDFDOM::IgnURDFDOM)
   endif()
 
-  ign_build_tests(
-    TYPE UNIT
-    SOURCES ${gtest_sources}
-    INCLUDE_DIRS
-      ${CMAKE_CURRENT_SOURCE_DIR}
-      ${PROJECT_SOURCE_DIR}/test
-  )
+if (WIN32 AND USE_INTERNAL_URDF)
+  target_compile_definitions(${PROJECT_LIBRARY_TARGET_NAME} PRIVATE URDFDOM_STATIC)
+endif()
 
-  if (TARGET UNIT_Converter_TEST)
-    target_link_libraries(UNIT_Converter_TEST
-      TINYXML2::TINYXML2)
-    target_sources(UNIT_Converter_TEST PRIVATE
-      Converter.cc
-      EmbeddedSdf.cc
-      XmlUtils.cc)
-  endif()
+target_include_directories(${PROJECT_LIBRARY_TARGET_NAME}
+  PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}
+)
 
-  if (TARGET UNIT_FrameSemantics_TEST)
-    target_sources(UNIT_FrameSemantics_TEST PRIVATE FrameSemantics.cc Utils.cc)
-    target_link_libraries(UNIT_FrameSemantics_TEST TINYXML2::TINYXML2)
+if (BUILD_TESTING)
+  # Build this test file only if Ignition Tools is installed.
+  if (NOT IGN_PROGRAM)
+    list(REMOVE_ITEM gtest_sources ign_TEST.cc)
   endif()
 
-  if (TARGET UNIT_ParamPassing_TEST)
-    target_link_libraries(UNIT_ParamPassing_TEST
-      TINYXML2::TINYXML2
-      using_parser_urdf)
-    target_sources(UNIT_ParamPassing_TEST PRIVATE
+  add_library(library_for_tests OBJECT
       Converter.cc
       EmbeddedSdf.cc
       FrameSemantics.cc
@@ -78,53 +66,45 @@ if (BUILD_TESTING)
       Utils.cc
       XmlUtils.cc
       parser.cc
-      parser_urdf.cc)
-  endif()
+      parser_urdf.cc
+  )
 
-  if (TARGET UNIT_Utils_TEST)
-    target_sources(UNIT_Utils_TEST PRIVATE Utils.cc)
-    target_link_libraries(UNIT_Utils_TEST TINYXML2::TINYXML2)
+  if (USE_INTERNAL_URDF)
+    target_sources(library_for_tests PRIVATE ${urdf_internal_sources})
   endif()
 
-  if (TARGET UNIT_XmlUtils_TEST)
-    target_link_libraries(UNIT_XmlUtils_TEST
-      TINYXML2::TINYXML2)
-    target_sources(UNIT_XmlUtils_TEST PRIVATE XmlUtils.cc)
-  endif()
+  # Link against the publically and privately linked libraries of the core library
+  target_link_libraries(library_for_tests
+    $<TARGET_PROPERTY:${PROJECT_LIBRARY_TARGET_NAME},LINK_LIBRARIES>
+  )
 
-  if (TARGET UNIT_parser_urdf_TEST)
-    target_link_libraries(UNIT_parser_urdf_TEST
-      TINYXML2::TINYXML2
-      using_parser_urdf)
-    target_sources(UNIT_parser_urdf_TEST PRIVATE
-      SDFExtension.cc
-      XmlUtils.cc
-      parser_urdf.cc)
-  endif()
-endif()
+  # Use the include flags from the core library
+  target_include_directories(library_for_tests PUBLIC
+    $<TARGET_PROPERTY:${PROJECT_LIBRARY_TARGET_NAME},INCLUDE_DIRECTORIES>
+  )
 
-ign_create_core_library(SOURCES ${sources}
-  CXX_STANDARD 17
-  LEGACY_PROJECT_PREFIX SDFormat
+  # Use the private compile flags from the core library. Also set IGNITION_SDFORMAT_STATIC_DEFINE to avoid
+  # inconsistent linkage issues on windows. Setting the define will cause the SDFORMAT_VISIBLE/SDFORMAT_HIDDEN  macros
+  # to expand to nothing when building a static library
+  target_compile_definitions(library_for_tests PUBLIC
+    $<TARGET_PROPERTY:${PROJECT_LIBRARY_TARGET_NAME},COMPILE_DEFINITIONS>
+    -DIGNITION_SDFORMAT_STATIC_DEFINE
   )
 
-target_link_libraries(${PROJECT_LIBRARY_TARGET_NAME}
-  PUBLIC
-    ignition-math${IGN_MATH_VER}::ignition-math${IGN_MATH_VER}
-    ignition-utils${IGN_UTILS_VER}::ignition-utils${IGN_UTILS_VER}
-  PRIVATE
-    TINYXML2::TINYXML2
-    using_parser_urdf)
+  ign_build_tests(
+    TYPE UNIT
+    SOURCES ${gtest_sources}
+    INCLUDE_DIRS
+      ${CMAKE_CURRENT_SOURCE_DIR}
+      ${PROJECT_SOURCE_DIR}/test
+    LIB_DEPS
+      library_for_tests
+    TEST_LIST
+      test_targets
+  )
 
-if (WIN32)
-  target_compile_definitions(${PROJECT_LIBRARY_TARGET_NAME} PRIVATE URDFDOM_STATIC)
 endif()
 
-target_include_directories(${PROJECT_LIBRARY_TARGET_NAME}
-  PRIVATE
-    ${CMAKE_CURRENT_SOURCE_DIR}
-)
-
 if(NOT WIN32)
   add_subdirectory(cmd)
 endif()