Skip to content

Commit

Permalink
Clean up test configuration creation.
Browse files Browse the repository at this point in the history
Work towards platform-based flags: bazelbuild#19409.

PiperOrigin-RevId: 617202986
Change-Id: Ifd0d8d897b43963837236a956b4a57f07a959168
  • Loading branch information
katre authored and pull[bot] committed Apr 19, 2024
1 parent 2a0651b commit 5556482
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,19 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
.map(ToolchainTypeRequirement::toolchainType)
.collect(toImmutableSet()));

// Create keys for all platforms that will be used, and validate them early.
// Do this early, to catch platform errors early.
PlatformKeys platformKeys =
loadPlatformKeys(
env,
debug,
configuration.getKey(),
platformConfiguration,
key.execConstraintLabels());
if (env.valuesMissing()) {
return null;
}

// Load the configured target for the toolchain types to ensure that they are valid and
// resolve aliases.
ImmutableMap<Label, ToolchainTypeInfo> resolvedToolchainTypeInfos =
Expand All @@ -103,18 +116,6 @@ public UnloadedToolchainContext compute(SkyKey skyKey, Environment env)
ImmutableSet<ToolchainType> resolvedToolchainTypes =
loadToolchainTypes(resolvedToolchainTypeInfos, key.toolchainTypes());

// Create keys for all platforms that will be used, and validate them early.
PlatformKeys platformKeys =
loadPlatformKeys(
env,
debug,
configuration.getKey(),
platformConfiguration,
key.execConstraintLabels());
if (env.valuesMissing()) {
return null;
}

// Determine the actual toolchain implementations to use.
determineToolchainImplementations(
env,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,11 @@ java_test(
name = "ToolchainResolutionFunctionTest",
srcs = ["ToolchainResolutionFunctionTest.java"],
deps = [
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement",
"//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/skyframe/config",
"//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:constraint_value_lookup_util",
"//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:platform_lookup_util",
"//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:toolchain_context_key",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,12 @@
import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.assertThatEvaluationResult;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.rules.platform.ToolchainTestCase;
import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey;
import com.google.devtools.build.lib.skyframe.toolchains.ConstraintValueLookupUtil.InvalidConstraintValueException;
import com.google.devtools.build.lib.skyframe.toolchains.PlatformLookupUtil.InvalidPlatformException;
import com.google.devtools.build.lib.skyframe.toolchains.ToolchainTypeLookupUtil.InvalidToolchainTypeException;
Expand Down Expand Up @@ -540,13 +543,22 @@ public void resolve_unavailableToolchainType_multiple() throws Exception {
@Test
public void resolve_invalidTargetPlatform_badTarget() throws Exception {
scratch.file("invalid/BUILD", "filegroup(name = 'not_a_platform')");
useConfiguration("--platforms=//invalid:not_a_platform");

// Manually create a configuration key: trying to call `useConfiguration` will immediately throw
// the exception this is checking for.
BuildOptions newOptions = targetConfigKey.getOptions().clone();
newOptions.get(PlatformOptions.class).platforms =
ImmutableList.of(Label.parseCanonicalUnchecked("//invalid:not_a_platform"));
BuildConfigurationKey configKey = BuildConfigurationKey.create(newOptions);

// Create the toolchain context key and evaluate it.
ToolchainContextKey key =
ToolchainContextKey.key()
.configurationKey(targetConfigKey)
.configurationKey(configKey)
.toolchainTypes(testToolchainType)
.build();

reporter.removeHandler(failFastHandler); // expect errors
EvaluationResult<UnloadedToolchainContext> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasError();
Expand All @@ -566,13 +578,22 @@ public void resolve_invalidTargetPlatform_badTarget() throws Exception {
@Test
public void resolve_invalidTargetPlatform_badPackage() throws Exception {
scratch.resolve("invalid").delete();
useConfiguration("--platforms=//invalid:not_a_platform");

// Manually create a configuration key: trying to call `useConfiguration` will immediately throw
// the exception this is checking for.
BuildOptions newOptions = targetConfigKey.getOptions().clone();
newOptions.get(PlatformOptions.class).platforms =
ImmutableList.of(Label.parseCanonicalUnchecked("//invalid:not_a_platform"));
BuildConfigurationKey configKey = BuildConfigurationKey.create(newOptions);

// Create the toolchain context key and evaluate it.
ToolchainContextKey key =
ToolchainContextKey.key()
.configurationKey(targetConfigKey)
.configurationKey(configKey)
.toolchainTypes(testToolchainType)
.build();

reporter.removeHandler(failFastHandler); // expect errors
EvaluationResult<UnloadedToolchainContext> result = invokeToolchainResolution(key);

assertThatEvaluationResult(result).hasError();
Expand Down

0 comments on commit 5556482

Please sign in to comment.