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

build(cmake): Add VELOX_BUILD_FUZZERS option #10802

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

majetideepak
Copy link
Collaborator

@majetideepak majetideepak commented Aug 21, 2024

Fixes: #11797

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 21, 2024
Copy link

netlify bot commented Aug 21, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit ca3a22f
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66c6408041c72400089bd49e

@@ -461,8 +461,13 @@ add_compile_definitions(FOLLY_HAVE_INT128_T=1)
set_source(folly)
resolve_dependency(folly)

if(${VELOX_BUILD_TESTING})
# Spark qury runner depends on absl, gRPC.
if(VELOX_BUILD_FUZZERS)
Copy link
Collaborator

@rui-mo rui-mo Aug 22, 2024

Choose a reason for hiding this comment

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

Is the fuzzer considered part of testing? Wondering if it makes sense to check both VELOX_BUILD_TESTING and VELOX_BUILD_FUZZERS. Thanks.

Copy link
Collaborator Author

@majetideepak majetideepak Aug 22, 2024

Choose a reason for hiding this comment

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

My thinking is that fuzzers and unit tests should be independent.
VELOX_BUILD_TESTING should enable/disable unit tests.
VELOX_BUILD_FUZZERS should enable/disable fuzzers.

Copy link
Collaborator

Choose a reason for hiding this comment

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

While we might build fuzzers when we also build testing it does make sense to make them independent, would save us build time in the fuzzer builds and test builds respectivly.

Copy link

stale bot commented Nov 22, 2024

This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the PR, make sure you've addressed reviewer comments, and rebase on the latest main. Thank you for your contributions!

@stale stale bot added the stale label Nov 22, 2024
@majetideepak majetideepak changed the title Add VELOX_BUILD_FUZZERS option build(cmake): Add VELOX_BUILD_FUZZERS option Dec 9, 2024
@stale stale bot removed the stale label Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move fuzzer dependencies under CMake option to improve build time
4 participants