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

Remove protoc requirement #3073

Open
emilk opened this issue Oct 31, 2024 · 5 comments
Open

Remove protoc requirement #3073

emilk opened this issue Oct 31, 2024 · 5 comments
Labels
good first issue Good for newcomers

Comments

@emilk
Copy link

emilk commented Oct 31, 2024

Trying to add lance as a dependency currently results in a build error:

error: failed to run custom build command for `lance-encoding v0.18.2`
Caused by:
  process didn't exit successfully: `lance-encoding-b1cb34df4c6a6aa9/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-changed=protos

  --- stderr
  Error: Custom { kind: NotFound, error: "Could not find `protoc`. If `protoc` is installed, try setting the `PROTOC` environment variable to the path of the `protoc` binary. To install it on Debian, run `apt-get install protobuf-compiler`. It is also available at https://github.com/protocolbuffers/protobuf/releases  For more information: https://docs.rs/prost-build/#sourcing-protoc" }

You should instead check in the generated code, and include it in the crate. The build.rs should do nothing (or ideally, not exist).

@westonpace westonpace added the good first issue Good for newcomers label Oct 31, 2024
@loloxwg
Copy link

loloxwg commented Nov 22, 2024

error: failed to run custom build command for lance-encoding v0.20.0 (https://github.com/lancedb/lance.git?tag=v0.20.0-beta.2#920b1918)
note: To improve backtraces for build dependencies, set the CARGO_PROFILE_DEV_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.
Caused by:
process didn't exit successfully: /Users/xwg/dev/rust/lancedb/target/debug/build/lance-encoding-b8d4df13f280b9ab/build-script-build (exit status: 1)
--- stdout
cargo:rerun-if-changed=protos
--- stderr
Error: Custom { kind: Other, error: "protoc failed: google/protobuf/empty.proto: File not found.\nencodings.proto:8:1: Import "google/protobuf/empty.proto" was not found or had errors.\nencodings.proto:303:5: "google.protobuf.Empty" is not defined.\n" }

@loloxwg
Copy link

loloxwg commented Nov 25, 2024

@emilk How to temporarily alleviate this problem

@westonpace
Copy link
Contributor

Note an additional wrinkle: Datafusion requires protoc as well. Not just for Substrait but also for Arrow flight and for Datafusion's own internal plan serialization format. It does not appear to be an optional dependency and Datafusion lists protobuf-compiler as a required dependency.

So even if we can checkin the generated code there will still be a need for protoc to build datafusion.

@emilk
Copy link
Author

emilk commented Dec 5, 2024

Sounds like we should open an issue on https://github.com/apache/datafusion too then

@westonpace
Copy link
Contributor

westonpace commented Dec 5, 2024

Sounds like we should open an issue on https://github.com/apache/datafusion too then

Yes, another alternative is modifying datafusion to mask flight and its internal representation behind feature flags. We don't use or need those features (and I don't expect we will anytime soon) and this might be a more palatable change.

(which, to be clear, would also require opening an issue)

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

No branches or pull requests

3 participants