ros / urdfdom

URDF parser
http://ros.org/wiki/urdf
Other
96 stars 132 forks source link

CMake improvements for packaging #197

Open valgur opened 8 months ago

valgur commented 8 months ago

A few minor fixes to CMake. Mostly to aid packaging for the Conan recipe and Vcpkg port.

valgur commented 8 months ago

I applied all the suggestions. Thanks!

I also noticed that only the include/urdfdom path was being exported by urdfdom_INCLUDE_DIRS and added include to it as well.

Here's what the resulting urdfdom-config.cmake looks like after installation:


####### Expanded from @PACKAGE_INIT@ by configure_package_config_file() #######
####### Any changes to this file will be overwritten by the next CMake run ####
####### The input file was urdfdom-config.cmake.in                            ########

get_filename_component(PACKAGE_PREFIX_DIR "${CMAKE_CURRENT_LIST_DIR}/../../" ABSOLUTE)

macro(set_and_check _var _file)
  set(${_var} "${_file}")
  if(NOT EXISTS "${_file}")
    message(FATAL_ERROR "File or directory ${_file} referenced by variable ${_var} does not exist !")
  endif()
endmacro()

macro(check_required_components _NAME)
  foreach(comp ${${_NAME}_FIND_COMPONENTS})
    if(NOT ${_NAME}_${comp}_FOUND)
      if(${_NAME}_FIND_REQUIRED_${comp})
        set(${_NAME}_FOUND FALSE)
      endif()
    endif()
  endforeach()
endmacro()

####################################################################################

if (urdfdom_CONFIG_INCLUDED)
  return()
endif()
set(urdfdom_CONFIG_INCLUDED TRUE)

set(CMAKE_MODULE_PATH_BACKUP_URDFDOM ${CMAKE_MODULE_PATH})
list(APPEND CMAKE_MODULE_PATH "${urdfdom_DIR}")

set(urdfdom_INCLUDE_DIRS "${PACKAGE_PREFIX_DIR}/include/urdfdom")
if(ON)
  list(APPEND urdfdom_INCLUDE_DIRS "${PACKAGE_PREFIX_DIR}/include/urdfdom/..")
endif()

foreach(lib urdfdom_sensor;urdfdom_model_state;urdfdom_model;urdfdom_world)
  set(onelib "${lib}-NOTFOUND")
  set(onelibd "${lib}-NOTFOUND")
  find_library(onelib ${lib}
    PATHS "${PACKAGE_PREFIX_DIR}/lib"
    NO_DEFAULT_PATH)
  find_library(onelibd ${lib}d
    PATHS "${PACKAGE_PREFIX_DIR}/lib"
    NO_DEFAULT_PATH)
  if(onelib-NOTFOUND AND onelibd-NOTFOUND)
    message(FATAL_ERROR "Library '${lib}' in package urdfdom is not installed properly")
  endif()
  if(onelib AND onelibd)
    list(APPEND urdfdom_LIBRARIES $<$<NOT:$<CONFIG:Debug>>:${onelib}>)
    list(APPEND urdfdom_LIBRARIES $<$<CONFIG:Debug>:${onelibd}>)
  else()
    if(onelib)
      list(APPEND urdfdom_LIBRARIES ${onelib})
    else()
      list(APPEND urdfdom_LIBRARIES ${onelibd})
    endif()
  endif()
  list(APPEND urdfdom_TARGETS urdfdom::${lib})
endforeach()

include(CMakeFindDependencyMacro)
if(OFF)
  find_dependency(tinyxml2_vendor QUIET)
  find_dependency(console_bridge_vendor QUIET)
else()
  find_dependency(TinyXML2 REQUIRED)
  find_dependency(console_bridge REQUIRED)
endif()

find_dependency(urdfdom_headers REQUIRED)
list(APPEND urdfdom_INCLUDE_DIRS "${urdfdom_headers_INCLUDE_DIRS}")

foreach(exp urdfdom)
  include(${urdfdom_DIR}/${exp}Export.cmake)
endforeach()

set(urdfdom_LIBRARIES ${urdfdom_TARGETS})

set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH_BACKUP_URDFDOM})
traversaro commented 8 months ago

If now urdfdom_LIBRARIES does not contain absolute path to libraries anymore but just targets, can we remove all the logic from the for except for list(APPEND urdfdom_TARGETS urdfdom::${lib})?

valgur commented 8 months ago

@traversaro urdfdom_TARGETS is indeed not needed anymore, but I would keep it for backwards compatibility.

traversaro commented 8 months ago

@traversaro urdfdom_TARGETS is indeed not needed anymore, but I would keep it for backwards compatibility.

Yes, that is why I suggested to keep just the line related to it in the for, and remove everything else (see https://github.com/ros/urdfdom/pull/197/files#diff-d7dc97268b73a1c3d45d0b2ef6e480309066f17cc38ab85b02121ccc7729ecb3R17-R37).

valgur commented 5 months ago

@sloretz A friendly ping for a review. The PR should be ready to be merged.