Conversation
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
sloretz
left a comment
There was a problem hiding this comment.
Thank you for the PR! These look like good improvements to me.
I have just a couple requested changes.
|
I applied all the suggestions. Thanks! I also noticed that only the Here's what the resulting ####### 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}) |
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
|
If now |
|
@traversaro |
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). |
Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
There are no .cmake modules installed, so it has no effect. Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
|
@sloretz A friendly ping for a review. The PR should be ready to be merged. |
CMakeLists.txt
Outdated
| include(GNUInstallDirs) | ||
|
|
||
| option(BUILD_APPS "Build applications" ON) | ||
| option(BUILD_TESTING "Build tests" OFF) |
There was a problem hiding this comment.
what is the motivation for making BUILD_TESTING an explicit option? Including CTest will automatically add a BUILD_TESTING option that is ON by default.
There was a problem hiding this comment.
That's just because of plain ignorance on my part. Thanks! I'll drop it.
Not needed as it's added automatically by CTest. Signed-off-by: Martin Valgur <martin.valgur@gmail.com>
A few minor fixes to CMake. Mostly to aid packaging for the Conan recipe and Vcpkg port.
BUILD_APPSandBUILD_TESTINGoptions._USE_MATH_DEFINESfor MSVC.ADDITIONAL_MAKE_CLEAN_FILESproperty withADDITIONAL_CLEAN_FILES.find_dependency()in CMake config files.USE_VENDORED_DEPSoption and update theconfig.cmake.infile accordingly. Also modified the exportedurdfdom_LIBRARIESto point to the exported CMake targets instead of library files.