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

[VL] Disable protobuf build by default #6297

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

PHILO-HE
Copy link
Contributor

@PHILO-HE PHILO-HE commented Jul 1, 2024

What changes were proposed in this pull request?

Disable protobuf by default as it is not required for each build.
Also removed the build protobuf option from get_velox.sh since it is confusing and unnecessary. This option just controls whether to change velox's setup script. So it depends on the enabling of --run_setup_script.

How was this patch tested?

CI.

@PHILO-HE PHILO-HE changed the title [VL] Disable protobuf by default [VL] Disable protobuf build by default Jul 1, 2024
Copy link

github-actions bot commented Jul 1, 2024

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

@FelixYBW
Copy link
Contributor

FelixYBW commented Jul 1, 2024

when do we need run_setup_script to be enabled? is it used for dynamic link only, right?

@PHILO-HE
Copy link
Contributor Author

PHILO-HE commented Jul 2, 2024

when do we need run_setup_script to be enabled? is it used for dynamic link only, right?

@FelixYBW, yes, vcpkg build doesn't require this script.

@PHILO-HE PHILO-HE merged commit 7a3c129 into apache:main Jul 2, 2024
40 checks passed
@FelixYBW
Copy link
Contributor

FelixYBW commented Jul 2, 2024

with ep_cache=on, the protobuf is build every time when I build Velox. The PR can solve the issue, right?

@PHILO-HE
Copy link
Contributor Author

PHILO-HE commented Jul 3, 2024

with ep_cache=on, the protobuf is build every time when I build Velox. The PR can solve the issue, right?

@FelixYBW, this pr addresses the repeated build of protobuf by Gluten. If it's Velox that builds protobuf every time, it's because the required protobuf is not installed in the system. Does it happen in vcpkg build?

@FelixYBW
Copy link
Contributor

FelixYBW commented Jul 3, 2024

Does it happen in vcpkg build?

Yes

@PHILO-HE
Copy link
Contributor Author

PHILO-HE commented Jul 3, 2024

Does it happen in vcpkg build?

Yes

@FelixYBW, as enable_ep_cache=ON, I guess it's Gluten that triggers protobuf build every time. This pr should have fixed it already. If protobuf is still built every time, please let me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants