-
Notifications
You must be signed in to change notification settings - Fork 456
Update Earthfile to use uvx cmake #2083
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
Conversation
kevinAlbs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with one comment. The Earthfile maintenance and improvements are much appreciated.
| "alpine3.16", | ||
| "alpine3.17", | ||
| "alpine3.18", | ||
| "u22", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate:
| "u22", |
vector-of-bool
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM, but pending comments regarding migrating away from CentOS.
Earthfile
Outdated
| RUN pipx install uv | ||
| ENV PATH="/root/.local/bin:$PATH" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: Can this be modified to install uv elsewhere? e.g. set PIPX_BIN_DIR=/opt/uv/bin or use --global to install to /usr/local/.... I don't actually feel strongly about this, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated all pipx commands to install to /opt/uv/bin instead.
| "centos9", | ||
| "centos10", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following Red Hat's (wildly unpopular) decision to kill CentOS, Rocky Linux and AlmaLinux were created to take up the mantle as "drop-in-replacements" for RHEL. It may be better to switch to Rocky or Alma rather than use CentOS Steram as stand-ins RHEL 9+. Rocky and Alma are also available in docker.io, allowing us to continue to use ECR.
(RedHat killing CentOS is also the reason why CentOS 8 EOL is earlier then 7: They kicked the project to the curb at version 8 and didn't want to support it any more).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This isn't a blocking request for changes, just an alternative to consider.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep CentOS Stream coverage for official Red Hat LTS compatibility coverage, but I'm fine with extending coverage to AlmaLinux as well, which cursory research suggests may have better community activity and support than Rocky Linux. Some additional context included below for reference.
Since AlmaLinux is meant to (almost) mirror CentOS Stream, a common CENTOS_STREAM_ENV is used to define both env.centos* and env.almalinux*.
Per Red Hat:
CentOS Stream will now be the sole repository for public RHEL-related source code releases.
Per AlmaLinux:
Red Hat announced they will no longer be providing the means for downstream clones to continue to be 1:1 binary copies of Red Hat Enterprise Linux (RHEL). [...] After much discussion, the AlmaLinux OS Foundation board today has decided to drop the aim to be 1:1 with RHEL. AlmaLinux OS will instead aim to be binary compatible with RHEL*.
* Binary/ABI compatibility in our case means working to ensure that applications built to run on RHEL (or RHEL clones) can run without issue on AlmaLinux. Adjusting to this expectation removes our need to ensure that everything we release is an exact copy of the source code that you would get with RHEL. This includes kernel compatibility and application compatibility.
The most remarkable potential impact of the change is that we will no longer be held to the line of “bug-for-bug compatibility” with Red Hat, and that means that we can now accept bug fixes outside of Red Hat’s release cycle. While that means some AlmaLinux OS users may encounter bugs that are not in Red Hat, we may also accept patches for bugs that have not yet been accepted upstream, or shipped downstream.
Per Rocky Linux:
Red Hat has recently expressed their perspective that they "do not find value in a RHEL rebuild." While we believe this view is narrow-minded, Red Hat has taken a strong stance and limited access to the sources for RHEL to only their paying customers.
Previously, we obtained the source code for Rocky Linux exclusively from the CentOS Git repository as they recommended. However, this repository no longer hosts all of the versions corresponding to RHEL. Consequently, we now have to gather the source code from multiple sources, including CentOS Stream, pristine upstream packages, and RHEL SRPMs. [...] we refuse to agree with them, which means we must obtain the SRPMs through channels that adhere to our principles and uphold our rights.
vector-of-bool
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR replaces
find-cmake-latest.shwithuvx cmakefor Earthly tasks. To supportuvx cmake, this PR also updates the Earthfile to migrate away from old/EOL/unsupported operating systems. A followup PR will migrate the rest of the EVG configuration to useuvto obtain preferred build tools as done in mongodb/mongo-cxx-driver#1428.First, concerning the current Earthfile, the following operating system versions are removed due to being EOL:
Note
Although we do not have our own platform support policy document, both Evergreen and Server do not include "Extended Security Maintenance" mode in their range of support. Therefore, I do not think we should include them either.
These EOL versions are replaced with a full set of up-to-date and supported versions for their respective OS: up to Alpine Linux 3.22, Ubuntu 24.04, and CentOS Stream 10 (more on this below).
Second, to avoid the need to copy mongo-c-driver scripts into the Earthly environment, to better test OS compatibility, and to take advantage of Earthly containerization, the OS package managers are used to acquire
uvinstead of auv-installer.shscript (which will be needed by EVG tasks). This led to some complications with usingsnap/systemdin containers, wheresnapis typically the OS-recommended alternative to obtain a missing or newer package than what is available via the system package manager. However,pipxis available as the second uv-recommended method to installuv(aftercurl | sh) on all relevant distros as a system package (whenuvitself is not available via the system package manager), and so it is used instead when needed.Third, Red Hat apparently ended support for CentOS in 2024 in favor of CentOS Stream (confusingly not the same thing as CentOS, but more or less its equivalent successor). Red Hat strongly recommends against continuing to use CentOS as it is "now exposed to significant risks". As part of this migration, official OCI images for CentOS Stream are now provided via quay.io rather than DockerHub. Therefore, the CentOS Stream images are specified using fully-qualified image names to
quay.ioinstead of using$default_search_registry(at this time, the DevProd-provided Amazon ECR instance does not seem to support pull-through caching ofquay.ioimages, only DockerHub images).This PR also includes a drive-by addition of a
-Wpoison-system-directoriessuppression with Apple Clang for thepublic-header-warningstarget:This warning is strange. I do not know why the changes in this PR (or the upcoming PR to apply
uvto EVG scripts, I forget which) seem to trigger this warning. According to this comment, it may be related to the implicit use of the--sysrootflag on recent versions of macOS...? (There is no explicit--sysrootflag in the compilation command above.) We are definitely not attempting any cross-compilation in our macOS EVG tasks (that I am aware of), so I've opted to treat this as a false-positive and disabled it.