diff options
8 files changed, 118 insertions, 87 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java index c7c0866c32..2a0f751dd2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java @@ -56,7 +56,7 @@ public interface ConfiguredTarget extends TransitiveInfoCollection, ClassObject, BuildConfiguration getConfiguration(); default String getConfigurationChecksum() { - return getConfiguration().checksum(); + return getConfiguration() == null ? null : getConfiguration().checksum(); } /** diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PrintActionVisitor.java b/src/main/java/com/google/devtools/build/lib/analysis/PrintActionVisitor.java index abe62c1474..3f2f6aea37 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PrintActionVisitor.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PrintActionVisitor.java @@ -20,7 +20,6 @@ import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionGraph; import com.google.devtools.build.lib.actions.ActionGraphVisitor; import com.google.devtools.build.lib.actions.ActionOwner; - import java.util.List; /** @@ -42,7 +41,7 @@ public final class PrintActionVisitor extends ActionGraphVisitor { this.target = target; this.actionMnemonicMatcher = actionMnemonicMatcher; actions = Lists.newArrayList(); - targetConfigurationChecksum = target.getConfiguration().checksum(); + targetConfigurationChecksum = target.getConfigurationChecksum(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java index aeba06b462..97dde9c546 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetCompleteEvent.java @@ -46,9 +46,9 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.AttributeMap; import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper; import com.google.devtools.build.lib.packages.Rule; -import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.packages.TestSize; import com.google.devtools.build.lib.rules.AliasConfiguredTarget; +import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.syntax.Type; import com.google.devtools.build.skyframe.SkyValue; import java.util.Collection; @@ -60,8 +60,7 @@ public final class TargetCompleteEvent BuildEventWithOrderConstraint, EventReportingArtifacts, BuildEventWithConfiguration { - private final ConfiguredTarget target; - private final Target actualTarget; + private final ConfiguredTargetAndData targetAndData; private final NestedSet<Cause> rootCauses; private final ImmutableList<BuildEventId> postedAfter; private final Iterable<ArtifactsInOutputGroup> outputs; @@ -69,20 +68,18 @@ public final class TargetCompleteEvent private final boolean isTest; private TargetCompleteEvent( - ConfiguredTarget target, - Target actualTarget, + ConfiguredTargetAndData targetAndData, NestedSet<Cause> rootCauses, Iterable<ArtifactsInOutputGroup> outputs, boolean isTest) { - this.target = target; - this.actualTarget = actualTarget; + this.targetAndData = targetAndData; this.rootCauses = (rootCauses == null) ? NestedSetBuilder.<Cause>emptySet(Order.STABLE_ORDER) : rootCauses; ImmutableList.Builder<BuildEventId> postedAfterBuilder = ImmutableList.builder(); Label label = getTarget().getLabel(); - if (target instanceof AliasConfiguredTarget) { - label = ((AliasConfiguredTarget) target).getOriginalLabel(); + if (targetAndData.getConfiguredTarget() instanceof AliasConfiguredTarget) { + label = ((AliasConfiguredTarget) targetAndData.getConfiguredTarget()).getOriginalLabel(); } postedAfterBuilder.add(BuildEventId.targetConfigured(label)); for (Cause cause : getRootCauses()) { @@ -92,7 +89,7 @@ public final class TargetCompleteEvent this.outputs = outputs; this.isTest = isTest; InstrumentedFilesProvider instrumentedFilesProvider = - this.target.getProvider(InstrumentedFilesProvider.class); + this.targetAndData.getConfiguredTarget().getProvider(InstrumentedFilesProvider.class); if (instrumentedFilesProvider == null) { this.baselineCoverageArtifacts = null; } else { @@ -108,29 +105,28 @@ public final class TargetCompleteEvent /** Construct a successful target completion event. */ public static TargetCompleteEvent successfulBuild( - ConfiguredTarget ct, Target target, NestedSet<ArtifactsInOutputGroup> outputs) { - return new TargetCompleteEvent(ct, target, null, outputs, false); + ConfiguredTargetAndData ct, NestedSet<ArtifactsInOutputGroup> outputs) { + return new TargetCompleteEvent(ct, null, outputs, false); } /** Construct a successful target completion event for a target that will be tested. */ public static TargetCompleteEvent successfulBuildSchedulingTest( - ConfiguredTarget ct, Target target, NestedSet<ArtifactsInOutputGroup> outputs) { - return new TargetCompleteEvent(ct, target, null, outputs, true); + ConfiguredTargetAndData ct, NestedSet<ArtifactsInOutputGroup> outputs) { + return new TargetCompleteEvent(ct, null, outputs, true); } /** * Construct a target completion event for a failed target, with the given non-empty root causes. */ public static TargetCompleteEvent createFailed( - ConfiguredTarget ct, Target target, NestedSet<Cause> rootCauses) { + ConfiguredTargetAndData ct, NestedSet<Cause> rootCauses) { Preconditions.checkArgument(!Iterables.isEmpty(rootCauses)); - return new TargetCompleteEvent( - ct, target, rootCauses, ImmutableList.<ArtifactsInOutputGroup>of(), false); + return new TargetCompleteEvent(ct, rootCauses, ImmutableList.of(), false); } /** Returns the target associated with the event. */ public ConfiguredTarget getTarget() { - return target; + return targetAndData.getConfiguredTarget(); } /** Determines whether the target has failed or succeeded. */ @@ -146,10 +142,10 @@ public final class TargetCompleteEvent @Override public BuildEventId getEventId() { Label label = getTarget().getLabel(); - if (target instanceof AliasConfiguredTarget) { - label = ((AliasConfiguredTarget) target).getOriginalLabel(); + if (targetAndData.getConfiguredTarget() instanceof AliasConfiguredTarget) { + label = ((AliasConfiguredTarget) targetAndData.getConfiguredTarget()).getOriginalLabel(); } - BuildConfiguration config = getTarget().getConfiguration(); + BuildConfiguration config = targetAndData.getConfiguration(); BuildEventId configId = config == null ? BuildEventId.nullConfigurationId() : config.getEventId(); return BuildEventId.targetCompleted(label, configId); @@ -165,15 +161,18 @@ public final class TargetCompleteEvent // For tests, announce all the test actions that will minimally happen (except for // interruption). If after the result of a test action another attempt is necessary, // it will be announced with the action that made the new attempt necessary. - Label label = target.getLabel(); - TestProvider.TestParams params = target.getProvider(TestProvider.class).getTestParams(); + Label label = targetAndData.getConfiguredTarget().getLabel(); + TestProvider.TestParams params = + targetAndData.getConfiguredTarget().getProvider(TestProvider.class).getTestParams(); for (int run = 0; run < Math.max(params.getRuns(), 1); run++) { for (int shard = 0; shard < Math.max(params.getShards(), 1); shard++) { childrenBuilder.add( - BuildEventId.testResult(label, run, shard, target.getConfiguration().getEventId())); + BuildEventId.testResult( + label, run, shard, targetAndData.getConfiguration().getEventId())); } } - childrenBuilder.add(BuildEventId.testSummary(label, target.getConfiguration().getEventId())); + childrenBuilder.add( + BuildEventId.testSummary(label, targetAndData.getConfiguration().getEventId())); } return childrenBuilder.build(); } @@ -205,14 +204,14 @@ public final class TargetCompleteEvent BuildEventStreamProtos.TargetComplete.newBuilder(); builder.setSuccess(!failed()); - builder.setTargetKind(actualTarget.getTargetKind()); + builder.setTargetKind(targetAndData.getTarget().getTargetKind()); builder.addAllTag(getTags()); builder.addAllOutputGroup(getOutputFilesByGroup(converters.artifactGroupNamer())); if (isTest) { builder.setTestSize( TargetConfiguredEvent.bepTestSize( - TestSize.getTestSize(actualTarget.getAssociatedRule()))); + TestSize.getTestSize(targetAndData.getTarget().getAssociatedRule()))); } // TODO(aehlig): remove direct reporting of artifacts as soon as clients no longer @@ -251,9 +250,9 @@ public final class TargetCompleteEvent @Override public Collection<BuildEvent> getConfigurations() { - BuildConfiguration configuration = target.getConfiguration(); + BuildConfiguration configuration = targetAndData.getConfiguration(); if (configuration != null) { - return ImmutableList.of(target.getConfiguration().toBuildEvent()); + return ImmutableList.of(targetAndData.getConfiguration().toBuildEvent()); } else { return ImmutableList.<BuildEvent>of(); } @@ -261,12 +260,13 @@ public final class TargetCompleteEvent private Iterable<String> getTags() { // We are only interested in targets that are rules. - if (!(target instanceof RuleConfiguredTarget)) { + if (!(targetAndData.getConfiguredTarget() instanceof RuleConfiguredTarget)) { return ImmutableList.<String>of(); } AttributeMap attributes = ConfiguredAttributeMapper.of( - (Rule) actualTarget, ((RuleConfiguredTarget) target).getConfigConditions()); + (Rule) targetAndData.getTarget(), + ((RuleConfiguredTarget) targetAndData.getConfiguredTarget()).getConfigConditions()); // Every rule (implicitly) has a "tags" attribute. return attributes.get("tags", Type.STRING_LIST); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java index 1a95454370..28cc7d6130 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/AggregatingTestListener.java @@ -222,7 +222,7 @@ public class AggregatingTestListener { @AllowConcurrentEvents public void targetComplete(TargetCompleteEvent event) { if (event.failed()) { - targetFailure(ConfiguredTargetKey.of(event.getTarget())); + targetFailure(asKey(event.getTarget())); } } @@ -263,6 +263,9 @@ public class AggregatingTestListener { private ConfiguredTargetKey asKey(ConfiguredTarget target) { return ConfiguredTargetKey.of( - AliasProvider.getDependencyLabel(target), target.getConfiguration()); + // A test is never in the host configuration. + AliasProvider.getDependencyLabel(target), + target.getConfigurationKey(), + /*isHostConfiguration=*/ false); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java index 7481bd5ec8..05a4fbe1a3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java @@ -29,8 +29,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.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; -import com.google.devtools.build.lib.packages.NoSuchTargetException; -import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.skyframe.AspectCompletionValue.AspectCompletionKey; import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey; import com.google.devtools.build.lib.skyframe.TargetCompletionValue.TargetCompletionKey; @@ -129,9 +127,13 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S @Override public Event getRootCauseError(ConfiguredTargetValue ctValue, Cause rootCause, Environment env) throws InterruptedException { - Target target = getTargetFromConfiguredTarget(ctValue.getConfiguredTarget(), env); + ConfiguredTargetAndData configuredTargetAndData = + ConfiguredTargetAndData.fromConfiguredTargetInSkyframe( + ctValue.getConfiguredTarget(), env); return Event.error( - target == null ? null : target.getLocation(), + configuredTargetAndData == null + ? null + : configuredTargetAndData.getTarget().getLocation(), String.format( "%s: missing input file '%s'", ctValue.getConfiguredTarget().getLabel(), rootCause)); } @@ -141,13 +143,17 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S public MissingInputFileException getMissingFilesException( ConfiguredTargetValue value, int missingCount, Environment env) throws InterruptedException { - Target target = getTargetFromConfiguredTarget(value.getConfiguredTarget(), env); - if (target == null) { + ConfiguredTargetAndData configuredTargetAndData = + ConfiguredTargetAndData.fromConfiguredTargetInSkyframe(value.getConfiguredTarget(), env); + if (configuredTargetAndData == null) { return null; } return new MissingInputFileException( - target.getLocation() + " " + missingCount + " input file(s) do not exist", - target.getLocation()); + configuredTargetAndData.getTarget().getLocation() + + " " + + missingCount + + " input file(s) do not exist", + configuredTargetAndData.getTarget().getLocation()); } @Override @@ -160,12 +166,12 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S public ExtendedEventHandler.Postable createFailed( ConfiguredTargetValue value, NestedSet<Cause> rootCauses, Environment env) throws InterruptedException { - Target actualTarget = getTargetFromConfiguredTarget(value.getConfiguredTarget(), env); - if (actualTarget == null) { + ConfiguredTargetAndData configuredTargetAndData = + ConfiguredTargetAndData.fromConfiguredTargetInSkyframe(value.getConfiguredTarget(), env); + if (configuredTargetAndData == null) { return null; } - return TargetCompleteEvent.createFailed( - value.getConfiguredTarget(), actualTarget, rootCauses); + return TargetCompleteEvent.createFailed(configuredTargetAndData, rootCauses); } @Override @@ -183,36 +189,20 @@ public final class CompletionFunction<TValue extends SkyValue, TResult extends S Environment env) throws InterruptedException { ConfiguredTarget target = value.getConfiguredTarget(); - Target actualTarget = getTargetFromConfiguredTarget(target, env); - if (target == null) { + ConfiguredTargetAndData configuredTargetAndData = + ConfiguredTargetAndData.fromConfiguredTargetInSkyframe(target, env); + if (configuredTargetAndData == null) { return null; } ArtifactsToBuild artifactsToBuild = TopLevelArtifactHelper.getAllArtifactsToBuild(target, topLevelArtifactContext); if (((TargetCompletionKey) skyKey.argument()).willTest()) { return TargetCompleteEvent.successfulBuildSchedulingTest( - target, actualTarget, artifactsToBuild.getAllArtifactsByOutputGroup()); + configuredTargetAndData, artifactsToBuild.getAllArtifactsByOutputGroup()); } else { return TargetCompleteEvent.successfulBuild( - target, actualTarget, artifactsToBuild.getAllArtifactsByOutputGroup()); - } - } - - @Nullable - private Target getTargetFromConfiguredTarget(ConfiguredTarget ct, Environment env) - throws InterruptedException { - Target target = null; - try { - PackageValue packageValue = - (PackageValue) env.getValue(PackageValue.key(ct.getLabel().getPackageIdentifier())); - if (packageValue != null) { - target = packageValue.getPackage().getTarget(ct.getLabel().getName()); - } - } catch (NoSuchTargetException e) { - throw new IllegalStateException( - "Failed to retrieve target to create MissingFilesException.", e); + configuredTargetAndData, artifactsToBuild.getAllArtifactsByOutputGroup()); } - return target; } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java index 81a95f64b9..d8d0f6751c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetAndData.java @@ -14,11 +14,19 @@ package com.google.devtools.build.lib.skyframe; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.packages.NoSuchTargetException; import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.skyframe.SkyFunction; +import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyValue; +import java.util.Map; +import javax.annotation.Nullable; /** * A container class for a {@link ConfiguredTarget} and associated data, {@link Target} and {@link @@ -35,7 +43,8 @@ public class ConfiguredTargetAndData { private final Target target; private final BuildConfiguration configuration; - ConfiguredTargetAndData( + @VisibleForTesting + public ConfiguredTargetAndData( ConfiguredTarget configuredTarget, Target target, BuildConfiguration configuration) { this.configuredTarget = configuredTarget; this.target = target; @@ -75,6 +84,41 @@ public class ConfiguredTargetAndData { } } + @Nullable + static ConfiguredTargetAndData fromConfiguredTargetInSkyframe( + ConfiguredTarget ct, SkyFunction.Environment env) throws InterruptedException { + BuildConfiguration configuration = null; + ImmutableSet<SkyKey> packageAndMaybeConfiguration; + PackageValue.Key packageKey = PackageValue.key(ct.getLabel().getPackageIdentifier()); + BuildConfigurationValue.Key configurationKeyMaybe = ct.getConfigurationKey(); + if (configurationKeyMaybe == null) { + packageAndMaybeConfiguration = ImmutableSet.of(packageKey); + } else { + packageAndMaybeConfiguration = ImmutableSet.of(packageKey, configurationKeyMaybe); + } + Map<SkyKey, SkyValue> packageAndMaybeConfigurationValues = + env.getValues(packageAndMaybeConfiguration); + // Don't test env.valuesMissing(), because values may already be missing from the caller. + PackageValue packageValue = (PackageValue) packageAndMaybeConfigurationValues.get(packageKey); + if (packageValue == null) { + return null; + } + if (configurationKeyMaybe != null) { + BuildConfigurationValue buildConfigurationValue = + (BuildConfigurationValue) packageAndMaybeConfigurationValues.get(configurationKeyMaybe); + if (buildConfigurationValue == null) { + return null; + } + configuration = buildConfigurationValue.getConfiguration(); + } + try { + return new ConfiguredTargetAndData( + ct, packageValue.getPackage().getTarget(ct.getLabel().getName()), configuration); + } catch (NoSuchTargetException e) { + throw new IllegalStateException("Failed to retrieve target for " + ct, e); + } + } + /** * For use with {@code MergedConfiguredTarget} and similar, where we create a virtual {@link * ConfiguredTarget} corresponding to the same {@link Target}. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java index 657eeee232..50d86008fc 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java @@ -70,7 +70,7 @@ public class ConfiguredTargetKey extends ActionLookupKey { } @AutoCodec.Instantiator - static ConfiguredTargetKey of( + public static ConfiguredTargetKey of( Label label, @Nullable BuildConfigurationValue.Key configurationKey, boolean isHostConfiguration) { 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 2a35b9f186..6464b45a76 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 @@ -31,12 +31,9 @@ import com.google.devtools.build.lib.analysis.config.InvalidConfigurationExcepti import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.BuildType; -import com.google.devtools.build.lib.packages.NoSuchTargetException; -import com.google.devtools.build.lib.packages.Package; import com.google.devtools.build.lib.packages.RawAttributeMapper; 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.skyframe.SkyframeActionExecutor.ConflictException; import com.google.devtools.build.lib.syntax.EvalException; import com.google.devtools.build.lib.util.OrderedSetMultimap; @@ -95,18 +92,14 @@ public class PostConfiguredTargetFunction implements SkyFunction { } ConfiguredTarget ct = ctValue.getConfiguredTarget(); - - Package targetPkg = - ((PackageValue) env.getValue(PackageValue.key(ct.getLabel().getPackageIdentifier()))) - .getPackage(); - Target target = null; - try { - target = targetPkg.getTarget(ct.getLabel().getName()); - } catch (NoSuchTargetException e) { - throw new IllegalStateException("Name already verified", e); + ConfiguredTargetAndData configuredTargetAndData = + ConfiguredTargetAndData.fromConfiguredTargetInSkyframe(ct, env); + if (configuredTargetAndData == null) { + return null; } - - TargetAndConfiguration ctgValue = new TargetAndConfiguration(target, ct.getConfiguration()); + TargetAndConfiguration ctgValue = + new TargetAndConfiguration( + configuredTargetAndData.getTarget(), configuredTargetAndData.getConfiguration()); ImmutableMap<Label, ConfigMatchingProvider> configConditions = getConfigurableAttributeConditions(ctgValue, env); @@ -117,7 +110,9 @@ public class PostConfiguredTargetFunction implements SkyFunction { OrderedSetMultimap<Attribute, Dependency> deps; try { BuildConfiguration hostConfiguration = - buildViewProvider.getSkyframeBuildView().getHostConfiguration(ct.getConfiguration()); + buildViewProvider + .getSkyframeBuildView() + .getHostConfiguration(configuredTargetAndData.getConfiguration()); SkyframeDependencyResolver resolver = buildViewProvider.getSkyframeBuildView().createDependencyResolver(env); // We don't track root causes here - this function is only invoked for successfully analyzed @@ -131,7 +126,7 @@ public class PostConfiguredTargetFunction implements SkyFunction { configConditions, /*toolchainLabels*/ ImmutableSet.of(), defaultBuildOptions); - if (ct.getConfiguration() != null) { + if (configuredTargetAndData.getConfiguration() != null) { deps = ConfigurationResolver.resolveConfigurations( env, ctgValue, deps, hostConfiguration, ruleClassProvider, defaultBuildOptions); |