Skip to content

Commit

Permalink
Change PlatformConfiguration to only have a single target platform.
Browse files Browse the repository at this point in the history
This is the actual situation that has existed and will continue to be supported: each ConfiguredTarget has a single target platform. When we update to support multi-platform builds, the configuration will be split at the top level so that each split still only sees a single target platform.

Multi-arch platforms (such as fat binaries) will need a single platform to represent the multi-arch situation, and rules will need to know how to handle generating and combining the proper single-target dependencies: this is the same situation we have currently.

Change-Id: I904d74d136c761fa9c8da329040dd20920db00b7

Closes #5755.

Change-Id: Icaa539fa864d005c754986f7c98dac1bb8dc7e35
PiperOrigin-RevId: 209939618
  • Loading branch information
katre authored and Copybara-Service committed Aug 23, 2018
1 parent 1d70434 commit 5b30745
Show file tree
Hide file tree
Showing 6 changed files with 227 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,20 +29,20 @@ public class PlatformConfiguration extends BuildConfiguration.Fragment
implements PlatformConfigurationApi {
private final Label hostPlatform;
private final ImmutableList<String> extraExecutionPlatforms;
private final ImmutableList<Label> targetPlatforms;
private final Label targetPlatform;
private final ImmutableList<String> extraToolchains;
private final ImmutableList<Label> enabledToolchainTypes;

@AutoCodec.Instantiator
PlatformConfiguration(
Label hostPlatform,
ImmutableList<String> extraExecutionPlatforms,
ImmutableList<Label> targetPlatforms,
Label targetPlatform,
ImmutableList<String> extraToolchains,
ImmutableList<Label> enabledToolchainTypes) {
this.hostPlatform = hostPlatform;
this.extraExecutionPlatforms = extraExecutionPlatforms;
this.targetPlatforms = targetPlatforms;
this.targetPlatform = targetPlatform;
this.extraToolchains = extraToolchains;
this.enabledToolchainTypes = enabledToolchainTypes;
}
Expand All @@ -60,9 +60,19 @@ public ImmutableList<String> getExtraExecutionPlatforms() {
return extraExecutionPlatforms;
}

/**
* Returns the single target platform used in this configuration. The flag is multi-valued for
* future handling of multiple target platforms but any given configuration should only be
* concerned with a single target platform.
*/
@Override
public Label getTargetPlatform() {
return targetPlatform;
}

@Override
public ImmutableList<Label> getTargetPlatforms() {
return targetPlatforms;
return ImmutableList.of(targetPlatform);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,14 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.cmdline.Label;

/** A loader that creates {@link PlatformConfiguration} instances based on command-line options. */
public class PlatformConfigurationLoader implements ConfigurationFragmentFactory {
Expand All @@ -44,10 +46,18 @@ public Class<? extends BuildConfiguration.Fragment> creates() {

private PlatformConfiguration create(PlatformOptions options)
throws InvalidConfigurationException {

if (options.hostPlatform == null) {
throw new InvalidConfigurationException("Host platform not set");
}
Label targetPlatform = Iterables.getFirst(options.platforms, null);
if (targetPlatform == null) {
throw new InvalidConfigurationException("Target platform not set");
}
return new PlatformConfiguration(
options.hostPlatform,
ImmutableList.copyOf(options.extraExecutionPlatforms),
ImmutableList.copyOf(options.platforms),
targetPlatform,
ImmutableList.copyOf(options.extraToolchains),
ImmutableList.copyOf(options.enabledToolchainTypes));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ static ToolchainContext createToolchainContext(
return null;
}
Label hostPlatformLabel = platformConfiguration.getHostPlatform();
Label targetPlatformLabel = platformConfiguration.getTargetPlatforms().get(0);
Label targetPlatformLabel = platformConfiguration.getTargetPlatform();

ConfiguredTargetKey hostPlatformKey = ConfiguredTargetKey.of(hostPlatformLabel, configuration);
ConfiguredTargetKey targetPlatformKey =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,22 @@
public interface PlatformConfigurationApi {

@SkylarkCallable(name = "host_platform", structField = true, doc = "The current host platform")
public Label getHostPlatform();
Label getHostPlatform();

@SkylarkCallable(name = "platforms", structField = true, doc = "The current target platforms")
public ImmutableList<Label> getTargetPlatforms();
@SkylarkCallable(name = "platform", structField = true, doc = "The current target platform")
Label getTargetPlatform();

@SkylarkCallable(
name = "enabled_toolchain_types",
structField = true,
doc = "The set of toolchain types enabled for platform-based toolchain selection."
)
public List<Label> getEnabledToolchainTypes();
name = "platforms",
structField = true,
doc = "The current target platforms",
documented = false)
@Deprecated
ImmutableList<Label> getTargetPlatforms();

@SkylarkCallable(
name = "enabled_toolchain_types",
structField = true,
doc = "The set of toolchain types enabled for platform-based toolchain selection.")
List<Label> getEnabledToolchainTypes();
}
27 changes: 13 additions & 14 deletions src/test/java/com/google/devtools/build/lib/rules/platform/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -12,36 +12,35 @@ filegroup(

TESTUTIL_SRCS = ["ToolchainTestCase.java"]

java_test(
name = "PlatformRulesTests",
java_library(
name = "PlatformRulesTests_lib",
srcs = glob(
["*.java"],
exclude = TESTUTIL_SRCS,
),
test_class = "com.google.devtools.build.lib.AllTests",
deps = [
"//src/main/java/com/google/devtools/build/lib:build-base",
"//src/main/java/com/google/devtools/build/lib:os_util",
"//src/main/java/com/google/devtools/build/lib:packages-internal",
"//src/main/java/com/google/devtools/build/lib:syntax",
"//src/main/java/com/google/devtools/build/lib:util",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/platform",
"//src/test/java/com/google/devtools/build/lib:analysis_testutil",
"//src/test/java/com/google/devtools/build/lib:packages_testutil",
"//src/test/java/com/google/devtools/build/lib/skyframe:testutil",
"//src/test/java/com/google/devtools/build/lib/skylark:testutil",
"//src/test/java/com/google/devtools/build/lib:testutil",
"//third_party:guava",
"//third_party:guava-testlib",
"//third_party:junit4",
"//third_party:truth",
],
)

java_test(
name = "PlatformRulesTests",
test_class = "com.google.devtools.build.lib.AllTests",
runtime_deps = [
":PlatformRulesTests_lib",
"//src/test/java/com/google/devtools/build/lib:test_runner",
],
)

java_library(
name = "testutil",
srcs = TESTUTIL_SRCS,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
// Copyright 2018 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.rules.platform;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.SkylarkProvider.SkylarkKey;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.skylarkbuildapi.platform.PlatformConfigurationApi;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

/** Tests Skylark API for Platform configuration fragments. */
@RunWith(JUnit4.class)
public class PlatformConfigurationApiTest extends BuildViewTestCase {

@Test
public void testHostPlatform() throws Exception {
scratch.file("platforms/BUILD", "platform(name = 'test_platform')");

useConfiguration("--host_platform=//platforms:test_platform");
ruleBuilder().build();
scratch.file(
"foo/BUILD",
"load(':extension.bzl', 'my_rule')",
"my_rule(",
" name = 'my_skylark_rule',",
")");
assertNoEvents();

PlatformConfigurationApi platformConfiguration = fetchPlatformConfiguration();
assertThat(platformConfiguration).isNotNull();
assertThat(platformConfiguration.getHostPlatform())
.isEqualTo(Label.parseAbsoluteUnchecked("//platforms:test_platform"));
}

@Test
public void testHostPlatform_unset() {
assertThrows(InvalidConfigurationException.class, () -> useConfiguration("--host_platform="));
}

@Test
public void testTargetPlatform_single() throws Exception {
scratch.file("platforms/BUILD", "platform(name = 'test_platform')");

useConfiguration("--platforms=//platforms:test_platform");
ruleBuilder().build();
scratch.file(
"foo/BUILD",
"load(':extension.bzl', 'my_rule')",
"my_rule(",
" name = 'my_skylark_rule',",
")");
assertNoEvents();

PlatformConfigurationApi platformConfiguration = fetchPlatformConfiguration();
assertThat(platformConfiguration).isNotNull();
assertThat(platformConfiguration.getTargetPlatform())
.isEqualTo(Label.parseAbsoluteUnchecked("//platforms:test_platform"));
assertThat(platformConfiguration.getTargetPlatforms())
.containsExactly(Label.parseAbsoluteUnchecked("//platforms:test_platform"));
}

@Test
public void testTargetPlatform_unset() {
assertThrows(InvalidConfigurationException.class, () -> useConfiguration("--platforms="));
}

@Test
public void testTargetPlatform_multiple() throws Exception {
scratch.file(
"platforms/BUILD",
"platform(name = 'test_platform1')",
"platform(name = 'test_platform2')");

useConfiguration("--platforms=//platforms:test_platform1,//platforms:test_platform2");
ruleBuilder().build();
scratch.file(
"foo/BUILD",
"load(':extension.bzl', 'my_rule')",
"my_rule(",
" name = 'my_skylark_rule',",
")");
assertNoEvents();

PlatformConfigurationApi platformConfiguration = fetchPlatformConfiguration();
assertThat(platformConfiguration).isNotNull();
// Despite setting two platforms in the flag, only a single platform should be visible to the
// target.
assertThat(platformConfiguration.getTargetPlatform())
.isEqualTo(Label.parseAbsoluteUnchecked("//platforms:test_platform1"));
assertThat(platformConfiguration.getTargetPlatforms())
.containsExactly(Label.parseAbsoluteUnchecked("//platforms:test_platform1"));
}

@Test
public void testEnabledToolchainTypes() throws Exception {
scratch.file(
"toolchains/BUILD",
"toolchain_type(name = 'test_toolchain_type1')",
"toolchain_type(name = 'test_toolchain_type2')",
"toolchain_type(name = 'test_toolchain_type3')");

useConfiguration(
"--enabled_toolchain_types="
+ "//toolchains:test_toolchain_type1,//toolchains:test_toolchain_type3");
ruleBuilder().build();
scratch.file(
"foo/BUILD",
"load(':extension.bzl', 'my_rule')",
"my_rule(",
" name = 'my_skylark_rule',",
")");
assertNoEvents();

PlatformConfigurationApi platformConfiguration = fetchPlatformConfiguration();
assertThat(platformConfiguration).isNotNull();
assertThat(platformConfiguration.getEnabledToolchainTypes())
.containsExactly(
Label.parseAbsoluteUnchecked("//toolchains:test_toolchain_type1"),
Label.parseAbsoluteUnchecked("//toolchains:test_toolchain_type3"));
}

private RuleBuilder ruleBuilder() {
return new RuleBuilder();
}

private class RuleBuilder {
private void build() throws Exception {
ImmutableList.Builder<String> lines = ImmutableList.builder();
lines.add(
"result = provider()",
"def _impl(ctx):",
" platformConfig = ctx.fragments.platform",
" return [result(property = platformConfig)]");
lines.add("my_rule = rule(", " implementation = _impl,", " fragments = ['platform'],", ")");

scratch.file("foo/extension.bzl", lines.build().toArray(new String[] {}));
}
}

private PlatformConfigurationApi fetchPlatformConfiguration() throws Exception {
ConfiguredTarget myRuleTarget = getConfiguredTarget("//foo:my_skylark_rule");
StructImpl info =
(StructImpl)
myRuleTarget.get(
new SkylarkKey(
Label.parseAbsolute("//foo:extension.bzl", ImmutableMap.of()), "result"));

@SuppressWarnings("unchecked")
PlatformConfigurationApi javaInfo = (PlatformConfigurationApi) info.getValue("property");
return javaInfo;
}
}

0 comments on commit 5b30745

Please sign in to comment.