Skip to content

Commit

Permalink
Rename ToolchainLookup rule to ToolchainType, to better explain the u…
Browse files Browse the repository at this point in the history
…sage of

the rule.

Example:
tools/newlang/BUILD:
  toolchain_type(name = "toolchain_type")
Allows users to refer to //tools/newlang:toolchain_type when discussing the
newlang toolchain.

The previous usage:
tools/newlang/BUILD:
  toolchain_lookup(name = "lookup")
Lead to labels like //tools/newlang:lookup, which was unclear that each
language's toolchain needs one unique instance of toolchain_(lookup|type).

--
PiperOrigin-RevId: 151326827
MOS_MIGRATED_REVID=151326827
  • Loading branch information
katre authored and philwo committed Mar 28, 2017
1 parent 6b60718 commit 34efd79
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.ConfigRuleClasses;
import com.google.devtools.build.lib.analysis.constraints.EnvironmentRule;
import com.google.devtools.build.lib.bazel.rules.BazelToolchainLookup.BazelToolchainLookupRule;
import com.google.devtools.build.lib.bazel.rules.BazelToolchainType.BazelToolchainTypeRule;
import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryRule;
import com.google.devtools.build.lib.bazel.rules.android.AndroidSdkRepositoryRule;
import com.google.devtools.build.lib.bazel.rules.android.BazelAarImportRule;
Expand Down Expand Up @@ -297,7 +297,7 @@ public static void setup(ConfiguredRuleClassProvider.Builder builder) {

// These rules are a little special: they need to depend on every configuration fragment that
// has Make variables, so we can't put them in any of the above buckets.
builder.addRuleDefinition(new BazelToolchainLookupRule());
builder.addRuleDefinition(new BazelToolchainTypeRule());
builder.addRuleDefinition(new GenRuleBaseRule());
builder.addRuleDefinition(new BazelGenRuleRule());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,25 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.rules.ToolchainLookup;
import com.google.devtools.build.lib.rules.ToolchainType;
import com.google.devtools.build.lib.rules.android.AndroidConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.java.Jvm;

/**
* Implementation of {@code toolchain_lookup}.
* Implementation of {@code toolchain_type}.
*/
public class BazelToolchainLookup extends ToolchainLookup {
public class BazelToolchainType extends ToolchainType {

/**
* Definition for {@code toolchain_lookup}.
* Definition for {@code toolchain_type}.
*/
public static class BazelToolchainLookupRule implements RuleDefinition {
public static class BazelToolchainTypeRule implements RuleDefinition {

@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
return builder
// This means that *every* toolchain_lookup rule depends on every configuration fragment
// This means that *every* toolchain_type rule depends on every configuration fragment
// that contributes Make variables, regardless of which one it is.
.requiresConfigurationFragments(
CppConfiguration.class, Jvm.class, AndroidConfiguration.class)
Expand All @@ -51,21 +51,21 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
@Override
public Metadata getMetadata() {
return Metadata.builder()
.name("toolchain_lookup")
.factoryClass(BazelToolchainLookup.class)
.name("toolchain_type")
.factoryClass(BazelToolchainType.class)
.ancestors(BaseRuleClasses.BaseRule.class)
.build();
}
}

public BazelToolchainLookup() {
public BazelToolchainType() {
super(
ImmutableMap.<Label, Class<? extends BuildConfiguration.Fragment>>builder()
.put(Label.parseAbsoluteUnchecked("@bazel_tools//tools/cpp:lookup"),
.put(Label.parseAbsoluteUnchecked("@bazel_tools//tools/cpp:toolchain_type"),
CppConfiguration.class)
.put(Label.parseAbsoluteUnchecked("@bazel_tools//tools/jdk:lookup"),
.put(Label.parseAbsoluteUnchecked("@bazel_tools//tools/jdk:toolchain_type"),
Jvm.class)
.put(Label.parseAbsoluteUnchecked("@bazel_tools//tools/android:lookup"),
.put(Label.parseAbsoluteUnchecked("@bazel_tools//tools/android:toolchain_type"),
AndroidConfiguration.class)
.build(),
ImmutableMap.<Label, ImmutableMap<String, String>>of());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@
import java.util.TreeMap;

/**
* Abstract base class for {@code toolchain_lookup}.
* Abstract base class for {@code toolchain_type}.
*/
public class ToolchainLookup implements RuleConfiguredTargetFactory {
public class ToolchainType implements RuleConfiguredTargetFactory {
private final ImmutableMap<Label, Class<? extends BuildConfiguration.Fragment>> fragmentMap;
private final ImmutableMap<Label, ImmutableMap<String, String>> hardcodedVariableMap;

protected ToolchainLookup(
protected ToolchainType(
ImmutableMap<Label, Class<? extends BuildConfiguration.Fragment>> fragmentMap,
ImmutableMap<Label, ImmutableMap<String, String>> hardcodedVariableMap) {
this.fragmentMap = fragmentMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void setup(MockToolsConfig config) throws IOException {
config.create(
"/bazel_tools_workspace/tools/cpp/BUILD",
"package(default_visibility=['//visibility:public'])",
"toolchain_lookup(name = 'lookup')",
"toolchain_type(name = 'toolchain_type')",
"cc_library(name = 'stl')",
"cc_library(name = 'malloc')",
"cc_toolchain_suite(",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,7 @@ protected static void createToolsCppPackage(MockToolsConfig config) throws IOExc
config.create(
"tools/cpp/BUILD",
"package(default_visibility = ['//visibility:public'])",
"toolchain_lookup(name = 'lookup')",
"toolchain_type(name = 'toolchain_type')",
"cc_library(name = 'stl')",
"alias(name='toolchain', actual='//third_party/crosstool')",
"cc_library(name = 'malloc')",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@
import org.junit.Test;

/**
* Unit tests for the {@code toolchain_lookup} rule.
* Unit tests for the {@code toolchain_type} rule.
*/
public class ToolchainLookupTest extends BuildViewTestCase {
public class ToolchainTypeTest extends BuildViewTestCase {
@Test
public void testSmoke() throws Exception {
ConfiguredTarget cc = getConfiguredTarget(getRuleClassProvider().getToolsRepository()
+ "//tools/cpp:lookup");
+ "//tools/cpp:toolchain_type");
assertThat(cc.getProvider(ToolchainProvider.class).getMakeVariables())
.containsKey("TARGET_CPU");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,13 @@
import com.google.devtools.build.lib.analysis.util.AnalysisMock;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.bazel.rules.BazelRuleClassProvider;
import com.google.devtools.build.lib.bazel.rules.BazelToolchainLookup;
import com.google.devtools.build.lib.bazel.rules.BazelToolchainType;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.flags.InvocationPolicyEnforcer;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.rules.ToolchainLookup;
import com.google.devtools.build.lib.rules.ToolchainType;
import com.google.devtools.build.lib.testutil.MoreAsserts;
import com.google.devtools.build.lib.util.FileType;
import com.google.devtools.build.lib.util.OsUtils;
Expand Down Expand Up @@ -905,24 +905,24 @@ public void testSymlinkActionIsNotRegisteredWhenIncludePrefixDoesntChangePath()
}

/**
* A {@code toolchain_lookup} rule for testing that only supports C++.
* A {@code toolchain_type} rule for testing that only supports C++.
*/
public static class OnlyCppToolchainLookup extends ToolchainLookup {
public OnlyCppToolchainLookup() {
public static class OnlyCppToolchainType extends ToolchainType {
public OnlyCppToolchainType() {
super(
ImmutableMap.<Label, Class<? extends BuildConfiguration.Fragment>>of(),
ImmutableMap.<Label, ImmutableMap<String, String>>of());
}
}

/**
* A {@code toolchain_lookup} rule for testing that only supports C++.
* A {@code toolchain_type} rule for testing that only supports C++.
*/
public static class OnlyCppToolchainLookupRule implements RuleDefinition {
public static class OnlyCppToolchainTypeRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) {
return builder
// This means that *every* toolchain_lookup rule depends on every configuration fragment
// This means that *every* toolchain_type rule depends on every configuration fragment
// that contributes Make variables, regardless of which one it is.
.requiresConfigurationFragments(CppConfiguration.class)
.removeAttribute("licenses")
Expand All @@ -933,8 +933,8 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
@Override
public Metadata getMetadata() {
return Metadata.builder()
.name("toolchain_lookup")
.factoryClass(BazelToolchainLookup.class)
.name("toolchain_type")
.factoryClass(BazelToolchainType.class)
.ancestors(BaseRuleClasses.BaseRule.class)
.build();
}
Expand Down Expand Up @@ -966,7 +966,7 @@ public ConfiguredRuleClassProvider createRuleClassProvider() {
BazelRuleClassProvider.CORE_WORKSPACE_RULES.init(builder);
BazelRuleClassProvider.BASIC_RULES.init(builder);
BazelRuleClassProvider.CPP_RULES.init(builder);
builder.addRuleDefinition(new OnlyCppToolchainLookupRule());
builder.addRuleDefinition(new OnlyCppToolchainTypeRule());
return builder.build();
}

Expand Down

0 comments on commit 34efd79

Please sign in to comment.