diff options
7 files changed, 92 insertions, 68 deletions
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 e2fbee956e..a329a4c78f 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 @@ -52,6 +52,7 @@ import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.packages.RuleVisibility; +import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier; import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder; import com.google.devtools.build.lib.rules.fileset.FilesetProvider; @@ -383,16 +384,16 @@ public final class ConfiguredTargetFactory { } } - for (String providerName : advertisedProviders.getSkylarkProviders()) { + for (SkylarkProviderIdentifier providerId : advertisedProviders.getSkylarkProviders()) { SkylarkProviders skylarkProviders = configuredAspect.getProvider(SkylarkProviders.class); - if (skylarkProviders == null || skylarkProviders.getValue(providerName) == null) { + if (skylarkProviders == null || skylarkProviders.get(providerId) == null) { eventHandler.handle(Event.error( target.getLocation(), String.format( "Aspect '%s', applied to '%s', does not provide advertised provider '%s'", configuredAspect.getName(), target.getLabel(), - providerName + providerId ))); } } 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 a568329754..05c9c347a1 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,6 +20,7 @@ import com.google.devtools.build.lib.analysis.MergedConfiguredTarget.DuplicateEx 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.SkylarkProviderIdentifier; import com.google.devtools.build.lib.rules.SkylarkApiProvider; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.SkylarkType; @@ -88,6 +89,13 @@ public final class SkylarkProviders implements TransitiveInfoProvider { return declaredProviders.get(key); } + public Object get(SkylarkProviderIdentifier id) { + if (id.isLegacy()) { + return getValue(id.getLegacyId()); + } + return getDeclaredProvider(id.getKey()); + } + private static final Function<SkylarkProviders, Map<String, Object>> SKYLARK_PROVIDERS_MAP_FUNCTION = new Function<SkylarkProviders, Map<String, Object>>() { diff --git a/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java b/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java index 59c25830bd..1f1ddd6d85 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AdvertisedProviderSet.java @@ -38,12 +38,11 @@ import java.util.Objects; public final class AdvertisedProviderSet { private final boolean canHaveAnyProvider; private final ImmutableSet<Class<?>> nativeProviders; - // todo(dslomov,vladmos): support declared providers - private final ImmutableSet<String> skylarkProviders; + private final ImmutableSet<SkylarkProviderIdentifier> skylarkProviders; private AdvertisedProviderSet(boolean canHaveAnyProvider, ImmutableSet<Class<?>> nativeProviders, - ImmutableSet<String> skylarkProviders) { + ImmutableSet<SkylarkProviderIdentifier> skylarkProviders) { this.canHaveAnyProvider = canHaveAnyProvider; this.nativeProviders = nativeProviders; this.skylarkProviders = skylarkProviders; @@ -52,15 +51,15 @@ public final class AdvertisedProviderSet { public static final AdvertisedProviderSet ANY = new AdvertisedProviderSet(true, ImmutableSet.<Class<?>>of(), - ImmutableSet.<String>of()); + ImmutableSet.<SkylarkProviderIdentifier>of()); public static final AdvertisedProviderSet EMPTY = new AdvertisedProviderSet(false, ImmutableSet.<Class<?>>of(), - ImmutableSet.<String>of()); + ImmutableSet.<SkylarkProviderIdentifier>of()); public static AdvertisedProviderSet create( ImmutableSet<Class<?>> nativeProviders, - ImmutableSet<String> skylarkProviders) { + ImmutableSet<SkylarkProviderIdentifier> skylarkProviders) { if (nativeProviders.isEmpty() && skylarkProviders.isEmpty()) { return EMPTY; } @@ -106,7 +105,7 @@ public final class AdvertisedProviderSet { /** * Get all advertised Skylark providers. */ - public ImmutableSet<String> getSkylarkProviders() { + public ImmutableSet<SkylarkProviderIdentifier> getSkylarkProviders() { return skylarkProviders; } @@ -118,7 +117,7 @@ public final class AdvertisedProviderSet { public static class Builder { private boolean canHaveAnyProvider; private final ArrayList<Class<?>> nativeProviders; - private final ArrayList<String> skylarkProviders; + private final ArrayList<SkylarkProviderIdentifier> skylarkProviders; private Builder() { nativeProviders = new ArrayList<>(); skylarkProviders = new ArrayList<>(); @@ -156,7 +155,17 @@ public final class AdvertisedProviderSet { } public Builder addSkylark(String providerName) { - skylarkProviders.add(providerName); + skylarkProviders.add(SkylarkProviderIdentifier.forLegacy(providerName)); + return this; + } + + public Builder addSkylark(SkylarkProviderIdentifier id) { + skylarkProviders.add(id); + return this; + } + + public Builder addSkylark(ClassObjectConstructor.Key id) { + skylarkProviders.add(SkylarkProviderIdentifier.forKey(id)); return this; } } diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeClassObjectConstructor.java b/src/main/java/com/google/devtools/build/lib/packages/NativeClassObjectConstructor.java index 011bf3bcfe..b3fd87e73f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/NativeClassObjectConstructor.java +++ b/src/main/java/com/google/devtools/build/lib/packages/NativeClassObjectConstructor.java @@ -18,6 +18,7 @@ import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.syntax.FunctionSignature; import com.google.devtools.build.lib.syntax.SkylarkType; +import com.google.devtools.build.lib.util.Pair; import java.util.Map; import javax.annotation.Nullable; @@ -134,6 +135,17 @@ public abstract class NativeClassObjectConstructor extends ClassObjectConstructo String.format("'%s' cannot be constructed from Skylark", getPrintableName())); } + public static Pair<String, String> getSerializedRepresentationForNativeKey(NativeKey key) { + return Pair.of(key.name, key.aClass.getName()); + } + + public static NativeKey getNativeKeyFromSerializedRepresentation(Pair<String, String> serialized) + throws ClassNotFoundException { + Class<? extends NativeClassObjectConstructor> aClass = + Class.forName(serialized.second).asSubclass(NativeClassObjectConstructor.class); + return new NativeKey(serialized.first, aClass); + } + /** * A serializable representation of {@link NativeClassObjectConstructor}. * diff --git a/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java b/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java index 371006b6b3..24719c6bf2 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java +++ b/src/main/java/com/google/devtools/build/lib/packages/RequiredProviders.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.packages; import com.google.common.base.Predicate; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; @@ -120,16 +121,7 @@ public final class RequiredProviders { return advertisedProviderSet.getNativeProviders().contains(aClass); } }, - new Predicate<SkylarkProviderIdentifier>() { - @Override - public boolean apply(SkylarkProviderIdentifier skylarkProviderIdentifier) { - if (!skylarkProviderIdentifier.isLegacy()) { - return false; - } - return advertisedProviderSet.getSkylarkProviders() - .contains(skylarkProviderIdentifier.getLegacyId()); - } - }, + Predicates.in(advertisedProviderSet.getSkylarkProviders()), requiredProviders ); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProviderIdentifier.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProviderIdentifier.java index 42684f7f34..d9e663c43c 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProviderIdentifier.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProviderIdentifier.java @@ -88,7 +88,7 @@ public final class SkylarkProviderIdentifier { @Override public int hashCode() { - return Objects.hash(legacyId, key); + return legacyId != null ? legacyId.hashCode() * 2 : key.hashCode(); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java b/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java index 18a7bf366b..abc3ec917d 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java @@ -16,8 +16,11 @@ package com.google.devtools.build.lib.packages; import static com.google.common.truth.Truth.assertThat; -import com.google.common.base.Predicate; +import com.google.common.base.Predicates; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.RequiredProviders.Builder; import org.junit.Test; import org.junit.runner.RunWith; @@ -28,27 +31,37 @@ import org.junit.runners.JUnit4; */ @RunWith(JUnit4.class) public class RequiredProvidersTest { - private static boolean satisfies(final AdvertisedProviderSet providers, + private static final class P1 {} + private static final class P2 {} + private static final class P3 {} + + private static final ClassObjectConstructor P_NATIVE = + new NativeClassObjectConstructor("p_native") {}; + + private static final SkylarkClassObjectConstructor P_SKYLARK = + new SkylarkClassObjectConstructor("p_skylark", Location.BUILTIN); + static { + try { + P_SKYLARK.export(Label.create("foo/bar", "x.bzl"), "p_skylark"); + } catch (LabelSyntaxException e) { + throw new AssertionError(e); + } + } + + private static final SkylarkProviderIdentifier ID_NATIVE = + SkylarkProviderIdentifier.forKey(P_NATIVE.getKey()); + private static final SkylarkProviderIdentifier ID_SKYLARK = + SkylarkProviderIdentifier.forKey(P_SKYLARK.getKey()); + private static final SkylarkProviderIdentifier ID_LEGACY = + SkylarkProviderIdentifier.forLegacy("p_legacy"); + + private static boolean satisfies(AdvertisedProviderSet providers, RequiredProviders requiredProviders) { boolean result = requiredProviders.isSatisfiedBy(providers); assertThat(requiredProviders.isSatisfiedBy( - new Predicate<Class<?>>() { - @Override - public boolean apply(Class<?> aClass) { - return providers.getNativeProviders().contains(aClass); - } - }, - new Predicate<SkylarkProviderIdentifier>() { - @Override - public boolean apply(SkylarkProviderIdentifier skylarkProviderIdentifier) { - if (!skylarkProviderIdentifier.isLegacy()) { - return false; - } - return providers.getSkylarkProviders() - .contains(skylarkProviderIdentifier.getLegacyId()); - } - } + Predicates.in(providers.getNativeProviders()), + Predicates.in(providers.getSkylarkProviders()) )).isEqualTo(result); return result; } @@ -89,10 +102,6 @@ public class RequiredProvidersTest { )).isFalse(); } - private static final class P1 {} - private static final class P2 {} - private static final class P3 {} - @Test public void nativeProvidersAllMatch() { AdvertisedProviderSet providerSet = AdvertisedProviderSet.builder() @@ -130,10 +139,13 @@ public class RequiredProvidersTest { @Test public void skylarkProvidersAllMatch() { AdvertisedProviderSet providerSet = AdvertisedProviderSet.builder() - .addSkylark("p1") - .addSkylark("p2") + .addSkylark(ID_LEGACY) + .addSkylark(ID_NATIVE) + .addSkylark(ID_SKYLARK) .build(); - assertThat(validateSkylark(providerSet, ImmutableSet.of("p1", "p2"))) + assertThat(validateSkylark(providerSet, + ImmutableSet.of( + ID_LEGACY, ID_SKYLARK, ID_NATIVE))) .isTrue(); } @@ -142,10 +154,10 @@ public class RequiredProvidersTest { assertThat( validateSkylark( AdvertisedProviderSet.builder() - .addSkylark("p1") + .addSkylark(ID_LEGACY) .build(), - ImmutableSet.of("p1"), - ImmutableSet.of("p2") + ImmutableSet.of(ID_LEGACY), + ImmutableSet.of(ID_NATIVE) )).isTrue(); } @@ -154,10 +166,10 @@ public class RequiredProvidersTest { assertThat( validateSkylark( AdvertisedProviderSet.builder() - .addSkylark("p3") + .addSkylark(ID_SKYLARK) .build(), - ImmutableSet.of("p1"), - ImmutableSet.of("p2") + ImmutableSet.of(ID_LEGACY), + ImmutableSet.of(ID_NATIVE) )).isFalse(); } @@ -178,25 +190,15 @@ public class RequiredProvidersTest { @SafeVarargs private static boolean validateSkylark( AdvertisedProviderSet providerSet, - ImmutableSet<String>... sets) { + ImmutableSet<SkylarkProviderIdentifier>... sets) { Builder anyBuilder = RequiredProviders.acceptAnyBuilder(); Builder noneBuilder = RequiredProviders.acceptNoneBuilder(); - for (ImmutableSet<String> set : sets) { - ImmutableSet<SkylarkProviderIdentifier> idSet = toIdSet(set); - anyBuilder.addSkylarkSet(idSet); - noneBuilder.addSkylarkSet(idSet); + for (ImmutableSet<SkylarkProviderIdentifier> set : sets) { + anyBuilder.addSkylarkSet(set); + noneBuilder.addSkylarkSet(set); } boolean result = satisfies(providerSet, anyBuilder.build()); assertThat(satisfies(providerSet, noneBuilder.build())).isEqualTo(result); return result; } - - private static ImmutableSet<SkylarkProviderIdentifier> toIdSet(ImmutableSet<String> set) { - ImmutableSet.Builder<SkylarkProviderIdentifier> builder = ImmutableSet.builder(); - for (String id : set) { - builder.add(SkylarkProviderIdentifier.forLegacy(id)); - } - return builder.build(); - } - } |