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

Add a proto_library rule for well-known types and update the proto_lang_toolchain rules #2763

Closed
cgrushko opened this issue Feb 26, 2017 · 19 comments
Assignees

Comments

@cgrushko
Copy link
Contributor

Bazel now has built-in rules for building Java and C++ protos (proto_libarry, java_proto_library, java_lite_proto_library and cc_proto_library).

For users to import well-known types (e.g., descriptor.proto) from their .proto files, they must depend on a proto_library that srcs the former.

This FR is about adding such a proto_library.

#2598 is a blocker.

@xfxyjwf xfxyjwf self-assigned this Mar 1, 2017
@kamalmarhubi
Copy link

@xfxyjwf do you know if you'll get to this in the near term?

@cgrushko
Copy link
Contributor Author

bazelbuild/bazel#2265 is a blocker.

@kamalmarhubi
Copy link

@cgrushko ah, I kept running into that error and didn't realize it was a bug in bazel.

@buchgr
Copy link
Contributor

buchgr commented Apr 26, 2017

bazelbuild/bazel#2265 has been fixed and should be part of the next bazel release.

buchgr added a commit to buchgr/bazel that referenced this issue May 1, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

[1] bazelbuild#2265
[2] protocolbuffers/protobuf#2763
buchgr added a commit to buchgr/bazel that referenced this issue May 1, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

Bazel's proto_library bug [1] is also the reason why this change has to
duplicate the well known protos under google/protobuf.

[1] bazelbuild#2265
[2] protocolbuffers/protobuf#2763

Change-Id: I5e16d437f2f581f11553049be0b4bcc564f4ad96
buchgr added a commit to buchgr/bazel that referenced this issue May 2, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

Bazel's proto_library bug [1] is also the reason why this change has to
duplicate the well known protos under google/protobuf.

[1] bazelbuild#2265
[2] protocolbuffers/protobuf#2763

Change-Id: I5e16d437f2f581f11553049be0b4bcc564f4ad96
buchgr added a commit to buchgr/bazel that referenced this issue May 3, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

Bazel's proto_library bug [1] is also the reason why this change has to
duplicate the well known protos under google/protobuf.

[1] bazelbuild#2265
[2] protocolbuffers/protobuf#2763

Change-Id: I5e16d437f2f581f11553049be0b4bcc564f4ad96
buchgr added a commit to buchgr/bazel that referenced this issue May 4, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

Bazel's proto_library bug [1] is also the reason why this change has to
duplicate the well known protos under google/protobuf.

[1] bazelbuild#2265
[2] protocolbuffers/protobuf#2763

Change-Id: I5e16d437f2f581f11553049be0b4bcc564f4ad96
@thaidn
Copy link

thaidn commented May 5, 2017

So is this issue fixed? If so, can I build bazel from HEAD and depend on well-known protos in my proto_library rules?

@buchgr
Copy link
Contributor

buchgr commented May 5, 2017

hey @thaidn

bazelbuild/bazel#2265 has been fixed and reverted again, because it caused trouble inside of google's programming environment. See the bug for details.

It's really embarrassing and I apologize for the trouble's this causes.

Inside bazel we also need well known types and thus made a simple hack that copies the well known types from src/com/google/protobuf to com/google/protobuf and then it works. Here is the patch that does that: https://bazel-review.googlesource.com/c/10690/

@xfxyjwf
Copy link
Contributor

xfxyjwf commented May 5, 2017

@buchgr Is a proper fix of this issue depending on b/37798101 (making both internal and external protoc accepts the same file path)?

@cgrushko
Copy link
Contributor Author

cgrushko commented May 5, 2017 via email

buchgr added a commit to buchgr/bazel that referenced this issue May 11, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

Bazel's proto_library bug [1] is also the reason why this change has to
duplicate the well known protos under google/protobuf.

[1] bazelbuild#2265
[2] protocolbuffers/protobuf#2763

Change-Id: I5e16d437f2f581f11553049be0b4bcc564f4ad96
@junghoahnsc
Copy link

Is there any workaround to import well-known protos for now?

@buchgr
Copy link
Contributor

buchgr commented May 18, 2017

@junghoahnsc
You can do what I described in my previous comment that bazel does.

@junghoahnsc
Copy link

@buchgr you mean using a patch https://bazel-review.googlesource.com/c/10690 ?

Would it be a workaround to have all well-known protos in local repo with BUILD?

@buchgr
Copy link
Contributor

buchgr commented May 18, 2017

@junghoahnsc yes to both :-).

buchgr added a commit to buchgr/bazel that referenced this issue May 19, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

Bazel's proto_library bug [1] is also the reason why this change has to
duplicate the well known protos under google/protobuf.

[1] bazelbuild#2265
[2] protocolbuffers/protobuf#2763

Change-Id: I5e16d437f2f581f11553049be0b4bcc564f4ad96
buchgr added a commit to buchgr/bazel that referenced this issue May 19, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

Bazel's proto_library bug [1] is also the reason why this change has to
duplicate the well known protos under google/protobuf.

[1] bazelbuild#2265
[2] protocolbuffers/protobuf#2763

Change-Id: I5e16d437f2f581f11553049be0b4bcc564f4ad96
bazel-io pushed a commit to bazelbuild/bazel that referenced this issue May 30, 2017
In order to use the build event service protobufs we need to support
protobufs well known types inside bazel. Unfortunately, due to a bug in
bazel's proto_library [1], protobuf can't provide such a proto_library
yet [2]. We expect this patch to be committed to protobuf upstream once
[1] is resolved.

Bazel's proto_library bug [1] is also the reason why this change has to
duplicate the well known protos under google/protobuf.

[1] #2265
[2] protocolbuffers/protobuf#2763

Change-Id: I5e16d437f2f581f11553049be0b4bcc564f4ad96
@kamalmarhubi
Copy link

I have an example repo defining an extra com_google_protobuf_well_known_protos repository: https://github.com/kamalmarhubi/bazel-well-known-protos-example

@buchgr
Copy link
Contributor

buchgr commented Aug 7, 2017

@kamalmarhubi smart to use strip-prefix :-).

Anyways, the fix is in bazel upstream again and will be part of the 0.5.4 release.

@perezd
Copy link
Contributor

perezd commented Aug 15, 2017

So, once 0.5.4 is available, what should we do to make use of this? Will it start to just work automatically?

@buchgr
Copy link
Contributor

buchgr commented Aug 16, 2017

Someone needs to add proto_library rules to protobuf's BUILD file :-). I can do that next week.

@aj-michael
Copy link
Contributor

Now that bazel 0.5.4 is released, can someone please fix this 😄

@buchgr
Copy link
Contributor

buchgr commented Sep 5, 2017

I am working on a change.

@xfxyjwf xfxyjwf closed this as completed in 699c0eb Sep 5, 2017
xfxyjwf added a commit that referenced this issue Sep 5, 2017
bazel: Add proto_library rules for well known types. Fixes #2763
@buchgr
Copy link
Contributor

buchgr commented Sep 5, 2017

Thanks @xfxyjwf !

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

No branches or pull requests

8 participants