diff options
author | Greg Estren <gregce@google.com> | 2017-01-04 18:43:08 +0000 |
---|---|---|
committer | John Cater <jcater@google.com> | 2017-01-04 20:39:43 +0000 |
commit | f21d18e1b2643c5b9814b3f2803619ed15550beb (patch) | |
tree | 571318f9df61ae712c91cda090d226f368db0b0a /src/main/java/com | |
parent | a75ba263afb0219bb635cb725fea57d321d9aaaf (diff) |
Fix an obscure crash scenario with static configs and BuildConfiguration.equals.
See OutputFileConfiguredTargetTest for deep details.
Also more strongly enforce the expectation that all output files have generating rules.
--
PiperOrigin-RevId: 143570028
MOS_MIGRATED_REVID=143570028
Diffstat (limited to 'src/main/java/com')
4 files changed, 16 insertions, 8 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java index d7205a5bf5..8ba3211da1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputFileConfiguredTarget.java @@ -35,7 +35,7 @@ public class OutputFileConfiguredTarget extends FileConfiguredTarget TransitiveInfoCollection generatingRule, Artifact outputArtifact) { super(targetContext, outputArtifact); Preconditions.checkArgument(targetContext.getTarget() == outputFile); - this.generatingRule = generatingRule; + this.generatingRule = Preconditions.checkNotNull(generatingRule); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java index c8b26b97bf..343e69878f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/PackageGroupConfiguredTarget.java @@ -40,7 +40,7 @@ public final class PackageGroupConfiguredTarget extends AbstractConfiguredTarget NestedSetBuilder<PackageSpecification> builder = NestedSetBuilder.stableOrder(); for (Label label : packageGroup.getIncludes()) { - TransitiveInfoCollection include = targetContext.findDirectPrerequisite( + TransitiveInfoCollection include = targetContext.maybeFindDirectPrerequisite( label, targetContext.getConfiguration()); PackageSpecificationProvider provider = include == null ? null : include.getProvider(PackageSpecificationProvider.class); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TargetContext.java b/src/main/java/com/google/devtools/build/lib/analysis/TargetContext.java index cf1afb2de6..fc263efbab 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TargetContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TargetContext.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.analysis; +import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.cmdline.Label; @@ -88,7 +89,20 @@ public class TargetContext { return visibility; } + /** + * Returns the prerequisite with the given label and configuration. Throws a RuntimeException if + * no such prerequisite exists. + */ TransitiveInfoCollection findDirectPrerequisite(Label label, BuildConfiguration config) { + return Verify.verifyNotNull(maybeFindDirectPrerequisite(label, config), + "Could not find prerequisite %s in the expected configuration", label); + } + + /** + * Returns the prerequisite with the given label and configuration, or null if no such + * prerequisite exists. + */ + TransitiveInfoCollection maybeFindDirectPrerequisite(Label label, BuildConfiguration config) { for (ConfiguredTarget prerequisite : directPrerequisites) { if (prerequisite.getLabel().equals(label) && (prerequisite.getConfiguration() == config)) { return prerequisite; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java index a3c63cf2dd..86d736d60a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/ConstraintSemantics.java @@ -763,12 +763,6 @@ public class ConstraintSemantics { // Note this reassignment means constraint violation errors reference the generating // rule, not the file. This makes the source of the environmental mismatch more clear. dep = ((OutputFileConfiguredTarget) dep).getGeneratingRule(); - if (dep == null) { - // This shouldn't happen, but this is causing spurious Bazel test failures because the - // getProvider call below throws a NullPointerException. See b/33385302 for details. - // TODO(gregce): fix the underlying bug, then remove this condition. - continue; - } } // Input files don't support environments. We may subsequently opt them into constraint // checking, but for now just pass them by. |