[proto] Add extra_outputs to go_proto_compile #3631
Conversation
| @@ -55,13 +55,14 @@ to structs produced by other compilers will set this to False.""", | |||
| }, | |||
| ) | |||
|
|
|||
| def go_proto_compile(go, compiler, protos, imports, importpath): | |||
| def go_proto_compile(go, compiler, protos, extra_outputs, imports, importpath): | |||
There was a problem hiding this comment.
Adding this as a positional argument is a breaking change. Could you make it an optional keyword argument instead, e.g. like this:
| def go_proto_compile(go, compiler, protos, extra_outputs, imports, importpath): | |
| def go_proto_compile(go, compiler, protos, imports, importpath, *, extra_outputs = {}): |
| for _, v in extra_outputs.items(): | ||
| if not v.endswith("pb.go"): |
There was a problem hiding this comment.
| for _, v in extra_outputs.items(): | |
| if not v.endswith("pb.go"): | |
| if any([not v.endswith(".pb.go") for v in extra_outputs.values()]): |
Checking for .pb.go seems more appropriate than pb.go, also .values() directly gives you the right list.
| """Invokes protoc to generate Go sources for a given set of protos | ||
|
|
||
| Args: | ||
| go: the go object, returned by go_context. | ||
| compiler: a GoProtoCompiler provider. | ||
| protos: list of ProtoInfo providers for protos to compile. | ||
| extra_outputs: dict of additional output files to be compiled. |
There was a problem hiding this comment.
Could you add a description of what the keys and values are in this dict?
| file_name_without_suffix = v[:-len(".pb.go")] | ||
| out = go.declare_file( | ||
| go, | ||
| path = importpath + "/" + file_name_without_suffix, |
There was a problem hiding this comment.
Is it clear that this is always the correct importpath or would we have to provide a way for users to override this?
| @@ -93,6 +98,17 @@ def go_proto_compile(go, compiler, protos, imports, importpath): | |||
| ext = compiler.internal.suffix, | |||
| ) | |||
| go_srcs.append(out) | |||
|
|
|||
| if extra_outputs.get(src.basename): | |||
There was a problem hiding this comment.
You can assign this to a variable so that you don't need to get it again in the next line.
| @@ -165,6 +166,9 @@ go_proto_library = rule( | |||
| providers = [GoLibrary], | |||
| aspects = [_go_proto_aspect], | |||
| ), | |||
| "extra_outputs": attr.string_dict( | |||
| doc = "Extra output files to be included in the expected generated output", | |||
There was a problem hiding this comment.
Same as for the other doc, please explain the role of keys and values.
|
Abandoning this in favor of #3650 |
What type of PR is this?
What does this PR do? Why is it needed?
Which issues(s) does this PR fix?
Fixes #3630
Other notes for review