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

Bazel build recompiles protoc every time PATH changes #3213

Open
nfelt opened this issue Feb 4, 2020 · 0 comments
Open

Bazel build recompiles protoc every time PATH changes #3213

nfelt opened this issue Feb 4, 2020 · 0 comments

Comments

@nfelt
Copy link
Contributor

nfelt commented Feb 4, 2020

This is a tracking issue for the fact that our bazel build triggers an unnecessary C++ compilation step for protoc, the protocol buffer compiler, every time the value of PATH changes, which most notably happens when switching virtualenvs. This is a time sink since the recompilation can take 30-50 seconds and makes switching virtualenvs to test TensorBoard against various versions of dependencies (in particular different TF dependencies) more time consuming than necessary.

The upstream issue for this in Bazel is bazelbuild/bazel#7095, but it hasn't seen much progress lately, and it's unclear long-term whether there's any prospect for it being solved.

As brief context, the reason we use virtualenv in combination with Bazel is because when running tests, we often want to test TensorBoard against different python environments. In particular we often want to test against different versions of key dependencies like TensorFlow, and it's much easier to do this by swapping between venvs with different TF pip packages than by, for example, trying to build multiple copies of TensorFlow hermetically within our own build (it's also a somewhat more realistic replica of how we ultimately ship our pip package, since it depends on other pip packages at installation time rather than hermetically bundling everything; there is experimental support from rules_python for combining pip and bazel natively, but it doesn't support the TensorFlow pip packages). Another reason for virtualenv usage in the past was to test against py2 vs py3, and while we've since dropped py2 support, testing against different python minor versions is still occasionally relevant.

For the record, one of the approaches I tried is trying to use a precompiled protoc binary as a workspace dep instead of compiling it locally, based on the idea mentioned in the bazel thread linked from the issue above: https://groups.google.com/g/bazel-discuss/c/UKLK7Y-jsW4/m/18-puhbfDAAJ

The problem we run into there is that now we need to build gRPC proto python bindings as well, which requires running protoc with the grpc_python_plugin alongside it, and grpc_python_plugin is not distributed in prebuilt form, so we end up building that locally, and it depends on most of protoc so we're back at square 1 recompiling protoc. There is a prebuilt grpcio-tools pip package that combines protoc and grpc_python_plugin as described here: grpc/grpc#15675 (comment) So one possible way to get that working could be to rewrite the python proto bazel rules to shell out to the grpcio-tools precompiled binary instead, but this seemed tricky to get working and also hard to maintain, so I didn't try doing it.

One other approach I tried was attempting to avoid the recompilation by passing a fixed PATH via --action_env=PATH=/bin:/usr/bin (you need to pass at least some path since Bazel itself needs some basic shell tools available). However, for reasons not understood by me, this doesn't actually stop recompilation from triggering. It looks like what protoc is sensitive to is PATH in the actual shell environment that runs bazel, and freezing --action_env doesn't fix that.

One last approach is changing how we use virtualenvs so that we don't actually change the PATH at all in the local shell that runs builds (thus avoiding recompilation), but instead figure out a different way to get py_binary targets to run using the desired virtualenv's python. I think the old way to do this was to pass --python_path manually; the new way is to use python toolchains. I prototyped a custom python toolchain that reads the VIRTUAL_ENV env variable to determine which interpreter binary to use, and this seems to work - changing the value of VIRTUAL_ENV but not PATH in the local shell seems to ensure the binaries to use the desired virtual env's interpreter when executing, but doesn't trigger protoc recompilation. The main downside here is that we can no longer just use the normal virtualenv activate scripts but would instead have to do something more customized to logically switch virtual envs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant