Skip to content

Fix protobuf builds when Velox is used as a sub project. #3062

Closed
kgpai wants to merge 1 commit intofacebookincubator:mainfrom
kgpai:fix_proto_again
Closed

Fix protobuf builds when Velox is used as a sub project. #3062
kgpai wants to merge 1 commit intofacebookincubator:mainfrom
kgpai:fix_proto_again

Conversation

@kgpai
Copy link
Contributor

@kgpai kgpai commented Nov 2, 2022 •
edited
Loading

Using CMAKE_BINARY_DIR instead of PROJECT_BINARY_DIR allows us to build protos even in cases when velox is a subproject.

@netlify
Copy link

netlify bot commented Nov 2, 2022 •
edited
Loading

✅ Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit f66f0f8
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/6362917db16a1c000881e9b2

@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 Nov 2, 2022
@kgpai kgpai changed the title [WIP] Add fix for protobuf builds on presto_cpp. Fix protobuf builds when Velox is used as a sub project. Nov 2, 2022
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Nov 2, 2022

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

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Nov 2, 2022

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

gggrace14
gggrace14 approved these changes Nov 2, 2022
View reviewed changes
Copy link
Contributor

@gggrace14 gggrace14 left a comment

Choose a reason for hiding this comment

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

Great investigation and fix!

@mbasmanova
Copy link
Contributor

mbasmanova commented Nov 3, 2022

@kgpai I'm getting an error building Prestissimo

/Users/mbasmanova/java/presto/presto-native-execution/cmake-build-debug/velox/velox/dwio/dwrf/proto/dwrf_proto.pb.cc:4:10: fatal error: 'velox/velox/dwio/dwrf/proto/dwrf_proto.pb.h' file not found
#include "velox/velox/dwio/dwrf/proto/dwrf_proto.pb.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/mbasmanova/java/presto/presto-native-execution/cmake-build-debug/velox/velox/dwio/dwrf/proto/orc_proto.pb.cc:4:10: fatal error: 'velox/velox/dwio/dwrf/proto/orc_proto.pb.h' file not found
#include "velox/velox/dwio/dwrf/proto/orc_proto.pb.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Wondering if there is a fix or workaround.

@kgpai
Copy link
Contributor Author

kgpai commented Nov 3, 2022

@mbasmanova Investigating.

@amitkdutta
Copy link
Contributor

amitkdutta commented Nov 4, 2022

@kgpai I am seeing the same error as well in Presto C++

/Users/adutta/Documents/code/presto/presto-native-execution/cmake-build-debug/velox/velox/dwio/dwrf/proto/dwrf_proto.pb.cc:4:10: fatal error: 'velox/velox/dwio/dwrf/proto/dwrf_proto.pb.h' file not found
#include "velox/velox/dwio/dwrf/proto/dwrf_proto.pb.h"

@gggrace14 gggrace14 mentioned this pull request Nov 5, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gggrace14 gggrace14 gggrace14 approved these changes

@kevinwilfong kevinwilfong Awaiting requested review from kevinwilfong

@pedroerp pedroerp Awaiting requested review from pedroerp

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kgpai @facebook-github-bot @mbasmanova @amitkdutta @gggrace14