From ef4912bd29d7ad019489060b5234d2358b54c970 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 28 Jul 2019 18:10:44 +0200 Subject: [PATCH 1/6] Expose proto fragment fields necessary for writing proto_toolchain to Starlark --- .../java/com/google/devtools/build/lib/BUILD | 1 + .../lib/rules/proto/ProtoConfiguration.java | 27 +++++++++++++++++++ .../ProtoConfigurationApi.java | 23 +++++++++++++++- .../skylarkbuildapi/proto/ProtoModuleApi.java | 11 +++++++- 4 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index 76aefeae9918cc..abe8bda8049624 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -1039,6 +1039,7 @@ java_library( ":util", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", + "//src/main/java/com/google/devtools/build/lib/analysis/skylark/annotations", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java index 8eac3feb2cd846..8899848bec4a1c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.proto; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; @@ -23,6 +24,7 @@ import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.StrictDepsMode; import com.google.devtools.build.lib.analysis.config.FragmentOptions; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; +import com.google.devtools.build.lib.analysis.skylark.annotations.SkylarkConfigurationField; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skylarkbuildapi.ProtoConfigurationApi; @@ -279,6 +281,7 @@ public boolean enableProtoSourceroot() { return !options.disableProtoSourceRoot; } + @Override public ImmutableList protocOpts() { return protocOpts; } @@ -292,6 +295,12 @@ public boolean runExperimentalProtoExtraActions() { return options.experimentalProtoExtraActions; } + @SkylarkConfigurationField( + name = "protoc", + doc = "Returns the target denoted by the value of the --proto_compiler flag. " + + "Do not use this field, its only puprose is to help with migration of " + + "Protobuf rules to Starlark." + ) public Label protoCompiler() { return options.protoCompiler; } @@ -316,6 +325,24 @@ public StrictDepsMode strictProtoDeps() { return options.strictProtoDeps; } + @Override + public String starlarkStrictDeps() { + switch (strictProtoDeps()) { + case OFF: + return "OFF"; + + case WARN: + return "WARN"; + + case ERROR: // fall-through + case STRICT: + case DEFAULT: + return "ERROR"; + } + Preconditions.checkState(false, "NOTREACHED"); + return null; + } + public List ccProtoLibraryHeaderSuffixes() { return ccProtoLibraryHeaderSuffixes; } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java index 6082ba850b35af..3e8d5027bae41d 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java @@ -14,6 +14,8 @@ package com.google.devtools.build.lib.skylarkbuildapi; +import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory; @@ -23,7 +25,26 @@ @SkylarkModule( name = "proto", category = SkylarkModuleCategory.CONFIGURATION_FRAGMENT, - doc = "A configuration fragment representing protocol buffers." + doc = "A configuration fragment representing protocol buffers. " + + "Do not use these fields, they will be removed after migrating " + + "Protobuf rules to Starlark." ) public interface ProtoConfigurationApi { + @SkylarkCallable( + name = "protoc_opts", + doc = "Additional options to pass to the protobuf compiler. " + + "Do not use this field, its only puprose is to help with migration of " + + "Protobuf rules to Starlark.", + structField = true) + ImmutableList protocOpts(); + + @SkylarkCallable( + name = "strict_deps", + doc = "A string that specifies how to handle strict deps. Possible values: 'OFF', 'WARN', " + + "'ERROR'. For more details see https://docs.bazel.build/versions/master/" + + "command-line-reference.html#flag--strict_proto_deps" + + "Do not use this field, its only puprose is to help with migration of " + + "Protobuf rules to Starlark.", + structField = true) + String starlarkStrictDeps(); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java index fdfe4aefb5cccd..b73c4a4157390e 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skylarkbuildapi.proto; +import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; /** @@ -30,4 +31,12 @@ + "to load this symbol from " + "rules_proto" + "

") -public interface ProtoModuleApi {} +public interface ProtoModuleApi { + @Deprecated + @SkylarkCallable( + name = "do_not_use_has_configuration_field_protoc", + doc = "Do not use this field, its only puprose is to help with migration of " + + "Protobuf rules to Starlark.", + structField = true) + default void configurationFieldProtocExists() {} +} From be2dc0606f2c105b29c2c4058dc83ab0e298728c Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 13 Oct 2019 17:03:53 +0200 Subject: [PATCH 2/6] Update fields to match [1] and add docs [1] https://github.com/bazelbuild/rules_proto/pull/25/files#diff-ef5b868c0a77c08e46b4cab4105c4470R18 --- .../lib/rules/proto/ProtoConfiguration.java | 16 +++-- .../ProtoConfigurationApi.java | 63 +++++++++++++++---- .../skylarkbuildapi/proto/ProtoModuleApi.java | 2 +- 3 files changed, 64 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java index 3babba3f0a8f8a..43f9a856f66001 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java @@ -247,10 +247,18 @@ public boolean runExperimentalProtoExtraActions() { } @SkylarkConfigurationField( - name = "protoc", - doc = "Returns the target denoted by the value of the --proto_compiler flag. " - + "Do not use this field, its only puprose is to help with migration of " - + "Protobuf rules to Starlark." + // Must match the value of `_protoc_key` + // in `@rules_proto//proto/private/rules:proto_toolchain.bzl`. + name = "protoc_do_not_use_or_we_will_break_you_without_mercy", + doc = "Exposes the value of `--proto_compiler`." + + "

Do not use this field directly, its only puprose is to help with " + + "migration of Protobuf rules to Starlark.

" + + "

Instead, you can access the value through proto_toolchain

" + + "

pre class=\"language-python\">\n" + + "def _my_rule_impl(ctx):" + + " proto_toolchain = ctx.toolchains[\"@rules_proto//proto:toolchain\"]\n" + + " protoc = proto_toolchain.compiler\n" + + "

" ) public Label protoCompiler() { return options.protoCompiler; diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java index 3e8d5027bae41d..dea1b5225e71c4 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java @@ -26,25 +26,64 @@ name = "proto", category = SkylarkModuleCategory.CONFIGURATION_FRAGMENT, doc = "A configuration fragment representing protocol buffers. " - + "Do not use these fields, they will be removed after migrating " - + "Protobuf rules to Starlark." + + "

Do not use these fields directly, they are considered an implementation " + + "detail and will be removed after migrating Protobuf rules to Starlark.

" + // TODO(yannic): Link to generated docs of `proto_toolchain`. + // https://github.com/bazelbuild/bazel/issues/9203 + + "

Instead, you can access them through proto_toolchain

" + + "

pre class=\"language-python\">\n" + + "def _my_rule_impl(ctx):\n" + + " proto_toolchain = ctx.toolchains[\"@rules_proto//proto:toolchain\"]\n" + + "\n" + + " # Contains the protoc binary, as specified by `--proto_compiler`.\n" + + " protoc = proto_toolchain.compiler\n" + + "\n" + + " # Contains extra args to pass to protoc, as specified by `--protocopt`.\n" + + " compiler_options = proto_toolchain.compiler_options\n" + + "\n" + + " # Contains the strict-dependency mode to use for Protocol Buffers\n" + + " # (i.e. 'OFF`, `WARN`, `ERROR`), as specified by `--strict_proto_deps`.\n" + + " strict_deps = proto_toolchain.strict_deps\n" + + "\n" + + "my_rule = rule(\n" + + " implementation = _my_rule_impl,\n" + + " attrs = {},\n" + + " toolchains = [\n" + + " \"@rules_proto//proto:toolchain\",\n" + + " ],\n" + + ")\n" + + "

" ) public interface ProtoConfigurationApi { @SkylarkCallable( - name = "protoc_opts", - doc = "Additional options to pass to the protobuf compiler. " - + "Do not use this field, its only puprose is to help with migration of " - + "Protobuf rules to Starlark.", + // Must match the value of `_protocopt_key` + // in `@rules_proto//proto/private/rules:proto_toolchain.bzl`. + name = "protocopt_do_not_use_or_we_will_break_you_without_mercy", + doc = "Exposes the value of `--protocopt`." + + "

Do not use this field directly, its only puprose is to help with " + + "migration of Protobuf rules to Starlark.

" + + "

Instead, you can access the value through proto_toolchain

" + + "

pre class=\"language-python\">\n" + + "def _my_rule_impl(ctx):" + + " proto_toolchain = ctx.toolchains[\"@rules_proto//proto:toolchain\"]\n" + + " compiler_options = proto_toolchain.compiler_options\n" + + "

", structField = true) ImmutableList protocOpts(); @SkylarkCallable( - name = "strict_deps", - doc = "A string that specifies how to handle strict deps. Possible values: 'OFF', 'WARN', " - + "'ERROR'. For more details see https://docs.bazel.build/versions/master/" - + "command-line-reference.html#flag--strict_proto_deps" - + "Do not use this field, its only puprose is to help with migration of " - + "Protobuf rules to Starlark.", + // Must match the value of `_strict_deps_key` + // in `@rules_proto//proto/private/rules:proto_toolchain.bzl`. + name = "strict_deps_do_not_use_or_we_will_break_you_without_mercy", + doc = "Exposes the value of `--strict_proto_deps`." + + "

Do not use this field directly, its only puprose is to help with " + + "migration of Protobuf rules to Starlark.

" + + "

Instead, you can access the value through proto_toolchain

" + + "

pre class=\"language-python\">\n" + + "def _my_rule_impl(ctx):" + + " proto_toolchain = ctx.toolchains[\"@rules_proto//proto:toolchain\"]\n" + + " strict_deps = proto_toolchain.strict_deps\n" + + "

", structField = true) String starlarkStrictDeps(); } diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java index b9e7c2cb5be6fe..f8b8438553f4ac 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java @@ -34,7 +34,7 @@ public interface ProtoModuleApi { @Deprecated @SkylarkCallable( - name = "do_not_use_has_configuration_field_protoc", + name = "has_protoc_do_not_use_or_we_will_break_you_without_mercy", doc = "Do not use this field, its only puprose is to help with migration of " + "Protobuf rules to Starlark.", structField = true) From 0f2ce4e7024e5e83d8d5271f5a0257f451dc14fb Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Sun, 13 Oct 2019 17:06:39 +0200 Subject: [PATCH 3/6] Fix typo --- .../devtools/build/lib/rules/proto/ProtoConfiguration.java | 2 +- .../build/lib/skylarkbuildapi/ProtoConfigurationApi.java | 4 ++-- .../build/lib/skylarkbuildapi/proto/ProtoModuleApi.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java index 43f9a856f66001..2cdd01d8e4e213 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java @@ -251,7 +251,7 @@ public boolean runExperimentalProtoExtraActions() { // in `@rules_proto//proto/private/rules:proto_toolchain.bzl`. name = "protoc_do_not_use_or_we_will_break_you_without_mercy", doc = "Exposes the value of `--proto_compiler`." - + "

Do not use this field directly, its only puprose is to help with " + + "

Do not use this field directly, its only purpose is to help with " + "migration of Protobuf rules to Starlark.

" + "

Instead, you can access the value through proto_toolchain

" + "

pre class=\"language-python\">\n" diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java index dea1b5225e71c4..92d0e590bcdcd4 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java @@ -60,7 +60,7 @@ public interface ProtoConfigurationApi { // in `@rules_proto//proto/private/rules:proto_toolchain.bzl`. name = "protocopt_do_not_use_or_we_will_break_you_without_mercy", doc = "Exposes the value of `--protocopt`." - + "

Do not use this field directly, its only puprose is to help with " + + "

Do not use this field directly, its only purpose is to help with " + "migration of Protobuf rules to Starlark.

" + "

Instead, you can access the value through proto_toolchain

" + "

pre class=\"language-python\">\n" @@ -76,7 +76,7 @@ public interface ProtoConfigurationApi { // in `@rules_proto//proto/private/rules:proto_toolchain.bzl`. name = "strict_deps_do_not_use_or_we_will_break_you_without_mercy", doc = "Exposes the value of `--strict_proto_deps`." - + "

Do not use this field directly, its only puprose is to help with " + + "

Do not use this field directly, its only purpose is to help with " + "migration of Protobuf rules to Starlark.

" + "

Instead, you can access the value through proto_toolchain

" + "

pre class=\"language-python\">\n" diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java index f8b8438553f4ac..90164a3a189adb 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/proto/ProtoModuleApi.java @@ -35,7 +35,7 @@ public interface ProtoModuleApi { @Deprecated @SkylarkCallable( name = "has_protoc_do_not_use_or_we_will_break_you_without_mercy", - doc = "Do not use this field, its only puprose is to help with migration of " + doc = "Do not use this field, its only purpose is to help with migration of " + "Protobuf rules to Starlark.", structField = true) default void configurationFieldProtocExists() {} From f4bef507bd12601f2aba3047f18a90ac52a65ba8 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Tue, 15 Oct 2019 19:30:27 +0200 Subject: [PATCH 4/6] Add tests --- .../lib/rules/proto/ProtoConfiguration.java | 7 +- .../ProtoConfigurationApi.java | 6 +- .../java/com/google/devtools/build/lib/BUILD | 17 +++ .../rules/proto/ProtoConfigurationTest.java | 142 ++++++++++++++++++ 4 files changed, 165 insertions(+), 7 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/rules/proto/ProtoConfigurationTest.java diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java index 2cdd01d8e4e213..3ae304b2a7fa73 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java @@ -287,7 +287,8 @@ public StrictDepsMode strictProtoDeps() { @Override public String starlarkStrictDeps() { switch (strictProtoDeps()) { - case OFF: + case OFF: // fall-through + case DEFAULT: return "OFF"; case WARN: @@ -295,11 +296,9 @@ public String starlarkStrictDeps() { case ERROR: // fall-through case STRICT: - case DEFAULT: return "ERROR"; } - Preconditions.checkState(false, "NOTREACHED"); - return null; + throw new IllegalStateException(); } public List ccProtoLibraryHeaderSuffixes() { diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java index 92d0e590bcdcd4..2a603beb12660c 100644 --- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java +++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/ProtoConfigurationApi.java @@ -31,7 +31,7 @@ // TODO(yannic): Link to generated docs of `proto_toolchain`. // https://github.com/bazelbuild/bazel/issues/9203 + "

Instead, you can access them through proto_toolchain

" - + "

pre class=\"language-python\">\n" + + "

\n"
               + "def _my_rule_impl(ctx):\n"
               + "    proto_toolchain = ctx.toolchains[\"@rules_proto//proto:toolchain\"]\n"
               + "\n"
@@ -63,7 +63,7 @@ public interface ProtoConfigurationApi {
                 + "
Do not use this field directly, its only purpose is to help with "
                 + "migration of Protobuf rules to Starlark.
"
                 + "
Instead, you can access the value through proto_toolchain
"
-                + "
pre class=\"language-python\">\n"
+                + "
\n"
                 + "def _my_rule_impl(ctx):"
                 + "    proto_toolchain = ctx.toolchains[\"@rules_proto//proto:toolchain\"]\n"
                 + "    compiler_options = proto_toolchain.compiler_options\n"
@@ -79,7 +79,7 @@ public interface ProtoConfigurationApi {
                 + "
Do not use this field directly, its only purpose is to help with "
                 + "migration of Protobuf rules to Starlark.
"
                 + "
Instead, you can access the value through proto_toolchain
"
-                + "
pre class=\"language-python\">\n"
+                + "
\n"
                 + "def _my_rule_impl(ctx):"
                 + "    proto_toolchain = ctx.toolchains[\"@rules_proto//proto:toolchain\"]\n"
                 + "    strict_deps = proto_toolchain.strict_deps\n"
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 454f11ab322cad..2bbbd8f1af7f51 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1501,6 +1501,23 @@ java_test(
     ],
 )
 
+java_test(
+    name = "ProtoConfigurationTest",
+    srcs = ["rules/proto/ProtoConfigurationTest.java"],
+    deps = [
+        ":actions_testutil",
+        ":analysis_testutil",
+        ":guava_junit_truth",
+        ":packages_testutil",
+        "//src/main/java/com/google/devtools/build/lib:build-base",
+        "//src/main/java/com/google/devtools/build/lib:proto-rules",
+        "//src/main/java/com/google/devtools/build/lib:util",
+        "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/test/java/com/google/devtools/build/lib:testutil",
+    ],
+)
+
 java_test(
     name = "BazelProtoLibraryTest",
     srcs = ["rules/proto/BazelProtoLibraryTest.java"],
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoConfigurationTest.java
new file mode 100644
index 00000000000000..0f5494a138bb8c
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoConfigurationTest.java
@@ -0,0 +1,142 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.rules.proto;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith;
+import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.eventbus.EventBus;
+import com.google.devtools.build.lib.actions.Action;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
+import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.util.MockProtoSupport;
+import com.google.devtools.build.lib.testutil.TestConstants;
+import com.google.devtools.build.lib.analysis.DefaultInfo;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class ProtoConfigurationTest extends BuildViewTestCase {
+  @Before
+  public void setUp() throws Exception {
+    MockProtoSupport.setupWorkspace(scratch);
+    invalidatePackages();
+
+    scratch.file("proto/BUILD", "cc_binary(name='compiler')");
+    scratch.file(
+        "proto/defs.bzl",
+        "def check_proto_common():",
+        "    if not hasattr(proto_common,",
+        "                   'has_protoc_do_not_use_or_we_will_break_you_without_mercy'):",
+        "        fail('missing has_protoc_... attribute on proto_common')",
+        "check_proto_common()",
+        "",
+        "def _echo_protocopt_impl(ctx):",
+        "    opts = ctx.fragments.proto.protocopt_do_not_use_or_we_will_break_you_without_mercy",
+        "    f = ctx.actions.declare_file(ctx.attr.name)",
+        "    ctx.actions.run(executable='echo',outputs=[f], arguments=opts)",
+        "    return [DefaultInfo(files=depset([f]))]",
+        "echo_protocopt = rule(_echo_protocopt_impl, fragments=['proto'])",
+        "",
+        "def _echo_strict_deps_impl(ctx):",
+        "    s = ctx.fragments.proto.strict_deps_do_not_use_or_we_will_break_you_without_mercy",
+        "    f = ctx.actions.declare_file(ctx.attr.name)",
+        "    ctx.actions.run(executable='echo',outputs=[f], arguments=[s])",
+        "    return [DefaultInfo(files=depset([f]))]",
+        "echo_strict_deps = rule(_echo_strict_deps_impl, fragments=['proto'])",
+        "",
+        "def _echo_proto_compiler_impl(ctx):",
+        "    return [DefaultInfo(files=depset([ctx.executable._compiler]))]",
+        "_protoc_key = 'protoc_do_not_use_or_we_will_break_you_without_mercy'",
+        "echo_proto_compiler = rule(",
+        "    implementation = _echo_proto_compiler_impl,",
+        "    attrs = {",
+        "        '_compiler': attr.label(",
+        "            executable = True,",
+        "            cfg = 'host',",
+        "            default = configuration_field('proto', _protoc_key),",
+        "        ),",
+        "    },",
+        "    fragments=['proto']",
+        ")");
+  }
+
+  private Artifact getArtifact(String target, String file) throws Exception {
+    return getFirstArtifactEndingWith(getFilesToBuild(getConfiguredTarget(target)), file);
+  }
+
+  @Test
+  public void readProtocopt() throws Exception {
+    scratch.file(
+        "x/BUILD",
+        "load('//proto:defs.bzl', 'echo_protocopt')",
+        "echo_protocopt(name = 'x')");
+    assertThat(getGeneratingSpawnAction(getArtifact("//x", "x")).getRemainingArguments())
+        .isEmpty();
+
+    useConfiguration("--protocopt=--foo", "--protocopt=--bar=10");
+    assertThat(getGeneratingSpawnAction(getArtifact("//x", "x")).getRemainingArguments())
+        .containsExactly("--foo", "--bar=10");
+  }
+
+  @Test
+  public void readStrictDeps() throws Exception {
+    scratch.file(
+        "x/BUILD",
+        "load('//proto:defs.bzl', 'echo_strict_deps')",
+        "echo_strict_deps(name = 'x')");
+    assertThat(getGeneratingSpawnAction(getArtifact("//x", "x")).getRemainingArguments())
+        .containsExactly("ERROR");
+
+    useConfiguration("--strict_proto_deps=OFF");
+    assertThat(getGeneratingSpawnAction(getArtifact("//x", "x")).getRemainingArguments())
+        .containsExactly("OFF");
+
+    useConfiguration("--strict_proto_deps=DEFAULT");
+    assertThat(getGeneratingSpawnAction(getArtifact("//x", "x")).getRemainingArguments())
+        .containsExactly("OFF");
+
+    useConfiguration("--strict_proto_deps=WARN");
+    assertThat(getGeneratingSpawnAction(getArtifact("//x", "x")).getRemainingArguments())
+        .containsExactly("WARN");
+
+    useConfiguration("--strict_proto_deps=ERROR");
+    assertThat(getGeneratingSpawnAction(getArtifact("//x", "x")).getRemainingArguments())
+        .containsExactly("ERROR");
+
+    useConfiguration("--strict_proto_deps=DEFAULT");
+    assertThat(getGeneratingSpawnAction(getArtifact("//x", "x")).getRemainingArguments())
+        .containsExactly("OFF");
+  }
+
+  @Test
+  public void readProtoCompiler() throws Exception {
+    scratch.file(
+        "x/BUILD",
+        "load('//proto:defs.bzl', 'echo_proto_compiler')",
+        "echo_proto_compiler(name = 'x')");
+
+    useConfiguration("--proto_compiler=//proto:compiler");
+    assertThat(getArtifact("//x", "compiler").getRootRelativePathString())
+        .startsWith("proto/compiler");  // Ends with `.exe` on Windows.
+  }
+}

From 63ba7e5c7741df011ebe0c83eb4591631ebe40d0 Mon Sep 17 00:00:00 2001
From: Yannic Bonenberger 
Date: Tue, 15 Oct 2019 19:34:17 +0200
Subject: [PATCH 5/6] Remove extra imports/deps

---
 src/test/java/com/google/devtools/build/lib/BUILD   |  6 ------
 .../lib/rules/proto/ProtoConfigurationTest.java     | 13 -------------
 2 files changed, 19 deletions(-)

diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 2bbbd8f1af7f51..451d407d7f59df 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -1508,13 +1508,7 @@ java_test(
         ":actions_testutil",
         ":analysis_testutil",
         ":guava_junit_truth",
-        ":packages_testutil",
-        "//src/main/java/com/google/devtools/build/lib:build-base",
-        "//src/main/java/com/google/devtools/build/lib:proto-rules",
-        "//src/main/java/com/google/devtools/build/lib:util",
         "//src/main/java/com/google/devtools/build/lib/actions",
-        "//src/main/java/com/google/devtools/build/lib/cmdline",
-        "//src/test/java/com/google/devtools/build/lib:testutil",
     ],
 )
 
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoConfigurationTest.java
index 0f5494a138bb8c..5c69ef299b94f7 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoConfigurationTest.java
@@ -16,19 +16,9 @@
 
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.getFirstArtifactEndingWith;
-import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames;
 
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.common.eventbus.EventBus;
-import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.Artifact;
-import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
-import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.packages.util.MockProtoSupport;
-import com.google.devtools.build.lib.testutil.TestConstants;
-import com.google.devtools.build.lib.analysis.DefaultInfo;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -38,9 +28,6 @@
 public class ProtoConfigurationTest extends BuildViewTestCase {
   @Before
   public void setUp() throws Exception {
-    MockProtoSupport.setupWorkspace(scratch);
-    invalidatePackages();
-
     scratch.file("proto/BUILD", "cc_binary(name='compiler')");
     scratch.file(
         "proto/defs.bzl",

From 551ca2f0cf778e47f9b1dfe08d13a8d46d5651b4 Mon Sep 17 00:00:00 2001
From: Yannic Bonenberger 
Date: Tue, 15 Oct 2019 19:48:08 +0200
Subject: [PATCH 6/6] Add missing <

---
 .../devtools/build/lib/rules/proto/ProtoConfiguration.java      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
index 3ae304b2a7fa73..6b34db7a6a9e29 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java
@@ -254,7 +254,7 @@ public boolean runExperimentalProtoExtraActions() {
                 + "
Do not use this field directly, its only purpose is to help with "
                 + "migration of Protobuf rules to Starlark.
"
                 + "
Instead, you can access the value through proto_toolchain
"
-                + "
pre class=\"language-python\">\n"
+                + "
\n"
                 + "def _my_rule_impl(ctx):"
                 + "    proto_toolchain = ctx.toolchains[\"@rules_proto//proto:toolchain\"]\n"
                 + "    protoc = proto_toolchain.compiler\n"