diff options
8 files changed, 224 insertions, 30 deletions
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 402668a10a..86a0d54216 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 @@ -26,13 +26,13 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.SkylarkClassObject; +import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor.Key; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.util.Preconditions; - import java.util.LinkedHashMap; import java.util.Map; import java.util.TreeMap; - import javax.annotation.Nullable; /** @@ -174,7 +174,8 @@ public final class ConfiguredAspect implements Iterable<TransitiveInfoProvider> ImmutableMap<String, Object> skylarkProvidersMap = skylarkProviderBuilder.build(); if (!skylarkProvidersMap.isEmpty()) { - providers.put(SkylarkProviders.class, new SkylarkProviders(skylarkProvidersMap)); + providers.put(SkylarkProviders.class, new SkylarkProviders(skylarkProvidersMap, + ImmutableMap.<Key, SkylarkClassObject>of())); } addProvider( 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 fc29d3e82e..91af120e62 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 @@ -21,9 +21,7 @@ import com.google.devtools.build.lib.analysis.config.RunUnder; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.OutputFile; import com.google.devtools.build.lib.packages.Rule; -import com.google.devtools.build.lib.rules.SkylarkApiProvider; import com.google.devtools.build.lib.util.Preconditions; - import java.util.LinkedHashMap; import java.util.Map; @@ -51,8 +49,8 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget { private final ImmutableMap<Label, ConfigMatchingProvider> configConditions; RuleConfiguredTarget(RuleContext ruleContext, - ImmutableMap<String, Object> skylarkProviders, - Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> providers) { + Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> providers, + SkylarkProviders skylarkProviders1) { super(ruleContext); // We don't use ImmutableMap.Builder here to allow augmenting the initial list of 'default' // providers by passing them in. @@ -63,13 +61,8 @@ public final class RuleConfiguredTarget extends AbstractConfiguredTarget { Preconditions.checkState(providerBuilder.containsKey(FilesToRunProvider.class)); // Initialize every SkylarkApiProvider - for (Object provider : skylarkProviders.values()) { - if (provider instanceof SkylarkApiProvider) { - ((SkylarkApiProvider) provider).init(this); - } - } - - providerBuilder.put(SkylarkProviders.class, new SkylarkProviders(skylarkProviders)); + skylarkProviders1.init(this); + providerBuilder.put(SkylarkProviders.class, skylarkProviders1); this.providers = ImmutableMap.copyOf(providerBuilder); this.configConditions = ruleContext.getConfigConditions(); 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 ee56c4fbe1..d111fd6f58 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 @@ -29,6 +29,8 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.SkylarkClassObject; +import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.test.ExecutionInfoProvider; @@ -40,7 +42,6 @@ import com.google.devtools.build.lib.rules.test.TestProvider.TestParams; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.lib.util.Preconditions; - import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; @@ -61,6 +62,8 @@ public final class RuleConfiguredTargetBuilder { private final Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> providers = new LinkedHashMap<>(); private final ImmutableMap.Builder<String, Object> skylarkProviders = ImmutableMap.builder(); + private final ImmutableMap.Builder<SkylarkClassObjectConstructor.Key, SkylarkClassObject> + skylarkDeclaredProviders = ImmutableMap.builder(); private final Map<String, NestedSetBuilder<Artifact>> outputGroupBuilders = new TreeMap<>(); /** These are supported by all configured targets and need to be specially handled. */ @@ -131,7 +134,10 @@ public final class RuleConfiguredTargetBuilder { addRegisteredProvidersToSkylarkProviders(); - return new RuleConfiguredTarget(ruleContext, skylarkProviders.build(), providers); + return new RuleConfiguredTarget( + ruleContext, + providers, + new SkylarkProviders(skylarkProviders.build(), skylarkDeclaredProviders.build())); } /** @@ -271,6 +277,19 @@ public final class RuleConfiguredTargetBuilder { return this; } + public RuleConfiguredTargetBuilder addSkylarkDeclaredProvider( + SkylarkClassObject provider, Location loc) throws EvalException { + SkylarkClassObjectConstructor constructor = provider.getConstructor(); + SkylarkProviderValidationUtil.validateAndThrowEvalException( + constructor.getPrintableName(), provider, loc); + if (!constructor.isExported()) { + throw new EvalException(constructor.getLocation(), + "All providers must be top level values"); + } + skylarkDeclaredProviders.put(constructor.getKey(), provider); + return this; + } + /** * Add a Skylark transitive info. The provider value must be safe. */ 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 1f8f1280a0..6ed41d9009 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 @@ -13,15 +13,19 @@ // limitations under the License. package com.google.devtools.build.lib.analysis; +import com.google.common.base.Function; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.SkylarkClassObject; +import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor; +import com.google.devtools.build.lib.rules.SkylarkApiProvider; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.SkylarkType; import com.google.devtools.build.lib.util.Preconditions; - import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -29,11 +33,23 @@ import java.util.Set; */ @Immutable public final class SkylarkProviders implements TransitiveInfoProvider { + private final ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject> + declaredProviders; private final ImmutableMap<String, Object> skylarkProviders; - SkylarkProviders(ImmutableMap<String, Object> skylarkProviders) { - Preconditions.checkNotNull(skylarkProviders); - this.skylarkProviders = skylarkProviders; + SkylarkProviders( + ImmutableMap<String, Object> skylarkProviders, + ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject> declaredProviders) { + this.declaredProviders = Preconditions.checkNotNull(declaredProviders); + this.skylarkProviders = Preconditions.checkNotNull(skylarkProviders); + } + + public void init(ConfiguredTarget target) { + for (Object o : skylarkProviders.values()) { + if (o instanceof SkylarkApiProvider) { + ((SkylarkApiProvider) o).init(target); + } + } } /** @@ -62,12 +78,35 @@ public final class SkylarkProviders implements TransitiveInfoProvider { return type.cast(obj); } + public SkylarkClassObject getDeclaredProvider(SkylarkClassObjectConstructor.Key key) { + return declaredProviders.get(key); + } + + + private static final Function<SkylarkProviders, Map<String, Object>> + SKYLARK_PROVIDERS_MAP_FUNCTION = new Function<SkylarkProviders, Map<String, Object>>() { + @Override + public Map<String, Object> apply(SkylarkProviders skylarkProviders) { + return skylarkProviders.skylarkProviders; + } + }; + + public static final Function<SkylarkProviders, + Map<SkylarkClassObjectConstructor.Key, SkylarkClassObject>> + DECLARED_PROVIDERS_MAP_FUNCTION = + new Function<SkylarkProviders, Map<SkylarkClassObjectConstructor.Key, SkylarkClassObject>>() { + @Override + public Map<SkylarkClassObjectConstructor.Key, SkylarkClassObject> apply( + SkylarkProviders skylarkProviders) { + return skylarkProviders.declaredProviders; + } + }; + /** * Merges skylark providers. The set of providers must be disjoint. * * @param providers providers to merge {@code this} with. */ - public static SkylarkProviders merge(List<SkylarkProviders> providers) { if (providers.size() == 0) { return null; @@ -76,18 +115,29 @@ public final class SkylarkProviders implements TransitiveInfoProvider { return providers.get(0); } - ImmutableMap.Builder<String, Object> resultBuilder = new ImmutableMap.Builder<>(); - Set<String> seenKeys = new HashSet<>(); + ImmutableMap<String, Object> skylarkProviders = mergeMaps(providers, + SKYLARK_PROVIDERS_MAP_FUNCTION); + ImmutableMap<SkylarkClassObjectConstructor.Key, SkylarkClassObject> declaredProviders = + 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) { + ImmutableMap.Builder<K, V> resultBuilder = new ImmutableMap.Builder<>(); + Set<K> seenKeys = new HashSet<>(); for (SkylarkProviders provider : providers) { - for (String key : provider.skylarkProviders.keySet()) { + Map<K, V> map = mapGetter.apply(provider); + for (K key : map.keySet()) { if (!seenKeys.add(key)) { // TODO(dslomov): add better diagnostics. throw new IllegalStateException("Skylark provider " + key + " provided twice"); } - resultBuilder.put(key, provider.getValue(key)); + resultBuilder.put(key, map.get(key)); } } - return new SkylarkProviders(resultBuilder.build()); + return resultBuilder.build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java index 5e5183cbc6..7afcbd5a1a 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java @@ -123,7 +123,7 @@ public class SkylarkClassObject implements ClassObject, SkylarkValue, Concatable public Concatter getConcatter() { return StructConcatter.INSTANCE; } - + public SkylarkClassObjectConstructor getConstructor() { return constructor; } diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObjectConstructor.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObjectConstructor.java index d090c950a3..c3b991d566 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObjectConstructor.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObjectConstructor.java @@ -79,8 +79,8 @@ public final class SkylarkClassObjectConstructor extends BaseFunction implements return key != null; } - @Nullable public Key getKey() { + Preconditions.checkState(isExported()); return key; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java index 52e4750c7c..064554809c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java @@ -88,8 +88,13 @@ public final class SkylarkRuleConfiguredTargetBuilder { if (ruleContext.hasErrors()) { return null; - } else if (!(target instanceof SkylarkClassObject) && target != Runtime.NONE) { - ruleContext.ruleError("Rule implementation doesn't return a struct"); + } else if ( + !(target instanceof SkylarkClassObject) && target != Runtime.NONE + && !(target instanceof Iterable)) { + ruleContext.ruleError( + String.format( + "Rule should return a return a struct or a list, but got %s", + SkylarkType.typeOf(target))); return null; } else if (!expectFailure.isEmpty()) { ruleContext.ruleError("Expected failure not found: " + expectFailure); @@ -248,6 +253,7 @@ public final class SkylarkRuleConfiguredTargetBuilder { Location insLoc = insStruct.getCreationLoc(); FileTypeSet fileTypeSet = FileTypeSet.ANY_FILE; if (insStruct.getKeys().contains("extensions")) { + @SuppressWarnings("unchecked") List<String> exts = cast( "extensions", insStruct, SkylarkList.class, String.class, insLoc); if (exts.isEmpty()) { @@ -285,11 +291,26 @@ public final class SkylarkRuleConfiguredTargetBuilder { Class<? extends TransitiveInfoProvider> providerType = registeredProviderTypes.get(key); TransitiveInfoProvider provider = cast(key, struct, providerType, loc); builder.addProvider(providerType, provider); + } else if (key.equals("providers")) { + Iterable iterable = cast(key, struct, Iterable.class, loc); + for (Object o : iterable) { + SkylarkClassObject declaredProvider = SkylarkType.cast(o, SkylarkClassObject.class, loc, + "The value of 'providers' should be a sequence of declared providers"); + builder.addSkylarkDeclaredProvider(declaredProvider, loc); + } } else if (!key.equals("executable")) { // We handled executable already. builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc); } } + } else if (target instanceof Iterable) { + loc = ruleContext.getRule().getRuleClassObject().getConfiguredTargetFunction().getLocation(); + for (Object o : (Iterable) target) { + SkylarkClassObject declaredProvider = SkylarkType.cast(o, SkylarkClassObject.class, loc, + "A return value of rule implementation function should be " + + "a sequence of declared providers"); + builder.addSkylarkDeclaredProvider(declaredProvider, declaredProvider.getCreationLoc()); + } } if ((statelessRunfiles != null) && (dataRunfiles != null || defaultRunfiles != null)) { diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java new file mode 100644 index 0000000000..7a4b06c78d --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java @@ -0,0 +1,110 @@ +// Copyright 2016 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.skylark; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.devtools.build.lib.analysis.BuildView.AnalysisResult; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.SkylarkProviders; +import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.SkylarkClassObject; +import com.google.devtools.build.lib.packages.SkylarkClassObjectConstructor; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * Integration tests for Skylark. + */ +@RunWith(JUnit4.class) +public class SkylarkIntegrationTest extends AnalysisTestCase { + protected boolean keepGoing() { + return false; + } + + @Test + public void rulesReturningDeclaredProviders() throws Exception { + scratch.file( + "test/extension.bzl", + "my_provider = provider()", + "def _impl(ctx):", + " return [my_provider(x = 1)]", + "my_rule = rule(_impl)" + ); + scratch.file( + "test/BUILD", + "load(':extension.bzl', 'my_rule')", + "my_rule(name = 'r')" + ); + + AnalysisResult analysisResult = update("//test:r"); + ConfiguredTarget configuredTarget = analysisResult.getTargetsToBuild().iterator().next(); + SkylarkClassObjectConstructor.Key key = new SkylarkClassObjectConstructor.Key( + Label.create(configuredTarget.getLabel().getPackageIdentifier(), "extension.bzl"), + "my_provider"); + SkylarkProviders skylarkProviders = configuredTarget.getProvider(SkylarkProviders.class); + assertThat(skylarkProviders).isNotNull(); + SkylarkClassObject declaredProvider = skylarkProviders.getDeclaredProvider(key); + assertThat(declaredProvider).isNotNull(); + assertThat(declaredProvider.getConstructor().getKey()).isEqualTo(key); + assertThat(declaredProvider.getValue("x")).isEqualTo(1); + } + + @Test + public void rulesReturningDeclaredProvidersCompatMode() throws Exception { + scratch.file( + "test/extension.bzl", + "my_provider = provider()", + "def _impl(ctx):", + " return struct(providers = [my_provider(x = 1)])", + "my_rule = rule(_impl)" + ); + scratch.file( + "test/BUILD", + "load(':extension.bzl', 'my_rule')", + "my_rule(name = 'r')" + ); + + AnalysisResult analysisResult = update("//test:r"); + ConfiguredTarget configuredTarget = analysisResult.getTargetsToBuild().iterator().next(); + SkylarkClassObjectConstructor.Key key = new SkylarkClassObjectConstructor.Key( + Label.create(configuredTarget.getLabel().getPackageIdentifier(), "extension.bzl"), + "my_provider"); + SkylarkProviders skylarkProviders = configuredTarget.getProvider(SkylarkProviders.class); + assertThat(skylarkProviders).isNotNull(); + SkylarkClassObject declaredProvider = skylarkProviders.getDeclaredProvider(key); + assertThat(declaredProvider).isNotNull(); + assertThat(declaredProvider.getConstructor().getKey()).isEqualTo(key); + assertThat(declaredProvider.getValue("x")).isEqualTo(1); + } + + /** + * Same test with "keep going". + */ + @RunWith(JUnit4.class) + public static final class WithKeepGoing extends SkylarkIntegrationTest { + @Override + protected FlagBuilder defaultFlags() { + return new FlagBuilder().with(Flag.KEEP_GOING); + } + + @Override + protected boolean keepGoing() { + return true; + } + } + +} |