FindIgnURDFDOM cmake module

[koonpeng]

🎉 New feature

Summary

Adds a minimal FindURDF.cmake module, this will help migrate sdformat to ign-cmake gazebosim/sdformat#181.

Test it

cmake_minimum_required(VERSION 3.10.2 FATAL_ERROR)
find_package(ignition-cmake2 REQUIRED)
ign_find_package(URDF REQUIRED)

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

[traversaro]

Could it make sense to call it FindIgnURDFDOM? In the past we had problems with collision between ign-cmake provided Find<pkg>.cmake and similarly named files distributed by other projects (see for example #40). Naming (at least new) all Find**.cmake scripts as FindIgn***.cmake should avoid these kind of problems. Furthermore, the library that this scripts finds is called urdfdom (see https://github.com/ros/urdfdom and https://repology.org/project/urdfdom/versions), so using urdfdom also in the Find script may avoid confusion.

[scpeters]

Could it make sense to call it FindIgnURDFDOM? In the past we had problems with collision between ign-cmake provided Find<pkg>.cmake and similarly named files distributed by other projects (see for example #40). Naming (at least new) all Find**.cmake scripts as FindIgn***.cmake should avoid these kind of problems. Furthermore, the library that this scripts finds is called urdfdom (see https://github.com/ros/urdfdom and https://repology.org/project/urdfdom/versions), so using urdfdom also in the Find script may avoid confusion.

I agree; please rename to FindIgnURDFDOM

[j-rivero]

I'm afraid that the current implementation is not valid for Windows/MSVC builds.

Is there any particular reason not to use the urdfdom CMake modules instead of using pkg-config? I think that we could support Windows and other system well by using CMake if possible.

[scpeters]

I'm afraid that the current implementation is not valid for Windows/MSVC builds.

Is there any particular reason not to use the urdfdom CMake modules instead of using pkg-config? I think that we could support Windows and other system well by using CMake if possible.

the SDFormat search logic has a fall-back to find_package if pkg_check_modules does not find it:

• https://github.com/ignitionrobotics/sdformat/blob/sdformat12_12.1.0/cmake/SearchForStuff.cmake#L13-L51

it looks like DART now uses only find_package:

• https://github.com/dartsim/dart/blob/main/cmake/DARTFindurdfdom.cmake

[koonpeng]

I'm afraid that the current implementation is not valid for Windows/MSVC builds.

Is there any particular reason not to use the urdfdom CMake modules instead of using pkg-config? I think that we could support Windows and other system well by using CMake if possible.

I missed that urdfdom supports cmake. But sdformat prefers the pkgconfig and fallbacks to cmake, should we change it to prefer cmake first? One downside with the cmake config is that it does not support version checking.

[j-rivero]

I missed that urdfdom supports cmake. But sdformat prefers the pkgconfig and fallbacks to cmake, should we change it to prefer cmake first? One downside with the cmake config is that it does not support version checking.

It is my impression that we should prioritize CMake over pkg-config as much as we can. The problem with no version checking with urdfdom CMake is unfortunate and seems a big thing to me. In this case we can document that we try pkg-config first for that reason.

[j-rivero]

It is my impression that we should prioritize CMake over pkg-config as much as we can. The problem with no version checking with urdfdom CMake is unfortunate and seems a big thing to me. In this case we can document that we try pkg-config first for that reason.

I've implemented the suggestion on d46f7cc branch jrivero/add-find-urdf . Let me know if that works for you Koon.

[koonpeng]

I've implemented the suggestion on d46f7cc branch jrivero/add-find-urdf . Let me know if that works for you Koon.

Thanks, I just merged that commit in.

[ahcorde]

@osrf-jenkins retest this please

[osrf-triage]

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-01-10/1228/1