Automated rollback of commit 9a00c789ea46dcb8c73e513ad1ba234a8417fff3. *** Reason for rollback *** Added fix for unconfigured dependencies causing analysis failure. PiperOrigin-RevId: 310361242
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java index ea0e769..02c37bf 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisFailureEvent.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.analysis; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.buildeventstream.BuildEvent; @@ -51,6 +52,15 @@ this.rootCauses = rootCauses; } + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("failedTarget", failedTarget) + .add("configuration", configuration) + .add("legacyFailureReason", getLegacyFailureReason()) + .toString(); + } + public ConfiguredTargetKey getFailedTarget() { return failedTarget; }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisRootCauseEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisRootCauseEvent.java index 8556f82..7be2f31 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisRootCauseEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisRootCauseEvent.java
@@ -25,7 +25,9 @@ import com.google.devtools.build.lib.buildeventstream.BuildEventWithConfiguration; import com.google.devtools.build.lib.buildeventstream.GenericBuildEvent; import com.google.devtools.build.lib.cmdline.Label; +import java.util.ArrayList; import java.util.Collection; +import javax.annotation.Nullable; /** * Error message of an analysis root cause. This is separate from {@link AnalysisFailureEvent} to @@ -38,7 +40,7 @@ private final String errorMessage; public AnalysisRootCauseEvent( - BuildConfiguration configuration, Label label, String errorMessage) { + @Nullable BuildConfiguration configuration, Label label, String errorMessage) { this.configuration = configuration; this.label = label; this.errorMessage = errorMessage; @@ -76,6 +78,12 @@ @Override public Collection<BuildEvent> getConfigurations() { - return ImmutableList.<BuildEvent>of(configuration.toBuildEvent()); + ArrayList<BuildEvent> result = new ArrayList<>(); + if (configuration == null) { + result.add(null); + } else { + result.add(configuration.toBuildEvent()); + } + return result; } }
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 0958ed4..47c7131 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
@@ -261,7 +261,7 @@ throw new IllegalStateException(target.getLabel().toString()); } - Map<Label, Target> targetMap = getTargets(outgoingLabels, target, rootCauses); + Map<Label, Target> targetMap = getTargets(outgoingLabels, node, rootCauses); if (targetMap == null) { // Dependencies could not be resolved. Try again when they are loaded by Skyframe. return OrderedSetMultimap.create(); @@ -713,8 +713,7 @@ */ protected abstract Map<Label, Target> getTargets( OrderedSetMultimap<DependencyKind, Label> labelMap, - Target fromTarget, + TargetAndConfiguration fromNode, NestedSetBuilder<Cause> rootCauses) throws InterruptedException; - }
diff --git a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java index c8dd51b..5cf5207 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java +++ b/src/main/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterCallback.java
@@ -125,7 +125,7 @@ // DependencyResolver but passing to avoid passing a null and since we have the information // anyway. deps = - new FormatterDependencyResolver(eventHandler) + new FormatterDependencyResolver() .dependentNodeMap( new TargetAndConfiguration(target, config), hostConfiguration, @@ -201,16 +201,11 @@ } private class FormatterDependencyResolver extends DependencyResolver { - private final ExtendedEventHandler eventHandler; - - private FormatterDependencyResolver(ExtendedEventHandler eventHandler) { - this.eventHandler = eventHandler; - } @Override protected Map<Label, Target> getTargets( OrderedSetMultimap<DependencyKind, Label> labelMap, - Target fromTarget, + TargetAndConfiguration fromNode, NestedSetBuilder<Cause> rootCauses) { return labelMap.values().stream() .distinct()
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 f8e96c0..70772d6 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
@@ -309,14 +309,14 @@ transitivePackagesForPackageRootResolution, transitiveRootCauses, defaultBuildOptions); - if (env.valuesMissing()) { - return null; - } if (!transitiveRootCauses.isEmpty()) { throw new ConfiguredTargetFunctionException( new ConfiguredValueCreationException( "Analysis failed", configuration, transitiveRootCauses.build())); } + if (env.valuesMissing()) { + return null; + } Preconditions.checkNotNull(depValueMap); // Load the requested toolchains into the ToolchainContext, now that we have dependencies. @@ -614,7 +614,6 @@ env.getListener().handle(Event.error(e.getLocation(), e.getMessage())); throw new DependencyEvaluationException(e); } - // Trim each dep's configuration so it only includes the fragments needed by its transitive // closure. depValueNames =
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 98bc456..12e5595 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
@@ -57,7 +57,9 @@ import com.google.devtools.build.lib.analysis.config.CoreOptions; import com.google.devtools.build.lib.analysis.config.FragmentClassSet; import com.google.devtools.build.lib.bugreport.BugReport; +import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ConfigurationId; import com.google.devtools.build.lib.buildtool.BuildRequestOptions; +import com.google.devtools.build.lib.causes.AnalysisFailedCause; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.causes.LabelCause; import com.google.devtools.build.lib.causes.LoadingFailedCause; @@ -716,6 +718,15 @@ ((ActionConflictException) cause).reportTo(eventHandler); // TODO(ulfjack): Report the action conflict. rootCauses = NestedSetBuilder.emptySet(Order.STABLE_ORDER); + } else if (cause instanceof NoSuchPackageException) { + // This branch is only taken in --nokeep_going builds. In a --keep_going build, the + // AnalysisFailedCause is properly reported through the ConfiguredValueCreationException. + BuildConfiguration configuration = + configurationLookupSupplier.get().get(label.getConfigurationKey()); + ConfigurationId configId = configuration.getEventId().getConfiguration(); + AnalysisFailedCause analysisFailedCause = + new AnalysisFailedCause(topLevelLabel, configId, cause.getMessage()); + rootCauses = NestedSetBuilder.create(Order.STABLE_ORDER, analysisFailedCause); } else { // TODO(ulfjack): Report something! rootCauses = NestedSetBuilder.emptySet(Order.STABLE_ORDER);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java index a487fa8..e39da2e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeDependencyResolver.java
@@ -16,9 +16,13 @@ import static com.google.devtools.build.lib.cmdline.LabelConstants.EXTERNAL_PACKAGE_IDENTIFIER; import com.google.common.collect.Maps; +import com.google.devtools.build.lib.analysis.AnalysisRootCauseEvent; import com.google.devtools.build.lib.analysis.DependencyKind; import com.google.devtools.build.lib.analysis.DependencyResolver; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId.ConfigurationId; +import com.google.devtools.build.lib.causes.AnalysisFailedCause; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.causes.LoadingFailedCause; import com.google.devtools.build.lib.cmdline.Label; @@ -84,10 +88,9 @@ @Override protected Map<Label, Target> getTargets( OrderedSetMultimap<DependencyKind, Label> labelMap, - Target fromTarget, + TargetAndConfiguration fromNode, NestedSetBuilder<Cause> rootCauses) throws InterruptedException { - Map<PackageIdentifier, SkyKey> packageKeys = new HashMap<>(labelMap.size()); for (Label label : labelMap.values()) { packageKeys.computeIfAbsent(label.getPackageIdentifier(), id -> PackageValue.key(id)); @@ -96,6 +99,8 @@ Map<SkyKey, ValueOrException<NoSuchPackageException>> packages = env.getValuesOrThrow(packageKeys.values(), NoSuchPackageException.class); + Target fromTarget = fromNode.getTarget(); + // As per the comment in SkyFunctionEnvironment.getValueOrUntypedExceptions(), we are supposed // to prefer reporting errors to reporting null, we first check for errors in our dependencies. // This, of course, results in some wasted work in case this will need to be restarted later. @@ -144,7 +149,13 @@ e.getMessage()))); continue; } - rootCauses.add(new LoadingFailedCause(label, e.getMessage())); + @Nullable BuildConfiguration configuration = fromNode.getConfiguration(); + @Nullable ConfigurationId configId = null; + if (configuration != null) { + configId = configuration.getEventId().getConfiguration(); + } + env.getListener().post(new AnalysisRootCauseEvent(configuration, label, e.getMessage())); + rootCauses.add(new AnalysisFailedCause(label, configId, e.getMessage())); missingEdgeHook(fromTarget, entry.getKey(), label, e); continue; }