From 7e918290f5d2aee85347ea45d89d05675a40817f Mon Sep 17 00:00:00 2001
From: mstaib <mstaib@google.com>
Date: Tue, 26 Sep 2017 01:42:15 +0200
Subject: Move --plugin to JavaOptions and remove --plugin_copt.

--plugin, though once used for C++, is currently a Java-specific flag.
--plugin_copt is currently a total no-op, and has been for a long time.

Moving these to the Java fragment is a little neater and helps get one
step closer to enforcing LateBoundDefault fragment access.

Additionally, since the "no plugins with duplicate names" restriction
was added to work with plugin_copt, this restriction can be lifted.
It no longer adds any value.

RELNOTES: None.
PiperOrigin-RevId: 169981221
---
 .../lib/analysis/config/BuildConfiguration.java    | 46 ----------------------
 .../build/lib/rules/java/JavaConfiguration.java    |  6 +++
 .../devtools/build/lib/rules/java/JavaOptions.java | 17 ++++++++
 .../build/lib/rules/java/JavaSemantics.java        |  5 +--
 4 files changed, 25 insertions(+), 49 deletions(-)

(limited to 'src/main/java')

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 51e6883457..bc0ba445a6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -520,34 +520,6 @@ public final class BuildConfiguration implements BuildEvent {
     )
     public boolean strictFilesets;
 
-    // Plugins are build using the host config. To avoid cycles we just don't propagate
-    // this option to the host config. If one day we decide to use plugins when building
-    // host tools, we can improve this by (for example) creating a compiler configuration that is
-    // used only for building plugins.
-    @Option(
-      name = "plugin",
-      converter = LabelListConverter.class,
-      allowMultiple = true,
-      defaultValue = "",
-      category = "flags",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help = "Plugins to use in the build. Currently works with java_plugin."
-    )
-    public List<Label> pluginList;
-
-    @Option(
-      name = "plugin_copt",
-      converter = PluginOptionConverter.class,
-      allowMultiple = true,
-      category = "flags",
-      defaultValue = ":",
-      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-      effectTags = {OptionEffectTag.UNKNOWN},
-      help = "Plugin options"
-    )
-    public List<Map.Entry<String, String>> pluginCoptList;
-
     @Option(
       name = "stamp",
       defaultValue = "false",
@@ -1260,20 +1232,6 @@ public final class BuildConfiguration implements BuildEvent {
       fragment.reportInvalidOptions(reporter, this.buildOptions);
     }
 
-    Set<String> plugins = new HashSet<>();
-    for (Label plugin : options.pluginList) {
-      String name = plugin.getName();
-      if (plugins.contains(name)) {
-        reporter.handle(Event.error("A build cannot have two plugins with the same name"));
-      }
-      plugins.add(name);
-    }
-    for (Map.Entry<String, String> opt : options.pluginCoptList) {
-      if (!plugins.contains(opt.getKey())) {
-        reporter.handle(Event.error("A plugin_copt must refer to an existing plugin"));
-      }
-    }
-
     if (options.outputDirectoryName != null) {
       reporter.handle(Event.error(
           "The internal '--output directory name' option cannot be used on the command line"));
@@ -1667,10 +1625,6 @@ public final class BuildConfiguration implements BuildEvent {
     return options.strictFilesets;
   }
 
-  public List<Label> getPlugins() {
-    return options.pluginList;
-  }
-
   public String getMainRepositoryName() {
     return mainRepositoryName.strippedName();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
index faaf5d0e06..e902005435 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
@@ -164,6 +164,7 @@ public final class JavaConfiguration extends Fragment {
   private final boolean explicitJavaTestDeps;
   private final boolean experimentalTestRunner;
   private final boolean jplPropagateCcLinkParamsStore;
+  private final ImmutableList<Label> pluginList;
 
   // TODO(dmarting): remove once we have a proper solution for #2539
   private final boolean legacyBazelJavaTest;
@@ -218,6 +219,7 @@ public final class JavaConfiguration extends Fragment {
       optimizersBuilder.put(mnemonic, Optional.fromNullable(optimizer.getValue()));
     }
     this.bytecodeOptimizers = optimizersBuilder.build();
+    this.pluginList = ImmutableList.copyOf(javaOptions.pluginList);
   }
 
   @SkylarkCallable(name = "default_javac_flags", structField = true,
@@ -420,4 +422,8 @@ public final class JavaConfiguration extends Fragment {
   public boolean jplPropagateCcLinkParamsStore() {
     return jplPropagateCcLinkParamsStore;
   }
+
+  public List<Label> getPlugins() {
+    return pluginList;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
index 7877b78da9..1a608a5e8f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
@@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelConverter;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelListConverter;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.LabelMapConverter;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsConverter;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsMode;
@@ -514,6 +515,22 @@ public class JavaOptions extends FragmentOptions {
   )
   public boolean disableAbsoluteJavabase;
 
+  // Plugins are built using the host config. To avoid cycles we just don't propagate
+  // this option to the host config. If one day we decide to use plugins when building
+  // host tools, we can improve this by (for example) creating a compiler configuration that is
+  // used only for building plugins.
+  @Option(
+    name = "plugin",
+    converter = LabelListConverter.class,
+    allowMultiple = true,
+    defaultValue = "",
+    category = "flags",
+    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+    effectTags = {OptionEffectTag.UNKNOWN},
+    help = "Plugins to use in the build. Currently works with java_plugin."
+  )
+  public List<Label> pluginList;
+
   @Override
   public FragmentOptions getHost() {
     JavaOptions host = (JavaOptions) getDefault();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
index ca43c7e92e..0dd4ab0b87 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSemantics.java
@@ -171,12 +171,11 @@ public interface JavaSemantics {
             return javaConfig.getJavaLauncherLabel();
           });
 
-  // TODO(b/65746853): provide a way to do this without passing the entire configuration
   LateBoundDefault<?, List<Label>> JAVA_PLUGINS =
       LateBoundDefault.fromTargetConfiguration(
-          BuildConfiguration.class,
+          JavaConfiguration.class,
           ImmutableList.of(),
-          (rule, attributes, configuration) -> ImmutableList.copyOf(configuration.getPlugins()));
+          (rule, attributes, javaConfig) -> ImmutableList.copyOf(javaConfig.getPlugins()));
 
   /** Implementation for the :proguard attribute. */
   LateBoundDefault<?, Label> PROGUARD =
-- 
cgit v1.2.3