Skip to content

Commit

Permalink
Remove isTargetExplicit check -- now that filtering exists, only act…
Browse files Browse the repository at this point in the history
…ual platform/toolchain targets will actually get into those lists.

    RELNOTES: None
    PiperOrigin-RevId: 241788572
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent 64807e1 commit fb83100
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@

package com.google.devtools.build.lib.skyframe;

import static com.google.common.base.Predicates.equalTo;
import static com.google.common.base.Predicates.not;
import static com.google.common.collect.ImmutableList.toImmutableList;

import com.google.auto.value.AutoValue;
Expand Down Expand Up @@ -94,8 +92,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)

// Load the configured target for each, and get the declared execution platforms providers.
ImmutableList<ConfiguredTargetKey> registeredExecutionPlatformKeys =
configureRegisteredExecutionPlatforms(
env, configuration, configuration.trimConfigurationsRetroactively(), platformLabels);
configureRegisteredExecutionPlatforms(env, configuration, platformLabels);
if (env.valuesMissing()) {
return null;
}
Expand Down Expand Up @@ -125,7 +122,6 @@ public static List<String> getWorkspaceExecutionPlatforms(Environment env)
private ImmutableList<ConfiguredTargetKey> configureRegisteredExecutionPlatforms(
Environment env,
BuildConfiguration configuration,
boolean sanityCheckConfiguration,
List<Label> labels)
throws InterruptedException, RegisteredExecutionPlatformsFunctionException {

Expand All @@ -149,22 +145,6 @@ private ImmutableList<ConfiguredTargetKey> configureRegisteredExecutionPlatforms
}
ConfiguredTarget target =
((ConfiguredTargetValue) valueOrException.get()).getConfiguredTarget();
// This check is necessary because trimming for other rules assumes that platform resolution
// uses the platform fragment and _only_ the platform fragment. Without this check, it's
// possible another fragment could slip in without us realizing, and thus break this
// assumption.
if (sanityCheckConfiguration
&& target.getConfigurationKey().getFragments().stream()
.anyMatch(not(equalTo(PlatformConfiguration.class)))) {
// Only the PlatformConfiguration fragment may be present on a platform rule in
// retroactive trimming mode.
throw new RegisteredExecutionPlatformsFunctionException(
new InvalidPlatformException(
target.getLabel(),
"has fragments other than PlatformConfiguration, "
+ "which is forbidden in retroactive trimming mode"),
Transience.PERSISTENT);
}
PlatformInfo platformInfo = PlatformProviderUtils.platform(target);
if (platformInfo == null) {
throw new RegisteredExecutionPlatformsFunctionException(
Expand Down Expand Up @@ -235,9 +215,13 @@ public boolean shouldRetain(Target target, boolean explicit) {
return true;
}

// If the rule requires platforms or toolchain resolution, it can't be used as a platform.
// If the rule requires platforms, it can't be used as a platform.
RuleClass ruleClass = target.getAssociatedRule().getRuleClassObject();
if (ruleClass == null || ruleClass.useToolchainResolution()) {
if (ruleClass == null) {
return false;
}

if (ruleClass.supportsPlatforms()) {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.devtools.build.lib.skyframe;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static java.util.stream.Collectors.joining;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
Expand Down Expand Up @@ -146,31 +145,8 @@ private ImmutableList<DeclaredToolchainInfo> configureRegisteredToolchains(
valuesMissing = true;
continue;
}

ConfiguredTarget target =
((ConfiguredTargetValue) valueOrException.get()).getConfiguredTarget();
if (configuration.trimConfigurationsRetroactively()
&& !target.getConfigurationKey().getFragments().isEmpty()) {
// No fragment may be present on a toolchain rule in retroactive trimming mode.
// This is because trimming expects that platform and toolchain resolution uses only
// the platform configuration. In theory, this means toolchains could use platforms, but
// the current expectation is that toolchains should not use anything at all, so better
// to go with the stricter expectation for now.
String extraFragmentDescription =
target.getConfigurationKey().getFragments().stream()
.map(cl -> cl.getSimpleName())
.collect(joining(","));

throw new RegisteredToolchainsFunctionException(
new InvalidToolchainLabelException(
toolchainLabel,
"this toolchain uses configuration, which is forbidden in retroactive trimming "
+ "mode: "
+ "extra fragments are ["
+ extraFragmentDescription
+ "]"),
Transience.PERSISTENT);
}
DeclaredToolchainInfo toolchainInfo = target.getProvider(DeclaredToolchainInfo.class);

if (toolchainInfo == null) {
Expand Down Expand Up @@ -209,10 +185,6 @@ public InvalidToolchainLabelException(Label invalidLabel) {
"target does not provide the DeclaredToolchainInfo provider"));
}

public InvalidToolchainLabelException(Label invalidLabel, String reason) {
super(formatMessage(invalidLabel.getCanonicalForm(), reason));
}

public InvalidToolchainLabelException(TargetPatternUtil.InvalidTargetPatternException e) {
this(e.getInvalidPattern(), e.getTpe());
}
Expand Down

0 comments on commit fb83100

Please sign in to comment.