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

Updates for noble #53

Merged
merged 16 commits into from
Apr 24, 2024
Merged

Updates for noble #53

merged 16 commits into from
Apr 24, 2024

Conversation

christophfroehlich
Copy link
Contributor

@christophfroehlich christophfroehlich commented Apr 18, 2024

Using the last version of setup-ros fixed it for noble, if the correct container is used. These PR adds changes for all usages of setup-ros to make that work for our workflows.

Needs container: ubuntu:24.04 until the github runner will be updated, but the workflows should work automatically if ubuntu-latest is bumped to 24.04 and no container input is given.

Fixes #51 and #13

@christophfroehlich
Copy link
Contributor Author

We need a noble image for fixing that, see https://github.com/ros-controls/realtime_tools/actions/runs/8745182563/job/23999673020?pr=163

The official github image will probably be released next week actions/runner-images#9691, I'd wait for that.
@fmauch Or should we use one from osrf and skip the setup-ros step?

@fmauch
Copy link
Contributor

fmauch commented Apr 19, 2024

As far as I understand the discussion the runner image will be in public beta next week, so it probably won't be available through runs-on: ubuntu-24.04. Hence, it might actually be a good idea to use the OSRF one.

@christophfroehlich
Copy link
Contributor Author

I had a quick look, but that's not that easy..

  • using a bare ubuntu:noble container needs lots of steps to get ROS running (no surprise)
  • using OSRF's ros:rolling-ros-core-noble still needs lots of additional steps, I don't understand why for example even after setup-ros rosdep is not available.

Not sure if we should merge this, or wait until a simpler workflow will work again

jobs:
  coverage:
    name: coverage build
    uses: ros-controls/ros2_control_ci/.github/workflows/reusable-build-coverage.yml@revert_coverage_hack
    secrets: inherit
    with:
      ros_distro: rolling
      os_name: ubuntu-22.04
      container: ros:rolling-ros-core-noble

@christophfroehlich christophfroehlich marked this pull request as ready for review April 22, 2024 20:50
@christophfroehlich
Copy link
Contributor Author

christophfroehlich commented Apr 22, 2024

[email protected] now supports noble (#55). Needs container: ubuntu:24.04 or container: ros:rolling-ros-core-noble until ubuntu-latest runner is available from github.

Should we merge this and change all calling workflows, or wait until ubuntu-latest is noble? Same question for all the source build workflows.

@fmauch
Copy link
Contributor

fmauch commented Apr 23, 2024

Great news :-)

I don't necessarily see a downside from merging this in now.

@christophfroehlich
Copy link
Contributor Author

Then I'll merge this and create PRs for the calling workflows to add the images. I don't see any problems to leave the images there until the next ubuntu?

@christophfroehlich
Copy link
Contributor Author

or does this significantly increase bandwidth/CI time to pull the images instead of using the GH runner directly?

@christophfroehlich christophfroehlich linked an issue Apr 23, 2024 that may be closed by this pull request
@christophfroehlich christophfroehlich changed the title coverage-workflow: Deactivate ROSDISTRO_INDEX_URL again Updates for noble Apr 23, 2024
Copy link
Contributor

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for all the work you put into this!

@christophfroehlich christophfroehlich merged commit b7592c8 into master Apr 24, 2024
2 checks passed
@christophfroehlich christophfroehlich deleted the revert_coverage_hack branch April 24, 2024 12:42
@saikishor
Copy link
Member

@christophfroehlich Thank you :)

@christophfroehlich
Copy link
Contributor Author

now come the next 10 PRs for setting the correct image :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Go back to default rolling settings for coverage build Deprecated ::set-output
3 participants