Protobuf fixes for Bazel 0.29 #288
Conversation
|
It might be a good idea to backport this on 0.12, since |
register_module_mapping_write_action expects a string, not a target. Signed-off-by: Steeve Morin <[email protected]>
The change was introduced in #1. 1. bazelbuild/bazel#7157 Signed-off-by: Steeve Morin <[email protected]>
| @@ -270,7 +270,7 @@ def _swift_protoc_gen_aspect_impl(target, aspect_ctx): | |||
| minimal_module_mappings.extend(_gather_transitive_module_mappings(proto_deps)) | |||
|
|
|||
| transitive_module_mapping_file = register_module_mapping_write_action( | |||
| target, | |||
| target.label.name, | |||
There was a problem hiding this comment.
This is not needed for Bazel 0.29, but I figure I would take the change and fix it anyway, that's why it's in its own commit
register_module_mapping_write_action expects a string, not a target
The bug was that the file was named path/to/the/file/<generated //xxxxx>.yyy
|
Also if you want I can change the commit/PR to be: Since things have changed. |
| # paths (e.g. using strip_import_prefix / import_prefix attributes) | ||
| virtual_imports = "/_virtual_imports/" | ||
| if virtual_imports in file.path: | ||
| return file.path.split(virtual_imports)[1].split("/", 1)[1] |
There was a problem hiding this comment.
Given the ongoing discussion on bazelbuild/bazel#9215, I'm not sure this is right since it seems like we'd need to pick up a different root also to properly handle generated proto files.
There was a problem hiding this comment.
I'm not sure:
When this flag is flipped, the two .proto files are both symlinked into a directory bazel-bin/x/virtual_imports/p) and that's what proto_source_root will be.
In the above case, it depends on whether foo.proto is generated or not. If it is not, the proto path of the proto compile action of :bar will be . and bazel-bin/x/_virtual_imports/p. If it is, in addition to that, bazel-bin/m/n/o/_virtual_imports/foo will also be there.
Also, the fix is from:
- https://github.com/grpc/grpc-java/pull/5959/files
- https://github.com/bazelbuild/rules_rust/pull/239/files
So it would mean they are not ok ?
There was a problem hiding this comment.
From what is being said, yea, they also won't handle a generated proto file being used (unless they are hardcoding other output paths into the proto search path for their protoc invokes)
|
I believe #298 covers this using the data from the provider vs. hardcoding things. |
Fixes #287
The fix was stolen from various patches found in bazelbuild/bazel#7157