Skip to content

[VL] Fix gflags mode in folly on Centos9#12265

Merged
zhouyuan merged 1 commit into
apache:mainfrom
zhouyuan:wip_fix_centos9_folly_build
Jun 9, 2026
Merged

[VL] Fix gflags mode in folly on Centos9#12265
zhouyuan merged 1 commit into
apache:mainfrom
zhouyuan:wip_fix_centos9_folly_build

Conversation

@zhouyuan

@zhouyuan zhouyuan commented Jun 8, 2026

Copy link
Copy Markdown
Member

What changes are proposed in this pull request?

Remove the workaround for folly gflags mode on centos9 image introduced in #12223

How was this patch tested?

pass GHA

Was this patch authored or co-authored using generative AI tooling?

Copilot AI review requested due to automatic review settings June 8, 2026 18:07
@zhouyuan zhouyuan changed the title [VL] Fix gflags mdoe in folly on Centos9 [VL] Fix gflags mode in folly on Centos9 Jun 8, 2026
on dynamic lib testing env, the gflags_shared need to enabled in folly

Signed-off-by: Yuan <yuanzhou@apache.org>
@zhouyuan zhouyuan force-pushed the wip_fix_centos9_folly_build branch from 248258c to 55321d9 Compare June 8, 2026 18:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR removes CentOS 9–specific workaround logic that was patching Velox/Folly’s gflags linkage behavior in CI and in the Velox fetch/setup helper, and instead relies on upstream Velox’s VELOX_BUILD_SHARED mechanism when building the “dynamic-build” Docker images. This aligns Gluten’s build with the intended upstream configuration path and drops now-stale post-install sed patches in workflows.

Changes:

  • Remove the CentOS9 sed patch in get-velox.sh that forced -DGFLAGS_SHARED=TRUE in Velox’s scripts/setup-common.sh.
  • Set VELOX_BUILD_SHARED=ON during CentOS8/9 dynamic Docker image builds to drive upstream Velox dependency builds in shared mode.
  • Remove CI workflow sed hacks that rewrote Folly CMake targets from gflags_static to gflags_shared.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
ep/build-velox/src/get-velox.sh Drops the CentOS9-specific patching of Velox’s setup script.
dev/docker/Dockerfile.centos9-dynamic-build Exports VELOX_BUILD_SHARED=ON for shared-deps builds in the CentOS9 dynamic image.
dev/docker/Dockerfile.centos8-dynamic-build Exports VELOX_BUILD_SHARED=ON for shared-deps builds in the CentOS8 dynamic image.
.github/workflows/velox_backend_x86.yml Removes the Folly targets sed workaround from the x86 CI job.
.github/workflows/velox_backend_arm.yml Removes the Folly targets sed workaround from the ARM CI job.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@zhouyuan zhouyuan requested a review from marin-ma June 8, 2026 18:12
fi; \
cd /opt/gluten; \
source /opt/rh/gcc-toolset-12/enable; \
export VELOX_BUILD_SHARED=ON; \

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@marin-ma marin-ma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks!

@zhouyuan zhouyuan merged commit 06e0008 into apache:main Jun 9, 2026
59 checks passed
@prestodb-ci

prestodb-ci commented Jun 9, 2026

Copy link
Copy Markdown

Rebase job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants