[GLUTEN-12313][VL] followup to cleanup CI image#12384
Conversation
Signed-off-by: Yuan <yuanzhou@apache.org>
This reverts commit c3d0cdb.
There was a problem hiding this comment.
Pull request overview
This PR updates Gluten’s CI/Docker image build setup by removing some no-longer-used Docker image build jobs and adjusting how CMake is provisioned in CentOS-based build environments (notably for vcpkg images).
Changes:
- Remove Docker image build jobs for
vcpkg-centos-7andvcpkg-centos-8, and update the manifest-merge job accordingly. - Update CentOS 9 static-build image to install CMake via the OS package manager instead of pip.
- Update CentOS 8 GCC13 static-build image to install a specific Kitware CMake release via the
.shinstaller, and remove pip-based CMake install from the CentOS 8/9 vcpkg dependency setup script.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| dev/vcpkg/setup-build-depends.sh | Removes pip-pinned CMake install for CentOS 8/9 dependency setup. |
| dev/docker/Dockerfile.centos9-static-build | Switches CMake provisioning from pip-pinned to distro cmake package. |
| dev/docker/Dockerfile.centos8-gcc13-static-build | Switches to installing a pinned Kitware CMake .sh installer. |
| .github/workflows/docker_image.yml | Removes unused image build jobs and updates digest/manifest merge to match. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
philo-he
left a comment
There was a problem hiding this comment.
LGTM. Thanks.
Do we need to remove all other centos-7 based images, e.g., build-vcpkg-centos-7-gcc13?
|
vcpkg-centos-7-gcc13 is in use https://github.com/apache/gluten/blob/main/.github/workflows/velox_backend_x86.yml#L71 We should be safe to remove it after migrated CI to almalinux-8 |
What changes are proposed in this pull request?
This patch removed the unused docker images. Also fixed the cmake issue in vcpkg-centos-8/9 image
How was this patch tested?
manually verified
Was this patch authored or co-authored using generative AI tooling?
Related issue: #12313