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.

Sign up for GitHub

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

Jump to bottom

Any objection to add a ignore_go_package option in go_proto_library rule? #323

Closed
rosun82 opened this issue Mar 22, 2017 · 3 comments · Fixed by #325
Closed

Any objection to add a ignore_go_package option in go_proto_library rule? #323

rosun82 opened this issue Mar 22, 2017 · 3 comments · Fixed by #325

Comments

@rosun82
Copy link
Contributor

rosun82 commented Mar 22, 2017

Background:

We have a bunch of protos put in a git repository, and for historical reasons they have go_package option specified. However, when switching to Bazel, the go_package option gets in the way, as the output .pb.go file is in the wrong place and wrong package hence cannot be used as "deps" for other go_proto_library rules.

Proposal, add an ignore_go_package option, when specified, simply remove "option go_package" line from the proto source code then compile.

I can handle the modification if there is no objection.
@pmbethe09
Copy link
Contributor

Long-term: we should fix go_proto_library. But that might take some effort.

So short-term I would be ok with this hack if you want to write it.

@jayconrod
Copy link
Contributor

@rosun82 Would this work for you: write a genrule that removes option go_package with grep -v or something similar, then use the output as srcs for go_proto_library instead of the original.

@rosun82
Copy link
Contributor Author

rosun82 commented Mar 23, 2017

I have tried that. The trouble is there are proto imports. A genrule has to produce a file with different name, say we rename file a.proto to a_fix.proto through genrule. If b.proto imports a.proto, it will not build

@rosun82 rosun82 mentioned this issue Mar 23, 2017
Merged
jayconrod pushed a commit that referenced this issue Mar 23, 2017
@rosun82 @jayconrod
Adding a ignore_go_package_option option to the go_proto_library. (#325)
9069f96 
Once the option is on, we will first strip the option go_package = ... line from
the .proto file then compile, which avoids the problem where the compiled .pb.go
file is in the directory unexpected by bazel.

This partially resolves #323.
@rosun82 rosun82 mentioned this issue Dec 5, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Adding a ignore_go_package_option option to the go_proto_library.
3 participants
@jayconrod @rosun82 @pmbethe09