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

Remove use of proto_library.proto_source_root #14

Closed
aaliddell opened this issue Aug 5, 2019 · 6 comments
Closed

Remove use of proto_library.proto_source_root #14

aaliddell opened this issue Aug 5, 2019 · 6 comments
Labels
resolved-next-release Code to resolve issue is pending release
Milestone

Comments

@aaliddell
Copy link
Member

This attribute is destined to be removed in Bazel 1.0: bazelbuild/bazel#7153

The test workspace for this attribute can then also be removed.

@aaliddell aaliddell added this to the 1.0.0 milestone Aug 5, 2019
@lberki
Copy link

lberki commented Aug 8, 2019

Note that the functionality of stripping prefixes from proto import paths will still be there, it's just that the attribute was replaced with strip_import_prefix / import_prefix so that prefixes can also be added and so that it's more coherent with our C++ rules..

@aaliddell
Copy link
Member Author

Ah, so does the proto_source_root property on the ProtoInfo provider remain then?

Those other two new attributes are presently supported and strip_import_prefix is tested here (although may need the '/' adding). A test workspace could be added for import_prefix though.

@lberki
Copy link

lberki commented Aug 8, 2019

Yep, the dep[ProtoInfo].proto_source_root is going to stay, it's just proto_library.proto_source_root that's going away.

@aaliddell
Copy link
Member Author

Ok, thanks. I've restored the use of the provider property and added a bunch more test workspaces on the branch for various combinations of those attributes. However, whilst doing so I came across an issue with duplicate files appearing in ProtoInfo.direct_sources when using import_prefix: bazelbuild/bazel#9127

@aaliddell aaliddell changed the title Remove proto_source_root support Remove use of proto_library.proto_source_root Aug 8, 2019
@lberki
Copy link

lberki commented Aug 9, 2019

Commented on the bug. TL;DR: that will be fixed in Bazel 1.0 as an excellent example of acausal development (We fix bugs even before you report them!)

In the meantime, can you live with the duplication?

@aaliddell
Copy link
Member Author

Serendipity! It's fine as-is, I've put in a check to skip duplicates in that list.

@aaliddell aaliddell added the resolved-next-release Code to resolve issue is pending release label Nov 6, 2019
@aaliddell aaliddell mentioned this issue Dec 1, 2019
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolved-next-release Code to resolve issue is pending release
Projects
None yet
Development

No branches or pull requests

2 participants