Replace custom cmake code with ign-cmake2 by scpeters · Pull Request #780 · gazebosim/sdformat

scpeters · 2021-12-09T19:53:32Z

🎉 New feature

Closes #181, borrowing from #747 and targeting the sdf10 branch. It should be possible to backport these changes to sdf9 if an appropriate find-module is provided for TinyXML.

Summary

This replaces most of the custom cmake code in libsdformat with the functionality provided by ignition-cmake2. The root CMakeLists.txt is much shorter now and most of the cmake folder has been deleted. This is made possible by the NO_IGNITION_PREFIX and REPLACE_IGNITION_INCLUDE_PATH parameters added to ign_configure_project in gazebosim/gz-cmake#190 and gazebosim/gz-cmake#191. Some other details are listed below:

Use ign-cmake find modules for Tinyxml2 and URDFDOM (added in FindIgnURDFDOM cmake module gz-cmake#193), which provide cmake targets
the sdf_config.h.in template is renamed to config.hh.in to match the ign-cmake convention, and an sdf_config.h static header is added to preserve API
the autogenerated header is now sdformat.hh instead of sdf/sdf.hh, so a static header sdf/sdf.hh is added to preserve API
the SDFORMAT_* visibility macros in system_util.h are now based on the IGNITION_SDFORMAT_* macros from the auto-generated sdf/Export.hh
the LEGACY_PROJECT_PREFIX option from Add LEGACY_PROJECT_PREFIX option gz-cmake#199 is used to ensure the legacy cmake variables from the cmake config file retain the SDFormat_ prefix

~~This is currently a draft pull request since~~ There are some differences between the output of this branch and the currently installed versions:

headers are installed to {prefix}/include/ignition/sdformat10 rather than {prefix}/include/sdformat-10.6. EDIT: I think this is ok because the header folder changes with each minor release anyways. As long as the cmake config and pkgconfig files point to the correct place, it should be fine.
the name / description in the .pc file have changed from SDF / Robot Modeling Language (SDF) to Ignition sdformat / A set of sdformat classes for robot applications
the SDFORMAT_RUN_VALGRIND_TESTS cmake option is removed since ign_build_tests does not support valgrind

There ~~are~~ is also ~~two~~ one remaining custom cmake function in the cmake folder:

find_python_module: this function is used to find the psutil python module that is used by an integration test.

Test it

It should continue to work normally. Testing in a colcon workspace with other packages would be very helpful.

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

Fixes #181. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-12-09T20:18:32Z

testing gazebo-tooling/action-gz-ci#46

Needed by gazebosim/sdformat#780 Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-12-09T20:31:27Z

the ABI-checker needs gazebo-tooling/release-tools#589

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

This may represent a breakage for downstream users of the SDFormat_* cmake variables. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-12-09T23:08:02Z

@osrf-jenkins run tests please

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-12-10T00:11:03Z

the warnings in the Ubuntu build should be fixed by gazebo-tooling/release-tools#590

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

* sdformat-abichecker: set GZDEV_PROJECT_NAME Needed by gazebosim/sdformat#780 Signed-off-by: Steve Peters <scpeters@openrobotics.org> * detect_ci_matching_branch: use format() This is compatible with more verisons of python3. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-12-10T17:24:02Z

the SDFormat_* cmake variables are no longer provided by our cmake configuration file, which breaks our example CMakeLists.txt (see results of bionic GitHub Actions workflow)

the SDFormat_LIBRARIES variable was still in use in ignition-physics, so I'm updating it to use the proper cmake targets in gazebosim/gz-physics#314

koonpeng · 2021-12-13T09:23:33Z

There are some additional changes not in #747, will this be automatically forward ported to other branches or should I start cherry-picking the changes.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-12-21T23:51:31Z

Minor comments added. Overall looks great, a wonderful work. I'm not able to make it fail. As a final layout compatibility test we could try generating a nightly from this branch to check that files are named and located where expected.

I really like the changes guys.

what if we make a prerelease from sdf10 before a stable release?

scpeters · 2021-12-21T23:53:01Z

since it looks like no more changes are needed from ign-cmake, I'll open a PR for a stable 2.10.0 release

scpeters · 2021-12-22T00:03:01Z

since it looks like no more changes are needed from ign-cmake, I'll open a PR for a stable 2.10.0 release

gazebosim/gz-cmake#201

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Needed since gazebosim/sdformat#780 Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters · 2021-12-22T07:18:08Z

preparing sdformat10-release debian metadata:

Depend on libignition-cmake2-dev >= 2.10.0 gazebo-release/sdformat10-release#6

scpeters · 2021-12-22T07:22:09Z

since it looks like no more changes are needed from ign-cmake, I'll open a PR for a stable 2.10.0 release

ignitionrobotics/ign-cmake#201

the release is done and I just deleted the matching branch on gzdev that was enabling prerelease repositories, so it should build from stable packages now

scpeters · 2021-12-22T07:22:51Z

@osrf-jenkins run tests please

chapulina

I just took a really quick look and didn't see anything fishy. I think we can merge this and make a pre-release to see if anything breaks downstream.

The only thing to keep in mind is that we should make the last 10.X release in the coming days for Dome's EOL. So in the worst case scenario, if we identify issues after the pre-release, we may need to revert this PR soon.

chapulina · 2021-12-22T21:03:02Z


 if [ "$ans" = "y" ] || [ "$ans" = "Y" ]; then
-  s3cmd sync @CMAKE_BINARY_DIR@/doxygen/html/* s3://osrf-distributions/sdformat/api/@SDF_VERSION_FULL@/ -v
+  s3cmd sync @CMAKE_BINARY_DIR@/doxygen/html/* s3://osrf-distributions/sdformat/api/@PROJECT_VERSION_FULL@/ -v


Just checking, did anyone try uploading docs from this branch?

no, sorry I haven't tried that yet

koonpeng · 2021-12-29T06:14:10Z

@ahcorde @scpeters how do i go about forward porting this to main? I wans't able to find any docs on this.

scpeters · 2021-12-29T09:26:50Z

@ahcorde @scpeters how do i go about forward porting this to main? I wans't able to find any docs on this.

we forward port by merging forward and resolving conflicts. I'm testing a forward port to sdf11

chapulina · 2021-12-29T16:39:33Z

how do i go about forward porting this to main? I wans't able to find any docs on this.

It's documented here: https://ignitionrobotics.org/docs/all/contributing#process

scpeters · 2021-12-29T19:08:43Z

@ahcorde @scpeters how do i go about forward porting this to main? I wans't able to find any docs on this.

we forward port by merging forward and resolving conflicts. I'm testing a forward port to sdf11

merging #789 forward first since its test needs a few changes with SDFormat 1.8: #807

scpeters · 2021-12-29T20:51:06Z

@ahcorde @scpeters how do i go about forward porting this to main? I wans't able to find any docs on this.

we forward port by merging forward and resolving conflicts. I'm testing a forward port to sdf11

merging #789 forward first since its test needs a few changes with SDFormat 1.8: #807

merging the rest forward to sdf11 in #808

osrf-triage · 2022-03-25T22:29:30Z

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

https://community.gazebosim.org/t/new-ignition-releases-2022-03-25-fortress-edifice-citadel/1343/1

osrf-triage · 2022-04-13T21:31:00Z

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

https://community.gazebosim.org/t/new-ignition-releases-2022-04-13-fortress-edifice/1367/1

scpeters added 5 commits December 9, 2021 09:51

Configure as ignition package using ign-cmake2

2697f42

Fixes #181. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Find packages in root CMakeLists.txt

920296f

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

system_util.hh: use macros from Export.hh

cd75113

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

try ci_matching_branch_bionic branch in workflow

dc2587a

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Add anyone to todo comments

74b44f9

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

github-actions bot added the 🔮 dome Ignition Dome label Dec 9, 2021

scpeters mentioned this pull request Dec 9, 2021

Build: use ign cmake #747

Closed

8 tasks

Fix versioned_symbols test

407b760

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters mentioned this pull request Dec 9, 2021

Try checking out ci_matching_branch for gzdev gazebo-tooling/action-gz-ci#46

Merged

scpeters added a commit to gazebo-tooling/release-tools that referenced this pull request Dec 9, 2021

sdformat-abichecker: set GZDEV_PROJECT_NAME

702d5ed

Needed by gazebosim/sdformat#780 Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters mentioned this pull request Dec 9, 2021

sdformat-abichecker: set GZDEV_PROJECT_NAME gazebo-tooling/release-tools#589

Merged

scpeters mentioned this pull request Dec 9, 2021

Use ign-cmake. #181

Closed

scpeters added 3 commits December 9, 2021 14:56

Revert to bionic branch for action

aa33fe5

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Restore sdf_setup_windows

9847dbc

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Update examples/CMakeLists.txt to use targets

08bda50

This may represent a breakage for downstream users of the SDFormat_* cmake variables. Signed-off-by: Steve Peters <scpeters@openrobotics.org>

fix )

0a32128

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Filesystem: try adding windows header

775ebb9

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters requested review from azeey, chapulina and j-rivero December 10, 2021 00:57

scpeters marked this pull request as ready for review December 10, 2021 01:08

Try removing sdf_setup_windows

1db0122

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters mentioned this pull request Dec 10, 2021

Link to sdformat10 cmake target gazebosim/gz-physics#314

Merged

7 tasks

Use python3-psutils as dependency

17aa0d0

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters mentioned this pull request Dec 21, 2021

Prepare for 2.10.0 stable release gazebosim/gz-cmake#201

Merged

7 tasks

scpeters added 2 commits December 21, 2021 16:11

Add backwards compatibility comment in headers

6865f45

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

Move find_python_module to root CMakeLists.txt

4a7e7e3

Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters added a commit to gazebo-release/sdformat10-release that referenced this pull request Dec 22, 2021

Depend on libignition-cmake2-dev >= 2.10.0

caa2cf7

Needed since gazebosim/sdformat#780 Signed-off-by: Steve Peters <scpeters@openrobotics.org>

scpeters mentioned this pull request Dec 22, 2021

Depend on libignition-cmake2-dev >= 2.10.0 gazebo-release/sdformat10-release#6

Merged

j-rivero approved these changes Dec 22, 2021

View reviewed changes

chapulina approved these changes Dec 22, 2021

View reviewed changes

scpeters merged commit 92d20d2 into sdf10 Dec 22, 2021

scpeters deleted the ci_matching_branch/use-ign-cmake-10 branch December 22, 2021 22:56

scpeters mentioned this pull request Dec 29, 2021

Merge part of 10 to 11 #807

Merged

scpeters mentioned this pull request Dec 29, 2021

Improve static library support gazebosim/gz-cmake#203

Closed

7 tasks

adlarkin mentioned this pull request Jan 7, 2022

Make SDF -> USD code a separate component of SDF #816

Closed

8 tasks

This was referenced Mar 21, 2022

Install sdf/1.8 to versioned path #890

Merged

Install sdf/1.8 and sdf/1.9 to versioned paths #891

Merged

chapulina mentioned this pull request Apr 14, 2022

sdformat[11,12,13]: Explicitly depend on ign-cmake2 osrf/homebrew-simulation#1902

Merged

scpeters mentioned this pull request Nov 8, 2023

Use visibility hidden by default gazebosim/gz-cmake#392

Merged

9 tasks

Conversation

scpeters commented Dec 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎉 New feature

Summary

Test it

Checklist

Uh oh!

scpeters commented Dec 9, 2021

Uh oh!

scpeters commented Dec 9, 2021

Uh oh!

scpeters commented Dec 9, 2021

Uh oh!

scpeters commented Dec 10, 2021

Uh oh!

scpeters commented Dec 10, 2021

Uh oh!

koonpeng commented Dec 13, 2021

Uh oh!

scpeters commented Dec 21, 2021

Uh oh!

scpeters commented Dec 21, 2021

Uh oh!

scpeters commented Dec 22, 2021

Uh oh!

scpeters commented Dec 22, 2021

Uh oh!

scpeters commented Dec 22, 2021

Uh oh!

scpeters commented Dec 22, 2021

Uh oh!

chapulina left a comment

Choose a reason for hiding this comment

Uh oh!

chapulina Dec 22, 2021

Choose a reason for hiding this comment

Uh oh!

scpeters Dec 23, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

koonpeng commented Dec 29, 2021

Uh oh!

scpeters commented Dec 29, 2021

Uh oh!

chapulina commented Dec 29, 2021

Uh oh!

scpeters commented Dec 29, 2021

Uh oh!

scpeters commented Dec 29, 2021

Uh oh!

osrf-triage commented Mar 25, 2022

Uh oh!

osrf-triage commented Apr 13, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

scpeters commented Dec 9, 2021 •

edited

Loading