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

(Backport) Enable 24.04 CI, remove distutils dependency (#1408) #1413

Merged
merged 3 commits into from
May 14, 2024

Conversation

Blast545
Copy link
Contributor

  • distutils is no longer required since this branch requires a new enough version of cmake.
  • Remove outdated cmake code: setting python debug libraries is not needed as of cmake 3.12.
  • Require cmake 3.22.1

🦟 Bug fix

Should fix CI on sdformat for Harmonic.
See reference job: https://build.osrfoundation.org/job/sdformat-ci-sdf14-noble-amd64/

Summary

Backporting the removal of the distutils dependency

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.

* distutils is no longer required since this branch requires a
 new enough version of cmake.
* Remove outdated cmake code: setting python
  debug libraries is not needed as of cmake 3.12.
* Require cmake 3.22.1

Signed-off-by: Steve Peters <[email protected]>
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label May 13, 2024
Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.44%. Comparing base (ff9b3ad) to head (e100e03).
Report is 12 commits behind head on sdf14.

❗ Current head e100e03 differs from pull request most recent head 47e82ba. Consider uploading reports for the commit 47e82ba to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##            sdf14    #1413      +/-   ##
==========================================
+ Coverage   92.42%   92.44%   +0.01%     
==========================================
  Files         134      135       +1     
  Lines       17751    17837      +86     
==========================================
+ Hits        16406    16489      +83     
- Misses       1345     1348       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Blast545
Copy link
Contributor Author

@scpeters ABI checker is failing on the CMake version :/

Signed-off-by: Jorge Perez <[email protected]>
@scpeters
Copy link
Member

@scpeters ABI checker is failing on the CMake version :/

I think this is a problem with infra and not this PR; let's try to fix the infra

examples/CMakeLists.txt Outdated Show resolved Hide resolved
if(USE_SYSTEM_PATHS_FOR_PYTHON_INSTALLATION)
if(${CMAKE_VERSION} VERSION_LESS "3.12.0")
execute_process(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should revert this. We can be sure Jammy has CMake > 3.12, but since the whole project sets the minimum version to 3.10, we can't assume that this is being built by a newer version of CMake.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes related to CMakeLists were removed in 47e82ba

@scpeters scpeters merged commit 321d85e into sdf14 May 14, 2024
12 checks passed
@scpeters scpeters deleted the blast545/BP_1408_remove_distutils branch May 14, 2024 08:10
aagrawal05 pushed a commit to aagrawal05/sdformat that referenced this pull request Aug 16, 2024
* Enable 24.04 CI, remove distutils dependency (gazebosim#1408)
  distutils is no longer required since this branch requires a
 new enough version of cmake.

Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Jorge Perez <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
aagrawal05 pushed a commit to aagrawal05/sdformat that referenced this pull request Aug 16, 2024
* Enable 24.04 CI, remove distutils dependency (gazebosim#1408)
  distutils is no longer required since this branch requires a
 new enough version of cmake.

Signed-off-by: Steve Peters <[email protected]>
Signed-off-by: Jorge Perez <[email protected]>
Co-authored-by: Steve Peters <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants