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

ProtoInfo.transitive_proto_path does not add genfiles path prefixes #7964

Closed
EdSchouten opened this issue Apr 6, 2019 · 4 comments
Closed
Labels
P2 We'll consider working on this in future. (Assignee optional) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Server Issues for serverside rules included with Bazel type: bug

Comments

@EdSchouten
Copy link
Contributor

EdSchouten commented Apr 6, 2019

Description of the problem / feature request:

I am currently working on rules_elm: a set of Bazel rules for Elm, a Haskell-like programming language that transpiles to Javascript. As there exists a Protobuf compiler for Elm, I also want to add an elm_proto_library().

While implementing this function and testing it on the REv2 protocol, I observed the following build failure:

external/com_google_protobuf: warning: directory does not exist.
google/protobuf/descriptor.proto: File not found.
google/api/annotations.proto: Import "google/protobuf/descriptor.proto" was not found or had errors.
google/api/annotations.proto:28:8: "google.protobuf.MethodOptions" is not defined.
google/protobuf/any.proto: File not found.
google/protobuf/empty.proto: File not found.
google/rpc/status.proto: Import "google/protobuf/any.proto" was not found or had errors.
[ lots of errors follow ]

The Protobuf compiler was being invoked by my build rules as follows:

bazel‑out/host/bin/external/com_google_protobuf/protoc ‑‑plugin bazel‑out/host/bin/external/com_github_tiziano88_elm_protobuf/protoc‑gen‑elm/linux_amd64_stripped/protoc‑gen‑elm ‑‑elm_out '' ‑I external/com_github_bazelbuild_remote_apis ‑I external/com_google_protobuf ‑I external/googleapis external/com_github_bazelbuild_remote_apis/build/bazel/remote/execution/v2/remote_execution.proto

As I'm using remote builds, I managed to extract a tarball of the input root. As you can see, the .proto files that are reported as missing are stored underneath bazel-out/k8-fastbuild/genfiles/external/com_google_protobuf/google/protobuf, while the compiler is invoked to include external/com_google_protobuf.

To obtain the include paths I'm passing to protoc, I'm using ProtoInfo.transitive_proto_path, which I thought would be sufficient. The question is: is Bazel incorrect in that it omits the genfiles prefixes where applicable, or am I supposed to do some post-processing on ProtoInfo.transitive_proto_path?

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Check out bb-browser, commit e40af2807d5bd4bd5bc80e010f0b880172351415 in branch elm. Then try to build //cmd/bb_browser/frontend/....

What operating system are you running Bazel on?

Ubuntu 18.04 LTS

What's the output of bazel info release?

release 0.24.1

Have you found anything relevant by searching the web?

No. :-(

@EdSchouten
Copy link
Contributor Author

Might it be the case that this issue is weakly related to #7157?

@EdSchouten
Copy link
Contributor Author

I guess that the issue is that the Library class in ProtoCommon only allows setting a single sourceRoot. In theory, you could have multiple of those in case you'd declare a proto_library() that has some source files and some generated ones. The getProtoSourceRoot function only computes the path of source files; not generated ones.

+cc @lberki, as there already is a TODO with his name right above this code. Do you think this analysis is correct?

@aiuto aiuto added team-Rules-Server Issues for serverside rules included with Bazel untriaged labels Apr 15, 2019
@lberki lberki self-assigned this May 6, 2019
@lberki lberki added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels May 6, 2019
@lberki
Copy link
Contributor

lberki commented May 6, 2019

There are many TODOs with my name...

The way this works with built-in rules is that we pass -I<import path>=<execpath> parameters to protoc so that it knows exactly where to find which files. Unfortunately, that map is not yet available in Starlark. I'm afraid the only workaround I can think of is to look at the path of the input artifact, try to determine if it's an output artifact and then brew your own -I arguments.

Not very satisfying, I know :(

@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

@sgowroji sgowroji closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-Server Issues for serverside rules included with Bazel type: bug
Projects
None yet
Development

No branches or pull requests

4 participants