Skip to content

proto: allow shell vars to propagate to protoc.#779

Closed
htuch wants to merge 1 commit intobazel-contrib:masterfrom
htuch:use-default-shell
Closed

proto: allow shell vars to propagate to protoc.#779
htuch wants to merge 1 commit intobazel-contrib:masterfrom
htuch:use-default-shell

Conversation

@htuch
Copy link
Contributor

@htuch htuch commented Aug 31, 2017

Similar to protocolbuffers/protobuf#2508, when using
protoc with non-standard LD_LIBRARY_PATH, we need to be able to have it
pick up from the environment the correct LD_LIBRARY_PATH.

Similar to protocolbuffers/protobuf#2508, when using
protoc with non-standard LD_LIBRARY_PATH, we need to be able to have it
pick up from the environment the correct LD_LIBRARY_PATH.
@bazel-io
Copy link

Can one of the admins verify this patch?

@ianthehat
Copy link
Contributor

I think that use_default_shell_env is fundamentally incompatible with remote execution, this is probably not the right way to solve the problem.
Why is protoc special (compared to any other host binary), why does it need a custom LD_LIBRARY_PATH?

@htuch
Copy link
Contributor Author

htuch commented Aug 31, 2017

It's built under Bazel using a C++ toolchain configured with CC/CXX/LD_LIBRARY_PATH. Other host binaries are built with whatever the host base image is using. FWIW, we're building on Goobuntu with CXX/LD_LIBRARY_PATH pointing at crosstools. Would anyone want to remotely execute this rule when building with a custom local toolchain?

@htuch
Copy link
Contributor Author

htuch commented Aug 31, 2017

Closing this out based on offline discussion with @ianthehat. We have a bunch of needs (this PR, arbitrary plugins) that don't match well with the current go_proto_library rule, which is in a state of transition. The recommended approach is to fork go_proto_library.bzl in our repo (CC: @rodaine).

@htuch htuch closed this Aug 31, 2017
@htuch htuch deleted the use-default-shell branch August 31, 2017 21:09
yushan26 pushed a commit to yushan26/rules_go that referenced this pull request Jun 16, 2025
…es (bazel-contrib#779)

Update Python minor toolchain versions to allow smaller Python toolchains

Co-authored-by: Matt Mackay <mattem@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants