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

Fix action conflict on exec transitioned cc_proto_library #13875

Closed

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Aug 19, 2021

When the proto rules' depdencies were switched over to the exec config in
7acf9ea, the dependency of CcProtoAspect on the
proto C++ toolchain was not updated. Due to this, cc_proto_library had two
dependencies on @com_google_protobuf//:protobuf (through :protoc and directly)
with potentially incompatible configurations, which lead to conflicting actions.

Fixes #13464.

@google-cla google-cla bot added the cla: yes label Aug 19, 2021
@oquenchil oquenchil added the team-Rules-CPP Issues for C++ rules label Aug 20, 2021
@katre
Copy link
Member

katre commented Aug 20, 2021

Is this PR ready to be reviewed? It's still in the DRAFT state.

If so, please attach an issue describing the problem being fixed, ideally with a reproduction of the issue, so that I can verify what's happening.

@fmeum fmeum marked this pull request as ready for review August 21, 2021 15:24
@fmeum fmeum requested a review from lberki as a code owner August 21, 2021 15:24
@fmeum
Copy link
Collaborator Author

fmeum commented Aug 21, 2021

This is ready for review now. I failed to come up with a Bazel integration test that would actually reproduce the conflicting actions without this fix, perhaps because @com_google_protobuf//:protobuf always ended up being cached before the test ran.

When the proto rules' depdencies were switched over to the exec config in
7acf9ea, the dependency of CcProtoAspect on the
proto C++ toolchain was not updated. Due to this, cc_proto_library had two
dependencies on @com_google_protobuf//:protobuf (through :protoc and directly)
with potentially incompatible configurations, which lead to conflicting actions.

Fixes bazelbuild#13464.
@fmeum
Copy link
Collaborator Author

fmeum commented Aug 21, 2021

@sdtwigg I'm sorry, it seems that I got ahead of myself here: While the tests pass and the reproducer for #13464 no longer errors out with this patch, it is probably not semantically correct as it seems it would end up building the runtime in the exec rather than the target config. If that is correct, I will close the PR.

@fmeum fmeum closed this Aug 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes team-Rules-CPP Issues for C++ rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Conflicting actions for cc_proto_library with transition in Starlark rule dependency
3 participants