Clean up after transition of java_xxx_proto_library rules to proto-toolchains.

This hardcodes usage of proto-toolchains, which triggers strict-proto-deps. 
Since strict-proto-deps relies on a proto-compiler feature which doesn't exist yet, I've also changed the default of --strict_proto_deps to 'default', which won't trigger the check unless specifically requested.

--
PiperOrigin-RevId: 141347426
MOS_MIGRATED_REVID=141347426

3 files changed, 31 insertions, 182 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
index 62205d5edb..164b582c0d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspect.java
@@ -21,7 +21,6 @@ import static com.google.devtools.build.lib.cmdline.Label.parseAbsoluteUnchecked
 import static com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition.HOST;
 import static com.google.devtools.build.lib.packages.Attribute.attr;
 import static com.google.devtools.build.lib.packages.BuildType.LABEL;
-import static com.google.devtools.build.lib.packages.BuildType.LABEL_LIST;
 import static com.google.devtools.build.lib.rules.java.proto.JavaCompilationArgsAspectProvider.GET_PROVIDER;
 import static com.google.devtools.build.lib.rules.java.proto.JavaProtoLibraryTransitiveFilesToBuildProvider.GET_JARS;
 
@@ -31,7 +30,6 @@ import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.ConfiguredAspect;
 import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
 import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
@@ -45,7 +43,6 @@ import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.packages.AspectDefinition;
 import com.google.devtools.build.lib.packages.AspectParameters;
 import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.NativeAspectClass;
 import com.google.devtools.build.lib.packages.Rule;
@@ -63,7 +60,6 @@ import com.google.devtools.build.lib.rules.proto.ProtoSourceFileBlacklist;
 import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider;
 import com.google.devtools.build.lib.rules.proto.ProtoSupportDataProvider;
 import com.google.devtools.build.lib.rules.proto.SupportData;
-import java.util.List;
 import javax.annotation.Nullable;
 
 /** An Aspect which JavaProtoLibrary injects to build Java SPEED protos. */
@@ -71,32 +67,6 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
 
   private static final String SPEED_PROTO_TOOLCHAIN_ATTR = ":aspect_java_proto_toolchain";
 
-  private static final String SPEED_PROTO_RUNTIME_ATTR = "$aspect_java_lib";
-  private static final String SPEED_PROTO_RUNTIME_LABEL = "//external:protobuf/java_runtime";
-
-  /**
-   * The attribute name for holding a list of protos for which no code should be generated because
-   * the proto-runtime already contains them.
-   */
-  private static final String PROTO_SOURCE_FILE_BLACKLIST_ATTR = ":proto_source_file_blacklist";
-
-  private static final Attribute.LateBoundLabelList<BuildConfiguration> BLACKLISTED_PROTOS =
-      new Attribute.LateBoundLabelList<BuildConfiguration>(
-          ImmutableList.<Label>of(), ProtoConfiguration.class) {
-        @Override
-        public List<Label> resolve(
-            Rule rule, AttributeMap attributes, BuildConfiguration configuration) {
-          return configuration
-              .getFragment(ProtoConfiguration.class)
-              .protoCompilerJavaBlacklistedProtos();
-        }
-
-        @Override
-        public boolean useHostConfiguration() {
-          return true;
-        }
-      };
-
   private static Attribute.LateBoundLabel<BuildConfiguration> getSpeedProtoToolchainLabel(
       String defaultValue) {
     return new Attribute.LateBoundLabel<BuildConfiguration>(
@@ -144,12 +114,9 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
         new Impl(
                 ruleContext,
                 supportData,
-                ruleContext
-                    .getFragment(ProtoConfiguration.class, ConfigurationTransition.HOST)
-                    .protoCompilerJavaFlags(),
-                javaSemantics,
-                rpcSupport,
-                ruleContext.getFragment(ProtoConfiguration.class).useToolchainForJavaProto())
+            javaSemantics,
+                rpcSupport
+        )
             .createProviders());
 
     return aspect.build();
@@ -163,14 +130,6 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
             .requiresConfigurationFragments(JavaConfiguration.class, ProtoConfiguration.class)
             .requireProviders(ProtoSourcesProvider.class)
             .add(
-                attr(SPEED_PROTO_RUNTIME_ATTR, LABEL)
-                    .legacyAllowAnyFileType()
-                    .value(parseAbsoluteUnchecked(SPEED_PROTO_RUNTIME_LABEL)))
-            .add(
-                attr(PROTO_SOURCE_FILE_BLACKLIST_ATTR, LABEL_LIST)
-                    .cfg(HOST)
-                    .value(BLACKLISTED_PROTOS))
-            .add(
                 attr(SPEED_PROTO_TOOLCHAIN_ATTR, LABEL)
                     // TODO(carmi): reinstate mandatoryNativeProviders(ProtoLangToolchainProvider)
                     // once it's in a Bazel release.
@@ -205,22 +164,16 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
      * Java compilation action.
      */
     private final JavaCompilationArgsProvider dependencyCompilationArgs;
-    private final String protoCompilerPluginOptions;
-    private final boolean useToolchainForJavaProto;
 
     Impl(
         final RuleContext ruleContext,
         final SupportData supportData,
-        String protoCompilerPluginOptions,
         JavaSemantics javaSemantics,
-        RpcSupport rpcSupport,
-        boolean useToolchainForJavaProto) {
+        RpcSupport rpcSupport) {
       this.ruleContext = ruleContext;
       this.supportData = supportData;
-      this.protoCompilerPluginOptions = protoCompilerPluginOptions;
       this.javaSemantics = javaSemantics;
       this.rpcSupport = rpcSupport;
-      this.useToolchainForJavaProto = useToolchainForJavaProto;
 
       dependencyCompilationArgs =
           JavaCompilationArgsProvider.merge(
@@ -281,51 +234,31 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
       }
 
       final ProtoSourceFileBlacklist protoBlackList;
-      if (useToolchainForJavaProto) {
-        NestedSetBuilder<Artifact> blacklistedProtos = NestedSetBuilder.stableOrder();
-        blacklistedProtos.addTransitive(getProtoToolchainProvider().blacklistedProtos());
-        blacklistedProtos.addTransitive(rpcSupport.getBlacklist(ruleContext));
+      NestedSetBuilder<Artifact> blacklistedProtos = NestedSetBuilder.stableOrder();
+      blacklistedProtos.addTransitive(getProtoToolchainProvider().blacklistedProtos());
+      blacklistedProtos.addTransitive(rpcSupport.getBlacklist(ruleContext));
 
-        protoBlackList = new ProtoSourceFileBlacklist(ruleContext, blacklistedProtos.build());
-      } else {
-        protoBlackList =
-            new ProtoSourceFileBlacklist(
-                ruleContext,
-                ruleContext
-                    .getPrerequisiteArtifacts(PROTO_SOURCE_FILE_BLACKLIST_ATTR, Mode.HOST)
-                    .list());
-      }
+      protoBlackList = new ProtoSourceFileBlacklist(ruleContext, blacklistedProtos.build());
 
       return protoBlackList.checkSrcs(supportData.getDirectProtoSources(), "java_proto_library");
     }
 
     private void createProtoCompileAction(Artifact sourceJar) {
-      if (useToolchainForJavaProto) {
-        ImmutableList.Builder<ToolchainInvocation> invocations = ImmutableList.builder();
-        invocations.add(
-            new ToolchainInvocation(
-                "java", checkNotNull(getProtoToolchainProvider()), sourceJar.getExecPathString()));
-        invocations.addAll(rpcSupport.getToolchainInvocation(ruleContext, sourceJar));
-        ProtoCompileActionBuilder.registerActions(
-            ruleContext,
-            invocations.build(),
-            supportData.getDirectProtoSources(),
-            supportData.getTransitiveImports(),
-            supportData.getProtosInDirectDeps(),
-            ruleContext.getLabel().getCanonicalForm(),
-            ImmutableList.of(sourceJar),
-            "Java (Immutable)",
-            rpcSupport.allowServices(ruleContext));
-      } else {
-        ProtoCompileActionBuilder actionBuilder =
-            new ProtoCompileActionBuilder(
-                    ruleContext, supportData, "Java", "java", ImmutableList.of(sourceJar))
-                .allowServices(true)
-                .setLangParameter(
-                    String.format(protoCompilerPluginOptions, sourceJar.getExecPathString()));
-        rpcSupport.mutateProtoCompileAction(ruleContext, sourceJar, actionBuilder);
-        ruleContext.registerAction(actionBuilder.build());
-      }
+      ImmutableList.Builder<ToolchainInvocation> invocations = ImmutableList.builder();
+      invocations.add(
+          new ToolchainInvocation(
+              "java", checkNotNull(getProtoToolchainProvider()), sourceJar.getExecPathString()));
+      invocations.addAll(rpcSupport.getToolchainInvocation(ruleContext, sourceJar));
+      ProtoCompileActionBuilder.registerActions(
+          ruleContext,
+          invocations.build(),
+          supportData.getDirectProtoSources(),
+          supportData.getTransitiveImports(),
+          supportData.getProtosInDirectDeps(),
+          ruleContext.getLabel().getCanonicalForm(),
+          ImmutableList.of(sourceJar),
+          "Java (Immutable)",
+          rpcSupport.allowServices(ruleContext));
     }
 
     private JavaCompilationArgsProvider createJavaCompileAction(
@@ -338,15 +271,9 @@ public class JavaProtoAspect extends NativeAspectClass implements ConfiguredAspe
       helper
           .addDep(dependencyCompilationArgs)
           .setCompilationStrictDepsMode(StrictDepsMode.OFF);
-      if (useToolchainForJavaProto) {
-        TransitiveInfoCollection runtime = getProtoToolchainProvider().runtime();
-        if (runtime != null) {
-          helper.addDep(runtime.getProvider(JavaCompilationArgsProvider.class));
-        }
-      } else {
-        helper.addDep(
-            ruleContext.getPrerequisite(
-                SPEED_PROTO_RUNTIME_ATTR, Mode.TARGET, JavaCompilationArgsProvider.class));
+      TransitiveInfoCollection runtime = getProtoToolchainProvider().runtime();
+      if (runtime != null) {
+        helper.addDep(runtime.getProvider(JavaCompilationArgsProvider.class));
       }
 
       rpcSupport.mutateJavaCompileAction(ruleContext, helper);
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 bb503fc202..c93b05e67e 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
@@ -72,48 +72,6 @@ public class ProtoConfiguration extends Fragment {
     )
     public Label protoCompiler;
 
-    // TODO(b/31775048): Replace with a toolchain
-    /** This is experimental, and is subject to change without warning. */
-    @Option(
-      name = "proto_compiler_java_flags",
-      defaultValue = "--java_out=shared,immutable:%s",
-      category = "experimental",
-      help = "The flags to pass to proto-compiler when generating Java protos."
-    )
-    public String protoCompilerJavaFlags;
-
-    // TODO(b/31775048): Replace with a toolchain
-    /** This is experimental, and is subject to change without warning. */
-    @Option(
-      name = "proto_compiler_java_blacklisted_protos",
-      defaultValue = "",
-      category = "experimental",
-      converter = BuildConfiguration.LabelListConverter.class,
-      help = "A label of a filegroup of .proto files that we shouldn't generate sources for."
-    )
-    public List<Label> protoCompilerJavaBlacklistedProtos;
-
-    // TODO(b/31775048): Replace with a toolchain
-    /** This is experimental, and is subject to change without warning. */
-    @Option(
-      name = "proto_compiler_javalite_flags",
-      defaultValue = "--javalite_out=%s",
-      category = "experimental",
-      help = "The flags to pass to proto-compiler when generating JavaLite protos."
-    )
-    public String protoCompilerJavaLiteFlags;
-
-    // TODO(b/31775048): Replace with a toolchain
-    /** This is experimental, and is subject to change without warning. */
-    @Option(
-      name = "proto_compiler_javalite_plugin",
-      defaultValue = "",
-      category = "experimental",
-      converter = BuildConfiguration.EmptyToNullLabelConverter.class,
-      help = "A label for the javalite proto-compiler plugin, if needed."
-    )
-    public Label protoCompilerJavaLitePlugin;
-
     @Option(
       name = "proto_toolchain_for_javalite",
       defaultValue = "@com_google_protobuf_javalite//:javalite_toolchain",
@@ -143,18 +101,15 @@ public class ProtoConfiguration extends Fragment {
 
     @Option(
       name = "use_toolchain_for_java_proto",
-      defaultValue = "false",
+      defaultValue = "true",
       category = "experimental",
-      help =
-          "If true, --proto_toolchain_for_java will be used for java_proto_library. "
-              + "This flag is an escape-hatch and should be removed once toolchain-based builds "
-              + "are tested."
+      help = "ignored."
     )
     public boolean useToolchainForJavaProto;
 
     @Option(
       name = "strict_proto_deps",
-      defaultValue = "error",
+      defaultValue = "strict",
       converter = BuildConfiguration.StrictDepsConverter.class,
       category = "semantics",
       help =
@@ -178,13 +133,8 @@ public class ProtoConfiguration extends Fragment {
       host.protocOpts = protocOpts;
       host.experimentalProtoExtraActions = experimentalProtoExtraActions;
       host.protoCompiler = protoCompiler;
-      host.protoCompilerJavaFlags = protoCompilerJavaFlags;
-      host.protoCompilerJavaBlacklistedProtos = protoCompilerJavaBlacklistedProtos;
-      host.protoCompilerJavaLiteFlags = protoCompilerJavaLiteFlags;
-      host.protoCompilerJavaLitePlugin = protoCompilerJavaLitePlugin;
       host.protoToolchainForJava = protoToolchainForJava;
       host.protoToolchainForJavaLite = protoToolchainForJavaLite;
-      host.useToolchainForJavaProto = useToolchainForJavaProto;
       host.strictProtoDeps = strictProtoDeps;
       host.outputDescriptorSet = outputDescriptorSet;
       return host;
@@ -215,13 +165,8 @@ public class ProtoConfiguration extends Fragment {
   private final boolean experimentalProtoExtraActions;
   private final ImmutableList<String> protocOpts;
   private final Label protoCompiler;
-  private final String protoCompilerJavaFlags;
-  private final List<Label> protoCompilerJavaBlacklistedProtos;
-  private final String protoCompilerJavaLiteFlags;
-  private final Label protoCompilerJavaLitePlugin;
   private final Label protoToolchainForJava;
   private final Label protoToolchainForJavaLite;
-  private final boolean useToolchainForJavaProto;
   private final Label protoToolchainForCc;
   private final StrictDepsMode strictProtoDeps;
   private final boolean outputDescriptorSet;
@@ -230,13 +175,8 @@ public class ProtoConfiguration extends Fragment {
     this.experimentalProtoExtraActions = options.experimentalProtoExtraActions;
     this.protocOpts = ImmutableList.copyOf(options.protocOpts);
     this.protoCompiler = options.protoCompiler;
-    this.protoCompilerJavaFlags = options.protoCompilerJavaFlags;
-    this.protoCompilerJavaLiteFlags = options.protoCompilerJavaLiteFlags;
-    this.protoCompilerJavaLitePlugin = options.protoCompilerJavaLitePlugin;
-    this.protoCompilerJavaBlacklistedProtos = options.protoCompilerJavaBlacklistedProtos;
     this.protoToolchainForJava = options.protoToolchainForJava;
     this.protoToolchainForJavaLite = options.protoToolchainForJavaLite;
-    this.useToolchainForJavaProto = options.useToolchainForJavaProto;
     this.protoToolchainForCc = options.protoToolchainForCc;
     this.strictProtoDeps = options.strictProtoDeps;
     this.outputDescriptorSet = options.outputDescriptorSet;
@@ -259,22 +199,6 @@ public class ProtoConfiguration extends Fragment {
     return protoCompiler;
   }
 
-  public String protoCompilerJavaFlags() {
-    return protoCompilerJavaFlags;
-  }
-
-  public String protoCompilerJavaLiteFlags() {
-    return protoCompilerJavaLiteFlags;
-  }
-
-  public Label protoCompilerJavaLitePlugin() {
-    return protoCompilerJavaLitePlugin;
-  }
-
-  public List<Label> protoCompilerJavaBlacklistedProtos() {
-    return protoCompilerJavaBlacklistedProtos;
-  }
-
   public Label protoToolchainForJava() {
     return protoToolchainForJava;
   }
@@ -283,10 +207,6 @@ public class ProtoConfiguration extends Fragment {
     return protoToolchainForJavaLite;
   }
 
-  public boolean useToolchainForJavaProto() {
-    return useToolchainForJavaProto;
-  }
-
   public Label protoToolchainForCc() {
     return protoToolchainForCc;
   }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
index 03edbf5d60..0202623d71 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoLibraryTest.java
@@ -77,6 +77,7 @@ public class BazelProtoLibraryTest extends BuildViewTestCase {
 
   @Test
   public void testDescriptorSetOutput_strictDeps() throws Exception {
+    useConfiguration("--strict_proto_deps=error");
     scratch.file(
         "x/BUILD",
         "proto_library(name='nodeps', srcs=['nodeps.proto'])",
@@ -101,6 +102,7 @@ public class BazelProtoLibraryTest extends BuildViewTestCase {
 
   @Test
   public void testDescriptorSetOutput_strictDeps_aliasLibrary() throws Exception {
+    useConfiguration("--strict_proto_deps=error");
     scratch.file(
         "x/BUILD",
         "proto_library(name='alias', deps=[':dep1', ':subalias'])",