Skip to content

Commit

Permalink
Update toolchain resolution to load requested toolchain types.
Browse files Browse the repository at this point in the history
This allows Bazel to validate toolchain types earleir, and to deal with
aliases to toolchain types.

Fixes #7404.

Closes #8381.

PiperOrigin-RevId: 248755418
  • Loading branch information
katre authored and copybara-github committed May 17, 2019
1 parent 438b885 commit 004099d
Show file tree
Hide file tree
Showing 8 changed files with 650 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import com.google.devtools.build.lib.skylarkinterface.StarlarkContext;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import java.util.Optional;
import java.util.Set;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -67,7 +66,9 @@ public static ResolvedToolchainContext load(
.setExecutionPlatform(unloadedToolchainContext.executionPlatform())
.setTargetPlatform(unloadedToolchainContext.targetPlatform())
.setRequiredToolchainTypes(unloadedToolchainContext.requiredToolchainTypes())
.setResolvedToolchainLabels(unloadedToolchainContext.resolvedToolchainLabels());
.setResolvedToolchainLabels(unloadedToolchainContext.resolvedToolchainLabels())
.setRequestedToolchainTypeLabels(
unloadedToolchainContext.requestedLabelToToolchainType());

ImmutableMap.Builder<ToolchainTypeInfo, ToolchainInfo> toolchains =
new ImmutableMap.Builder<>();
Expand Down Expand Up @@ -126,6 +127,10 @@ interface Builder {
/** Sets the toolchain types that were requested. */
Builder setRequiredToolchainTypes(Set<ToolchainTypeInfo> requiredToolchainTypes);

/** Sets the map from requested {@link Label} to toolchain type provider. */
Builder setRequestedToolchainTypeLabels(
ImmutableMap<Label, ToolchainTypeInfo> requestedToolchainTypeLabels);

/** Sets the map from toolchain type to toolchain provider. */
Builder setToolchains(ImmutableMap<ToolchainTypeInfo, ToolchainInfo> toolchains);

Expand All @@ -142,6 +147,9 @@ interface Builder {
/** Returns a description of the target being used, for error messaging. */
abstract String targetDescription();

/** Sets the map from requested {@link Label} to toolchain type provider. */
abstract ImmutableMap<Label, ToolchainTypeInfo> requestedToolchainTypeLabels();

abstract ImmutableMap<ToolchainTypeInfo, ToolchainInfo> toolchains();

/** Returns the template variables that these toolchains provide. */
Expand All @@ -153,15 +161,11 @@ interface Builder {
*/
@Nullable
public ToolchainInfo forToolchainType(Label toolchainTypeLabel) {
Optional<ToolchainTypeInfo> toolchainType =
toolchains().keySet().stream()
.filter(info -> info.typeLabel().equals(toolchainTypeLabel))
.findFirst();
if (toolchainType.isPresent()) {
return forToolchainType(toolchainType.get());
} else {
ToolchainTypeInfo toolchainTypeInfo = requestedToolchainTypeLabels().get(toolchainTypeLabel);
if (toolchainTypeInfo == null) {
return null;
}
return toolchains().get(toolchainTypeInfo);
}

@Nullable
Expand Down Expand Up @@ -215,6 +219,9 @@ public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
Label toolchainTypeLabel = transformKey(key, loc);

if (!containsKey(key, loc, context)) {
// TODO(bazel-configurability): The list of available toolchain types is confusing in the
// presence of aliases, since it only contains the actual label, not the alias passed to the
// rule definition.
throw new EvalException(
loc,
String.format(
Expand All @@ -233,12 +240,7 @@ public ToolchainInfo getIndex(Object key, Location loc, StarlarkContext context)
public boolean containsKey(Object key, Location loc, StarlarkContext context)
throws EvalException {
Label toolchainTypeLabel = transformKey(key, loc);
Optional<Label> matching =
toolchains().keySet().stream()
.map(ToolchainTypeInfo::typeLabel)
.filter(label -> label.equals(toolchainTypeLabel))
.findAny();
return matching.isPresent();
return requestedToolchainTypeLabels().containsKey(toolchainTypeLabel);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@
package com.google.devtools.build.lib.skyframe;

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

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Table;
import com.google.devtools.build.lib.analysis.PlatformConfiguration;
Expand All @@ -35,6 +37,7 @@
import com.google.devtools.build.lib.skyframe.PlatformLookupUtil.InvalidPlatformException;
import com.google.devtools.build.lib.skyframe.RegisteredToolchainsFunction.InvalidToolchainLabelException;
import com.google.devtools.build.lib.skyframe.SingleToolchainResolutionFunction.NoToolchainFoundException;
import com.google.devtools.build.lib.skyframe.ToolchainTypeLookupUtil.InvalidToolchainTypeException;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
Expand Down Expand Up @@ -76,6 +79,20 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
boolean debug =
configuration.getOptions().get(PlatformOptions.class).toolchainResolutionDebug;

// Load the configured target for the toolchain types to ensure that they are valid and
// resolve aliases.
ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypes =
loadToolchainTypes(
env,
configuration,
key.requiredToolchainTypeLabels(),
key.shouldSanityCheckConfiguration());
builder.setRequestedLabelToToolchainType(resolvedToolchainTypes);
ImmutableSet<Label> resolvedToolchainTypeLabels =
resolvedToolchainTypes.values().stream()
.map(ToolchainTypeInfo::typeLabel)
.collect(toImmutableSet());

// Create keys for all platforms that will be used, and validate them early.
PlatformKeys platformKeys =
loadPlatformKeys(
Expand All @@ -94,7 +111,7 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
determineToolchainImplementations(
env,
key.configurationKey(),
key.requiredToolchainTypeLabels(),
resolvedToolchainTypeLabels,
builder,
platformKeys,
key.shouldSanityCheckConfiguration());
Expand Down Expand Up @@ -123,6 +140,27 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
}
}

/** Returns a map from the requested toolchain type to the {@link ToolchainTypeInfo} provider. */
private ImmutableMap<Label, ToolchainTypeInfo> loadToolchainTypes(
Environment environment,
BuildConfiguration configuration,
ImmutableSet<Label> requestedToolchainTypeLabels,
boolean shouldSanityCheckConfiguration)
throws InvalidToolchainTypeException, InterruptedException, ValueMissingException {
ImmutableSet<ConfiguredTargetKey> toolchainTypeKeys =
requestedToolchainTypeLabels.stream()
.map(label -> ConfiguredTargetKey.of(label, configuration))
.collect(toImmutableSet());

ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypes =
ToolchainTypeLookupUtil.resolveToolchainTypes(
environment, toolchainTypeKeys, shouldSanityCheckConfiguration);
if (environment.valuesMissing()) {
throw new ValueMissingException();
}
return resolvedToolchainTypes;
}

@AutoValue
abstract static class PlatformKeys {
abstract ConfiguredTargetKey hostPlatformKey();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
// Copyright 2019 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

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

import static java.util.stream.Collectors.joining;

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.platform.PlatformProviderUtils;
import com.google.devtools.build.lib.analysis.platform.ToolchainTypeInfo;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction.ConfiguredValueCreationException;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.ValueOrException3;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;

/** Helper class that looks up {@link ToolchainTypeInfo} data. */
public class ToolchainTypeLookupUtil {

@Nullable
public static ImmutableMap<Label, ToolchainTypeInfo> resolveToolchainTypes(
Environment env,
Iterable<ConfiguredTargetKey> toolchainTypeKeys,
boolean sanityCheckConfiguration)
throws InterruptedException, InvalidToolchainTypeException {
Map<
SkyKey,
ValueOrException3<
ConfiguredValueCreationException, NoSuchThingException, ActionConflictException>>
values =
env.getValuesOrThrow(
toolchainTypeKeys,
ConfiguredValueCreationException.class,
NoSuchThingException.class,
ActionConflictException.class);
boolean valuesMissing = env.valuesMissing();
Map<Label, ToolchainTypeInfo> results = valuesMissing ? null : new HashMap<>();
for (ConfiguredTargetKey key : toolchainTypeKeys) {
Label originalLabel = key.getLabel();
ToolchainTypeInfo toolchainTypeInfo =
findToolchainTypeInfo(key, values.get(key), sanityCheckConfiguration);
if (!valuesMissing && toolchainTypeInfo != null) {
// These are only different if the toolchain type was aliased.
results.put(originalLabel, toolchainTypeInfo);
results.put(toolchainTypeInfo.typeLabel(), toolchainTypeInfo);
}
}
if (valuesMissing) {
return null;
}

return ImmutableMap.copyOf(results);
}

@Nullable
private static ToolchainTypeInfo findToolchainTypeInfo(
ConfiguredTargetKey key,
ValueOrException3<
ConfiguredValueCreationException, NoSuchThingException, ActionConflictException>
valueOrException,
boolean sanityCheckConfiguration)
throws InvalidToolchainTypeException {

try {
ConfiguredTargetValue ctv = (ConfiguredTargetValue) valueOrException.get();
if (ctv == null) {
return null;
}

ConfiguredTarget configuredTarget = ctv.getConfiguredTarget();
BuildConfigurationValue.Key configurationKey = configuredTarget.getConfigurationKey();
// 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 && !configurationKey.getFragments().isEmpty()) {
// No fragments may be present on a toolchain_type rule in retroactive
// trimming mode.
String extraFragmentDescription =
configurationKey.getFragments().stream()
.map(cl -> cl.getSimpleName())
.collect(joining(","));
throw new InvalidToolchainTypeException(
configuredTarget.getLabel(),
"has configuration fragments, "
+ "which is forbidden in retroactive trimming mode: "
+ "extra fragments are ["
+ extraFragmentDescription
+ "]");
}
ToolchainTypeInfo toolchainTypeInfo = PlatformProviderUtils.toolchainType(configuredTarget);
if (toolchainTypeInfo == null) {
throw new InvalidToolchainTypeException(configuredTarget.getLabel());
}

return toolchainTypeInfo;
} catch (ConfiguredValueCreationException e) {
throw new InvalidToolchainTypeException(key.getLabel(), e);
} catch (NoSuchThingException e) {
throw new InvalidToolchainTypeException(key.getLabel(), e);
} catch (ActionConflictException e) {
throw new InvalidToolchainTypeException(key.getLabel(), e);
}
}

/** Exception used when a toolchain type label is not a valid toolchain type. */
public static final class InvalidToolchainTypeException extends ToolchainException {
private static final String DEFAULT_ERROR = "does not provide ToolchainTypeInfo";

InvalidToolchainTypeException(Label label) {
super(formatError(label, DEFAULT_ERROR));
}

InvalidToolchainTypeException(Label label, ConfiguredValueCreationException e) {
super(formatError(label, DEFAULT_ERROR), e);
}

public InvalidToolchainTypeException(Label label, NoSuchThingException e) {
// Just propagate the inner exception, because it's directly actionable.
super(e);
}

public InvalidToolchainTypeException(Label label, ActionConflictException e) {
super(formatError(label, DEFAULT_ERROR), e);
}

InvalidToolchainTypeException(Label label, String error) {
super(formatError(label, error));
}

private static String formatError(Label label, String error) {
return String.format("Target %s was referenced as a toolchain type, but %s", label, error);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.ToolchainContext;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
Expand Down Expand Up @@ -92,16 +93,37 @@ public interface Builder {
/** Sets the toolchain types that were requested. */
Builder setRequiredToolchainTypes(Set<ToolchainTypeInfo> requiredToolchainTypes);

/**
* Maps from the actual toolchain type to the resolved toolchain implementation that should be
* used.
*/
Builder setToolchainTypeToResolved(
ImmutableBiMap<ToolchainTypeInfo, Label> toolchainTypeToResolved);

/**
* Maps from the actual requested {@link Label} to the discovered {@link ToolchainTypeInfo}.
*
* <p>Note that the key may be different from {@link ToolchainTypeInfo#typeLabel()} if the
* requested {@link Label} is an {@code alias}.
*/
Builder setRequestedLabelToToolchainType(
ImmutableMap<Label, ToolchainTypeInfo> requestedLabelToToolchainType);

UnloadedToolchainContext build();
}

/** The map of toolchain type to resolved toolchain to be used. */
// TODO(https://github.com/bazelbuild/bazel/issues/7935): Make this package-protected again.
public abstract ImmutableBiMap<ToolchainTypeInfo, Label> toolchainTypeToResolved();

/**
* Maps from the actual requested {@link Label} to the discovered {@link ToolchainTypeInfo}.
*
* <p>Note that the key may be different from {@link ToolchainTypeInfo#typeLabel()} if the
* requested {@link Label} is an {@code alias}. In this case, there will be two {@link Label
* labels} for the same {@link ToolchainTypeInfo}.
*/
public abstract ImmutableMap<Label, ToolchainTypeInfo> requestedLabelToToolchainType();

@Override
public ImmutableSet<Label> resolvedToolchainLabels() {
return toolchainTypeToResolved().values();
Expand Down
Loading

0 comments on commit 004099d

Please sign in to comment.