Do not add SkylarkProviders to target's provider map unless it is needed.

RELNOTES: None.
PiperOrigin-RevId: 154943665

4 files changed, 21 insertions, 35 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java
index cedb87bd32..f3e1535916 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AbstractConfiguredTarget.java
@@ -150,22 +150,20 @@ public abstract class AbstractConfiguredTarget
 
   @Override
   public ImmutableCollection<String> getKeys() {
+    ImmutableList.Builder<String> result = ImmutableList.builder();
+    result.addAll(ImmutableList.of(
+        DATA_RUNFILES_FIELD,
+        DEFAULT_RUNFILES_FIELD,
+        LABEL_FIELD,
+        FILES_FIELD,
+        FilesToRunProvider.SKYLARK_NAME));
     if (getProvider(OutputGroupProvider.class) != null) {
-      return ImmutableList.of(
-          DATA_RUNFILES_FIELD,
-          DEFAULT_RUNFILES_FIELD,
-          LABEL_FIELD,
-          FILES_FIELD,
-          FilesToRunProvider.SKYLARK_NAME,
-          OutputGroupProvider.SKYLARK_NAME);
-    } else {
-      return ImmutableList.of(
-          DATA_RUNFILES_FIELD,
-          DEFAULT_RUNFILES_FIELD,
-          LABEL_FIELD,
-          FILES_FIELD,
-          FilesToRunProvider.SKYLARK_NAME);
+      result.add(OutputGroupProvider.SKYLARK_NAME);
+    }
+    if (getProvider(SkylarkProviders.class) != null) {
+      result.addAll(getProvider(SkylarkProviders.class).getKeys());
     }
+    return result.build();
   }
 
   private DefaultProvider getDefaultProvider() {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
index 76223780b2..aff014fe55 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/MergedConfiguredTarget.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
-import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import java.util.ArrayList;
@@ -59,15 +58,6 @@ public final class MergedConfiguredTarget extends AbstractConfiguredTarget {
     return provider;
   }
 
-  @Override
-  public ImmutableCollection<String> getKeys() {
-    return ImmutableList.<String>builder()
-        .addAll(super.getKeys())
-        .addAll(getProvider(SkylarkProviders.class).getKeys())
-        .build();
-  }
-
-
   /** Creates an instance based on a configured target and a set of aspects. */
   public static ConfiguredTarget of(ConfiguredTarget base, Iterable<ConfiguredAspect> aspects)
       throws DuplicateException {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
index 54d1e46b1d..7c285aaf15 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTarget.java
@@ -13,8 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.analysis;
 
-import com.google.common.collect.ImmutableCollection;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
@@ -50,7 +48,7 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
   RuleConfiguredTarget(
       RuleContext ruleContext,
       TransitiveInfoProviderMap providers,
-      SkylarkProviders skylarkProviders1) {
+      SkylarkProviders skylarkProviders) {
     super(ruleContext);
     // We don't use ImmutableMap.Builder here to allow augmenting the initial list of 'default'
     // providers by passing them in.
@@ -60,8 +58,10 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
     Preconditions.checkState(providerBuilder.contains(FilesToRunProvider.class));
 
     // Initialize every SkylarkApiProvider
-    skylarkProviders1.init(this);
-    providerBuilder.add(skylarkProviders1);
+    if (!skylarkProviders.isEmpty()) {
+      skylarkProviders.init(this);
+      providerBuilder.add(skylarkProviders);
+    }
 
     this.providers = providerBuilder.build();
     this.configConditions = ruleContext.getConfigConditions();
@@ -110,10 +110,4 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget {
     return String.format("target (rule class of '%s') doesn't have provider '%s'.",
         getTarget().getRuleClass(), name);
   }
-
-  @Override
-  public ImmutableCollection<String> getKeys() {
-    return ImmutableList.<String>builder().addAll(super.getKeys())
-        .addAll(getProvider(SkylarkProviders.class).getKeys()).build();
-  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
index b306b7791b..78cb247164 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
@@ -61,6 +61,10 @@ public final class SkylarkProviders implements TransitiveInfoProvider {
     return skylarkProviders.keySet();
   }
 
+  public boolean isEmpty() {
+    return skylarkProviders.isEmpty() && declaredProviders.isEmpty();
+  }
+
   /** Returns the keys for the declared providers. */
   public ImmutableCollection<ClassObjectConstructor.Key> getDeclaredProviderKeys() {
     return declaredProviders.keySet();