General cleanup for the configured target / aspect creation code.
- update some comments
- add some comments to make it easier to follow
- delete some dead code, in particular the SkyframeDependencyResolver can
never be null; remove an non-applicable @Nullable annotation
I'm trying to figure out how the error handling code works, in order to add
support for interleaved loading+analysis, which requires handling loading
errors in this code path.
--
MOS_MIGRATED_REVID=112456674
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 992e7a2..3565324 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
@@ -243,6 +243,7 @@
ruleContext.ruleError(missingFragmentError(ruleContext, configurationFragmentPolicy));
return null;
}
+ // Otherwise missingFragmentPolicy == MissingFragmentPolicy.CREATE_FAIL_ACTIONS:
return createFailConfiguredTarget(ruleContext);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
index d5a515a..92f7fec 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java
@@ -485,7 +485,7 @@
"Late bound attribute '%s' is not a label or a label list",
attribute.getName()));
}
- } catch (ClassCastException e) {
+ } catch (ClassCastException e) { // From either of the cast calls above.
throw new EvalException(
rule.getLocation(),
String.format(
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 fad923c..aafa943 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
@@ -31,6 +31,7 @@
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.NativeAspectClass;
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.Rule;
import com.google.devtools.build.lib.packages.RuleClassProvider;
@@ -159,13 +160,9 @@
}
SkyframeDependencyResolver resolver = view.createDependencyResolver(env);
- if (resolver == null) {
- return null;
- }
TargetAndConfiguration ctgValue =
new TargetAndConfiguration(target, key.getConfiguration());
-
try {
// Get the configuration targets that trigger this rule's configurable attributes.
Set<ConfigMatchingProvider> configConditions = ConfiguredTargetFunction.getConfigConditions(
@@ -276,7 +273,7 @@
* Used to indicate errors during the computation of an {@link AspectValue}.
*/
private static final class AspectFunctionException extends SkyFunctionException {
- public AspectFunctionException(Exception e) {
+ public AspectFunctionException(AspectCreationException e) {
super(e, Transience.PERSISTENT);
}
@@ -284,6 +281,10 @@
public AspectFunctionException(SkyKey childKey, Exception transitiveError) {
super(transitiveError, childKey);
}
- }
+ /** Used to rethrow a child error that we cannot handle. */
+ public AspectFunctionException(SkyKey childKey, NoSuchThingException e) {
+ super(e, childKey);
+ }
+ }
}
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 70d7233..0532d72 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
@@ -65,7 +65,6 @@
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
-import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrException;
@@ -151,8 +150,7 @@
Package pkg = packageValue.getPackage();
if (pkg.containsErrors()) {
throw new ConfiguredTargetFunctionException(
- new BuildFileContainsErrorsException(lc.getLabel().getPackageIdentifier()),
- Transience.PERSISTENT);
+ new BuildFileContainsErrorsException(lc.getLabel().getPackageIdentifier()));
}
Target target;
try {
@@ -169,14 +167,11 @@
if (!target.isConfigurable()) {
configuration = null;
}
- TargetAndConfiguration ctgValue =
- new TargetAndConfiguration(target, configuration);
SkyframeDependencyResolver resolver = view.createDependencyResolver(env);
- if (resolver == null) {
- return null;
- }
+ TargetAndConfiguration ctgValue =
+ new TargetAndConfiguration(target, configuration);
try {
// Get the configuration targets that trigger this rule's configurable attributes.
Set<ConfigMatchingProvider> configConditions =
@@ -754,7 +749,7 @@
if (events.hasErrors()) {
analysisEnvironment.disable(target);
throw new ConfiguredTargetFunctionException(new ConfiguredValueCreationException(
- "Analysis of target '" + target.getLabel() + "' failed; build aborted"));
+ "Analysis of target '" + target.getLabel() + "' failed; build aborted"));
}
Preconditions.checkState(!analysisEnvironment.hasErrors(),
"Analysis environment hasError() but no errors reported");
@@ -765,13 +760,16 @@
analysisEnvironment.disable(target);
Preconditions.checkNotNull(configuredTarget, target);
+ Map<Artifact, Action> generatingActions;
+ // Check for conflicting actions within this configured target (that indicates a bug in the
+ // rule implementation).
try {
- return new ConfiguredTargetValue(configuredTarget,
- filterSharedActionsAndThrowIfConflict(analysisEnvironment.getRegisteredActions()),
- transitivePackages.build());
+ generatingActions = filterSharedActionsAndThrowIfConflict(analysisEnvironment.getRegisteredActions());
} catch (ActionConflictException e) {
throw new ConfiguredTargetFunctionException(e);
}
+ return new ConfiguredTargetValue(
+ configuredTarget, generatingActions, transitivePackages.build());
}
static Map<Artifact, Action> filterSharedActionsAndThrowIfConflict(Iterable<Action> actions)
@@ -794,7 +792,6 @@
* a ConfiguredTargetValue.
*/
public static final class ConfiguredValueCreationException extends Exception {
-
public ConfiguredValueCreationException(String message) {
super(message);
}
@@ -805,15 +802,10 @@
* {@link ConfiguredTargetFunction#compute}.
*/
public static final class ConfiguredTargetFunctionException extends SkyFunctionException {
- public ConfiguredTargetFunctionException(NoSuchTargetException e) {
+ public ConfiguredTargetFunctionException(NoSuchThingException e) {
super(e, Transience.PERSISTENT);
}
- public ConfiguredTargetFunctionException(
- BuildFileContainsErrorsException e, Transience transience) {
- super(e, transience);
- }
-
private ConfiguredTargetFunctionException(ConfiguredValueCreationException error) {
super(error, Transience.PERSISTENT);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
index a109dab..c5a54dc 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PostConfiguredTargetFunction.java
@@ -75,8 +75,6 @@
ImmutableMap<Action, ConflictException> badActions = PrecomputedValue.BAD_ACTIONS.get(env);
ConfiguredTargetValue ctValue = (ConfiguredTargetValue)
env.getValue(ConfiguredTargetValue.key((ConfiguredTargetKey) skyKey.argument()));
- SkyframeDependencyResolver resolver =
- buildViewProvider.getSkyframeBuildView().createDependencyResolver(env);
if (env.valuesMissing()) {
return null;
}
@@ -101,6 +99,8 @@
try {
BuildConfiguration hostConfiguration =
buildViewProvider.getSkyframeBuildView().getHostConfiguration(ct.getConfiguration());
+ SkyframeDependencyResolver resolver =
+ buildViewProvider.getSkyframeBuildView().createDependencyResolver(env);
deps =
resolver.dependentNodeMap(
ctgValue, hostConfiguration, /*aspect=*/ null, configConditions);
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 f60d880..be7ff9b 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
@@ -534,7 +534,6 @@
return trimmedConfig;
}
- @Nullable
SkyframeDependencyResolver createDependencyResolver(Environment env) {
return new SkyframeDependencyResolver(env);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
index 5cf7d97..cac4c02 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ToplevelSkylarkAspectFunction.java
@@ -89,7 +89,6 @@
* Exceptions thrown from ToplevelSkylarkAspectFunction.
*/
public class LoadSkylarkAspectFunctionException extends SkyFunctionException {
-
public LoadSkylarkAspectFunctionException(Exception cause, SkyKey childKey) {
super(cause, childKey);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Type.java b/src/main/java/com/google/devtools/build/lib/syntax/Type.java
index e543315..49d0b74 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Type.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Type.java
@@ -143,9 +143,9 @@
/**
* Implementation of concatenation for this type (e.g. "val1 + val2"). Returns null to
- * designate concatenation isn't supported.
+ * indicate concatenation isn't supported.
*/
- public T concat(Iterable<T> elements) {
+ public T concat(@SuppressWarnings("unused") Iterable<T> elements) {
return null;
}
