diff options
author | John Cater <jcater@google.com> | 2017-08-09 21:09:53 +0200 |
---|---|---|
committer | Marcel Hlopko <hlopko@google.com> | 2017-08-10 13:47:59 +0200 |
commit | 3a0ce7780a5707450d7fb013c006e90bf6861e27 (patch) | |
tree | 9a2f634c04fa570a69efb13a1bdc545f7ff43609 /src/main/java/com | |
parent | 6cc9d93a73d4cae58ab55748edde5be782cc0727 (diff) |
Ensure that invalid target errors are properly reported in
ConfiguredTargetFunction.
Fixes #3430.
Change-Id: Iac1ab3fb4958dc6fb23e92a43a32b81461dcf0f3
PiperOrigin-RevId: 164754851
Diffstat (limited to 'src/main/java/com')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/analysis/ToolchainContext.java | 14 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java | 40 |
2 files changed, 47 insertions, 7 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ToolchainContext.java b/src/main/java/com/google/devtools/build/lib/analysis/ToolchainContext.java index 23db96cfa6..9cbdd475ac 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ToolchainContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ToolchainContext.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils; import com.google.devtools.build.lib.analysis.platform.ToolchainInfo; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Location; @@ -39,6 +40,7 @@ import com.google.devtools.build.lib.util.OrderedSetMultimap; import com.google.devtools.build.lib.util.Preconditions; import java.util.List; import java.util.Optional; +import java.util.Set; import javax.annotation.Nullable; /** Contains toolchain-related information needed for a {@link RuleContext}. */ @@ -99,6 +101,14 @@ public class ToolchainContext { return resolvedToolchainLabels.getToolchainLabels(); } + /** Returns the {@link Label}s from the {@link NestedSet} that refer to toolchain dependencies. */ + public Set<Label> filterToolchainLabels(Set<Label> labels) { + return labels + .stream() + .filter(label -> resolvedToolchainLabels.isToolchainDependency(label)) + .collect(ImmutableSet.toImmutableSet()); + } + /** Tracks the mapping from toolchain type label to the label of the actual resolved toolchain. */ private static class ResolvedToolchainLabels { private final ImmutableBiMap<Label, Label> toolchainLabels; @@ -118,6 +128,10 @@ public class ToolchainContext { public ImmutableSet<Label> getToolchainLabels() { return toolchainLabels.values(); } + + public boolean isToolchainDependency(Label label) { + return toolchainLabels.containsValue(label); + } } private static ImmutableMap<Label, ToolchainInfo> findToolchains( 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 b208c08b8a..87de6cf5bf 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 @@ -20,6 +20,7 @@ import com.google.common.base.Verify; import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.LinkedHashMultimap; import com.google.common.collect.LinkedListMultimap; @@ -177,7 +178,7 @@ public final class ConfiguredTargetFunction implements SkyFunction { target = pkg.getTarget(lc.getLabel().getName()); } catch (NoSuchTargetException e) { throw new ConfiguredTargetFunctionException( - new ConfiguredValueCreationException(e.getMessage())); + new ConfiguredValueCreationException(e.getMessage(), lc.getLabel())); } if (pkg.containsErrors()) { transitiveLoadingRootCauses.add(lc.getLabel()); @@ -205,6 +206,8 @@ public final class ConfiguredTargetFunction implements SkyFunction { SkyframeDependencyResolver resolver = view.createDependencyResolver(env); + ToolchainContext toolchainContext = null; + // TODO(janakr): this acquire() call may tie up this thread indefinitely, reducing the // parallelism of Skyframe. This is a strict improvement over the prior state of the code, in // which we ran with #processors threads, but ideally we would call #tryAcquire here, and if we @@ -231,7 +234,6 @@ public final class ConfiguredTargetFunction implements SkyFunction { } // Determine what toolchains are needed by this target. - ToolchainContext toolchainContext = null; if (target instanceof Rule) { Rule rule = ((Rule) target); ImmutableList<Label> requiredToolchains = rule.getRuleClassObject().getRequiredToolchains(); @@ -277,18 +279,42 @@ public final class ConfiguredTargetFunction implements SkyFunction { return ans; } catch (DependencyEvaluationException e) { if (e.getCause() instanceof ConfiguredValueCreationException) { - throw new ConfiguredTargetFunctionException( - (ConfiguredValueCreationException) e.getCause()); + ConfiguredValueCreationException cvce = (ConfiguredValueCreationException) e.getCause(); + + // Check if this is caused by an unresolved toolchain, and report it as such. + if (toolchainContext != null) { + ImmutableSet.Builder<Label> causes = new ImmutableSet.Builder<Label>(); + if (cvce.getAnalysisRootCause() != null) { + causes.add(cvce.getAnalysisRootCause()); + } + if (!cvce.getRootCauses().isEmpty()) { + causes.addAll(cvce.getRootCauses()); + } + Set<Label> toolchainDependencyErrors = + toolchainContext.filterToolchainLabels(causes.build()); + if (!toolchainDependencyErrors.isEmpty()) { + env.getListener() + .handle( + Event.error( + String.format( + "While resolving toolchains for target %s: %s", + target.getLabel(), e.getCause().getMessage()))); + } + } + + throw new ConfiguredTargetFunctionException(cvce); } else if (e.getCause() instanceof InconsistentAspectOrderException) { InconsistentAspectOrderException cause = (InconsistentAspectOrderException) e.getCause(); throw new ConfiguredTargetFunctionException( new ConfiguredValueCreationException(cause.getMessage(), target.getLabel())); - } else { - // Cast to InvalidConfigurationException as a consistency check. If you add any - // DependencyEvaluationException constructors, you may need to change this code, too. + } else if (e.getCause() instanceof InvalidConfigurationException) { InvalidConfigurationException cause = (InvalidConfigurationException) e.getCause(); throw new ConfiguredTargetFunctionException( new ConfiguredValueCreationException(cause.getMessage(), target.getLabel())); + } else { + // Unknown exception type. + throw new ConfiguredTargetFunctionException( + new ConfiguredValueCreationException(e.getMessage(), target.getLabel())); } } catch (AspectCreationException e) { // getAnalysisRootCause may be null if the analysis of the aspect itself failed. |