Skip to content
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

[GLUTEN-7458][VL] Upgrade to GCC-11 for centos-7/8 and ubuntu-20.04 #7578

Merged
merged 4 commits into from
Oct 24, 2024

Conversation

PHILO-HE
Copy link
Contributor

@PHILO-HE PHILO-HE commented Oct 17, 2024

What changes were proposed in this pull request?

Since the recently upgraded folly in Velox requires GCC >=10, we need to upgrade it for OSes whose default gcc package doesn't meet this requirement, such as centos-7/8 and ubuntu-20.04.

Also remove build tool setup for ubuntu-18.04 from setup-build-depends.sh, as it is not officially supported now.

How was this patch tested?

CI.

Copy link

#7458

@PHILO-HE PHILO-HE force-pushed the upgrade-gcc branch 2 times, most recently from 0afb29d to b90724c Compare October 17, 2024 13:14
@zhouyuan
Copy link
Contributor

zhouyuan commented Oct 17, 2024

@PHILO-HE should we upgrade the docker image in a separate PR, and tag it with gcc-11?
otherwise it may break some old tests

@PHILO-HE
Copy link
Contributor Author

@PHILO-HE should we upgrade the docker image in a separate PR, and tag it with gcc-11? otherwise it may break some old tests

@zhouyuan, maybe, we can directly install upgraded GCC in this pr with the old docker images used. After docker images are updated with new gcc installed, we can remove unnecessary installation from yml.

Regarding updating docker image, I think we should keep both gcc-9/11 installed. Then, use required gcc version by sourcing corresponding SCL devtoolset. This is for considering branch-1.2 GHA workflow, where gcc-9 is still used.

@zhouyuan
Copy link
Contributor

@PHILO-HE should we upgrade the docker image in a separate PR, and tag it with gcc-11? otherwise it may break some old tests

@zhouyuan, maybe, we can directly install upgraded GCC in this pr with the old docker images used. After docker images are updated with new gcc installed, we can remove unnecessary installation from yml.

Regarding updating docker image, I think we should keep both gcc-9/11 installed. Then, use required gcc version by sourcing corresponding SCL devtoolset. This is for considering branch-1.2 GHA workflow, where gcc-9 is still used.

Yes, that's the issue. We will need to modify the branch-1.2 also if image is updated with same name

@PHILO-HE PHILO-HE force-pushed the upgrade-gcc branch 4 times, most recently from 426f5fb to fd9b430 Compare October 18, 2024 03:22
@github-actions github-actions bot added the TOOLS label Oct 18, 2024
@@ -135,60 +134,18 @@ install_centos_8() {
install_maven_from_source
}

install_ubuntu_18.04() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this, thanks.

@PHILO-HE
Copy link
Contributor Author

@Yohahaha, do you have any comment?

@Yohahaha
Copy link
Contributor

@Yohahaha, do you have any comment?

no comments, feel free to merge this PR.

@PHILO-HE PHILO-HE merged commit da66b58 into apache:main Oct 24, 2024
48 checks passed
@wForget
Copy link
Member

wForget commented Oct 24, 2024

The CI https://github.com/apache/incubator-gluten/actions/runs/11499412404/job/32007324721?pr=7648 for branch 1.2 seems to be failing due to this change, do we have a solution?

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.

6 participants