Do not put OutputGroupProvider into SkylarkProviders.
Almost every target has an OutputGroupProvider. Putting an
("output_groups", value) pair into SkylarkProviders creates an
unneccessary map. This CL removes it.
RELNOTES: None.
PiperOrigin-RevId: 154940624
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 cb7daee..cedb87b 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
@@ -79,15 +79,6 @@
return configuration;
}
- @Nullable
- @Override
- public Object get(SkylarkProviderIdentifier id) {
- if (id.isLegacy()) {
- return get(id.getLegacyId());
- }
- return get(id.getKey());
- }
-
@Override
public Label getLabel() {
return getTarget().getLabel();
@@ -159,12 +150,22 @@
@Override
public ImmutableCollection<String> getKeys() {
- return 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);
+ }
}
private DefaultProvider getDefaultProvider() {
@@ -179,6 +180,16 @@
return defaultProvider.get();
}
+ @Nullable
+ @Override
+ public Object get(SkylarkProviderIdentifier id) {
+ if (id.isLegacy()) {
+ return get(id.getLegacyId());
+ }
+ return get(id.getKey());
+ }
+
+
/** Returns a declared provider provided by this target. Only meant to use from Skylark. */
@Nullable
@Override
@@ -195,4 +206,17 @@
}
return null;
}
+
+ /**
+ * Returns a value provided by this target. Only meant to use from Skylark.
+ */
+ @Override
+ public final Object get(String providerKey) {
+ if (OutputGroupProvider.SKYLARK_NAME.equals(providerKey)) {
+ return getProvider(OutputGroupProvider.class);
+ }
+ SkylarkProviders skylarkProviders = getProvider(SkylarkProviders.class);
+ return skylarkProviders != null ? skylarkProviders.getValue(providerKey) : null;
+ }
+
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
index 18c00f9..116df61 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java
@@ -33,6 +33,7 @@
import com.google.devtools.build.lib.packages.ClassObjectConstructor.Key;
import com.google.devtools.build.lib.packages.SkylarkClassObject;
import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
+import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.util.Preconditions;
import java.util.Arrays;
@@ -92,6 +93,33 @@
return providers.getProvider(providerClass);
}
+ public Object getProvider(SkylarkProviderIdentifier id) {
+ if (id.isLegacy()) {
+ return get(id.getLegacyId());
+ } else {
+ return get(id.getKey());
+ }
+ }
+
+ public SkylarkClassObject get(ClassObjectConstructor.Key key) {
+ if (OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey().equals(key)) {
+ return getProvider(OutputGroupProvider.class);
+ }
+ SkylarkProviders skylarkProviders = getProvider(SkylarkProviders.class);
+ return skylarkProviders != null ? skylarkProviders.getDeclaredProvider(key) : null;
+ }
+
+ public Object get(String legacyKey) {
+ if (OutputGroupProvider.SKYLARK_NAME.equals(legacyKey)) {
+ return getProvider(OutputGroupProvider.class);
+ }
+ SkylarkProviders skylarkProviders = getProvider(SkylarkProviders.class);
+ return skylarkProviders != null
+ ? skylarkProviders.get(SkylarkProviderIdentifier.forLegacy(legacyKey))
+ : null;
+ }
+
+
@Override
public UnmodifiableIterator<TransitiveInfoProvider> iterator() {
return providers.values().iterator();
@@ -210,14 +238,23 @@
throw new EvalException(
constructor.getLocation(), "All providers must be top level values");
}
- skylarkDeclaredProvidersBuilder.put(constructor.getKey(), declaredProvider);
+ ClassObjectConstructor.Key key = constructor.getKey();
+ addDeclaredProvider(key, declaredProvider);
return this;
}
+ private void addDeclaredProvider(Key key, SkylarkClassObject declaredProvider) {
+ if (OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey().equals(key)) {
+ addProvider(OutputGroupProvider.class, (OutputGroupProvider) declaredProvider);
+ } else {
+ skylarkDeclaredProvidersBuilder.put(key, declaredProvider);
+ }
+ }
+
public Builder addNativeDeclaredProvider(SkylarkClassObject declaredProvider) {
ClassObjectConstructor constructor = declaredProvider.getConstructor();
Preconditions.checkState(constructor.isExported());
- skylarkDeclaredProvidersBuilder.put(constructor.getKey(), declaredProvider);
+ addDeclaredProvider(constructor.getKey(), declaredProvider);
return this;
}
@@ -234,8 +271,7 @@
throw new IllegalStateException(
"OutputGroupProvider was provided explicitly; do not use addOutputGroup");
}
- skylarkDeclaredProvidersBuilder.put(
- OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey(),
+ addDeclaredProvider(OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey(),
new OutputGroupProvider(outputGroups.build()));
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
index d560923..b1ec35d 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java
@@ -388,8 +388,7 @@
}
for (SkylarkProviderIdentifier providerId : advertisedProviders.getSkylarkProviders()) {
- SkylarkProviders skylarkProviders = configuredAspect.getProvider(SkylarkProviders.class);
- if (skylarkProviders == null || skylarkProviders.get(providerId) == null) {
+ if (configuredAspect.getProvider(providerId) == null) {
eventHandler.handle(Event.error(
target.getLocation(),
String.format(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java
index 5d3ca37..eb7a1e1 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/EnvironmentGroupConfiguredTarget.java
@@ -35,12 +35,6 @@
return (EnvironmentGroup) super.getTarget();
}
- @Override
- public Object get(String providerKey) {
- // No providers.
- return null;
- }
-
@Nullable
@Override
public SkylarkClassObject get(ClassObjectConstructor.Key providerKey) {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java
index 75495da..7f3f9f6 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/FileConfiguredTarget.java
@@ -77,9 +77,4 @@
AnalysisUtils.checkProvider(provider);
return providers.getProvider(provider);
}
-
- @Override
- public Object get(String providerKey) {
- return null;
- }
}
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 bff4b1c..7622378 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
@@ -15,10 +15,7 @@
import com.google.common.collect.ImmutableCollection;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
-import com.google.devtools.build.lib.packages.SkylarkClassObject;
-import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -50,12 +47,6 @@
this.providers = providers;
}
- /** Returns a value provided by this target. Only meant to use from Skylark. */
- @Override
- public Object get(String providerKey) {
- return getProvider(SkylarkProviders.class).getValue(providerKey);
- }
-
@Override
public <P extends TransitiveInfoProvider> P getProvider(Class<P> providerClass) {
AnalysisUtils.checkProvider(providerClass);
@@ -90,22 +81,8 @@
OutputGroupProvider.merge(getAllOutputGroupProviders(base, aspects));
// Merge Skylark providers.
- ImmutableMap<String, Object> premergedLegacyProviders =
- mergedOutputGroupProvider == null
- ? ImmutableMap.<String, Object>of()
- : ImmutableMap.<String, Object>of(
- OutputGroupProvider.SKYLARK_NAME, mergedOutputGroupProvider);
-
- ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject> premergedProviders =
- mergedOutputGroupProvider == null
- ? ImmutableMap.<SkylarkClassObjectConstructor.Key, SkylarkClassObject>of()
- : ImmutableMap.<SkylarkClassObjectConstructor.Key, SkylarkClassObject>of(
- OutputGroupProvider.SKYLARK_CONSTRUCTOR.getKey(), mergedOutputGroupProvider);
SkylarkProviders mergedSkylarkProviders =
- SkylarkProviders.merge(
- premergedLegacyProviders,
- premergedProviders,
- getAllProviders(base, aspects, SkylarkProviders.class));
+ SkylarkProviders.merge(getAllProviders(base, aspects, SkylarkProviders.class));
// Merge extra-actions provider.
ExtraActionArtifactsProvider mergedExtraActionProviders = ExtraActionArtifactsProvider.merge(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java
index 0606a1f..06bbfdc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java
@@ -127,17 +127,12 @@
@Nullable
public static OutputGroupProvider get(TransitiveInfoCollection collection) {
- return (OutputGroupProvider) collection.getProvider(OutputGroupProvider.class);
+ return collection.getProvider(OutputGroupProvider.class);
}
@Nullable
public static OutputGroupProvider get(ConfiguredAspect aspect) {
- SkylarkProviders skylarkProviders = aspect.getProvider(SkylarkProviders.class);
-
-
- return skylarkProviders != null
- ? (OutputGroupProvider) skylarkProviders.getDeclaredProvider(SKYLARK_CONSTRUCTOR.getKey())
- : null;
+ return (OutputGroupProvider) aspect.get(SKYLARK_CONSTRUCTOR.getKey());
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java
index 7a3a33a..3e0b855 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java
@@ -67,11 +67,6 @@
return packageSpecifications;
}
- @Override
- public Object get(String providerKey) {
- // No providers.
- return null;
- }
@Nullable
@Override
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 2879236..54d1e46 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
@@ -100,14 +100,6 @@
return providers.getProvider(providerClass);
}
- /**
- * Returns a value provided by this target. Only meant to use from Skylark.
- */
- @Override
- public Object get(String providerKey) {
- return getProvider(SkylarkProviders.class).getValue(providerKey);
- }
-
@Override
public final Rule getTarget() {
return (Rule) super.getTarget();
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
index 16baad7..512f576 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java
@@ -132,7 +132,6 @@
OutputGroupProvider outputGroupProvider = new OutputGroupProvider(outputGroups.build());
addProvider(OutputGroupProvider.class, outputGroupProvider);
- addSkylarkTransitiveInfo(OutputGroupProvider.SKYLARK_NAME, outputGroupProvider);
}
TransitiveInfoProviderMap providers = providersBuilder.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 99a4534..b306b77 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
@@ -20,7 +20,6 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.ClassObjectConstructor;
import com.google.devtools.build.lib.packages.SkylarkClassObject;
-import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor;
import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
import com.google.devtools.build.lib.rules.SkylarkApiProvider;
import com.google.devtools.build.lib.syntax.EvalException;
@@ -120,45 +119,34 @@
/**
* Merges skylark providers. The set of providers must be disjoint.
*
- * @param premergedProviders providers that has already been merged. They will
- * be put into the result as-is and their presence will be ignored among {@code providers}.
* @param providers providers to merge {@code this} with.
*/
- public static SkylarkProviders merge(
- ImmutableMap<String, Object> premergedLegacyProviders,
- ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject> premergedProviders,
- List<SkylarkProviders> providers)
+ public static SkylarkProviders merge(List<SkylarkProviders> providers)
throws DuplicateException {
- if (premergedProviders.size() == 0 && providers.size() == 0) {
+ if (providers.size() == 0) {
return null;
}
- if (premergedProviders.size() == 0 && providers.size() == 1) {
+ if (providers.size() == 1) {
return providers.get(0);
}
ImmutableMap<String, Object> skylarkProviders = mergeMaps(providers,
- SKYLARK_PROVIDERS_MAP_FUNCTION,
- premergedLegacyProviders);
+ SKYLARK_PROVIDERS_MAP_FUNCTION);
ImmutableMap<ClassObjectConstructor.Key, SkylarkClassObject> declaredProviders =
- mergeMaps(providers, DECLARED_PROVIDERS_MAP_FUNCTION,
- premergedProviders);
+ mergeMaps(providers, DECLARED_PROVIDERS_MAP_FUNCTION);
return new SkylarkProviders(skylarkProviders, declaredProviders);
}
private static <K, V> ImmutableMap<K, V> mergeMaps(List<SkylarkProviders> providers,
- Function<SkylarkProviders, Map<K, V>> mapGetter, Map<K, V> premerged)
+ Function<SkylarkProviders, Map<K, V>> mapGetter)
throws DuplicateException {
Set<K> seenKeys = new HashSet<>();
ImmutableMap.Builder<K, V> resultBuilder = ImmutableMap.builder();
- resultBuilder.putAll(premerged);
for (SkylarkProviders provider : providers) {
Map<K, V> map = mapGetter.apply(provider);
for (K key : map.keySet()) {
- if (premerged.containsKey(key)) {
- continue;
- }
if (!seenKeys.add(key)) {
// TODO(dslomov): add better diagnostics.
throw new DuplicateException("Provider " + key + " provided twice");
