Rollback of commit ac7195025b073948785db8c6975a53fc305c5087.
*** Reason for rollback ***
Uses tests that don't run on Bazel
*** Original change description ***
Implement aspect(...) Skylark function.
--
MOS_MIGRATED_REVID=105808850
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java b/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
index bb6c38d..fe97bfc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Aspect.java
@@ -22,7 +22,6 @@
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 java.util.LinkedHashMap;
import java.util.Map;
@@ -89,8 +88,6 @@
private final Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider>
providers = new LinkedHashMap<>();
private final Map<String, NestedSetBuilder<Artifact>> outputGroupBuilders = new TreeMap<>();
- private final ImmutableMap.Builder<String, Object> skylarkProviderBuilder =
- ImmutableMap.builder();
private final String name;
public Builder(String name) {
@@ -106,8 +103,6 @@
Preconditions.checkNotNull(value);
AnalysisUtils.checkProvider(key);
Preconditions.checkState(!providers.containsKey(key));
- Preconditions.checkArgument(!SkylarkProviders.class.equals(key),
- "Do not provide SkylarkProviders directly");
providers.put(key, value);
return this;
}
@@ -132,12 +127,6 @@
return this;
}
- public Builder addSkylarkTransitiveInfo(String name, Object value, Location loc) {
- // TODO(dslomov): add {@link RuleConfiguredTargetBuilder#checkSkylarkObjectSafe}
- skylarkProviderBuilder.put(name, value);
- return this;
- }
-
public Aspect build() {
if (!outputGroupBuilders.isEmpty()) {
ImmutableMap.Builder<String, NestedSet<Artifact>> outputGroups = ImmutableMap.builder();
@@ -152,11 +141,6 @@
addProvider(OutputGroupProvider.class, new OutputGroupProvider(outputGroups.build()));
}
- ImmutableMap<String, Object> skylarkProvidersMap = skylarkProviderBuilder.build();
- if (!skylarkProvidersMap.isEmpty()) {
- providers.put(SkylarkProviders.class, new SkylarkProviders(skylarkProvidersMap));
- }
-
return new Aspect(name, ImmutableMap.copyOf(providers));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index e05dd59..14302ac 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -42,7 +42,6 @@
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -76,7 +75,6 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.util.RegexFilter;
import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.WalkableGraph;
import com.google.devtools.common.options.Option;
@@ -452,43 +450,18 @@
List<AspectKey> aspectKeys = new ArrayList<>();
for (String aspect : aspects) {
-
- // Syntax: label%aspect
- int delimiterPosition = aspect.indexOf('%');
- if (delimiterPosition >= 0) {
- PackageIdentifier bzlFile;
- try {
- bzlFile =
- PackageIdentifier.create(
- PackageIdentifier.DEFAULT_REPOSITORY,
- new PathFragment(aspect.substring(0, delimiterPosition)));
- } catch (LabelSyntaxException e) {
- throw new ViewCreationFailedException("Error", e);
- }
-
- String skylarkFunctionName = aspect.substring(delimiterPosition + 1);
+ @SuppressWarnings("unchecked")
+ final Class<? extends ConfiguredAspectFactory> aspectFactoryClass =
+ (Class<? extends ConfiguredAspectFactory>)
+ ruleClassProvider.getAspectFactoryMap().get(aspect);
+ if (aspectFactoryClass != null) {
for (ConfiguredTargetKey targetSpec : targetSpecs) {
aspectKeys.add(
- AspectValue.createSkylarkAspectKey(
- targetSpec.getLabel(),
- targetSpec.getConfiguration(),
- bzlFile,
- skylarkFunctionName));
+ AspectValue.createAspectKey(
+ targetSpec.getLabel(), targetSpec.getConfiguration(), aspectFactoryClass));
}
} else {
- @SuppressWarnings("unchecked")
- final Class<? extends ConfiguredAspectFactory> aspectFactoryClass =
- (Class<? extends ConfiguredAspectFactory>)
- ruleClassProvider.getAspectFactoryMap().get(aspect);
- if (aspectFactoryClass != null) {
- for (ConfiguredTargetKey targetSpec : targetSpecs) {
- aspectKeys.add(
- AspectValue.createAspectKey(
- targetSpec.getLabel(), targetSpec.getConfiguration(), aspectFactoryClass));
- }
- } else {
- throw new ViewCreationFailedException("Aspect '" + aspect + "' is unknown");
- }
+ throw new ViewCreationFailedException("Aspect '" + aspect + "' is unknown");
}
}
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 a8cc9e4..6d97e86 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
@@ -286,14 +286,12 @@
* {@code aspectFactory} should call one of the error reporting methods of {@link RuleContext}.
*/
public Aspect createAspect(
- AnalysisEnvironment env,
- RuleConfiguredTarget associatedTarget,
+ AnalysisEnvironment env, RuleConfiguredTarget associatedTarget,
ConfiguredAspectFactory aspectFactory,
AspectParameters aspectParameters,
ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
Set<ConfigMatchingProvider> configConditions,
- BuildConfiguration hostConfiguration)
- throws InterruptedException {
+ BuildConfiguration hostConfiguration) {
RuleContext.Builder builder = new RuleContext.Builder(env,
associatedTarget.getTarget(),
associatedTarget.getConfiguration(),
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 d861459..9bb9365 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
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.RunUnder;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.OutputFile;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.rules.SkylarkApiProvider;
@@ -130,14 +131,20 @@
providers.addAll(base.providers.keySet());
// Merge output group providers.
- List<OutputGroupProvider> outputGroupProviders =
- getAllProviders(base, aspects, OutputGroupProvider.class);
- OutputGroupProvider mergedOutputGroupProvider = OutputGroupProvider.merge(outputGroupProviders);
+ OutputGroupProvider baseOutputGroupProvider = base.getProvider(OutputGroupProvider.class);
+ List<OutputGroupProvider> outputGroupProviders = new ArrayList<>();
+ if (baseOutputGroupProvider != null) {
+ outputGroupProviders.add(baseOutputGroupProvider);
+ }
- // Merge Skylark providers.
- List<SkylarkProviders> skylarkProviders =
- getAllProviders(base, aspects, SkylarkProviders.class);
- SkylarkProviders mergedSkylarkProviders = SkylarkProviders.merge(skylarkProviders);
+ for (Aspect aspect : aspects) {
+ final OutputGroupProvider aspectProvider = aspect.getProvider(OutputGroupProvider.class);
+ if (aspectProvider == null) {
+ continue;
+ }
+ outputGroupProviders.add(aspectProvider);
+ }
+ OutputGroupProvider outputGroupProvider = OutputGroupProvider.merge(outputGroupProviders);
// Validate that all other providers are only provided once.
for (Aspect aspect : aspects) {
@@ -146,17 +153,13 @@
if (OutputGroupProvider.class.equals(aClass)) {
continue;
}
- if (SkylarkProviders.class.equals(aClass)) {
- continue;
- }
if (!providers.add(aClass)) {
throw new IllegalStateException("Provider " + aClass + " provided twice");
}
}
}
- if (base.getProvider(OutputGroupProvider.class) == mergedOutputGroupProvider
- && base.getProvider(SkylarkProviders.class) == mergedSkylarkProviders) {
+ if (baseOutputGroupProvider == outputGroupProvider) {
this.providers = base.providers;
} else {
ImmutableMap.Builder<Class<? extends TransitiveInfoProvider>, Object> builder =
@@ -165,17 +168,9 @@
if (OutputGroupProvider.class.equals(aClass)) {
continue;
}
- if (SkylarkProviders.class.equals(aClass)) {
- continue;
- }
builder.put(aClass, base.providers.get(aClass));
}
- if (mergedOutputGroupProvider != null) {
- builder.put(OutputGroupProvider.class, mergedOutputGroupProvider);
- }
- if (mergedSkylarkProviders != null) {
- builder.put(SkylarkProviders.class, skylarkProviders);
- }
+ builder.put(OutputGroupProvider.class, outputGroupProvider);
this.providers = builder.build();
}
this.mandatoryStampFiles = base.mandatoryStampFiles;
@@ -183,26 +178,6 @@
this.aspects = ImmutableList.copyOf(aspects);
}
- private static <T extends TransitiveInfoProvider> List<T> getAllProviders(
- RuleConfiguredTarget base,
- Iterable<Aspect> aspects,
- Class<T> providerClass) {
- T baseProvider = base.getProvider(providerClass);
- List<T> providers = new ArrayList<>();
- if (baseProvider != null) {
- providers.add(baseProvider);
- }
-
- for (Aspect aspect : aspects) {
- final T aspectProvider = aspect.getProvider(providerClass);
- if (aspectProvider == null) {
- continue;
- }
- providers.add(aspectProvider);
- }
- return providers;
- }
-
/**
* The configuration conditions that trigger this rule's configurable attributes.
*/
@@ -233,7 +208,7 @@
*/
@Override
public Object get(String providerKey) {
- return getProvider(SkylarkProviders.class).getValue(providerKey);
+ return getProvider(SkylarkProviders.class).skylarkProviders.get(providerKey);
}
public ImmutableList<Artifact> getMandatoryStampFiles() {
@@ -245,6 +220,33 @@
return (Rule) super.getTarget();
}
+ /**
+ * A helper class for transitive infos provided by Skylark rule implementations.
+ */
+ @Immutable
+ public static final class SkylarkProviders implements TransitiveInfoProvider {
+ private final ImmutableMap<String, Object> skylarkProviders;
+
+ private SkylarkProviders(ImmutableMap<String, Object> skylarkProviders) {
+ Preconditions.checkNotNull(skylarkProviders);
+ this.skylarkProviders = skylarkProviders;
+ }
+
+ /**
+ * Returns the keys for the Skylark providers.
+ */
+ public ImmutableCollection<String> getKeys() {
+ return skylarkProviders.keySet();
+ }
+
+ /**
+ * Returns a Skylark provider; "key" must be one from {@link #getKeys()}.
+ */
+ public Object getValue(String key) {
+ return skylarkProviders.get(key);
+ }
+ }
+
@Override
public UnmodifiableIterator<TransitiveInfoProvider> iterator() {
Map<Class<? extends TransitiveInfoProvider>, TransitiveInfoProvider> allProviders =
@@ -271,6 +273,6 @@
@Override
public ImmutableCollection<String> getKeys() {
return ImmutableList.<String>builder().addAll(super.getKeys())
- .addAll(getProvider(SkylarkProviders.class).getKeys()).build();
+ .addAll(getProvider(SkylarkProviders.class).skylarkProviders.keySet()).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
deleted file mode 100644
index d4746c4..0000000
--- a/src/main/java/com/google/devtools/build/lib/analysis/SkylarkProviders.java
+++ /dev/null
@@ -1,79 +0,0 @@
-// Copyright 2014 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.analysis;
-
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableCollection;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-
-import java.util.HashSet;
-import java.util.List;
-import java.util.Set;
-
-/**
- * A helper class for transitive infos provided by Skylark rule implementations.
- */
-@Immutable
-public final class SkylarkProviders implements TransitiveInfoProvider {
- private final ImmutableMap<String, Object> skylarkProviders;
-
- SkylarkProviders(ImmutableMap<String, Object> skylarkProviders) {
- Preconditions.checkNotNull(skylarkProviders);
- this.skylarkProviders = skylarkProviders;
- }
-
- /**
- * Returns the keys for the Skylark providers.
- */
- public ImmutableCollection<String> getKeys() {
- return skylarkProviders.keySet();
- }
-
- /**
- * Returns a Skylark provider; "key" must be one from {@link #getKeys()}.
- */
- public Object getValue(String key) {
- return skylarkProviders.get(key);
- }
-
- /**
- * 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;
- }
- if (providers.size() == 1) {
- return providers.get(0);
- }
-
- ImmutableMap.Builder<String, Object> resultBuilder = new ImmutableMap.Builder<>();
- Set<String> seenKeys = new HashSet<>();
- for (SkylarkProviders provider : providers) {
- for (String key : provider.skylarkProviders.keySet()) {
- if (!seenKeys.add(key)) {
- // TODO(dslomov): add better diagnostics.
- throw new IllegalStateException("Skylark provider " + key + " provided twice");
- }
-
- resultBuilder.put(key, provider.getValue(key));
- }
- }
- return new SkylarkProviders(resultBuilder.build());
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java b/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java
index cd7ba2c..eea286d 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectFactory.java
@@ -29,8 +29,7 @@
* @param parameters information from attributes of the rule that have requested this
* aspect
*/
- TAspect create(TConfiguredTarget base, TRuleContext context, AspectParameters parameters)
- throws InterruptedException;
+ TAspect create(TConfiguredTarget base, TRuleContext context, AspectParameters parameters);
/**
* Returns the definition of the aspect.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
index 05d6100..87b5703 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleClassFunctions.java
@@ -69,7 +69,6 @@
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.FuncallExpression;
import com.google.devtools.build.lib.syntax.FunctionSignature;
-import com.google.devtools.build.lib.syntax.Printer;
import com.google.devtools.build.lib.syntax.Runtime;
import com.google.devtools.build.lib.syntax.SkylarkCallbackFunction;
import com.google.devtools.build.lib.syntax.SkylarkList;
@@ -77,7 +76,6 @@
import com.google.devtools.build.lib.syntax.SkylarkSignature;
import com.google.devtools.build.lib.syntax.SkylarkSignature.Param;
import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor;
-import com.google.devtools.build.lib.syntax.SkylarkValue;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -331,21 +329,6 @@
}
};
- @SkylarkSignature(
- name = "aspect",
- returnType = SkylarkAspect.class,
- documented = false, // todo(dslomov)
- mandatoryPositionals = {@Param(name = "implementation", type = BaseFunction.class)},
- useEnvironment = true
- )
- private static final BuiltinFunction aspect =
- new BuiltinFunction("aspect") {
- public SkylarkAspect invoke(BaseFunction implementation, Environment funcallEnv) {
- return new SkylarkAspect(implementation, funcallEnv);
- }
- };
-
-
// This class is needed for testing
public static final class RuleFunction extends BaseFunction {
// Note that this means that we can reuse the same builder.
@@ -533,36 +516,4 @@
static {
SkylarkSignatureProcessor.configureSkylarkFunctions(SkylarkRuleClassFunctions.class);
}
-
- /**
- * A Skylark value that is a result of 'aspect(..)' function call.
- */
- public static class SkylarkAspect implements SkylarkValue {
- private final BaseFunction implementation;
- private final Environment funcallEnv;
-
- public SkylarkAspect(BaseFunction implementation, Environment funcallEnv) {
- this.implementation = implementation;
- this.funcallEnv = funcallEnv;
- }
-
- public BaseFunction getImplementation() {
- return implementation;
- }
-
- public Environment getFuncallEnv() {
- return funcallEnv;
- }
-
- @Override
- public boolean isImmutable() {
- return implementation.isImmutable();
- }
-
- @Override
- public void write(Appendable buffer, char quotationMark) {
- Printer.append(buffer, "Aspect:");
- implementation.write(buffer, quotationMark);
- }
- }
}
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 9e7ba33..71619eb 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
@@ -62,8 +62,7 @@
.setGlobals(
ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironment().getGlobals())
.setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
- .build(); // NB: loading phase functions are not available: this is analysis already,
- // so we do *not* setLoadingPhase().
+ .build(); // NB: we do *not* setLoadingPhase()
Object target = ruleImplementation.call(
ImmutableList.<Object>of(skylarkRuleContext),
ImmutableMap.<String, Object>of(),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java b/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
index a0dc80b..3ffbede 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/workspace/Bind.java
@@ -20,9 +20,9 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTarget;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.SkylarkProviders;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
@@ -111,7 +111,7 @@
ImmutableList.Builder<String> result = ImmutableList.<String>builder().add("label", "files");
if (configuredTarget != null) {
result.addAll(
- configuredTarget.getProvider(SkylarkProviders.class).getKeys());
+ configuredTarget.getProvider(RuleConfiguredTarget.SkylarkProviders.class).getKeys());
}
return result.build();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index 57dc717..df627ce 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -35,14 +35,9 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
-import com.google.devtools.build.lib.skyframe.ASTFileLookupValue.ASTLookupInputException;
import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
-import com.google.devtools.build.lib.skyframe.AspectValue.NativeAspectKey;
-import com.google.devtools.build.lib.skyframe.AspectValue.SkylarkAspectKey;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.DependencyEvaluationException;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor.BuildViewProvider;
-import com.google.devtools.build.lib.syntax.Type.ConversionException;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
@@ -55,30 +50,13 @@
/**
* The Skyframe function that generates aspects.
*/
-public final class AspectFunction<T extends AspectKey> implements SkyFunction {
+public final class AspectFunction implements SkyFunction {
private final BuildViewProvider buildViewProvider;
private final RuleClassProvider ruleClassProvider;
- private final AspectFactoryCreator<T> aspectFactoryCreator;
- public static AspectFunction<NativeAspectKey> createNativeAspectFunction(
- BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
- return new AspectFunction<>(
- buildViewProvider, ruleClassProvider, new NativeAspectFactoryCreator());
- }
-
- public static AspectFunction<SkylarkAspectKey> createSkylarkAspectFunction(
- BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
- return new AspectFunction<>(
- buildViewProvider, ruleClassProvider, new SkylarkAspectFactoryCreator());
- }
-
- private AspectFunction(
- BuildViewProvider buildViewProvider,
- RuleClassProvider ruleClassProvider,
- AspectFactoryCreator<T> aspectFactoryCreator) {
+ public AspectFunction(BuildViewProvider buildViewProvider, RuleClassProvider ruleClassProvider) {
this.buildViewProvider = buildViewProvider;
this.ruleClassProvider = ruleClassProvider;
- this.aspectFactoryCreator = aspectFactoryCreator;
}
@Nullable
@@ -88,7 +66,8 @@
SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
NestedSetBuilder<Package> transitivePackages = NestedSetBuilder.stableOrder();
AspectKey key = (AspectKey) skyKey.argument();
- ConfiguredAspectFactory aspectFactory = aspectFactoryCreator.createAspectFactory(skyKey, env);
+ ConfiguredAspectFactory aspectFactory =
+ (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
PackageValue packageValue =
(PackageValue) env.getValue(PackageValue.key(key.getLabel().getPackageIdentifier()));
@@ -153,13 +132,7 @@
ruleClassProvider, view.getHostConfiguration(ctgValue.getConfiguration()),
transitivePackages);
- return createAspect(
- env,
- key,
- aspectFactory,
- associatedTarget,
- configConditions,
- depValueMap,
+ return createAspect(env, key, associatedTarget, configConditions, depValueMap,
transitivePackages);
} catch (DependencyEvaluationException e) {
throw new AspectFunctionException(e.getRootCauseSkyKey(), e.getCause());
@@ -169,15 +142,11 @@
}
@Nullable
- private AspectValue createAspect(
- Environment env,
- AspectKey key,
- ConfiguredAspectFactory aspectFactory,
- RuleConfiguredTarget associatedTarget,
- Set<ConfigMatchingProvider> configConditions,
+ private AspectValue createAspect(Environment env, AspectKey key,
+ RuleConfiguredTarget associatedTarget, Set<ConfigMatchingProvider> configConditions,
ListMultimap<Attribute, ConfiguredTarget> directDeps,
NestedSetBuilder<Package> transitivePackages)
- throws AspectFunctionException, InterruptedException {
+ throws AspectFunctionException {
SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
BuildConfiguration configuration = associatedTarget.getConfiguration();
@@ -189,6 +158,8 @@
return null;
}
+ ConfiguredAspectFactory aspectFactory =
+ (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
Aspect aspect = view.createAspect(
analysisEnvironment, associatedTarget, aspectFactory, directDeps, configConditions,
key.getParameters());
@@ -246,58 +217,4 @@
super(transitiveError, childKey);
}
}
-
- /**
- * Factory for {@link ConfiguredAspectFactory} given a particular kind of {@link AspectKey}.
- */
- private interface AspectFactoryCreator<T extends AspectKey> {
- ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env)
- throws AspectFunctionException;
- }
-
- /**
- * Factory for native aspects.
- */
- private static class NativeAspectFactoryCreator implements AspectFactoryCreator<NativeAspectKey> {
-
- @Override
- public ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env) {
- NativeAspectKey key = (NativeAspectKey) skyKey.argument();
- return (ConfiguredAspectFactory) AspectFactory.Util.create(key.getAspect());
- }
- }
-
- /**
- * Factory for Skylark aspects.
- */
- private static class SkylarkAspectFactoryCreator
- implements AspectFactoryCreator<SkylarkAspectKey> {
-
- @Override
- public ConfiguredAspectFactory createAspectFactory(SkyKey skyKey, Environment env)
- throws AspectFunctionException {
- SkylarkAspectKey skylarkAspectKey = (SkylarkAspectKey) skyKey.argument();
- SkyKey importFileKey;
- try {
- importFileKey = SkylarkImportLookupValue.key(skylarkAspectKey.getExtensionFile());
- } catch (ASTLookupInputException e) {
- throw new AspectFunctionException(skyKey, e);
- }
- SkylarkImportLookupValue skylarkImportLookupValue =
- (SkylarkImportLookupValue) env.getValue(importFileKey);
- if (skylarkImportLookupValue == null) {
- return null;
- }
- Object skylarkValue = skylarkImportLookupValue
- .getEnvironmentExtension()
- .get(skylarkAspectKey.getSkylarkValueName());
- if (!(skylarkValue instanceof SkylarkAspect)) {
- throw new AspectFunctionException(
- new ConversionException(skylarkAspectKey.getSkylarkValueName() + " from "
- + skylarkAspectKey.getExtensionFile().toString() + " is not an aspect"));
- }
- SkylarkAspect skylarkAspect = (SkylarkAspect) skylarkValue;
- return new SkylarkAspectFactory(skylarkAspectKey.getSkylarkValueName(), skylarkAspect);
- }
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index 5a4052f..16277c7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -22,7 +22,6 @@
import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.AspectParameters;
@@ -36,17 +35,21 @@
* An aspect in the context of the Skyframe graph.
*/
public final class AspectValue extends ActionLookupValue {
-
/**
- * A base class for a key representing an aspect applied to a particular target.
+ * The key of an action that is generated by an aspect.
*/
- public abstract static class AspectKey extends ActionLookupKey {
- protected final Label label;
- protected final BuildConfiguration configuration;
+ public static final class AspectKey extends ActionLookupKey {
+ private final Label label;
+ private final BuildConfiguration configuration;
+ private final AspectWithParameters aspect;
- protected AspectKey(Label label, BuildConfiguration configuration) {
+ private AspectKey(Label label, BuildConfiguration configuration,
+ Class<? extends ConfiguredAspectFactory> aspectFactory,
+ AspectParameters parameters) {
+ Preconditions.checkNotNull(parameters);
this.label = label;
this.configuration = configuration;
+ this.aspect = new AspectWithParameters(aspectFactory, parameters);
}
@Override
@@ -54,34 +57,14 @@
return label;
}
- public abstract AspectParameters getParameters();
-
public BuildConfiguration getConfiguration() {
return configuration;
}
- }
-
- /**
- * The key of an action that is generated by a native aspect.
- */
- public static final class NativeAspectKey extends AspectKey {
- private final AspectWithParameters aspect;
-
- private NativeAspectKey(
- Label label,
- BuildConfiguration configuration,
- Class<? extends ConfiguredAspectFactory> aspectFactory,
- AspectParameters parameters) {
- super(label, configuration);
- Preconditions.checkNotNull(parameters);
- this.aspect = new AspectWithParameters(aspectFactory, parameters);
- }
public Class<? extends ConfiguredAspectFactory> getAspect() {
return aspect.getAspectFactory();
}
- @Override
@Nullable
public AspectParameters getParameters() {
return aspect.getParameters();
@@ -89,7 +72,7 @@
@Override
SkyFunctionName getType() {
- return SkyFunctions.NATIVE_ASPECT;
+ return SkyFunctions.ASPECT;
}
@Override
@@ -103,11 +86,11 @@
return true;
}
- if (!(other instanceof NativeAspectKey)) {
+ if (!(other instanceof AspectKey)) {
return false;
}
- NativeAspectKey that = (NativeAspectKey) other;
+ AspectKey that = (AspectKey) other;
return Objects.equal(label, that.label)
&& Objects.equal(configuration, that.configuration)
&& Objects.equal(aspect, that.aspect);
@@ -121,43 +104,6 @@
}
}
- /**
- * The key of an action that is generated by a skylark aspect.
- */
- public static class SkylarkAspectKey extends AspectKey {
- private final PackageIdentifier extensionFile;
- private final String skylarkFunctionName;
-
- private SkylarkAspectKey(
- Label targetLabel,
- BuildConfiguration targetConfiguration,
- PackageIdentifier extensionFile,
- String skylarkFunctionName) {
- super(targetLabel, targetConfiguration);
- this.extensionFile = extensionFile;
- this.skylarkFunctionName = skylarkFunctionName;
- }
-
- public PackageIdentifier getExtensionFile() {
- return extensionFile;
- }
-
- public String getSkylarkValueName() {
- return skylarkFunctionName;
- }
-
- @Override
- public AspectParameters getParameters() {
- return AspectParameters.EMPTY;
- }
-
- @Override
- SkyFunctionName getType() {
- return SkyFunctions.SKYLARK_ASPECT;
- }
- }
-
-
private final Label label;
private final Location location;
private final AspectKey key;
@@ -198,27 +144,18 @@
public static SkyKey key(Label label, BuildConfiguration configuration,
Class<? extends ConfiguredAspectFactory> aspectFactory,
AspectParameters additionalConfiguration) {
- return new SkyKey(
- SkyFunctions.NATIVE_ASPECT,
- new NativeAspectKey(label, configuration, aspectFactory, additionalConfiguration));
+ return new SkyKey(SkyFunctions.ASPECT,
+ new AspectKey(label, configuration, aspectFactory, additionalConfiguration));
}
public static SkyKey key(AspectKey aspectKey) {
- return new SkyKey(aspectKey.getType(), aspectKey);
+ return new SkyKey(SkyFunctions.ASPECT, aspectKey);
}
- public static NativeAspectKey createAspectKey(
+ public static AspectKey createAspectKey(
Label label,
BuildConfiguration configuration,
Class<? extends ConfiguredAspectFactory> aspectFactory) {
- return new NativeAspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
- }
-
- public static SkylarkAspectKey createSkylarkAspectKey(
- Label targetLabel,
- BuildConfiguration targetConfiguration,
- PackageIdentifier bzlFile,
- String skylarkFunctionName) {
- return new SkylarkAspectKey(targetLabel, targetConfiguration, bzlFile, skylarkFunctionName);
+ return new AspectKey(label, configuration, aspectFactory, AspectParameters.EMPTY);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index addf514..c98da4e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -52,10 +52,12 @@
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.BuildType;
+import com.google.devtools.build.lib.packages.InputFile;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.packages.Package;
+import com.google.devtools.build.lib.packages.PackageGroup;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClassProvider;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
index ee3d395..6c17858 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyFunctions.java
@@ -63,10 +63,9 @@
SkyFunctionName.create("TRANSITIVE_TRAVERSAL");
public static final SkyFunctionName CONFIGURED_TARGET =
SkyFunctionName.create("CONFIGURED_TARGET");
+ public static final SkyFunctionName ASPECT = SkyFunctionName.create("ASPECT");
public static final SkyFunctionName POST_CONFIGURED_TARGET =
SkyFunctionName.create("POST_CONFIGURED_TARGET");
- public static final SkyFunctionName NATIVE_ASPECT = SkyFunctionName.create("NATIVE_ASPECT");
- public static final SkyFunctionName SKYLARK_ASPECT = SkyFunctionName.create("SKYLARK_ASPECT");
public static final SkyFunctionName TARGET_COMPLETION =
SkyFunctionName.create("TARGET_COMPLETION");
public static final SkyFunctionName ASPECT_COMPLETION =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index 49766ff..22e272e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -507,13 +507,11 @@
@Nullable
public Aspect createAspect(
- AnalysisEnvironment env,
- RuleConfiguredTarget associatedTarget,
+ AnalysisEnvironment env, RuleConfiguredTarget associatedTarget,
ConfiguredAspectFactory aspectFactory,
ListMultimap<Attribute, ConfiguredTarget> prerequisiteMap,
Set<ConfigMatchingProvider> configConditions,
- AspectParameters aspectParameters)
- throws InterruptedException {
+ AspectParameters aspectParameters) {
return factory.createAspect(env, associatedTarget, aspectFactory, aspectParameters,
prerequisiteMap, configConditions,
getHostConfiguration(associatedTarget.getConfiguration()));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index cc80950..0b5c5cb 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -351,12 +351,7 @@
map.put(SkyFunctions.TRANSITIVE_TRAVERSAL, new TransitiveTraversalFunction());
map.put(SkyFunctions.CONFIGURED_TARGET,
new ConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider));
- map.put(
- SkyFunctions.NATIVE_ASPECT,
- AspectFunction.createNativeAspectFunction(new BuildViewProvider(), ruleClassProvider));
- map.put(
- SkyFunctions.SKYLARK_ASPECT,
- AspectFunction.createSkylarkAspectFunction(new BuildViewProvider(), ruleClassProvider));
+ map.put(SkyFunctions.ASPECT, new AspectFunction(new BuildViewProvider(), ruleClassProvider));
map.put(SkyFunctions.POST_CONFIGURED_TARGET,
new PostConfiguredTargetFunction(new BuildViewProvider(), ruleClassProvider));
map.put(SkyFunctions.BUILD_CONFIGURATION,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
deleted file mode 100644
index f757847..0000000
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ /dev/null
@@ -1,95 +0,0 @@
-// Copyright 2014 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.skyframe;
-
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
-import com.google.devtools.build.lib.analysis.Aspect;
-import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
-import com.google.devtools.build.lib.analysis.ConfiguredTarget;
-import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.AspectDefinition;
-import com.google.devtools.build.lib.packages.AspectParameters;
-import com.google.devtools.build.lib.rules.SkylarkRuleClassFunctions.SkylarkAspect;
-import com.google.devtools.build.lib.rules.SkylarkRuleContext;
-import com.google.devtools.build.lib.syntax.ClassObject.SkylarkClassObject;
-import com.google.devtools.build.lib.syntax.Environment;
-import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.Mutability;
-
-/**
- * A factory for aspects that are defined in Skylark.
- */
-public class SkylarkAspectFactory implements ConfiguredAspectFactory {
-
- private final String name;
- private final SkylarkAspect aspectFunction;
-
- public SkylarkAspectFactory(String name, SkylarkAspect aspectFunction) {
- this.name = name;
- this.aspectFunction = aspectFunction;
- }
-
- @Override
- public Aspect create(ConfiguredTarget base, RuleContext ruleContext, AspectParameters parameters)
- throws InterruptedException {
- try (Mutability mutability = Mutability.create("aspect")) {
- SkylarkRuleContext skylarkRuleContext;
- try {
- skylarkRuleContext = new SkylarkRuleContext(ruleContext);
- } catch (EvalException e) {
- ruleContext.ruleError(e.getMessage());
- return null;
- }
- Environment env =
- Environment.builder(mutability)
- .setSkylark()
- .setGlobals(aspectFunction.getFuncallEnv().getGlobals())
- .setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
- .build(); // NB: loading phase functions are not available: this is analysis already,
- // so we do *not* setLoadingPhase().
- Object aspect;
- try {
- aspect =
- aspectFunction
- .getImplementation()
- .call(
- ImmutableList.<Object>of(base, skylarkRuleContext),
- ImmutableMap.<String, Object>of(),
- /*ast=*/ null,
- env);
- } catch (EvalException e) {
- ruleContext.ruleError(e.getMessage());
- return null;
- }
- // todo(dslomov): unify this code with
- // {@link com.google.devtools.build.lib.rules.SkylarkRuleConfiguredTargetBuilder}
- Aspect.Builder builder = new Aspect.Builder(name);
- if (aspect instanceof SkylarkClassObject) {
- SkylarkClassObject struct = (SkylarkClassObject) aspect;
- Location loc = struct.getCreationLoc();
- for (String key : struct.getKeys()) {
- builder.addSkylarkTransitiveInfo(key, struct.getValue(key), loc);
- }
- }
- return builder.build();
- }
- }
-
- @Override
- public AspectDefinition getDefinition() {
- return new AspectDefinition.Builder(name).build();
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
index 8530884..dc997a1 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupValue.java
@@ -67,7 +67,7 @@
}
@VisibleForTesting
- public static SkyKey key(PackageIdentifier pkgIdentifier) throws ASTLookupInputException {
+ static SkyKey key(PackageIdentifier pkgIdentifier) throws ASTLookupInputException {
return key(pkgIdentifier.getRepository(), pkgIdentifier.getPackageFragment());
}
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
index db45c51..db65324 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkIntegrationTest.java
@@ -14,20 +14,16 @@
package com.google.devtools.build.lib.skylark;
import static com.google.common.collect.Iterables.getOnlyElement;
-import static com.google.common.collect.Iterables.transform;
import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.baseArtifactNames;
-import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
-import com.google.common.eventbus.EventBus;
import com.google.devtools.build.lib.actions.Action;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
-import com.google.devtools.build.lib.analysis.BuildView.AnalysisResult;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileConfiguredTarget;
import com.google.devtools.build.lib.analysis.FileProvider;
@@ -59,8 +55,6 @@
import java.util.LinkedList;
import java.util.List;
-import javax.annotation.Nullable;
-
/**
* Integration tests for Skylark.
*/
@@ -2650,35 +2644,6 @@
}
}
- public void testAspect() throws Exception {
- scratch.file(
- "test/aspect.bzl",
- "def _impl(target, ctx):",
- " print('This aspect does nothing')",
- "MyAspect = aspect(implementation=_impl)");
- scratch.file("test/BUILD", "java_library(name = 'xxx',)");
-
- AnalysisResult analysisResult =
- update(
- ImmutableList.of("//test:xxx"),
- ImmutableList.<String>of("test/aspect.bzl%MyAspect"),
- false,
- LOADING_PHASE_THREADS,
- true,
- new EventBus());
- assertThat(
- transform(
- analysisResult.getTargetsToBuild(),
- new Function<ConfiguredTarget, String>() {
- @Nullable
- @Override
- public String apply(ConfiguredTarget configuredTarget) {
- return configuredTarget.getLabel().toString();
- }
- }))
- .containsExactly("//test:xxx");
- }
-
/**
* Skylark integration test that forces inlining.
*/
