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

Bump actions ubuntu 24.04 #770

Merged
merged 20 commits into from
Oct 10, 2024
Merged

Bump actions ubuntu 24.04 #770

merged 20 commits into from
Oct 10, 2024

Conversation

figueroa1395
Copy link
Contributor

@figueroa1395 figueroa1395 commented Oct 8, 2024

Partially solves #773. Only upgrading the clang-tidy workflow to ubuntu-24.04 and clang-tidy itself to version 18.1.3 are left; however, discussion on how to resolve each resulting issue of this upgrade is out of scope of this PR.


This in an experiment PR to try to bump to Ubuntu 24.04 in github actions.

In actions/runner-images#10636, it is stated that ubuntu-latest won't fully migrate to 24.04 until October 30th, hence the explicit versioning in 6032f72.

Relevant tools, packages, etc. are updated to the latest supported by this Ubuntu image as stated in https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@figueroa1395
Copy link
Contributor Author

figueroa1395 commented Oct 8, 2024

Just to keep track of the experiment, after a full migration attempt (Ubuntu 24.04 with clang 18, python 3.12, etc.), there seem to be three major places that would need work:

In addition, I would expect the nighly build to uncover even more errors from clang-tidy.

@TonyXiang8787
Copy link
Member

We need to discuss case-by-case about the clang-tidy stuff.

This reverts commit 6b70c04.

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
This reverts commit 07a54a6.

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
This reverts commit 37bad1f.

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@figueroa1395
Copy link
Contributor Author

figueroa1395 commented Oct 8, 2024

We need to discuss case-by-case about the clang-tidy stuff.

We can add it to the knowledge sharing session this Thursday (if time allows), so we can all take a look. Should I tell Peter about it?

Side note: Force pushed some commits because I forgot to sign off the ones reverting some changes. Moreover, all changes were reverted (except for only bumping Ubuntu to 24.04), to see what would be the effect of this isolated change in our CI.

@figueroa1395
Copy link
Contributor Author

It seems like the "isolated" experiment of only bumping to Ubuntu 24.04 arguably breaks things even more.

@mgovers
Copy link
Member

mgovers commented Oct 8, 2024 via email

.readthedocs.yaml Outdated Show resolved Hide resolved
@TonyXiang8787
Copy link
Member

It seems like the "isolated" experiment of only bumping to Ubuntu 24.04 arguably breaks things even more.

If you only bump Ubuntu 24.04, it still uses clang-15 compiler but with gcc-14 C++ std library. This causes the issue with many errors. The error should be less if you bump the clang version to clang-18.

@figueroa1395
Copy link
Contributor Author

If you only bump Ubuntu 24.04, it still uses clang-15 compiler but with gcc-14 C++ std library. This causes the issue with many errors. The error should be less if you bump the clang version to clang-18.

I'll try this and fix the ReadTheDocs version as well. Additionally I may try a fix for the validate-citations action.

@mgovers
Copy link
Member

mgovers commented Oct 9, 2024

It seems like the "isolated" experiment of only bumping to Ubuntu 24.04 arguably breaks things even more.

If you only bump Ubuntu 24.04, it still uses clang-15 compiler but with gcc-14 C++ std library. This causes the issue with many errors. The error should be less if you bump the clang version to clang-18.

see the first experiment: bumping to clang-18 works but clang-tidy-18 complains. we need to decide on the path going forward and refine - most likely in the afternoon.

We don't need to do that. Just use ubuntu-latest. It is already default 24.04.

it is released but not yet the default - migration from 22.04 to 24.04 is going gradually, with the migration expected to be finished on 2024 October 30. This experiment mainly checks whether it will work in the first place. Turns out it did for the check-code-quality.yml (so we can keep that action at ubuntu-latest) but not for the main.yml action (so we need to pin and migrate manually)

@TonyXiang8787
Copy link
Member

It seems like the "isolated" experiment of only bumping to Ubuntu 24.04 arguably breaks things even more.

If you only bump Ubuntu 24.04, it still uses clang-15 compiler but with gcc-14 C++ std library. This causes the issue with many errors. The error should be less if you bump the clang version to clang-18.

see the first experiment: bumping to clang-18 works but clang-tidy-18 complains. we need to decide on the path going forward and refine - most likely in the afternoon.

We don't need to do that. Just use ubuntu-latest. It is already default 24.04.

it is released but not yet the default - migration from 22.04 to 24.04 is going gradually, with the migration expected to be finished on 2024 October 30. This experiment mainly checks whether it will work in the first place. Turns out it did for the check-code-quality.yml (so we can keep that action at ubuntu-latest) but not for the main.yml action (so we need to pin and migrate manually)

If we bump to clang-18, most of the errors in other workflow should be gone, if not all of them. And yes, we still need to fix clang-tidy. To decide per category to either ignore the warning (like large enum type), or fix the issue.

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@figueroa1395
Copy link
Contributor Author

To keep track of the experiment:

  • Not upgrading clang-format behaves as expected: The code quality check passes.
  • Upgrading only clang to version 18.1.3. (but not clang-tidy) still breaks the clang-tidy action. However, sonar-cloud doesn't complain anymore.

Following I will upgrade clang-tidy (but not clang-format) and attempt to fix the failed validate-citations action.

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@figueroa1395
Copy link
Contributor Author

Conclusions at this point:

Furthermore, an issue was created as a result of this experiment.

Signed-off-by: Santiago Figueroa Manrique <[email protected]>
Signed-off-by: Santiago Figueroa Manrique <[email protected]>
@TonyXiang8787
Copy link
Member

@figueroa1395 please do a down merge before you run the clang-format

@figueroa1395 figueroa1395 marked this pull request as ready for review October 10, 2024 12:34
@figueroa1395
Copy link
Contributor Author

figueroa1395 commented Oct 10, 2024

The manual nightly build failed for reasons out of scope of this PR. This nightly build fail issue is to be tackled in a different PR. Merging now.

Copy link

sonarcloud bot commented Oct 10, 2024

@figueroa1395 figueroa1395 added this pull request to the merge queue Oct 10, 2024
@mgovers mgovers mentioned this pull request Oct 10, 2024
4 tasks
Merged via the queue into main with commit e29918c Oct 10, 2024
27 checks passed
@figueroa1395 figueroa1395 deleted the bump-actions-ubuntu-24.04 branch October 10, 2024 13:53
@mgovers mgovers mentioned this pull request Nov 5, 2024
27 tasks
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.

3 participants