diff options
author | 2017-08-09 21:09:53 +0200 | |
---|---|---|
committer | 2017-08-10 13:47:59 +0200 | |
commit | 3a0ce7780a5707450d7fb013c006e90bf6861e27 (patch) | |
tree | 9a2f634c04fa570a69efb13a1bdc545f7ff43609 /src | |
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')
3 files changed, 82 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. diff --git a/src/test/shell/bazel/toolchain_test.sh b/src/test/shell/bazel/toolchain_test.sh index 7c676254a8..206abe89f1 100755 --- a/src/test/shell/bazel/toolchain_test.sh +++ b/src/test/shell/bazel/toolchain_test.sh @@ -478,4 +478,39 @@ EOF expect_log "While resolving toolchains for target //demo:use: invalid registered toolchain '//demo:invalid': target does not provide the DeclaredToolchainInfo provider" } + +function test_toolchain_error_invalid_target() { + write_test_toolchain + write_test_rule +# write_toolchains + + # Write toolchain with an invalid target. + mkdir -p invalid + cat > invalid/BUILD <<EOF +toolchain( + name = 'invalid_toolchain', + toolchain_type = '//toolchain:test_toolchain', + exec_compatible_with = [], + target_compatible_with = [], + toolchain = '//toolchain:does_not_exist', + visibility = ['//visibility:public']) +EOF + + cat > WORKSPACE <<EOF +register_toolchains('//invalid:invalid_toolchain') +EOF + + mkdir -p demo + cat >> demo/BUILD <<EOF +load('//toolchain:rule.bzl', 'use_toolchain') +# Use the toolchain. +use_toolchain( + name = 'use', + message = 'this is the rule') +EOF + + bazel build //demo:use &> $TEST_log && fail "Build failure expected" + expect_log "While resolving toolchains for target //demo:use: no such target '//toolchain:does_not_exist': target 'does_not_exist' not declared in package 'toolchain'" +} + run_suite "toolchain tests" |