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): Clean up various issues #11751

Closed
wants to merge 11 commits into from

Conversation

assignUser
Copy link
Collaborator

@assignUser assignUser commented Dec 5, 2024

I have pulled these changes from #10732 as they are not specifically part of the shared build.

  • Add [<dependency name>] as an indicator that a dependency is running CMake. This makes it much easier to parse the log and track issues:
-- [simdjson] Using BUNDLED simdjson
-- Setting folly source to BUNDLED
-- [folly] Building Folly from source
-- [folly] Using Boost - Bundled
  • use targets instead of variables for Folly::follybenchmark and glog::glog
  • remove explicit linking of std::fs as this is no longer necessary for gcc >= 9 (our minimum is 11)
  • unify generation and linking of dwrf protobuf files
  • don't set C++ std for dependencies

@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 Dec 5, 2024
Copy link

netlify bot commented Dec 5, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 43285c4
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6752184e1e7b5e0008beb60b

@assignUser assignUser marked this pull request as ready for review December 5, 2024 02:18
@assignUser
Copy link
Collaborator Author

The test is failing on main

@assignUser
Copy link
Collaborator Author

I have moved the wrapped proto files into velox_dwrf_common as they are never used standalone, so there is no reason to make a standalone library.

@facebook-github-bot
Copy link
Contributor

@kgpai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@kgpai
Copy link
Contributor

kgpai commented Dec 6, 2024

@majetideepak cc

@facebook-github-bot
Copy link
Contributor

@kgpai merged this pull request in 24b41e5.

Copy link
Collaborator

@majetideepak majetideepak left a comment

Choose a reason for hiding this comment

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

Lazy +1

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. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants