From 8f5d1ee24463bb7ce3509a331ca15a1b93d85c11 Mon Sep 17 00:00:00 2001 From: John Cater Date: Fri, 29 Jan 2021 14:00:09 -0500 Subject: [PATCH] PlatformProviderUtils should ignore targets that don't have the needed provider. Also update PlatformProviderUtils to work directly with Lists. This prevents a few types of crash that can occur from having null values in a List. Fixes #12879. --- .../RuleContextConstraintSemantics.java | 7 ++--- .../platform/PlatformProviderUtils.java | 31 +++++++++++++------ .../build/lib/rules/platform/Platform.java | 14 ++++----- .../build/lib/rules/platform/Toolchain.java | 16 +++++----- 4 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java b/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java index 2af26aa02fc528..bb94ee44debd30 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/constraints/RuleContextConstraintSemantics.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.analysis.constraints; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.Streams.stream; import com.google.auto.value.AutoValue; import com.google.common.base.Joiner; @@ -912,9 +911,9 @@ public static ConfiguredTarget incompatibleConfiguredTarget( if (ruleContext.getRule().getRuleClassObject().useToolchainResolution() && ruleContext.attributes().has("target_compatible_with")) { ImmutableList invalidConstraintValues = - stream( - PlatformProviderUtils.constraintValues( - ruleContext.getPrerequisites("target_compatible_with"))) + PlatformProviderUtils.constraintValues( + ruleContext.getPrerequisites("target_compatible_with")) + .stream() .filter(cv -> !ruleContext.targetPlatformHasConstraint(cv)) .collect(toImmutableList()); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformProviderUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformProviderUtils.java index c0b0a3f7310cde..75deeaca3efd6d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformProviderUtils.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformProviderUtils.java @@ -14,8 +14,12 @@ package com.google.devtools.build.lib.analysis.platform; -import com.google.common.collect.Iterables; +import static com.google.common.base.Predicates.notNull; +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.ProviderCollection; +import java.util.List; import javax.annotation.Nullable; /** Utility methods to help locate platform-related providers. */ @@ -31,8 +35,11 @@ public static PlatformInfo platform(@Nullable ProviderCollection target) { } /** Retrieves and casts {@link PlatformInfo} providers from the given targets. */ - public static Iterable platforms(Iterable targets) { - return Iterables.transform(targets, PlatformProviderUtils::platform); + public static ImmutableList platforms(List targets) { + return targets.stream() + .map(PlatformProviderUtils::platform) + .filter(notNull()) + .collect(toImmutableList()); } /** Retrieves and casts the {@link ConstraintSettingInfo} provider from the given target. */ @@ -45,9 +52,12 @@ public static ConstraintSettingInfo constraintSetting(@Nullable ProviderCollecti } /** Retrieves and casts {@link ConstraintSettingInfo} providers from the given targets. */ - public static Iterable constraintSettings( - Iterable targets) { - return Iterables.transform(targets, PlatformProviderUtils::constraintSetting); + public static List constraintSettings( + List targets) { + return targets.stream() + .map(PlatformProviderUtils::constraintSetting) + .filter(notNull()) + .collect(toImmutableList()); } /** Retrieves and casts the {@link ConstraintValueInfo} provider from the given target. */ @@ -65,9 +75,12 @@ public static boolean hasConstraintValue(ProviderCollection target) { } /** Retrieves and casts {@link ConstraintValueInfo} providers from the given targets. */ - public static Iterable constraintValues( - Iterable targets) { - return Iterables.transform(targets, PlatformProviderUtils::constraintValue); + public static ImmutableList constraintValues( + List targets) { + return targets.stream() + .map(PlatformProviderUtils::constraintValue) + .filter(notNull()) + .collect(toImmutableList()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java index e3564983428601..c260d3d69ec2f8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java +++ b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java @@ -14,9 +14,9 @@ package com.google.devtools.build.lib.rules.platform; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; @@ -35,7 +35,6 @@ import com.google.devtools.build.lib.util.CPU; import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.Pair; -import java.util.List; import java.util.Map; /** Defines a platform for execution contexts. */ @@ -49,10 +48,9 @@ public ConfiguredTarget create(RuleContext ruleContext) PlatformInfo.Builder platformBuilder = PlatformInfo.builder().setLabel(ruleContext.getLabel()); - List parentPlatforms = - Lists.newArrayList( - PlatformProviderUtils.platforms( - ruleContext.getPrerequisites(PlatformRule.PARENTS_PLATFORM_ATTR))); + ImmutableList parentPlatforms = + PlatformProviderUtils.platforms( + ruleContext.getPrerequisites(PlatformRule.PARENTS_PLATFORM_ATTR)); if (parentPlatforms.size() > 1) { throw ruleContext.throwWithAttributeError( @@ -130,7 +128,7 @@ private void autodetectConstraints( // Add the CPU. CPU cpu = cpuValues.getFirst(); - Iterable cpuConstraintValues = + ImmutableList cpuConstraintValues = PlatformProviderUtils.constraintValues( ruleContext.getPrerequisites(PlatformRule.CPU_CONSTRAINTS_ATTR)); for (ConstraintValueInfo constraint : cpuConstraintValues) { @@ -142,7 +140,7 @@ private void autodetectConstraints( // Add the OS. OS os = cpuValues.getSecond(); - Iterable osConstraintValues = + ImmutableList osConstraintValues = PlatformProviderUtils.constraintValues( ruleContext.getPrerequisites(PlatformRule.OS_CONSTRAINTS_ATTR)); for (ConstraintValueInfo constraint : osConstraintValues) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/Toolchain.java b/src/main/java/com/google/devtools/build/lib/rules/platform/Toolchain.java index 05788279f6f314..a9a617e3169c2c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/platform/Toolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/platform/Toolchain.java @@ -14,7 +14,9 @@ package com.google.devtools.build.lib.rules.platform; -import com.google.common.collect.Iterables; +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileProvider; @@ -41,16 +43,16 @@ public ConfiguredTarget create(RuleContext ruleContext) ToolchainTypeInfo toolchainType = PlatformProviderUtils.toolchainType( ruleContext.getPrerequisite(ToolchainRule.TOOLCHAIN_TYPE_ATTR)); - Iterable execConstraints = + ImmutableList execConstraints = PlatformProviderUtils.constraintValues( ruleContext.getPrerequisites(ToolchainRule.EXEC_COMPATIBLE_WITH_ATTR)); - Iterable targetConstraints = + ImmutableList targetConstraints = PlatformProviderUtils.constraintValues( ruleContext.getPrerequisites(ToolchainRule.TARGET_COMPATIBLE_WITH_ATTR)); - Iterable targetSettings = - Iterables.transform( - ruleContext.getPrerequisites(ToolchainRule.TARGET_SETTING_ATTR), - target -> target.getProvider(ConfigMatchingProvider.class)); + ImmutableList targetSettings = + ruleContext.getPrerequisites(ToolchainRule.TARGET_SETTING_ATTR).stream() + .map(target -> target.getProvider(ConfigMatchingProvider.class)) + .collect(toImmutableList()); Label toolchainLabel = ruleContext.attributes().get(ToolchainRule.TOOLCHAIN_ATTR, BuildType.NODEP_LABEL);