Patch 1.0.6 breaks include paths for urdf

[tdejager]

This patch includes MR: #71

This moves the default include paths to be under the name urdfdom_headers. I believe this is a breaking change, because this moves all the headers to a new location, so should not have been a patch release.

This breaks our build using conda, which depend on the major version. Urdf: https://github.com/ros/urdf has also not been updated to reflect this.

I think we should revert the patch release and make a major release for this change.

[traversaro]

Hi @tdejager ! I was the one that merged the 1.0.6 release in conda-forge/urdfdom_headers-feedstock#10 that change the header location in the conda-forge package urdfdom_headers. I tought of setting the dedicated option APPEND_PROJECT_NAME_TO_INCLUDEDIR to OFF to keep the original header location, but then I decided not to do that for consistency. However, we can always turn it again to OFF at least at https://github.com/conda-forge/urdfdom_headers-feedstock .
Just to understand, how are you consuming urdfdom_headers in your downstream project? As long as you are consuming it via pkg-config or CMake config files, no change in downstream projects should be necessary to deal with this change.

[tdejager]

Hi @traversaro :) So we are just using urdf from robostack directly in one of our projects. This includes urfddom_headers. This resolves to the latest version of urdfdom_headers, with that mentioned change. But urdf has not been updated so still includes it from ‘urdf_model’ directly.

So to illustrate:

Our project <— urdf <— urdfdom_headers

And it fails in the urdf libarary on the urdfdom_headers include. Does that make sense? :)

And we include urdf in our cmakelist directly.

[traversaro]

I think I got it (I am a bit out of time for documenting why, but I will do it later today). Yes, that is a clear regression from the packaging perspective, let me fix this on the urdfdom related feedstocks.

[traversaro]

In a nutshell, I think that something like https://github.com/ros/sdformat_urdf/blob/1dc1532fc6243d88a802d8e2cc3f01c0e6ba774e/sdformat_urdf/CMakeLists.txt#L37 is the source of the problem.

[traversaro]

@tdejager the problem should be solved by conda-forge/urdfdom_headers-feedstock#12 . I also filled conda-forge/urdfdom-feedstock#27 for a similar problem in urdfdom. Once the CDN propagate the packages, your problem should be solved.

[tdejager]

@traversaro You are amazing, thank you!

For my info what would have been the correct way for including this headers? They way that is explained here: git-stage/manual/cmake-packages.7.html#id19 ?

[traversaro]

For my info what would have been the correct way for including this headers? They way that is explained here: git-stage/manual/cmake-packages.7.html#id19 ?

Yes, the idea is to use imported targets as much as possible, see https://cmake.org/cmake/help/v3.23/manual/cmake-packages.7.html#creating-relocatable-packages, see for example the quick fix from @sloretz in ros/sdformat_urdf#5 . Unfortunatly if you use catkin I do not think it is straightforward to use CMake imported targets, while it is easier if you use ament_cmake.

[tdejager]

Cool! That makes sense, I’ll close this issue then as it will be resolved. Will rest our side tomorrow. Thanks for your help!

[traversaro]

@tdejager just a curiosity, did you tested if the issue is actually fixed?

[tdejager]

Yes, I did and it is fixed :)

…

On Thu, 7 Apr 2022 at 18:31, Silvio Traversaro ***@***.***> wrote: @tdejager <https://github.com/tdejager> just a curiosity, did you tested if the issue is actually fixed? — Reply to this email directly, view it on GitHub <#73 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AADF4XXJXG4GGWQFMPTIYRLVD4EW5ANCNFSM5SS75JPA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[traversaro]

Yes, I did and it is fixed :)

On Thu, 7 Apr 2022 at 18:31, Silvio Traversaro @.***>
wrote:

@tdejager https://github.com/tdejager just a curiosity, did you tested
if the issue is actually fixed?

—
Reply to this email directly, view it on GitHub
#73 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AADF4XXJXG4GGWQFMPTIYRLVD4EW5ANCNFSM5SS75JPA
.
You are receiving this because you were mentioned.Message ID:
@.***>

Great!