Conversation
.github/workflows/rhel.yml
Outdated
| - os: 9.6 | ||
| gcc: 13 | ||
| - os: 9.6 | ||
| gcc: 14 |
There was a problem hiding this comment.
Shouldn't we also support 9.4 ? There's with GCC Toolset 13 for this release.
There was a problem hiding this comment.
There's no UBI base/tool image for 9.4, unfortunately. I can recreate it by looking at the RHEL UBI dockerfile and pull the same untagged image they use by its SHA, but I'm not so sure it's worth it given that we can support version 10 soon & supporting the last 2 versions should be more than adequate.
There was a problem hiding this comment.
I can see https://catalog.redhat.com/software/containers/ubi9/ubi/615bcf606feffc5384e8452e?image=671a4613385185df173cf395&architecture=amd64 , but it's moot if there's no gcc version more recent than 11 (I do not know if there is)
There was a problem hiding this comment.
Oh yeah I saw that one, but for some reason it didn't want to pull down with the 9.4 tag. Let me try again.
I'm using the s2i-base image - I didn't see one with the 9.4 tag. I recall there's one with a SHA but that made referencing it harder & it seems iffy if it doesn't have a proper tag.
|
Instead of switching to name |
Done for |
|
@mathbunnyru sent me the following to have the images conform to the following, so they can also be used in
|
| @@ -0,0 +1,93 @@ | |||
| name: RHEL | |||
There was a problem hiding this comment.
I think we should create a reusable workflow and just call it from these workflows, instead of copying files with minor changes
There was a problem hiding this comment.
I suggest a separate PR after this one; also if you could prepare it that would be great.
There was a problem hiding this comment.
... also, I am not convinced that the complexity would be justified (we have different set of compilers for each distro). This repo does not do anything else but just keep ci container images, I think having separate workflow for each supported Linux distro is a nice readability feature.
I wonder if it would be also simpler. I thought maybe not (hence the older version of this comment, crossed out above) but on second thought ... maybe yes ? Please show us 🙂
| # Install Conan. | ||
| ARG CONAN_VERSION | ||
| RUN pip install conan==${CONAN_VERSION} |
There was a problem hiding this comment.
Probably can be installed for a ci user only
There was a problem hiding this comment.
Unfortunately I still can't get it to work.
Even something like this:
USER ${NONROOT_USER}
WORKDIR /home/${NONROOT_USER}
ARG CONAN_VERSION
RUN pip install conan==${CONAN_VERSION}
USER root
keeps trying to install it in a root-owned directory:
> [base 8/8] RUN pip install conan==2.17.0:
0.189 WARNING: The directory '/opt/app-root/src/.cache/pip' or its parent directory is not owned or is not writable by the current user. The cache has been disabled. Check the permissions and owner of that directory. If executing pip with sudo, you should use sudo's -H flag.
ERROR: Could not install packages due to an OSError: [Errno 13] Permission denied: '/opt/app-root/src/.local'`
Should pip itself be installed differently?
| # Fix the Conan user home directory as it otherwise will point to the | ||
| # /opt/app-root/src/.conan2 directory. | ||
| ENV CONAN_HOME=/home/${NONROOT_USER}/.conan2 |
There was a problem hiding this comment.
Who sets it to /opt/app-root/src/.conan2?
Probably should remove it, instead of patching it here
There was a problem hiding this comment.
I was surprised too that the home directory didn't update after switching to the non-root user. At this point I don't know why it retains the root user's directory - I'm not very familiar with how the Red Hat / Fedora family does things.
There was a problem hiding this comment.
I don't see how it is set at all?
There was a problem hiding this comment.
@bthomee I encourage you to install conan python package for non-root user, and it's likely you won't have to change this at all.
| # Fix the Conan user home directory as it otherwise will point to the | ||
| # /opt/app-root/src/.conan2 directory. | ||
| ENV CONAN_HOME=/home/${NONROOT_USER}/.conan2 |
There was a problem hiding this comment.
This is done for both gcc/clang, if you can't remove it, at least I think we should move it to base
There was a problem hiding this comment.
It feels awkward to me to set an environment variable to the home directory of a different user (i.e. the non-root user) than the one that is currently executing the commands (i.e. the root user). If you don't think it matters, I can move this up.
There was a problem hiding this comment.
But you can switch to non-root user in base as well
I've checked and EOF statements are not always in the same style.
I don't know which packages you use in rippled, so I can't help here
👌
I think that's because you don't build aarch64 images and we do.
👌
https://github.com/XRPLF/clio/blob/develop/docker/tools/Dockerfile
This is how we build our GCC: https://github.com/XRPLF/clio/tree/develop/docker/compilers/gcc
https://github.com/XRPLF/clio/blob/develop/.github/workflows/update_docker_ci.yml#L103
https://github.com/XRPLF/clio/blob/develop/.github/workflows/update_docker_ci.yml#L103 |
I do not see
I think this means we need to build them as well, if we want
This old version won't be supported by future releases of
So this similar to how we add gcc to our
Another example here https://github.com/libfn/functional/blob/main/.github/workflows/ci-build.yml |
This is more or less how I handled the multi-arch builds when it was in GitLab. I had some issues getting the tags correct with the GitHub action a while back, maybe it works better now. https://gist.github.com/legleux/d7e023232415eef6f76c219623dd5aa6 |
f4bfed4 to
056bef5
Compare
thanks, this now works. |
Addressed main comments. Follow-up PR can address any remaining issues.
f168340 to
6ba2d3c
Compare
This PR adds Docker images for Red Hat Enterprise Linux (RHEL) 9.6 with supported GCC or Clang compilers.
dnfpackage manager with a very restricted set of repos and thus available packages to install.Small improvements are made to the Debian and Ubuntu images as well for consistency.