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

Replace most mentions of Ubuntu 20.04 with 22.04 #5156

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

jafingerhut
Copy link
Contributor

No description provided.

@jafingerhut jafingerhut added run-sanitizer Use this tag to run a Clang+Sanitzers CI run. run-validation Use this tag to trigger a Validation CI run. run-ubuntu18 Use this tag to trigger a Ubuntu-18 CI run. run-static Use this tag to trigger static build CI run. labels Mar 2, 2025
@jafingerhut jafingerhut removed run-sanitizer Use this tag to run a Clang+Sanitzers CI run. run-static Use this tag to trigger static build CI run. run-ubuntu18 Use this tag to trigger a Ubuntu-18 CI run. labels Mar 2, 2025
@jafingerhut
Copy link
Contributor Author

jafingerhut commented Mar 3, 2025

The changes in this PR update the following CI tests to use Ubuntu 22.04 instead of 20.04, and they all pass:

  • p4c-lint
  • test-p4c-debian / test-ubuntu20 (Unity OFF, GTest OFF) (pull_request)
    • Now has a new name: test-p4c-debian / test-ubuntu22 (Unity OFF, GTest OFF) (pull_request)
  • test-p4c-debian / test-ubuntu20 (Unity ON, GTest ON) (pull_request)
    • Now has a new name: test-p4c-debian / test-ubuntu22 (Unity ON, GTest ON) (pull_request)
  • static-build-test-p4c / Dynamic glibc (pull_request)
  • static-build-test-p4c / Dynamic stdlib (pull_request)

Also several mentions of Ubuntu 20.04 in the top level README.md have been updated to 22.04.

See this comment for links to other draft PRs demonstrating that for some other CI tests, it is not as simple as replacing 20.04 with 22.04: #5155 (comment)

@jafingerhut jafingerhut requested a review from fruffy March 3, 2025 16:45
@vlstill
Copy link
Contributor

vlstill commented Mar 3, 2025

Nice! If (when) we drop Ubuntu 20, do we also want to shift the minimal GCC version required from 9.2 to something newer? Ubuntu 22.04 has GCC 11, which would be nice for C++20. Not sure if any downstream targets depend on GCC 9/Ubuntu 20.04 though (Altera does not; Tofino is now open-source so presumably that is one less to worry about).

@fruffy fruffy added the infrastructure Topics related to code style and build and test infrastructure. label Mar 3, 2025
@fruffy
Copy link
Collaborator

fruffy commented Mar 3, 2025

Also remove the Ubuntu 20 sanitizer check?

If someone wants Ubuntu 20.04 tests in CI longer term, they should
create something like the current ci-ubuntu-18-nightly.yml CI test
that runs Ubuntu 18.04 inside a Docker container.

Signed-off-by: Andy Fingerhut <[email protected]>
@jafingerhut
Copy link
Contributor Author

Also remove the Ubuntu 20 sanitizer check?

I have done this with commit 10. Weirdly, if you look at commit 10 diffs on this PR, it shows that file as deleted. But for some strange reason when I look at the top-level list of files changed on this PR, it is not mentioned at all. I have never seen this before in a Github PR.

@fruffy
Copy link
Collaborator

fruffy commented Mar 3, 2025

Also remove the Ubuntu 20 sanitizer check?

I have done this with commit 10. Weirdly, if you look at commit 10 diffs on this PR, it shows that file as deleted. But for some strange reason when I look at the top-level list of files changed on this PR, it is not mentioned at all. I have never seen this before in a Github PR.

The change indicates that the file was renamed, so should be ok. I actually do not know how Git figures this out.

@jafingerhut
Copy link
Contributor Author

Nice! If (when) we drop Ubuntu 20, do we also want to shift the minimal GCC version required from 9.2 to something newer? Ubuntu 22.04 has GCC 11, which would be nice for C++20. Not sure if any downstream targets depend on GCC 9/Ubuntu 20.04 though (Altera does not; Tofino is now open-source so presumably that is one less to worry about).

I am thinking one tiny step at a time right now, which is: avoid our CI tests failing randomly in March, and deterministically in April.

I don't know whether we have "officially" dropped support for Ubuntu 18.04 yet, since there is still an on-demand CI test that builds p4c and probably runs some tests on Ubuntu 18.04 in a Docker container.

I am happy to find out whether there are companies who want Ubuntu 20.04 support for a while longer. If they do, and if they want Ubuntu 20.04 CI tests on this public repo, I would recommend that someone from one of those companies should create a CI test that works after April 1, 2025 on Github.

@vlstill
Copy link
Contributor

vlstill commented Mar 3, 2025

The change indicates that the file was renamed, so should be ok. I actually do not know how Git figures this out.

heuristics :-(, git has sadly no real way of tracking renames

@vlstill
Copy link
Contributor

vlstill commented Mar 3, 2025

I am happy to find out whether there are companies who want Ubuntu 20.04 support for a while longer. If they do, and if they want Ubuntu 20.04 CI tests on this public repo, I would recommend that someone from one of those companies should create a CI test that works after April 1, 2025 on Github.

Sadly, I am afraid we have no good way of communicating downstream and many will not track the changes diligently :-(. A good starting point might be community notes on the various working groups and e-mails in them sufficiently before we drop the support. But I think ultimately we should be open to dropping the support for unsupported OSes, otherwise we will eventually freeze the code to the point we are a) having trouble with dependencies b) other downstreams will be annoyed at use being behind. C++17 is now 8 years old :-). But so far we are on the C++17 with the likes of LLVM, so we are not terribly behind :-D.

There are (relatively easy) ways of getting newer GCC on old Ubuntu, but if that is acceptable will depend a lot on the company policies so I am afraid we have no way of knowing unless someone tells us.

@jafingerhut
Copy link
Contributor Author

This has been updated to latest after #5011 was merged in.

@vlstill vlstill added run-static Use this tag to trigger static build CI run. run-sanitizer Use this tag to run a Clang+Sanitzers CI run. labels Mar 4, 2025
Copy link
Contributor

@vlstill vlstill 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 to me. Please add e.g. an empty commit to trigger static & sanitizers suites (git commit --allow-empty ...).

Before this is merged, we will need to update the "Required" checks for PR passing in repo settings. Then all PRs would need to merge this before becoming green (curretly this PR is blocked as it is waiting for Ubuntu 20 test).

@fruffy fruffy added the p4tc Topics related to the P4-TC back end. On PRs, also triggers p4tc CI tests to run. label Mar 4, 2025
Signed-off-by: Andy Fingerhut <[email protected]>
@jafingerhut
Copy link
Contributor Author

Looks good to me. Please add e.g. an empty commit to trigger static & sanitizers suites (git commit --allow-empty ...).

Before this is merged, we will need to update the "Required" checks for PR passing in repo settings. Then all PRs would need to merge this before becoming green (curretly this PR is blocked as it is waiting for Ubuntu 20 test).

I have forced CI to run again, and the sanitizer and static tests run and pass, as they did earlier on this PR when I tried them out. I have verified in the CI logs that they are running on Ubuntu 22.04.

@fruffy I do not know whether the order of merging this vs. updating the list of required check names is critical. Let me know if you want me to try updating the names of the two required tests that now have 22 in their names instead of 20.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Topics related to code style and build and test infrastructure. p4tc Topics related to the P4-TC back end. On PRs, also triggers p4tc CI tests to run. run-sanitizer Use this tag to run a Clang+Sanitzers CI run. run-static Use this tag to trigger static build CI run. run-validation Use this tag to trigger a Validation CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants