-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Forward strict_proto_deps
in java_proto_library
.
#16146
Conversation
Hi @uri-canva, It looks like this PR's git history may contain some unrelated cherry-picked commits (notably, the 5.3.0 release). Could you prune down the diff here (there are 31 files touched, most of which seem unrelated) and fix the presubmit issues? This also seems like a proto-related PR, which AFAICT is unrelated to Java and Android, so I'll remove all the currently assigned reviewers. |
Oh you're right, sorry, must have created the branch off the wrong base. |
Oh yeah I remember now why, it was because of #16147. |
156bc81
to
efd4c43
Compare
Unable to reproduce: I bootstrapped bazel and successfully built //src/tools/execlog:parser. strict_proto_deps flag is used in proto_library rule and passed to proto compiler. It shouldn't be in java_proto_library, those are suppose to be strict. I checked the configuration in com_google_protobuf and duration is on a list that should prevent it's compilation in java_proto_library (it is compiled as part of the runtime). Do you mind investigating what's going on there? |
I got it because of this separate issue: #16109. |
Are Duration isn't even supposed to be compiled in? I'll have to figure out why it's being picked up then. |
Yeah I think you're right, I narrowed down a repro and it's due to my own system configuration, sorry for the noise. |
strict_proto_deps
is not forwarded tojava_proto_library
rule, which causes//src/tools/execlog:parser
to fail to build, regardless of the values ofexperimental_strict_java_deps
andstrict_proto_deps
.