chore(devcontainer): use debian's protobuf-compiler package
#16687
+4
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
unfortunately, the current `Dockerfile` does not set the `PATH` variable correctly. the `RUN` directive is evaluated at build time. thus, the `$HOME` variable is resolved correctly to `/root/` when installing the `protoc` binary; the binary ends up in `/root/.local/bin/protoc`. in contrast, an `ENV` directive is evaluated at a container's runtime, so if `$HOME` is not correctly set at runtime, the `PATH` addition `$HOME/.local/bin` resolves to `/.local/bin`, which does not exist. since the linked docs now also recommend to use a package manager to install the `protoc` compiler, the `Dockerfile` does the same now. at the time of writing, bookworm's version is `libprotoc 3.21.12`, which is higher than the required version of `3.15`.
alamb
reviewed
Jul 5, 2025
There was a problem hiding this comment.
Thank you for this @fvj
I verified that trying to build datafusion in the current dev contianer did not work. For anyone who tries this, here is the error I got:
Caused by:
process didn't exit successfully: `/datafusion/target/debug/build/substrait-d198aa99ad943137/build-script-build` (exit status: 1)
--- stdout
cargo:rerun-if-env-changed=FORCE_REBUILD
cargo:rerun-if-changed=substrait
cargo:rerun-if-changed=substrait/text/simple_extensions_schema.yaml
cargo:rerun-if-changed=substrait/proto/substrait/type.proto
cargo:rerun-if-changed=substrait/proto/substrait/parameterized_types.proto
cargo:rerun-if-changed=substrait/proto/substrait/extensions/extensions.proto
cargo:rerun-if-changed=substrait/proto/substrait/type_expressions.proto
cargo:rerun-if-changed=substrait/proto/substrait/plan.proto
cargo:rerun-if-changed=substrait/proto/substrait/capabilities.proto
cargo:rerun-if-changed=substrait/proto/substrait/extended_expression.proto
cargo:rerun-if-changed=substrait/proto/substrait/function.proto
cargo:rerun-if-changed=substrait/proto/substrait/algebra.proto
--- 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" }
However, even with your change the build still fails for me like this
error: failed to run custom build command for `substrait v0.58.0`
Caused by:
process didn't exit successfully: `/datafusion/target/docker2/debug/build/substrait-3971c227a7ff9527/build-script-build` (exit status: 1)
--- stdout
cargo:rerun-if-env-changed=FORCE_REBUILD
cargo:rerun-if-changed=substrait
cargo:rerun-if-changed=substrait/text/simple_extensions_schema.yaml
cargo:rerun-if-changed=substrait/proto/substrait/type.proto
cargo:rerun-if-changed=substrait/proto/substrait/parameterized_types.proto
cargo:rerun-if-changed=substrait/proto/substrait/extensions/extensions.proto
cargo:rerun-if-changed=substrait/proto/substrait/type_expressions.proto
cargo:rerun-if-changed=substrait/proto/substrait/plan.proto
cargo:rerun-if-changed=substrait/proto/substrait/capabilities.proto
cargo:rerun-if-changed=substrait/proto/substrait/extended_expression.proto
cargo:rerun-if-changed=substrait/proto/substrait/function.proto
cargo:rerun-if-changed=substrait/proto/substrait/algebra.proto
--- stderr
Error: Custom { kind: Other, error: "protoc failed: google/protobuf/empty.proto: File not found.\nsubstrait/type.proto:6:1: Import \"google/protobuf/empty.proto\" was not found or had errors.\nsubstrait/type.proto:234:7: \"google.protobuf.Empty\" is not defined.\n" }
warning: build failed, waiting for other jobs to finish...
did you find some way to work around that?
|
Weird, it worked on my machine (tm). Let me investigate! Should I close the PR for now and reopen when I fixed? edit: I think I had a dirty devcontainer running when testing. will ping again once confirmed it works on a clean one. |
|
latest patch should include the missing files: the bookworm package |
alamb
approved these changes
Jul 8, 2025
There was a problem hiding this comment.
Thank you @fvj -- I tried it locally now and everything seems to work great
cd .devcontainer/
docker build . -t datafusion-devcontainer:latest
docker run -v `pwd`/..:/datafusion --rm -it datafusion-devcontainer:latest bashAnd then in the container
root@c32993dd1926:/# cd datafusion/
root@c32993dd1926:/datafusion# CARGO_TARGET_DIR=`pwd`/target/linux cargo run -p datafusion-cli
|
🚀 |
|
Thanks again @fvj |
|
you're very welcome. I hope I can contribute more valuable stuff in the future :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
None. I didn't find any issue for this problem but it's too small of a change for an issue, in my opinion. Happy to retroactively create one, though.
Rationale for this change
unfortunately, the current
Dockerfiledoes not set thePATHvariable correctly. theRUNdirective is evaluated at build time. thus, the$HOMEvariable is resolved correctly to/root/when installing theprotocbinary; the binary ends up in/root/.local/bin/protoc. in contrast, anENVdirective is evaluated at a container's runtime, so if$HOMEis not correctly set at runtime, thePATHaddition$HOME/.local/binresolves to/.local/bin, which does not exist.What changes are included in this PR?
protocfrom "from-source" to using the base image's (debian bookworm) package repository, in line with [1].[1] https://datafusion.apache.org/contributor-guide/development_environment.html#protoc-installation
Are these changes tested?
Yes. I tested the container to check if the
protocbinary is found:Additionally, I used the devcontainer to build DataFusion inside the devcontainer.
Are there any user-facing changes?
No, none.