-
Notifications
You must be signed in to change notification settings - Fork 92
feat(core): Add support for building and running on manylinux_2_28; Add manylinux_2_28 dependency container images. #1041
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
Changes from 3 commits
64b7d8d
917cecd
53f5974
73563d5
0b81039
5ab5946
117ff00
9112825
b18c2f3
c6aea71
258ba7b
ab09ee3
af88773
8378fdb
9c5c624
c31a6c0
dfa89e9
c744237
8cb738f
a35809d
b1716df
e916ecb
e1809e8
502a31c
f96a13a
10dc855
02d257d
31e9874
8fd2fa0
57cda64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| FROM quay.io/pypa/manylinux_2_28_aarch64 | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| # Binaries built on manylinux_2_28 (AlmaLinux 8 based) is expected to be | ||
| # compatible with other distros using glibc 2.28 or later, including: | ||
| # - Debian 10+ | ||
| # - Ubuntu 18.10+ | ||
| # - Fedora 29+ | ||
| # - CentOS/RHEL 8+ | ||
|
|
||
| WORKDIR /root | ||
|
|
||
| RUN mkdir -p ./tools/scripts/lib_install | ||
| ADD ./tools/scripts/lib_install ./tools/scripts/lib_install | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| RUN ./tools/scripts/lib_install/manylinux_2_28/install-all.sh | ||
|
|
||
| # Remove cached files | ||
| RUN dnf clean all && rm -rf /var/cache/dnf /tmp/* /var/tmp/* | ||
|
|
||
| # DO NOT FLATTEN THE IMAGE!!! | ||
| # The consequence of flattening the image is that we lose shell startup scripts | ||
| # and environment modification from /etc/profile.d, /etc/bashrc, etc. | ||
| # These are important for manylinux, otherwise, default system compilers are used, etc. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider adding a non-root user Building images as 🧰 Tools🪛 Checkov (3.2.334)[LOW] 1-23: Ensure that HEALTHCHECK instructions have been added to container images (CKV_DOCKER_2) [LOW] 1-23: Ensure that a user for the container has been created (CKV_DOCKER_3) 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| # Get the directory this script is in | ||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||
| component_root=${script_dir}/../../../ | ||
|
|
||
| # Build aarch64 image, will automatically use QEMU emulate if not on native platform | ||
| build_cmd=( | ||
| docker buildx build | ||
| --platform linux/arm64 | ||
| --tag clp-core-dependencies-manylinux_2_28_aarch64:dev | ||
| "$component_root" | ||
| --file "${script_dir}/Dockerfile" | ||
| --load | ||
| ) | ||
|
|
||
| # If in a git repo, add labels for revision and source | ||
| if command -v git >/dev/null && git -C "$script_dir" rev-parse --is-inside-work-tree >/dev/null ; | ||
| then | ||
| build_cmd+=( | ||
| --label "org.opencontainers.image.revision=$(git -C "$script_dir" rev-parse HEAD)" | ||
| --label "org.opencontainers.image.source=$(git -C "$script_dir" remote get-url origin)" | ||
| ) | ||
| fi | ||
|
|
||
| echo "Running: ${build_cmd[*]}" | ||
|
|
||
| # Execute the docker buildx command | ||
| "${build_cmd[@]}" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| FROM quay.io/pypa/manylinux_2_28_x86_64 | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| # Binaries built on manylinux_2_28 (AlmaLinux 8 based) is expected to be | ||
| # compatible with other distros using glibc 2.28 or later, including: | ||
| # - Debian 10+ | ||
| # - Ubuntu 18.10+ | ||
| # - Fedora 29+ | ||
| # - CentOS/RHEL 8+ | ||
|
|
||
| WORKDIR /root | ||
|
|
||
| RUN mkdir -p ./tools/scripts/lib_install | ||
| ADD ./tools/scripts/lib_install ./tools/scripts/lib_install | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| RUN ./tools/scripts/lib_install/manylinux_2_28/install-all.sh | ||
|
|
||
| # Remove cached files | ||
| RUN dnf clean all && rm -rf /var/cache/dnf /tmp/* /var/tmp/* | ||
|
|
||
| # DO NOT FLATTEN THE IMAGE!!! | ||
| # The consequence of flattening the image is that we lose shell startup scripts | ||
| # and environment modification from /etc/profile.d, /etc/bashrc, etc. | ||
| # These are important for manylinux, otherwise, default system compilers are used, etc. | ||
|
jackluo923 marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| # Get the directory this script is in | ||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||
| component_root=${script_dir}/../../../ | ||
|
|
||
| # Build x86_64 image, will automatically use QEMU emulate if not on native platform | ||
| build_cmd=( | ||
| docker buildx build | ||
| --platform linux/amd64 | ||
| --tag clp-core-dependencies-manylinux_2_28_x86_64:dev | ||
| "$component_root" | ||
| --file "${script_dir}/Dockerfile" | ||
| --load | ||
| ) | ||
|
Comment on lines
+10
to
+
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Image context may be unintentionally broad Using 🤖 Prompt for AI Agents |
||
|
|
||
| # If in a git repo, add labels for revision and source | ||
| if command -v git >/dev/null && git -C "$script_dir" rev-parse --is-inside-work-tree >/dev/null ; | ||
| then | ||
| build_cmd+=( | ||
| --label "org.opencontainers.image.revision=$(git -C "$script_dir" rev-parse HEAD)" | ||
| --label "org.opencontainers.image.source=$(git -C "$script_dir" remote get-url origin)" | ||
| ) | ||
| fi | ||
|
|
||
| echo "Running: ${build_cmd[*]}" | ||
|
|
||
| # Execute the docker buildx command | ||
| "${build_cmd[@]}" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # Exit on any error | ||
| set -e | ||
|
|
||
| # Error on undefined variable | ||
| set -u | ||
|
|
||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||
|
|
||
|
Comment on lines
+3
to
+7
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Declare The value of -script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+readonly script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"🤖 Prompt for AI Agents |
||
| "${script_dir}/install-prebuilt-packages.sh" | ||
| "${script_dir}/install-packages-from-source.sh" | ||
|
|
||
| # TODO: https://github.com/y-scope/clp/issues/795 | ||
| "${script_dir}/../check-cmake-version.sh" | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,19 @@ | ||||||||||||||||||||||
| #!/usr/bin/env bash | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Exit on any error | ||||||||||||||||||||||
| set -e | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| # Error on undefined variable | ||||||||||||||||||||||
| set -u | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
jackluo923 marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||
| script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" | ||||||||||||||||||||||
| lib_install_scripts_dir="${script_dir}/.." | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
Comment on lines
+3
to
+8
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Declare constants as
-script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
-lib_install_scripts_dir="${script_dir}/.."
+readonly script_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+readonly lib_install_scripts_dir="${script_dir}/.."📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||
| # NOTE: The remaining installation scripts depend on boost, so we install it beforehand. | ||||||||||||||||||||||
| "${lib_install_scripts_dir}"/install-boost.sh 1.87.0 | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| "$lib_install_scripts_dir"/liblzma.sh 5.8.1 | ||||||||||||||||||||||
| "$lib_install_scripts_dir"/lz4.sh 1.10.0 | ||||||||||||||||||||||
| "$lib_install_scripts_dir"/zstandard.sh 1.5.7 | ||||||||||||||||||||||
| "$lib_install_scripts_dir"/libarchive.sh 3.8.0 | ||||||||||||||||||||||
| "${lib_install_scripts_dir}"/msgpack.sh 7.0.0 | ||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| #!/usr/bin/env bash | ||
|
|
||
| # Exit on any error | ||
| set -e | ||
|
|
||
| # Error on undefined variable | ||
| set -u | ||
|
|
||
|
jackluo923 marked this conversation as resolved.
Outdated
|
||
| dnf install -y \ | ||
| gcc-c++ \ | ||
| java-11-openjdk \ | ||
| libcurl-devel \ | ||
| mariadb-connector-c-devel \ | ||
| openssl-devel \ | ||
| zlib-devel \ | ||
| zlib-static \ | ||
| jq | ||
|
|
||
| # Determine architecture for `task` release to install | ||
| rpm_arch=$(rpm --eval "%{_arch}") | ||
| case "$rpm_arch" in | ||
| "x86_64") | ||
| task_pkg_arch="amd64" | ||
| ;; | ||
| "aarch64") | ||
| task_pkg_arch="arm64" | ||
| ;; | ||
| *) | ||
| echo "Error: Unsupported architecture - $rpm_arch" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
||
| # Install `task` | ||
| # NOTE: We lock `task` to a version < 3.43 to avoid https://github.com/y-scope/clp/issues/872 | ||
| task_pkg_path="$(mktemp -t --suffix ".rpm" task-pkg.XXXXXXXXXX)" | ||
| curl \ | ||
| --fail \ | ||
| --location \ | ||
| --output "$task_pkg_path" \ | ||
| --show-error \ | ||
| "https://github.com/go-task/task/releases/download/v3.42.1/task_linux_${task_pkg_arch}.rpm" | ||
| dnf install --assumeyes "$task_pkg_path" | ||
| rm "$task_pkg_path" | ||
|
|
||
|
jackluo923 marked this conversation as resolved.
|
||
| # The bundled CMake 4.x version is too new; downgrading to CMake 3.27 is | ||
| # necessary to resolve build issues with third-party libraries. Even after | ||
| # we added the "-DCMAKE_POLICY_VERSION_MINIMUM=3.5" flag, compilation issues | ||
| # in libraries such as googletest-src and yaml-cpp still persist. | ||
| # See sample error below: | ||
| # CMake Error at _deps/googletest-src/CMakeLists.txt:4 (cmake_minimum_required): | ||
| # Support for CMake versions below 3.5 has been dropped. | ||
| # | ||
| # Adjust the VERSION argument’s minimum value, or use the <min>...<max> syntax | ||
| # to indicate the required CMake version range. | ||
| # | ||
| # Alternatively, you can try adding -DCMAKE_POLICY_VERSION_MINIMUM=3.5 | ||
| export PLATFORM=$(uname -m) | ||
|
jackluo923 marked this conversation as resolved.
Outdated
|
||
| export CMAKE_VERSION=3.27.9 | ||
| export CMAKE_RELEASE_URL=https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION}/cmake-${CMAKE_VERSION}-linux-${PLATFORM}.tar.gz | ||
| echo "Downgrading CMAKE to 3.27.9" | ||
| curl -L ${CMAKE_RELEASE_URL} | tar -xzf - | ||
| mv cmake-${CMAKE_VERSION}-linux-${PLATFORM} /opt/cmake-${CMAKE_VERSION} | ||
| ln -sf /opt/cmake-${CMAKE_VERSION}/bin/cmake /usr/local/bin/cmake | ||
|
jackluo923 marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,15 @@ | ||||||||||||||||||||||||||||||||||||||
| This directory contains uncategorized utility scripts. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| * `build-and-run-unit-tests.sh` can be used to build all executables and run | ||||||||||||||||||||||||||||||||||||||
| the unit tests. | ||||||||||||||||||||||||||||||||||||||
| * `build.py` can be used to trigger a local platform build | ||||||||||||||||||||||||||||||||||||||
| * `build-with-docker.py` can be used to build all targets within a docker | ||||||||||||||||||||||||||||||||||||||
| image for x86_64 and arm64 | ||||||||||||||||||||||||||||||||||||||
| * manylinux_2_28 based images are used and the output binaries are | ||||||||||||||||||||||||||||||||||||||
| expected to be compatible with Debian 10+, Ubuntu 18.10+, Fedora 29+, | ||||||||||||||||||||||||||||||||||||||
| CentOS/RHEL 8+ and other platforms. | ||||||||||||||||||||||||||||||||||||||
| * Command to enable docker emulation: | ||||||||||||||||||||||||||||||||||||||
| `docker run --rm --privileged multiarch/qemu-user-static --reset -p yes` | ||||||||||||||||||||||||||||||||||||||
| * `build-and-run-unit-tests.py` can be used to perform a local platform | ||||||||||||||||||||||||||||||||||||||
| build of all executables and run the unit tests. | ||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Clean up list formatting & punctuation Improve grammar, remove trailing spaces, and conform to markdown-lint indentation. -* `build.py` can be used to trigger a local platform build
-* `build-with-docker.py` can be used to build all targets within a docker
- image for x86_64 and arm64
- * manylinux_2_28 based images are used and the output binaries are
- expected to be compatible with Debian 10+, Ubuntu 18.10+, Fedora 29+,
- CentOS/RHEL 8+ and other platforms.
- * Command to enable docker emulation:
- `docker run --rm --privileged multiarch/qemu-user-static --reset -p yes`
-* `build-and-run-unit-tests.py` can be used to perform a local platform
- build of all executables and run the unit tests.
+* `build.py` – trigger a build for the host platform.
+* `build-with-docker.py` – build all targets inside a Docker image for
+ both x86_64 and arm64.
+ * manylinux_2_28–based images are used, and the resulting binaries are
+ compatible with Debian 10+, Ubuntu 18.10+, Fedora 29+, CentOS/RHEL 8+, etc.
+ * To enable QEMU emulation run:
+ `docker run --rm --privileged multiarch/qemu-user-static --reset -p yes`
+* `build-and-run-unit-tests.py` – build all executables locally and run unit tests.📝 Committable suggestion
Suggested change
🧰 Tools🪛 LanguageTool[style] ~3-~3: To form a complete sentence, be sure to include a subject. (MISSING_IT_THERE) [uncategorized] ~6-~6: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2) [style] ~11-~11: To form a complete sentence, be sure to include a subject. (MISSING_IT_THERE) 🪛 markdownlint-cli2 (0.17.2)4-4: Trailing spaces (MD009, no-trailing-spaces) 6-6: Unordered list indentation (MD007, ul-indent) 6-6: Trailing spaces (MD009, no-trailing-spaces) 9-9: Unordered list indentation (MD007, ul-indent) 9-9: Trailing spaces (MD009, no-trailing-spaces) 11-11: Trailing spaces (MD009, no-trailing-spaces) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| * `run-in-container.sh` can be used to run a command (e.g., `make`), in a | ||||||||||||||||||||||||||||||||||||||
| container containing the core component's dependencies. | ||||||||||||||||||||||||||||||||||||||
| * All commands are run from the root of the core component. | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
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.
🛠️ Refactor suggestion
❓ Verification inconclusive
AlmaLinux detection can mis-classify other CentOS derivatives
file(READ "/etc/centos-release" …)will succeed on Rocky, Oracle and other RHEL8 clones that still ship/etc/centos-release.Those distributions will fail the
string(FIND … "AlmaLinux" …)check and be treated as “unsupported”, even though the toolchain is identical to AlmaLinux and static linking works.Consider matching the Red Hat release minor or looking for
/etc/os-releaseID_LIKE="rhel"instead.AlmaLinux detection can mis-classify other CentOS derivatives
file(READ "/etc/centos-release" …)will succeed on Rocky, Oracle and other RHEL8 clones that still ship/etc/centos-release.Those distributions will fail the
string(FIND … "AlmaLinux" …)check and be treated as “unsupported”, even though the toolchain is identical to AlmaLinux and static linking works.Consider matching the Red Hat release minor or looking for
/etc/os-releaseID_LIKE="rhel"instead.📝 Committable suggestion
🤖 Prompt for AI Agents