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

Declare all inputs of protoc action #2246

Merged
merged 1 commit into from
Oct 12, 2016
Merged

Conversation

fweikert
Copy link
Contributor

@fweikert fweikert commented Oct 12, 2016

ctx.executable.plugin must be in the inputs of protoc's action when using a plugin, otherwise the action will fail.

This bug has been hidden by a bug in Bazel: for every ctx.action, Bazel used to automatically add the runfiles of all executable inputs of the RULE instead of using the inputs of the specific ACTION. Consequently, we could get away with underspecifying the inputs of the action.

Fixes bazelbuild/bazel/issues/1929

ctx.executable.plugin must be in the inputs of protoc's action when using a plugin, otherwise the action will fail.

This bug has been hidden by a bug in Bazel: for every ctx.action, Bazel used to automatically add the runfiles of all executable inputs of the RULE instead of using the inputs of the specific ACTION. Consequently, we could get away with underspecifying the inputs of the action.
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

1 similar comment
@grpc-kokoro
Copy link

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok for us to run on our infrastructure.

Copy link
Contributor

@damienmg damienmg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is needed for incoming Bazel fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants