Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore CXX_STANDARD 17 in core library #1072

Merged
merged 1 commit into from
Jul 11, 2022

Conversation

scpeters
Copy link
Member

@scpeters scpeters commented Jul 9, 2022

🦟 Bug fix

Restore CXX_STANDARD export in core library that was removed in #1059 and also tests gazebosim/gz-cmake#273

Summary

This is mainly testing gazebosim/gz-cmake#273 using the ci_matching_branch/ trickery with osrf/homebrew-simulation@406b74a and gazebo-tooling/gazebodistro@b427433, but it does make a small improvement of its own, restoring the CXX_STANDARD export. The main things changed in gazebosim/gz-cmake#273 is finding IgnURDFDOM changes to GzURDFDOM, but there may be an issue with the target names not being migrated. This is a draft PR until that is confirmed.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

@codecov
Copy link

codecov bot commented Jul 9, 2022

Codecov Report

Merging #1072 (1fd9d56) into main (fdd9861) will not change coverage.
The diff coverage is n/a.

❗ Current head 1fd9d56 differs from pull request most recent head d74d0f3. Consider uploading reports for the commit d74d0f3 to get more accurate results

@@           Coverage Diff           @@
##             main    #1072   +/-   ##
=======================================
  Coverage   83.12%   83.12%           
=======================================
  Files         154      154           
  Lines       18702    18702           
=======================================
  Hits        15546    15546           
  Misses       3156     3156           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdd9861...d74d0f3. Read the comment docs.

@scpeters
Copy link
Member Author

scpeters commented Jul 9, 2022

the homebrew build using gazebosim/gz-cmake#273 didn't find the external URDFDOM package, so it built against the internal fork of URDFDOM, which apparently has some pragma's that cause compiler warnings.

CMake Deprecation Warning at /usr/local/share/cmake/gz-cmake3/cmake3/IgnUtils.cmake:231 (message):
  Ign prefixed package name [IgnURDFDOM] is deprecated! Automatically using
  the Gz prefix instead: [GzURDFDOM]
Call Stack (most recent call first):
  CMakeLists.txt:54 (gz_find_package)


-- Looking for  - found

-- Using internal URDF

@scpeters scpeters force-pushed the ci_matching_branch/ticktock_find_ign_cmake branch from 1fd9d56 to d74d0f3 Compare July 9, 2022 19:06
@scpeters
Copy link
Member Author

scpeters commented Jul 9, 2022

the homebrew build using gazebosim/gz-cmake#273 didn't find the external URDFDOM package, so it built against the internal fork of URDFDOM, which apparently has some pragma's that cause compiler warnings.

upon rebuilding, it appears to be working now, except for the *_pretty variable commented about in gazebosim/gz-cmake#273 (comment)

@scpeters scpeters marked this pull request as ready for review July 9, 2022 21:19
@scpeters scpeters requested a review from azeey as a code owner July 9, 2022 21:19
@scpeters scpeters merged commit 17544db into main Jul 11, 2022
@scpeters scpeters deleted the ci_matching_branch/ticktock_find_ign_cmake branch July 11, 2022 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants