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

Add ubuntu 2404 images to CI config #15422

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,14 @@ parameters:
type: string
# solbuildpackpusher/solidity-buildpack-deps:ubuntu2204.clang-8
default: "solbuildpackpusher/solidity-buildpack-deps@sha256:2662376fe0e1ec2d346495a19a6d64508c1048d5a7325d8600c33c343fa64a0f"
Copy link
Member

@cameel cameel Sep 11, 2024

Choose a reason for hiding this comment

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

Why was #15321 even merged without having a matching PR updating CI that passes all checks? Because of this we're in a really weird state now where the dockerfile source does not match what we're running and also it blocks us from doing any other changes to the images that we might need in the meantime (at least without reverting them). Good thing that we apparently did not need any so far.

Unless these are going to be bumped or removed very soon, we should really revert these images (with a version bump to avoid reusing the number).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This you mean? #15320

We had a ton of problems with the new evmone bump (and still do for ossfuzz) while you were off.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I mean why did we decide to merge #15321 even though #15320 wasn't passing CI? I remember that there were problems and that's fine, just saying that in that case the right procedure is to hold off on merging both.

ubuntu-2404-docker-image:
type: string
# solbuildpackpusher/solidity-buildpack-deps:ubuntu2404-1
default: "solbuildpackpusher/solidity-buildpack-deps@sha256:5d6d27551104321a30326d6024bd4e96d9d899a97a78eb9feea364996f1d18b5"
Copy link
Member

Choose a reason for hiding this comment

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

Just to let it documented, this image comes from: #15321 (comment)

ubuntu-2404-clang-docker-image:
type: string
# solbuildpackpusher/solidity-buildpack-deps:ubuntu2404.clang-1
default: "solbuildpackpusher/solidity-buildpack-deps@sha256:8df0086907cc1e57068ad48841f2284a87bfbd0a95006286a19a7132d84bb861"
Copy link
Member

Choose a reason for hiding this comment

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

And this image comes from here: #15370 (comment)

Copy link
Member

@cameel cameel Sep 11, 2024

Choose a reason for hiding this comment

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

By the way, I think it's a good convention to hide the comments with older hashes we ended up not using by marking them as "outdated".

ubuntu-clang-ossfuzz-docker-image:
type: string
# solbuildpackpusher/solidity-buildpack-deps:ubuntu.clang.ossfuzz-6
Expand Down Expand Up @@ -602,6 +610,56 @@ defaults:
MAKEFLAGS: -j 10
CPUs: 10

- base_ubuntu2404: &base_ubuntu2404
docker:
- image: << pipeline.parameters.ubuntu-2404-docker-image >>
environment: &base_ubuntu2404_env
TERM: xterm
MAKEFLAGS: -j 3
CPUs: 3

- base_ubuntu2404_clang: &base_ubuntu2404_clang
docker:
- image: << pipeline.parameters.ubuntu-2404-clang-docker-image >>
environment: &base_ubuntu2404_clang_env
TERM: xterm
CC: clang
CXX: clang++
MAKEFLAGS: -j 3
CPUs: 3

- base_ubuntu2404_clang_large: &base_ubuntu2404_clang_large
<<: *base_ubuntu2404_clang
resource_class: large
environment: &base_ubuntu2404_clang_large_env
<<: *base_ubuntu2404_clang_env
MAKEFLAGS: -j 5
CPUs: 5

- base_ubuntu2404_small: &base_ubuntu2404_small
<<: *base_ubuntu2404
resource_class: small
environment: &base_ubuntu2404_small_env
<<: *base_ubuntu2404_env
MAKEFLAGS: -j 2
CPUs: 2

- base_ubuntu2404_large: &base_ubuntu2404_large
<<: *base_ubuntu2404
resource_class: large
environment: &base_ubuntu2404_large_env
<<: *base_ubuntu2404_env
MAKEFLAGS: -j 5
CPUs: 5

- base_ubuntu2404_xlarge: &base_ubuntu2404_xlarge
<<: *base_ubuntu2404
resource_class: xlarge
environment: &base_ubuntu2404_xlarge_env
<<: *base_ubuntu2404_env
MAKEFLAGS: -j 10
CPUs: 10
Comment on lines +631 to +661
Copy link
Member

Choose a reason for hiding this comment

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

We generally add and/remove variants other than the default one as needed. So no need to add them already if nothing's using them.

Having them does not hurt, but the config is already quite long :)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I guess the assumption is that we'll switch all the 2204 ones to them anyway so they'll all be in use soon?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct - the first step is to merge this PR. Then @rodiazet will rebase his evmone bump PR and switch a few of the failing soltest jobs to use the 2404 images added here (required for the evmone bump to pass). Once his PR is merged, we can then switch the rest of the 2204 jobs to 2404, and perform any required clean-up (i.e. delete the 2204 images).


- base_win: &base_win
executor:
name: win/default
Expand Down