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

Create proto_toolchain rule #10937

Closed
wants to merge 17 commits into from
Closed

Conversation

Yannic
Copy link
Contributor

@Yannic Yannic commented Mar 10, 2020

This change adds a new proto_toolchain rule which we will use later to resolve
tools and options required to compile Protocol Buffers, thus eventually replacing
some of the command-line options from ProtoConfiguration.

To preclude inconsistencies in the migration period, we will start with a
proto_toolchain rule that doesn't have any arguments and populate
its provider from the existing command-line options. After all consumers
have been migrated to resolve the tools from the toolchain instead of
reading the command-line, we will replace all proto-specific flags with
attributes on proto_toolchain (or similar, language-specific toolchains
for flags that only affect a specific language).

This change also adds a dependency on the new toolchain type to
proto_library, cc_proto_library, java_proto_library, and java_lite_proto_library.
Unfortunately, there seems to be no existing way to add a dependency on a
toolchain based on the value of a flag. To avoid breaking everyone, we add
a (default) proto_toolchain target to @bazel_tools//tools/proto:toolchain that
is registered in a workspace suffix and add a migration flag to disable them (thus
requiring users to upgrade to a compatible @rules_proto version. At this point,
it is unclear whether we want to do this migration now (i.e. for Bazel 4.0) or
wait for a later release.

See bazelbuild/rules_proto#54

Updates #10005

@jin jin added the WIP label Mar 26, 2020
Yannic added a commit to Yannic/rules_proto_bazelbuild that referenced this pull request Mar 30, 2020
This change adds a proto toolchain configuration to `@rules_proto`.
To stay compatible with versions that do not have
`native.proto_toolchain` yet, this change also adds a API-compatible
Starlark `proto_toolchain`.

See bazelbuild/bazel#10937
@Yannic
Copy link
Contributor Author

Yannic commented Mar 30, 2020

This is ready for review now.

@Yannic Yannic marked this pull request as ready for review March 30, 2020 15:57
@meisterT
Copy link
Member

What's the state of this PR?

@philwo
Copy link
Member

philwo commented Nov 10, 2020

We're replacing the "WIP" label with the native GitHub "draft PRs". I thus converted this PR to a draft.

Please mark this as ready for review when it's ready, or close it if you no longer intend to work on it. Thanks! 👍

@philwo philwo removed the WIP label Nov 10, 2020
@Yannic
Copy link
Contributor Author

Yannic commented Nov 14, 2020

Things have changed since opening this, so I'll close this and submit a new (updated) one.

@Yannic Yannic closed this Nov 14, 2020
@Yannic Yannic deleted the proto_toolchain branch November 14, 2020 12:37
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