Skip to content

Add required packaging and pipeline tools. Update Conan#4

Merged
bthomee merged 8 commits intomainfrom
add_required_tools
Jul 3, 2025
Merged

Add required packaging and pipeline tools. Update Conan#4
bthomee merged 8 commits intomainfrom
add_required_tools

Conversation

@legleux
Copy link
Collaborator

@legleux legleux commented Jul 1, 2025

file utility is required for generating Debian package's dependencies.

gcovr is for the code coverage job.

Updates Conan version to latest 2.18.0 as of 07-01-25


with --no-install-recommends

0 upgraded, 100 newly installed, 0 to remove and 0 not upgraded.
Need to get 75.0 MB of archives.
After this operation, 311 MB of additional disk space will be used.

vs

0 upgraded, 249 newly installed, 0 to remove and 0 not upgraded.
Need to get 182 MB of archives.
After this operation, 723 MB of additional disk space will be used.

@legleux legleux requested a review from bthomee July 1, 2025 22:42
Copy link
Contributor

@bthomee bthomee left a comment

Choose a reason for hiding this comment

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

Can you also make the changes to the RHEL image?

@legleux legleux force-pushed the add_required_tools branch 4 times, most recently from ed12c19 to 009691d Compare July 2, 2025 04:18
@legleux legleux force-pushed the add_required_tools branch from 009691d to c6f6c52 Compare July 2, 2025 04:30
@legleux
Copy link
Collaborator Author

legleux commented Jul 2, 2025

@bthomee Maybe not?

@bthomee
Copy link
Contributor

bthomee commented Jul 2, 2025

@bthomee Maybe not?

In RHEL pipx isn't available, and there pip is the only alternative (but requires that weird workaround, even if installing as the non-root user). I pushed a commit here that fixes it.

Other changes made are to unset ARGs to require explicit setting, per previous suggestion of @mathbunnyru. I also updated the Gcovr version to 8.3, moved around a few env vars, and in RHEL added the dpkg-dev equivalents (namely, rpm-build and rpmdevtools) for consistency.

@bthomee bthomee requested a review from Bronek July 2, 2025 13:16
@bthomee
Copy link
Contributor

bthomee commented Jul 2, 2025

A follow-up PR I'll work on soon - probably next week - is to reduce duplication by moving the building, pushing, and merging into a local action, which then can be called by the three jobs.

Copy link
Collaborator

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

Overall, looks good, left some suggestions

@Bronek
Copy link
Collaborator

Bronek commented Jul 2, 2025

So far so good, but let's not fall into the trap of putting all potentially useful tools into a build image.

In this context, given that rippled has a clang-format workflow which needs very specific version 18.1.8 of clang-format, do we want to add clang-format with pip/pipx (thus decoupled from clang installation) ? I would argue that this should be best done in a separate image, for tools only (no choice of compilers and just one distro). I am not going to insist, though.

EDIT: One reason for having separate image for tools is that this would enable us to have different sets of tools for different projects to be used outside of build CI pipelines. That is, for build pipelines only, ideally clio and rippled should share images defined in this repo under debian rhel ubuntu (although obviously we are not going to get there quick, with clio still requiring Ubuntu Focal support). But there are also other pipelines which are project-specific (like mentioned above clang-format in rippled), for those there is no need for different projects to share the same CI containers.

In the same spirit, I am not 100% convinced we need gcovr in all these containers, but for now it's fine.

Copy link
Collaborator

@Bronek Bronek left a comment

Choose a reason for hiding this comment

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

Pls clean dnf cache in RHEL image; and apt cache in other two.

@bthomee
Copy link
Contributor

bthomee commented Jul 2, 2025

I cleaned the caches after each apt install or dnf install command. I also added the no-cache statement to pip install and put both Conan and Gcovr on the same line.

However, pipx does not seem to support installing multiple packages at once, despite pypa/pipx#1102 doing so:

[base 4/5] RUN pipx install conan==2.18.0 gcovr==8.3:                                                                                                                        
0.138 usage: pipx [-h] [--version]                                                                                                                                              
0.138             {install,inject,upgrade,upgrade-all,uninstall,uninstall-all,reinstall,reinstall-all,list,run,runpip,ensurepath,environment,completions}                       
0.138             ...
0.138 pipx: error: unrecognized arguments: gcovr==8.3

(removing the version pin results in the same error). There's also no no-cache option for the install command, only for the run command.

@bthomee
Copy link
Contributor

bthomee commented Jul 2, 2025

FWIW we should set up a package cache/proxy somewhere, since Ubuntu connections are currently timing out, suggesting that we've been throttled/blocked.

@mathbunnyru
Copy link
Collaborator

I also added the no-cache statement to pip install

Could you do it for pipx as well, please?
I'm not sure the flag will be the same, but there is probably something similar.

@bthomee
Copy link
Contributor

bthomee commented Jul 2, 2025

I also added the no-cache statement to pip install

Could you do it for pipx as well, please? I'm not sure the flag will be the same, but there is probably something similar.

As mentioned in my earlier comment, it is only available for pipx run, not for pipx install. See https://fig.io/manual/pipx/install.

@bthomee
Copy link
Contributor

bthomee commented Jul 2, 2025

I also added the no-cache statement to pip install

Could you do it for pipx as well, please? I'm not sure the flag will be the same, but there is probably something similar.

As mentioned in my earlier comment, it is only available for pipx run, not for pipx install. See https://fig.io/manual/pipx/install.

That being said... let me experiment with --pip-args.

@bthomee
Copy link
Contributor

bthomee commented Jul 2, 2025

I also added the no-cache statement to pip install

Could you do it for pipx as well, please? I'm not sure the flag will be the same, but there is probably something similar.

As mentioned in my earlier comment, it is only available for pipx run, not for pipx install. See https://fig.io/manual/pipx/install.

That being said... let me experiment with --pip-args.

I had to play a bit with it, but the command finally accepted --pip-args='--no-cache'. How can I verify that it did actually do what we want it to do?

@mathbunnyru
Copy link
Collaborator

I also added the no-cache statement to pip install

Could you do it for pipx as well, please? I'm not sure the flag will be the same, but there is probably something similar.

As mentioned in my earlier comment, it is only available for pipx run, not for pipx install. See https://fig.io/manual/pipx/install.

That being said... let me experiment with --pip-args.

I had to play a bit with it, but the command finally accepted --pip-args='--no-cache'. How can I verify that it did actually do what we want it to do?

You can use dive to take a look which files are getting added/deleted/modified on each docker command.
There should be less files when you don't use cache.

@Bronek
Copy link
Collaborator

Bronek commented Jul 2, 2025

EDIT: One reason for having separate image for tools is that this would enable us to have different sets of tools for different projects to be used outside of build CI pipelines. That is, for build pipelines only, ideally clio and rippled should share images defined in this repo under debian rhel ubuntu (although obviously we are not going to get there quick, with clio still requiring Ubuntu Focal support). But there are also other pipelines which are project-specific (like mentioned above clang-format in rippled), for those there is no need for different projects to share the same CI containers.

See DRAFT #5 for what I mean by above.

@bthomee
Copy link
Contributor

bthomee commented Jul 2, 2025

I also added the no-cache statement to pip install

Could you do it for pipx as well, please? I'm not sure the flag will be the same, but there is probably something similar.

As mentioned in my earlier comment, it is only available for pipx run, not for pipx install. See https://fig.io/manual/pipx/install.

That being said... let me experiment with --pip-args.

I had to play a bit with it, but the command finally accepted --pip-args='--no-cache'. How can I verify that it did actually do what we want it to do?

You can use dive to take a look which files are getting added/deleted/modified on each docker command. There should be less files when you don't use cache.

Yup, adding --pip-args='--no-cache' saved about 11MB in the image.

@Bronek Bronek requested a review from mathbunnyru July 3, 2025 08:45
Copy link
Collaborator

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

just a small style comment, but looks good overall

Copy link
Collaborator

@Bronek Bronek left a comment

Choose a reason for hiding this comment

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

Can you remove bizon and flex from the packages installed ? I do not think they are used for anything.

@bthomee
Copy link
Contributor

bthomee commented Jul 3, 2025

Can you remove bizon and flex from the packages installed ? I do not think they are used for anything.

@mathbunnyru requested them to be included, see #3 (comment). I propose just keeping them here, and then you can remove them in #5 by including them in a clio-tools image.

@Bronek Bronek self-requested a review July 3, 2025 13:42
@bthomee bthomee merged commit 0bc64db into main Jul 3, 2025
15 checks passed
@bthomee bthomee deleted the add_required_tools branch July 3, 2025 13:44
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.

4 participants