Skip to content

Commit

Permalink
Use platform mappings.
Browse files Browse the repository at this point in the history
Applying platform mapping to all BuildConfigurationValue.Key instances. This will cause all configurations to apply the platform mapping before being loaded.

Step 7/N towards the platforms mapping functionality for #6426

RELNOTES: New platform_mappings ability to allow gradual flag to platforms/toolchains migration. See also #6426
PiperOrigin-RevId: 244731653
  • Loading branch information
aragos authored and copybara-github committed Apr 22, 2019
1 parent 103a084 commit 3e77024
Show file tree
Hide file tree
Showing 15 changed files with 529 additions and 147 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.ConfigurationResolver;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.config.TransitionResolver;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
Expand Down Expand Up @@ -183,7 +184,8 @@ public static List<TargetAndConfiguration> getTargetsWithConfigs(
Collection<Target> targets,
ExtendedEventHandler eventHandler,
ConfiguredRuleClassProvider ruleClassProvider,
SkyframeExecutor skyframeExecutor) {
SkyframeExecutor skyframeExecutor)
throws InvalidConfigurationException {
// We use a hash set here to remove duplicate nodes; this can happen for input files and package
// groups.
LinkedHashSet<TargetAndConfiguration> nodes = new LinkedHashSet<>(targets.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.Dependency;
import com.google.devtools.build.lib.analysis.DependencyResolver.DependencyKind;
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
Expand All @@ -47,14 +48,17 @@
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetValue;
import com.google.devtools.build.lib.skyframe.PlatformMappingValue;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.skyframe.TransitiveTargetKey;
import com.google.devtools.build.lib.skyframe.TransitiveTargetValue;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrException;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -261,20 +265,38 @@ public static OrderedSetMultimap<DependencyKind, Dependency> resolveConfiguratio
}

// If we get here, we have to get the configuration from Skyframe.
for (BuildOptions options : toOptions) {
if (sameFragments) {
keysToEntries.put(
BuildConfigurationValue.key(
currentConfiguration.fragmentClasses(),
BuildOptions.diffForReconstruction(defaultBuildOptions, options)),
depsEntry);

} else {
keysToEntries.put(
BuildConfigurationValue.key(
depFragments, BuildOptions.diffForReconstruction(defaultBuildOptions, options)),
depsEntry);
PathFragment platformMappingPath =
currentConfiguration.getOptions().get(PlatformOptions.class).platformMappings;
PlatformMappingValue platformMappingValue =
(PlatformMappingValue) env.getValue(PlatformMappingValue.Key.create(platformMappingPath));
if (platformMappingValue == null) {
return null;
}

try {
for (BuildOptions options : toOptions) {
if (sameFragments) {
keysToEntries.put(
BuildConfigurationValue.keyWithPlatformMapping(
platformMappingValue,
defaultBuildOptions,
currentConfiguration.fragmentClasses(),
BuildOptions.diffForReconstruction(defaultBuildOptions, options)),
depsEntry);

} else {
keysToEntries.put(
BuildConfigurationValue.keyWithPlatformMapping(
platformMappingValue,
defaultBuildOptions,
depFragments,
BuildOptions.diffForReconstruction(defaultBuildOptions, options)),
depsEntry);
}
}
} catch (OptionsParsingException e) {
throw new ConfiguredTargetFunction.DependencyEvaluationException(
new InvalidConfigurationException(e));
}
}

Expand Down Expand Up @@ -631,7 +653,8 @@ public static LinkedHashSet<TargetAndConfiguration> getConfigurationsFromExecuto
Iterable<TargetAndConfiguration> defaultContext,
Multimap<BuildConfiguration, Dependency> targetsToEvaluate,
ExtendedEventHandler eventHandler,
SkyframeExecutor skyframeExecutor) {
SkyframeExecutor skyframeExecutor)
throws InvalidConfigurationException {

Map<Label, Target> labelsToTargets = new LinkedHashMap<>();
for (TargetAndConfiguration targetAndConfig : defaultContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,17 @@
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.common.options.OptionsParsingException;
import java.io.Serializable;
import java.util.Objects;
import java.util.Set;
import java.util.logging.Logger;

/** A Skyframe value representing a {@link BuildConfiguration}. */
// TODO(bazel-team): mark this immutable when BuildConfiguration is immutable.
// @Immutable
@AutoCodec
@ThreadSafe
public class BuildConfigurationValue implements SkyValue {
private static final Logger logger = Logger.getLogger(BuildConfigurationValue.class.getName());
private final BuildConfiguration configuration;

BuildConfigurationValue(BuildConfiguration configuration) {
Expand All @@ -48,15 +47,62 @@ public BuildConfiguration getConfiguration() {
return configuration;
}

/**
* Creates a new configuration key based on the given diff, after applying a platform mapping
* transformation.
*
* @param platformMappingValue sky value that can transform a configuration key based on a
* platform mapping
* @param defaultBuildOptions set of native build options without modifications based on parsing
* flags
* @param fragments set of options fragments this configuration should cover
* @param optionsDiff diff between the default options and the desired configuration
* @throws OptionsParsingException if the platform mapping cannot be parsed
*/
public static Key keyWithPlatformMapping(
PlatformMappingValue platformMappingValue,
BuildOptions defaultBuildOptions,
FragmentClassSet fragments,
BuildOptions.OptionsDiffForReconstruction optionsDiff)
throws OptionsParsingException {
return platformMappingValue.map(
keyWithoutPlatformMapping(fragments, optionsDiff), defaultBuildOptions);
}

/**
* Creates a new configuration key based on the given diff, after applying a platform mapping
* transformation.
*
* @param platformMappingValue sky value that can transform a configuration key based on a
* platform mapping
* @param defaultBuildOptions set of native build options without modifications based on parsing
* flags
* @param fragments set of options fragments this configuration should cover
* @param optionsDiff diff between the default options and the desired configuration
* @throws OptionsParsingException if the platform mapping cannot be parsed
*/
public static Key keyWithPlatformMapping(
PlatformMappingValue platformMappingValue,
BuildOptions defaultBuildOptions,
Set<Class<? extends BuildConfiguration.Fragment>> fragments,
BuildOptions.OptionsDiffForReconstruction optionsDiff)
throws OptionsParsingException {
return platformMappingValue.map(
keyWithoutPlatformMapping(fragments, optionsDiff), defaultBuildOptions);
}

/**
* Returns the key for a requested configuration.
*
* <p>Callers are responsible for applying the platform mapping or ascertaining that a platform
* mapping is not required.
*
* @param fragments the fragments the configuration should contain
* @param optionsDiff the {@link BuildOptions.OptionsDiffForReconstruction} object the {@link
* BuildOptions} should be rebuilt from
*/
@ThreadSafe
public static Key key(
static Key keyWithoutPlatformMapping(
Set<Class<? extends BuildConfiguration.Fragment>> fragments,
BuildOptions.OptionsDiffForReconstruction optionsDiff) {
return Key.create(
Expand All @@ -65,13 +111,23 @@ public static Key key(
optionsDiff);
}

public static Key key(
private static Key keyWithoutPlatformMapping(
FragmentClassSet fragmentClassSet, BuildOptions.OptionsDiffForReconstruction optionsDiff) {
return Key.create(fragmentClassSet, optionsDiff);
}

/**
* Returns a configuration key for the given configuration.
*
* <p>Note that this key creation method does not apply a platform mapping, it is assumed that the
* passed configuration was created with one such and thus its key does not need to be mapped
* again.
*
* @param buildConfiguration configuration whose key is requested
*/
public static Key key(BuildConfiguration buildConfiguration) {
return key(buildConfiguration.fragmentClasses(), buildConfiguration.getBuildOptionsDiff());
return keyWithoutPlatformMapping(
buildConfiguration.fragmentClasses(), buildConfiguration.getBuildOptionsDiff());
}

/** {@link SkyKey} for {@link BuildConfigurationValue}. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public BuildConfigurationValue.Key map(
}
}

return BuildConfigurationValue.key(
return BuildConfigurationValue.keyWithoutPlatformMapping(
original.getFragments(),
BuildOptions.diffForReconstruction(defaultBuildOptions, modifiedOptions));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.Dependency;
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.TargetAndConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
Expand All @@ -40,11 +41,13 @@
import com.google.devtools.build.lib.packages.NoSuchThingException;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.skyframe.PrepareAnalysisPhaseValue.PrepareAnalysisPhaseKey;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrException;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -82,19 +85,36 @@ public PrepareAnalysisPhaseValue compute(SkyKey key, Environment env)

ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> allFragments =
options.getFragments().fragmentClasses();
BuildConfigurationValue.Key hostConfigurationKey =
BuildConfigurationValue.key(
allFragments,
BuildOptions.diffForReconstruction(defaultBuildOptions, hostOptions));
ImmutableList<BuildConfigurationValue.Key> targetConfigurationKeys =
getTopLevelBuildOptions(targetOptions, options.getMultiCpu())
.stream()
.map(
elem ->
BuildConfigurationValue.key(
allFragments,
BuildOptions.diffForReconstruction(defaultBuildOptions, elem)))
.collect(ImmutableList.toImmutableList());

PathFragment platformMappingPath = targetOptions.get(PlatformOptions.class).platformMappings;
PlatformMappingValue platformMappingValue =
(PlatformMappingValue) env.getValue(PlatformMappingValue.Key.create(platformMappingPath));
if (platformMappingValue == null) {
return null;
}

BuildConfigurationValue.Key hostConfigurationKey = null;
ImmutableList.Builder<BuildConfigurationValue.Key> targetConfigurationKeysBuilder =
ImmutableList.builder();
try {
hostConfigurationKey =
BuildConfigurationValue.keyWithPlatformMapping(
platformMappingValue,
defaultBuildOptions,
allFragments,
BuildOptions.diffForReconstruction(defaultBuildOptions, hostOptions));
for (BuildOptions buildOptions :
getTopLevelBuildOptions(targetOptions, options.getMultiCpu())) {
targetConfigurationKeysBuilder.add(
BuildConfigurationValue.keyWithPlatformMapping(
platformMappingValue,
defaultBuildOptions,
allFragments,
BuildOptions.diffForReconstruction(defaultBuildOptions, buildOptions)));
}
} catch (OptionsParsingException e) {
throw new PrepareAnalysisPhaseFunctionException(new InvalidConfigurationException(e));
}

// We don't need the host configuration below, but we call this to get the error, if any.
try {
Expand All @@ -103,6 +123,8 @@ public PrepareAnalysisPhaseValue compute(SkyKey key, Environment env)
throw new PrepareAnalysisPhaseFunctionException(e);
}

ImmutableList<BuildConfigurationValue.Key> targetConfigurationKeys =
targetConfigurationKeysBuilder.build();
Map<SkyKey, SkyValue> configs = env.getValues(targetConfigurationKeys);

// We only report invalid options for the target configurations, and abort if there's an error.
Expand Down Expand Up @@ -148,7 +170,7 @@ public PrepareAnalysisPhaseValue compute(SkyKey key, Environment env)
LinkedHashSet<TargetAndConfiguration> topLevelTargetsWithConfigs;
try {
topLevelTargetsWithConfigs = resolveConfigurations(env, nodes, asDeps);
} catch (TransitionException e) {
} catch (TransitionException | OptionsParsingException e) {
throw new PrepareAnalysisPhaseFunctionException(new InvalidConfigurationException(e));
}
if (env.valuesMissing()) {
Expand Down Expand Up @@ -189,7 +211,7 @@ private LinkedHashSet<TargetAndConfiguration> resolveConfigurations(
SkyFunction.Environment env,
Iterable<TargetAndConfiguration> nodes,
Multimap<BuildConfiguration, Dependency> asDeps)
throws InterruptedException, TransitionException {
throws InterruptedException, TransitionException, OptionsParsingException {
Map<Label, Target> labelsToTargets = new LinkedHashMap<>();
for (TargetAndConfiguration node : nodes) {
labelsToTargets.put(node.getTarget().getLabel(), node.getTarget());
Expand Down Expand Up @@ -244,7 +266,7 @@ private static boolean useUntrimmedConfigs(BuildOptions options) {
// Note: this implementation runs inside Skyframe, so it has access to SkyFunction.Environment.
private Multimap<Dependency, BuildConfiguration> getConfigurations(
SkyFunction.Environment env, BuildOptions fromOptions, Iterable<Dependency> keys)
throws InterruptedException, TransitionException {
throws InterruptedException, TransitionException, OptionsParsingException {
Multimap<Dependency, BuildConfiguration> builder =
ArrayListMultimap.<Dependency, BuildConfiguration>create();
Set<Dependency> depsToEvaluate = new HashSet<>();
Expand Down Expand Up @@ -295,6 +317,13 @@ private Multimap<Dependency, BuildConfiguration> getConfigurations(
}

// Now get the configurations.
PathFragment platformMappingPath = fromOptions.get(PlatformOptions.class).platformMappings;
PlatformMappingValue platformMappingValue =
(PlatformMappingValue) env.getValue(PlatformMappingValue.Key.create(platformMappingPath));
if (platformMappingValue == null) {
return null;
}

final List<SkyKey> configSkyKeys = new ArrayList<>();
for (Dependency key : keys) {
if (labelsWithErrors.contains(key.getLabel()) || key.hasExplicitConfiguration()) {
Expand All @@ -306,6 +335,7 @@ private Multimap<Dependency, BuildConfiguration> getConfigurations(
ConfigurationTransition transition = key.getTransition();
ImmutableSortedSet<Class<? extends BuildConfiguration.Fragment>> depFragments =
fragmentsMap.get(key.getLabel());

if (depFragments != null) {
// TODO(juliexxia): combine these skyframe calls with other skyframe calls for this
// configured target.
Expand Down Expand Up @@ -333,8 +363,11 @@ private Multimap<Dependency, BuildConfiguration> getConfigurations(
StarlarkTransition.replayEvents(env.getListener(), transition);
for (BuildOptions toOption : toOptions) {
configSkyKeys.add(
BuildConfigurationValue.key(
depFragments, BuildOptions.diffForReconstruction(defaultBuildOptions, toOption)));
BuildConfigurationValue.keyWithPlatformMapping(
platformMappingValue,
defaultBuildOptions,
depFragments,
BuildOptions.diffForReconstruction(defaultBuildOptions, toOption)));
}
}
}
Expand Down Expand Up @@ -376,8 +409,11 @@ private Multimap<Dependency, BuildConfiguration> getConfigurations(
buildSettingOutputPackages);
for (BuildOptions toOption : toOptions) {
SkyKey configKey =
BuildConfigurationValue.key(
depFragments, BuildOptions.diffForReconstruction(defaultBuildOptions, toOption));
BuildConfigurationValue.keyWithPlatformMapping(
platformMappingValue,
defaultBuildOptions,
depFragments,
BuildOptions.diffForReconstruction(defaultBuildOptions, toOption));
BuildConfigurationValue configValue =
((BuildConfigurationValue) configsResult.get(configKey));
// configValue will be null here if there was an exception thrown during configuration
Expand Down
Loading

0 comments on commit 3e77024

Please sign in to comment.