[sdformat13] new port #31280

talregev · 2023-05-05T21:55:25Z

Add sdformat13 port:

talregev · 2023-05-05T21:58:09Z

I got this error:

CMake Error: install(EXPORT "sdformat13" ...) includes target "sdformat13" which requires target "using_parser_urdf" that is not in any export set.
CMake Error in src/CMakeLists.txt:
  export called with target "sdformat13" which requires target
  "using_parser_urdf" that is not in any export set.

talregev · 2023-05-05T22:04:44Z

logs:
sdformat13.zip

Adela0814 · 2023-05-06T10:33:51Z

Note: I will be converting your PR to draft status. If you fixed the issue with CI, please reactivate it.

talregev · 2023-05-06T17:21:45Z

@Adela0814 Can you explain why this error is happening? Is it come from vcpkg?
I try to look on it on the internet, and I didn't any export on the sdformat repo cod side.
vcpkg make export?

talregev · 2023-05-07T15:57:52Z

@autoantwort @dg0yt @traversaro
Can you help?

dg0yt · 2023-05-07T16:15:18Z

Grep for using_parser_urdf in the source code and see if it either can be added to the export set or recconstructed in the installed config file?

traversaro · 2023-05-07T16:18:42Z

I think it is a bug upstream in vcpkg in the case BUILD_SHARED_LIBS is set to OFF. In https://github.com/gazebosim/sdformat/blob/38f6435e095b5d82984aa26bbeac40ce08e34229/src/CMakeLists.txt#L118 the sdformat13 links as PRIVATE the using_parser_urdf INTERFACE target. As long as BUILD_SHARED_LIBS is set to ON, everything works fine as private linked libraries do not need to be found when find_package(sdformat13) is called, but if BUILD_SHARED_LIBS is set to OFF, then using_parser_urdf needs to be found when find_package(sdformat13) is called, and hence it should be installed. I guess the error went unnoticed upstream as there are not a lot of users for the case when BUILD_SHARED_LIBS is set to OFF.

talregev · 2023-05-07T16:19:23Z

Grep for using_parser_urdf in the source code and see if it either can be added to the export set or recconstructed in the installed config file?

It added on the cmake:
https://github.com/gazebosim/sdformat/blob/sdformat13_13.0.0/src/CMakeLists.txt#L26

talregev · 2023-05-07T16:23:44Z

I think it is a bug upstream in vcpkg in the case BUILD_SHARED_LIBS is set to OFF

It by design. Can we compile it only with static?
Can we patch it, or make gazebo upstream changes?

dg0yt · 2023-05-07T17:26:15Z

I think it is a bug upstream in vcpkg in the case BUILD_SHARED_LIBS is set to OFF

To be clear, AFAIU it is a bug in upstream which becomes visible in vcpkg's static triplets.

It by design. Can we compile it only with static? Can we patch it, or make gazebo upstream changes?

I don't see how this is "by design". It should be fixed upstream, and the same patch can be applied in the vcpkg port for the current release.

talregev · 2023-05-07T17:31:46Z

I think it is a bug upstream in vcpkg in the case BUILD_SHARED_LIBS is set to OFF

To be clear, AFAIU it is a bug in upstream which becomes visible in vcpkg's static triplets.

It by design. Can we compile it only with static? Can we patch it, or make gazebo upstream changes?

I don't see how this is "by design". It should be fixed upstream, and the same patch can be applied in the vcpkg port for the current release.

I meant it by design that vcpkg turn off the shared libs in Linux compilation.

dg0yt · 2023-05-07T17:35:12Z

I don't see how this is "by design". It should be fixed upstream, and the same patch can be applied in the vcpkg port for the current release.

I meant it by design that vcpkg turn off the shared libs in Linux compilation.

Oh, I don't think this is by design, but just the historic choice of the "supported triplet" x64-linux. There is also a x64-linux-dynamic "community triplet".

talregev · 2023-05-07T17:39:25Z

@traversaro But it also fail on windows that have shared libs on. Why?

dg0yt · 2023-05-07T17:56:54Z

The new gz port are still not mature.

Make Error at D:/installed/x64-windows/share/cmake/gz-cmake3/cmake3/GzConfigureBuild.cmake:73 (message):
  -- BUILD ERRORS: These must be resolved before compiling.
Call Stack (most recent call first):
  CMakeLists.txt:157 (gz_configure_build)


CMake Error at D:/installed/x64-windows/share/cmake/gz-cmake3/cmake3/GzConfigureBuild.cmake:75 (message):
  -- 	Couldn't find the urdfdom >= 1.0 system installation
Call Stack (most recent call first):
  CMakeLists.txt:157 (gz_configure_build)


CMake Error at D:/installed/x64-windows/share/cmake/gz-cmake3/cmake3/GzConfigureBuild.cmake:77 (message):
  -- END BUILD ERRORS

Call Stack (most recent call first):
  CMakeLists.txt:157 (gz_configure_build)


CMake Error at D:/installed/x64-windows/share/cmake/gz-cmake3/cmake3/GzConfigureBuild.cmake:82 (message):
  Errors encountered in build.  Please see BUILD ERRORS above.
Call Stack (most recent call first):
  CMakeLists.txt:157 (gz_configure_build)

talregev · 2023-05-07T19:34:44Z

@dg0yt Can you fix it?

dg0yt · 2023-05-08T03:07:05Z

I guess for Windows, urdfdom needs ros/urdfdom#169.

talregev · 2023-05-08T04:40:40Z

I guess for Windows, urdfdom needs ros/urdfdom#169.

ok. I will appreciate your fix.

talregev · 2023-05-12T12:30:10Z

I guess for Windows, urdfdom needs ros/urdfdom#169.

@dg0yt windows is still fail after update urdfdom with the patch you suggested:
ros/urdfdom#169
Can you help?

dg0yt · 2023-05-13T07:12:11Z

-- Configuring done
CMake Error: install(EXPORT "sdformat13" ...) includes target "sdformat13" which requires target "using_parser_urdf" that is not in any export set.
CMake Error in src/CMakeLists.txt:
 export called with target "sdformat13" which requires target
 "using_parser_urdf" that is not in any export set.

The remaining build error is a bug in the project's files and unrelated to windows.
The build system uses custom (gz) functions instead of pure CMake.
Look/ask upstream.

traversaro · 2023-05-13T08:47:43Z

The error is the one described in #31280 (comment) , nothing related to the use of gz_ CMake function.

talregev · 2023-05-13T09:30:40Z

The error is the one described in #31280 (comment) , nothing related to the use of gz_ CMake function.

@traversaro
Please create a PR in upstream, and I will make a patch from it.

dg0yt · 2023-05-13T09:34:26Z

The error is the one described in #31280 (comment) , nothing related to the use of gz_ CMake function.

using_parser_urdf is an interface target created in sdformat13 src/CMakeLists.txt, and linked to the main library target. This is the standard CMake stuff.
However, the export of CMake config is hidden in gz functions. This export would need to handle the interface target as well.

talregev · 2023-05-13T09:35:59Z

The error is the one described in #31280 (comment) , nothing related to the use of gz_ CMake function.

using_parser_urdf is an interface target created in sdformat13 src/CMakeLists.txt, and linked to the main library target. This is the standard CMake stuff. However, the export of CMake config is hidden in gz functions. This export would need to handle the interface target as well.

Do you know how to solve it?
Can we make a patch for it?

dg0yt · 2023-05-13T09:44:01Z

Do you know how to solve it?

That's the point I'm trying to make: I know how to use CMake. But once projects start to use their own language of macros and functions, it becomes tedious to find out where it breaks and how to fix it. We don't even have a CMake call stack to start from because the error shows too late.

traversaro · 2023-05-13T10:41:42Z

The error is the one described in #31280 (comment) , nothing related to the use of gz_ CMake function.

using_parser_urdf is an interface target created in sdformat13 src/CMakeLists.txt, and linked to the main library target. This is the standard CMake stuff. However, the export of CMake config is hidden in gz functions. This export would need to handle the interface target as well.

Ahh, sorry I misunderstdood, that's totally correct.

@traversaro
Please create a PR in upstream, and I will make a patch from it.

I am happy to give you the hints to understand how to fix it. To be honest I am not interest at the moment in building sdformat13 as part of vcpkg and in static mode, so I prefer not to prepare the PR and submit it.

The sdformat13 library is created in https://github.com/gazebosim/gz-cmake/blob/gz-cmake3_3.1.0/cmake/GzUtils.cmake#L1540 and installed in https://github.com/gazebosim/gz-cmake/blob/gz-cmake3_3.1.0/cmake/GzUtils.cmake#L1611 , so the EXPORT used for it is sdformat13 . The target installed with that export are installed in https://github.com/gazebosim/gz-cmake/blob/88e7dc5f727e2270f206f19f558ab8f7fb5b8df1/cmake/GzPackaging.cmake#L372 . So a simple workaround is to just install the using_parser_urdf with the sdformat13 export, close to where using_parser_urdf library is defined with add_library.

dg0yt · 2023-05-13T10:59:42Z

Okay, I managed to add the install line. I will send a PR for this port to @talregev, adding a few more changes.

Adela0814 · 2023-07-25T09:00:25Z

Note: I will be converting your PR to draft status. When you respond, please revert to "ready for review". That way, I can be aware that you've responded since you can't modify the tags.

Adela0814 · 2023-08-17T07:00:43Z

@talregev I'm a bit confused, does sdformat require so many different versions of ports(sdformat6, sdformat9, sdformat10, sdformat13)? Instead of updating existing ports?

talregev · 2023-08-17T07:07:11Z

@talregev I'm a bit confused, does sdformat require so many different versions of ports(sdformat6, sdformat9, sdformat10, sdformat13)? Instead of updating existing ports?

we already have this discussions on gz-ports:
#28656
#28656 (review)

The conclusion was to add new gz-port while maintain all the old other port with their version number.

ports/sdformat13/cmake-config.patch

talregev · 2023-08-17T09:26:15Z

The error is the one described in #31280 (comment) , nothing related to the use of gz_ CMake function.

using_parser_urdf is an interface target created in sdformat13 src/CMakeLists.txt, and linked to the main library target. This is the standard CMake stuff. However, the export of CMake config is hidden in gz functions. This export would need to handle the interface target as well.

Ahh, sorry I misunderstdood, that's totally correct.

@traversaro
Please create a PR in upstream, and I will make a patch from it.

I am happy to give you the hints to understand how to fix it. To be honest I am not interest at the moment in building sdformat13 as part of vcpkg and in static mode, so I prefer not to prepare the PR and submit it.

The sdformat13 library is created in https://github.com/gazebosim/gz-cmake/blob/gz-cmake3_3.1.0/cmake/GzUtils.cmake#L1540 and installed in https://github.com/gazebosim/gz-cmake/blob/gz-cmake3_3.1.0/cmake/GzUtils.cmake#L1611 , so the EXPORT used for it is sdformat13 . The target installed with that export are installed in https://github.com/gazebosim/gz-cmake/blob/88e7dc5f727e2270f206f19f558ab8f7fb5b8df1/cmake/GzPackaging.cmake#L372 . So a simple workaround is to just install the using_parser_urdf with the sdformat13 export, close to where using_parser_urdf library is defined with add_library.

@traversaro Will you consider the next version of sdformat will support vcpkg in sperate static and dynamic mode?

talregev · 2023-08-17T11:48:48Z

@Adela0814 adding unofficial- as a prefix, create to build failure:

CMake Error: install(EXPORT "sdformat13" ...) includes target "sdformat13" which requires target "using_parser_urdf" that is not in any export set.
CMake Error in src/CMakeLists.txt:
  export called with target "sdformat13" which requires target
  "using_parser_urdf" that is not in any export set.

Please read comments here:
#31280 (comment)

I want to revert this change.

traversaro · 2023-08-17T12:07:19Z

@traversaro Will you consider the next version of sdformat will support vcpkg in sperate static and dynamic mode?

I am not a mantainer of sdformat, but I guess that if you submit a PR based on #31280 (comment) explaining in which case it is useful I guess the mantainers would be happy to review it.

talregev · 2023-08-17T12:10:16Z

@traversaro Will you consider the next version of sdformat will support vcpkg in sperate static and dynamic mode?

I am not a mantainer of sdformat, but I guess that if you submit a PR based on #31280 (comment) explaining in which case it is useful I guess the mantainers would be happy to review it.

If I knew how to solve it, I was create a PR and a patch. I am lacking the knowledge how to do it.
It solve only for linux and not for windows. I am not sure why.

traversaro · 2023-08-17T12:13:22Z

@traversaro Will you consider the next version of sdformat will support vcpkg in sperate static and dynamic mode?

I am not a mantainer of sdformat, but I guess that if you submit a PR based on #31280 (comment) explaining in which case it is useful I guess the mantainers would be happy to review it.

If I knew how to solve it, I was create a PR and a patch. I am lacking the knowledge how to do it. It solve only for linux and not for windows. I am not sure why.

If you open an (even incomplete) PR on sdformat it may be easier to iterate on your solution and provide you suggestions.

talregev · 2023-08-17T16:28:51Z

@traversaro Will you consider the next version of sdformat will support vcpkg in sperate static and dynamic mode?

I am not a mantainer of sdformat, but I guess that if you submit a PR based on #31280 (comment) explaining in which case it is useful I guess the mantainers would be happy to review it.

If I knew how to solve it, I was create a PR and a patch. I am lacking the knowledge how to do it. It solve only for linux and not for windows. I am not sure why.

If you open an (even incomplete) PR on sdformat it may be easier to iterate on your solution and provide you suggestions.

I will copy the solution from here, and I will happy for your guidance and suggestion and test it
both on linux and windows. (also for mac with the vcpkg ci).

talregev · 2023-08-18T12:43:43Z

@Adela0814 adding unofficial- as a prefix, create to build failure:
CMake Error: install(EXPORT "sdformat13" ...) includes target "sdformat13" which requires target "using_parser_urdf" that is not in any export set.
CMake Error in src/CMakeLists.txt:
  export called with target "sdformat13" which requires target
  "using_parser_urdf" that is not in any export set.
Please read comments here: #31280 (comment)

I want to revert this change.

@Adela0814 I will happy to hear your comment about the build fail.

* Fix cmake config export * Cleanup * Add usage * Update versions

talregev · 2023-08-23T21:03:09Z

@Adela0814 It now compile also on windows.
Please review it again.

talregev · 2023-08-25T10:26:00Z

@Adela0814 Thank you for your approve.
I see this PR didn't get in the previous batch.
Are we waiting for more review?

talregev · 2023-08-26T07:39:28Z

Thank you!

Cheney-W assigned Adela0814 May 6, 2023

Adela0814 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label May 6, 2023

Adela0814 marked this pull request as draft May 6, 2023 10:33

talregev force-pushed the TalR/sdformat13 branch from 43a33b8 to 9f133b1 Compare May 11, 2023 22:02

Adela0814 marked this pull request as draft July 25, 2023 09:00

talregev force-pushed the TalR/sdformat13 branch from ca7d5d6 to 6007700 Compare August 16, 2023 21:57

talregev requested a review from Adela0814 August 16, 2023 21:59

talregev marked this pull request as ready for review August 16, 2023 21:59

Adela0814 suggested changes Aug 17, 2023

View reviewed changes

ports/sdformat13/cmake-config.patch Show resolved Hide resolved

talregev force-pushed the TalR/sdformat13 branch from 6007700 to ac60cf7 Compare August 17, 2023 09:14

talregev requested a review from Adela0814 August 17, 2023 09:17

talregev mentioned this pull request Aug 22, 2023

[vcpkg] install using_parser_urdf target gazebosim/sdformat#1309

Closed

talregev and others added 3 commits August 22, 2023 23:53

Add sdformat13 port

78f8fcf

sdformat13 fixes (#1)

0fba32f

* Fix cmake config export * Cleanup * Add usage * Update versions

Fix according comments review

46edb34

talregev force-pushed the TalR/sdformat13 branch from ac60cf7 to 46edb34 Compare August 23, 2023 21:02

talregev requested a review from traversaro August 23, 2023 21:02

Adela0814 approved these changes Aug 24, 2023

View reviewed changes

Adela0814 added the info:reviewed Pull Request changes follow basic guidelines label Aug 24, 2023

dan-shaw merged commit d71315b into microsoft:master Aug 25, 2023
15 checks passed

talregev deleted the TalR/sdformat13 branch August 25, 2023 19:23

[sdformat13] new port #31280

[sdformat13] new port #31280

Conversation

talregev commented May 5, 2023 • edited Loading

talregev commented May 5, 2023

talregev commented May 5, 2023

Adela0814 commented May 6, 2023

talregev commented May 6, 2023

talregev commented May 7, 2023 • edited Loading

dg0yt commented May 7, 2023

traversaro commented May 7, 2023

talregev commented May 7, 2023

talregev commented May 7, 2023 • edited Loading

dg0yt commented May 7, 2023

talregev commented May 7, 2023

dg0yt commented May 7, 2023

talregev commented May 7, 2023

dg0yt commented May 7, 2023

talregev commented May 7, 2023

dg0yt commented May 8, 2023

talregev commented May 8, 2023

talregev commented May 12, 2023

dg0yt commented May 13, 2023

traversaro commented May 13, 2023

talregev commented May 13, 2023 • edited Loading

dg0yt commented May 13, 2023

talregev commented May 13, 2023

dg0yt commented May 13, 2023

traversaro commented May 13, 2023

dg0yt commented May 13, 2023

Adela0814 commented Jul 25, 2023

Adela0814 commented Aug 17, 2023

talregev commented Aug 17, 2023 • edited Loading

talregev commented Aug 17, 2023 • edited Loading

talregev commented Aug 17, 2023

traversaro commented Aug 17, 2023

talregev commented Aug 17, 2023 • edited Loading

traversaro commented Aug 17, 2023

talregev commented Aug 17, 2023 • edited Loading

talregev commented Aug 18, 2023

talregev commented Aug 23, 2023

talregev commented Aug 25, 2023

talregev commented Aug 26, 2023

talregev commented May 5, 2023 •

edited

Loading

talregev commented May 7, 2023 •

edited

Loading

talregev commented May 7, 2023 •

edited

Loading

talregev commented May 13, 2023 •

edited

Loading

talregev commented Aug 17, 2023 •

edited

Loading

talregev commented Aug 17, 2023 •

edited

Loading

talregev commented Aug 17, 2023 •

edited

Loading

talregev commented Aug 17, 2023 •

edited

Loading