Build: use ign cmake by koonpeng · Pull Request #747 · gazebosim/sdformat

koonpeng · 2021-11-11T09:13:29Z

🎉 New feature

closes #181

Summary

Migrates the build scripts to use macros from ign-cmake as much as possible, this is dependent on a number of PRs

Project option: REPLACE_IGNITION_INCLUDE_PATH gz-cmake#190 (including the patch to fix config.hh still being referenced with ignition prefix in the master header)
FindIgnURDFDOM cmake module gz-cmake#193

Caveats

headers are installed to include/ignition/sdf. This shouldn't matter as long as the user is using cmake or pkgconfig to find sdformat.
the config header is installed to include/ignition/sdf/config.hh instead of include/sdf/sdf_config.h.
- A redirection header is added so that the old path still works.
the master header is installed to include/ignition/sdformat.hh instead of include/sdf/sdf.hh.
- A redirection header is added so that the old path still works.
docs are not using the standard ign_create_docs macro, I'm not sure if they are compatible enough to switch over.

Test it

git clone git@github.com:ignitionrobotics/ign-cmake.git src/ign-cmake
colcon build

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

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

scpeters

I've made a few comments

I'd recommend not deprecating the old headers in this pull request to reduce the noise in the diff. If we decide to do that, we can do it at a later time

scpeters · 2021-11-30T16:34:16Z

-
-sdf_install_includes("" ${headers}
-  ${CMAKE_CURRENT_BINARY_DIR}/sdf.hh)
+ign_install_all_headers(EXCLUDE_FILES sdf.hh sdf_config.h)


if you exclude these header files, they will not be installed. do they need to be excluded?

fixed in cbdc0de

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

ahcorde

CI is failing this should fix it

koonpeng · 2021-12-02T07:30:37Z

I think the CI is failing because it is using an "old" version of ignition-cmake, we need the recent prs which allow to remove the ignition prefix.

It's trying to look for src/cmd/cmdignition-sdformat.rb.in, but it should be looking for src/cmd/cmdsdformat.rb.in instead.

scpeters · 2021-12-06T19:47:26Z

I think the CI is failing because it is using an "old" version of ignition-cmake, we need the recent prs which allow to remove the ignition prefix.

It's trying to look for src/cmd/cmdignition-sdformat.rb.in, but it should be looking for src/cmd/cmdsdformat.rb.in instead.

the Ubuntu CI is using a nightly build of ignition-cmake2; you need to follow @ahcorde's suggestions above to fix the configuration

scpeters · 2021-12-06T19:52:48Z

 */
 #include <iostream>
-#include <sdf/sdf.hh>
+#include <ignition/sdformat.hh>


I think this can be reverted

done d1144d4

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com> Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

koonpeng · 2021-12-07T01:21:13Z

the Ubuntu CI is using a nightly build of ignition-cmake2; you need to follow @ahcorde's suggestions above to fix the configuration

I committed the suggested changes but still getting the same errors.

scpeters · 2021-12-08T22:10:17Z

the Ubuntu CI is using a nightly build of ignition-cmake2; you need to follow @ahcorde's suggestions above to fix the configuration

I committed the suggested changes but still getting the same errors.

my apologies, I forgot that gazebo-tooling/gzdev#44 would be needed

scpeters · 2021-12-09T06:16:02Z

I'll make an ign-cmake2 prerelease when gazebosim/gz-cmake#195 is approved

scpeters · 2021-12-09T19:55:40Z

I've opened an draft pull request in #780 that targets sdf10 that borrows heavily from your work here

adlarkin · 2022-01-04T23:08:32Z

Should we close this PR, since #780 was merged?

scpeters · 2022-01-05T00:24:53Z

alternative approach merged to sdf10 in #780 and merged forward to main in #813

Teo Koon Peng added 18 commits November 9, 2021 17:23

migrated main lib and unit tests

2efd6da

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

add cmd

109e5bc

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

update config file and master header location

6b6ca93

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

cleanup more stuffs that have been moved to ign-cmake

b56c9e4

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

build integration and performance tests

c6aeb0c

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

cleanup usage of link_directories

63c9027

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

remove unused files

b2f395d

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

use findURDF from ign-cmake

dfcdf84

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

update find urdf name

e681c41

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

reverse condition check

4d7e1a2

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

moar cleanups

ddce467

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

fix data install location

903f9e8

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

add redirection headers for sdf_config.h and sdf.hh

e25a163

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

use ign-cmake's visibility macros instead

2119536

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

do not use ign_create_docs as there are some differences

337a3cc

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

fix references to sdf_target

0fee3be

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

fix ign math and utils wrongly marked optional

355d296

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

fix missing patch version

116c95b

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

koonpeng marked this pull request as draft November 11, 2021 09:13

Teo Koon Peng added 2 commits November 12, 2021 09:41

use updated param name

591f379

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

remove urdfdom version as the cmake config does not support it

b478cb5

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

scpeters reviewed Nov 30, 2021

View reviewed changes

Teo Koon Peng added 3 commits December 1, 2021 10:01

install sdf.hh and sdf_config.h

cbdc0de

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

do not deprecate old headers; revert changes to use old headers

e695e3c

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

remove unnecessary configure_file

8b65844

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

koonpeng marked this pull request as ready for review December 1, 2021 02:28

Teo Koon Peng added 4 commits December 1, 2021 10:30

remove header changes in tests

4c37d73

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

Merge remote-tracking branch 'upstream/main' into build/use-ign-cmake

b5b02b5

remove unused template

ce47b3a

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

add header guards

f13f12c

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

ahcorde requested changes Dec 1, 2021

View reviewed changes

Comment thread src/cmd/CMakeLists.txt Outdated

Comment thread conf/CMakeLists.txt Outdated

koonpeng changed the title ~~WIP: Build: use ign cmake~~ Build: use ign cmake Dec 2, 2021

ahcorde added the needs upstream release Blocked by a release of an upstream library label Dec 2, 2021

scpeters reviewed Dec 6, 2021

View reviewed changes

Teo Koon Peng and others added 3 commits December 7, 2021 09:17

Update conf/CMakeLists.txt

4ec8bd4

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com> Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

Update src/cmd/CMakeLists.txt

d6aa259

Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com> Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

revert header change

d1144d4

Signed-off-by: Teo Koon Peng <koonpeng@openrobotics.org>

koonpeng force-pushed the build/use-ign-cmake branch from 4b07ccb to d1144d4 Compare December 7, 2021 01:18

scpeters mentioned this pull request Dec 8, 2021

Use nightlies for sdformat13 gazebo-tooling/gzdev#44

Merged

scpeters mentioned this pull request Dec 9, 2021

Prepare for 2.10.0~pre1 gazebosim/gz-cmake#195

Merged

7 tasks

scpeters mentioned this pull request Dec 9, 2021

Replace custom cmake code with ign-cmake2 #780

Merged

8 tasks

chapulina added the 🌱 garden Ignition Garden label Dec 30, 2021

scpeters closed this Jan 5, 2022

koonpeng deleted the build/use-ign-cmake branch January 5, 2022 07:52

Conversation

koonpeng commented Nov 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎉 New feature

Summary

Caveats

Test it

Checklist

Uh oh!

scpeters left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scpeters Nov 30, 2021

Choose a reason for hiding this comment

Uh oh!

koonpeng Dec 1, 2021

Choose a reason for hiding this comment

Uh oh!

ahcorde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

koonpeng commented Dec 2, 2021

Uh oh!

scpeters commented Dec 6, 2021

Uh oh!

scpeters Dec 6, 2021

Choose a reason for hiding this comment

Uh oh!

koonpeng Dec 7, 2021

Choose a reason for hiding this comment

Uh oh!

koonpeng commented Dec 7, 2021

Uh oh!

scpeters commented Dec 8, 2021

Uh oh!

scpeters commented Dec 9, 2021

Uh oh!

scpeters commented Dec 9, 2021

Uh oh!

adlarkin commented Jan 4, 2022

Uh oh!

scpeters commented Jan 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

koonpeng commented Nov 11, 2021 •

edited

Loading