-
Notifications
You must be signed in to change notification settings - Fork 254
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
feat: make it possible to build lance without protoc (except on Windows) #3363
Conversation
@@ -59,7 +59,7 @@ jobs: | |||
env: | |||
# Need up-to-date compilers for kernels | |||
CC: clang | |||
CXX: clang | |||
CXX: clang++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦
I thought the change was pushing us into "too long to build" but it looks like we were already riding that line. Turns out we were building the tests twice I think on stable builds so I disabled the second build. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3363 +/- ##
==========================================
- Coverage 78.63% 78.44% -0.20%
==========================================
Files 250 250
Lines 89836 90067 +231
Branches 89836 90067 +231
==========================================
+ Hits 70646 70650 +4
- Misses 16281 16496 +215
- Partials 2909 2921 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I want to run CI without the protoc feature (since we are already having issues with long build times). Regrettably there is no way to say "all features except this one" (related: rust-lang/cargo#11467) so the command is a little ugly but it seems to work. |
Inspired by substrait-io/substrait-validator#356 this PR adds a
protoc
feature tolance
. If the feature is specified then we will useprotobuf-src
to build a vendored copy ofprotoc
.This increases build times slightly (need to compile
protoc
as part of the build) but removes the need for an external copy ofprotoc
.At the moment it is not possible to build both the
protoc
andsubstrait
features becausedatafusion-substrait
does not yet have its ownprotoc
feature (but it will hopefully have one added soon).