Build: use ign cmake #747

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 [email protected]: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 <[email protected]>

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

include/sdf/sdf.hh

include/sdf/config.hh.in

include/sdf/CMakeLists.txt

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

include/sdf/CMakeLists.txt

-
-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 <[email protected]>

ahcorde

CI is failing this should fix it

src/cmd/CMakeLists.txt

conf/CMakeLists.txt

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

examples/simple.cc

@@ -15,7 +15,7 @@
 *
 */
 #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 <[email protected]> Signed-off-by: Teo Koon Peng <[email protected]>

Signed-off-by: Teo Koon Peng <[email protected]>

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 <[email protected]>

add cmd

109e5bc

Signed-off-by: Teo Koon Peng <[email protected]>

update config file and master header location

6b6ca93

Signed-off-by: Teo Koon Peng <[email protected]>

cleanup more stuffs that have been moved to ign-cmake

b56c9e4

Signed-off-by: Teo Koon Peng <[email protected]>

build integration and performance tests

c6aeb0c

Signed-off-by: Teo Koon Peng <[email protected]>

cleanup usage of link_directories

63c9027

Signed-off-by: Teo Koon Peng <[email protected]>

remove unused files

b2f395d

Signed-off-by: Teo Koon Peng <[email protected]>

use findURDF from ign-cmake

dfcdf84

Signed-off-by: Teo Koon Peng <[email protected]>

update find urdf name

e681c41

Signed-off-by: Teo Koon Peng <[email protected]>

reverse condition check

4d7e1a2

Signed-off-by: Teo Koon Peng <[email protected]>

moar cleanups

ddce467

Signed-off-by: Teo Koon Peng <[email protected]>

fix data install location

903f9e8

Signed-off-by: Teo Koon Peng <[email protected]>

add redirection headers for sdf_config.h and sdf.hh

e25a163

Signed-off-by: Teo Koon Peng <[email protected]>

use ign-cmake's visibility macros instead

2119536

Signed-off-by: Teo Koon Peng <[email protected]>

do not use ign_create_docs as there are some differences

337a3cc

Signed-off-by: Teo Koon Peng <[email protected]>

fix references to sdf_target

0fee3be

Signed-off-by: Teo Koon Peng <[email protected]>

fix ign math and utils wrongly marked optional

355d296

Signed-off-by: Teo Koon Peng <[email protected]>

fix missing patch version

116c95b

Signed-off-by: Teo Koon Peng <[email protected]>

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 <[email protected]>

remove urdfdom version as the cmake config does not support it

b478cb5

Signed-off-by: Teo Koon Peng <[email protected]>

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 <[email protected]>

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

e695e3c

Signed-off-by: Teo Koon Peng <[email protected]>

remove unnecessary configure_file

8b65844

Signed-off-by: Teo Koon Peng <[email protected]>

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

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

remove header changes in tests

4c37d73

Signed-off-by: Teo Koon Peng <[email protected]>

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

b5b02b5

Teo Koon Peng added 2 commits December 1, 2021 10:34

remove unused template

ce47b3a

Signed-off-by: Teo Koon Peng <[email protected]>

add header guards

f13f12c

Signed-off-by: Teo Koon Peng <[email protected]>

ahcorde requested changes Dec 1, 2021

View reviewed changes

src/cmd/CMakeLists.txt Outdated Show resolved Hide resolved

conf/CMakeLists.txt Outdated Show resolved Hide resolved

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 <[email protected]> Signed-off-by: Teo Koon Peng <[email protected]>

Update src/cmd/CMakeLists.txt

d6aa259

Co-authored-by: Alejandro Hernández Cordero <[email protected]> Signed-off-by: Teo Koon Peng <[email protected]>

revert header change

d1144d4

Signed-off-by: Teo Koon Peng <[email protected]>

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build: use ign cmake #747

Build: use ign cmake #747

koonpeng commented Nov 11, 2021 •

edited

Loading

scpeters left a comment

scpeters Nov 30, 2021

koonpeng Dec 1, 2021

ahcorde left a comment

koonpeng commented Dec 2, 2021

scpeters commented Dec 6, 2021

scpeters Dec 6, 2021

koonpeng Dec 7, 2021

koonpeng commented Dec 7, 2021

scpeters commented Dec 8, 2021

scpeters commented Dec 9, 2021

scpeters commented Dec 9, 2021

adlarkin commented Jan 4, 2022

scpeters commented Jan 5, 2022

Build: use ign cmake #747

Build: use ign cmake #747

Conversation

koonpeng commented Nov 11, 2021 • edited Loading

🎉 New feature

Summary

Caveats

Test it

Checklist

scpeters left a comment

Choose a reason for hiding this comment

scpeters Nov 30, 2021

Choose a reason for hiding this comment

koonpeng Dec 1, 2021

Choose a reason for hiding this comment

ahcorde left a comment

Choose a reason for hiding this comment

koonpeng commented Dec 2, 2021

scpeters commented Dec 6, 2021

scpeters Dec 6, 2021

Choose a reason for hiding this comment

koonpeng Dec 7, 2021

Choose a reason for hiding this comment

koonpeng commented Dec 7, 2021

scpeters commented Dec 8, 2021

scpeters commented Dec 9, 2021

scpeters commented Dec 9, 2021

adlarkin commented Jan 4, 2022

scpeters commented Jan 5, 2022

koonpeng commented Nov 11, 2021 •

edited

Loading