Skip to content

Commit

Permalink
PlatformProviderUtils should ignore targets that don't have the needed
Browse files Browse the repository at this point in the history
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.

Closes #12931.

PiperOrigin-RevId: 354595201
  • Loading branch information
katre authored and copybara-github committed Jan 29, 2021
1 parent 3e6e975 commit 24d0864
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -912,9 +911,9 @@ public static ConfiguredTarget incompatibleConfiguredTarget(
if (ruleContext.getRule().getRuleClassObject().useToolchainResolution()
&& ruleContext.attributes().has("target_compatible_with")) {
ImmutableList<ConstraintValueInfo> invalidConstraintValues =
stream(
PlatformProviderUtils.constraintValues(
ruleContext.getPrerequisites("target_compatible_with")))
PlatformProviderUtils.constraintValues(
ruleContext.getPrerequisites("target_compatible_with"))
.stream()
.filter(cv -> !ruleContext.targetPlatformHasConstraint(cv))
.collect(toImmutableList());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -31,8 +35,11 @@ public static PlatformInfo platform(@Nullable ProviderCollection target) {
}

/** Retrieves and casts {@link PlatformInfo} providers from the given targets. */
public static Iterable<PlatformInfo> platforms(Iterable<? extends ProviderCollection> targets) {
return Iterables.transform(targets, PlatformProviderUtils::platform);
public static ImmutableList<PlatformInfo> platforms(List<? extends ProviderCollection> targets) {
return targets.stream()
.map(PlatformProviderUtils::platform)
.filter(notNull())
.collect(toImmutableList());
}

/** Retrieves and casts the {@link ConstraintSettingInfo} provider from the given target. */
Expand All @@ -45,9 +52,12 @@ public static ConstraintSettingInfo constraintSetting(@Nullable ProviderCollecti
}

/** Retrieves and casts {@link ConstraintSettingInfo} providers from the given targets. */
public static Iterable<ConstraintSettingInfo> constraintSettings(
Iterable<? extends ProviderCollection> targets) {
return Iterables.transform(targets, PlatformProviderUtils::constraintSetting);
public static ImmutableList<ConstraintSettingInfo> constraintSettings(
List<? extends ProviderCollection> targets) {
return targets.stream()
.map(PlatformProviderUtils::constraintSetting)
.filter(notNull())
.collect(toImmutableList());
}

/** Retrieves and casts the {@link ConstraintValueInfo} provider from the given target. */
Expand All @@ -65,9 +75,12 @@ public static boolean hasConstraintValue(ProviderCollection target) {
}

/** Retrieves and casts {@link ConstraintValueInfo} providers from the given targets. */
public static Iterable<ConstraintValueInfo> constraintValues(
Iterable<? extends ProviderCollection> targets) {
return Iterables.transform(targets, PlatformProviderUtils::constraintValue);
public static ImmutableList<ConstraintValueInfo> constraintValues(
List<? extends ProviderCollection> targets) {
return targets.stream()
.map(PlatformProviderUtils::constraintValue)
.filter(notNull())
.collect(toImmutableList());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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. */
Expand All @@ -49,10 +48,9 @@ public ConfiguredTarget create(RuleContext ruleContext)

PlatformInfo.Builder platformBuilder = PlatformInfo.builder().setLabel(ruleContext.getLabel());

List<PlatformInfo> parentPlatforms =
Lists.newArrayList(
PlatformProviderUtils.platforms(
ruleContext.getPrerequisites(PlatformRule.PARENTS_PLATFORM_ATTR)));
ImmutableList<PlatformInfo> parentPlatforms =
PlatformProviderUtils.platforms(
ruleContext.getPrerequisites(PlatformRule.PARENTS_PLATFORM_ATTR));

if (parentPlatforms.size() > 1) {
throw ruleContext.throwWithAttributeError(
Expand Down Expand Up @@ -130,7 +128,7 @@ private void autodetectConstraints(

// Add the CPU.
CPU cpu = cpuValues.getFirst();
Iterable<ConstraintValueInfo> cpuConstraintValues =
ImmutableList<ConstraintValueInfo> cpuConstraintValues =
PlatformProviderUtils.constraintValues(
ruleContext.getPrerequisites(PlatformRule.CPU_CONSTRAINTS_ATTR));
for (ConstraintValueInfo constraint : cpuConstraintValues) {
Expand All @@ -142,7 +140,7 @@ private void autodetectConstraints(

// Add the OS.
OS os = cpuValues.getSecond();
Iterable<ConstraintValueInfo> osConstraintValues =
ImmutableList<ConstraintValueInfo> osConstraintValues =
PlatformProviderUtils.constraintValues(
ruleContext.getPrerequisites(PlatformRule.OS_CONSTRAINTS_ATTR));
for (ConstraintValueInfo constraint : osConstraintValues) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -41,16 +43,16 @@ public ConfiguredTarget create(RuleContext ruleContext)
ToolchainTypeInfo toolchainType =
PlatformProviderUtils.toolchainType(
ruleContext.getPrerequisite(ToolchainRule.TOOLCHAIN_TYPE_ATTR));
Iterable<ConstraintValueInfo> execConstraints =
ImmutableList<ConstraintValueInfo> execConstraints =
PlatformProviderUtils.constraintValues(
ruleContext.getPrerequisites(ToolchainRule.EXEC_COMPATIBLE_WITH_ATTR));
Iterable<ConstraintValueInfo> targetConstraints =
ImmutableList<ConstraintValueInfo> targetConstraints =
PlatformProviderUtils.constraintValues(
ruleContext.getPrerequisites(ToolchainRule.TARGET_COMPATIBLE_WITH_ATTR));
Iterable<ConfigMatchingProvider> targetSettings =
Iterables.transform(
ruleContext.getPrerequisites(ToolchainRule.TARGET_SETTING_ATTR),
target -> target.getProvider(ConfigMatchingProvider.class));
ImmutableList<ConfigMatchingProvider> 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);

Expand Down

0 comments on commit 24d0864

Please sign in to comment.