Skip to content

Commit

Permalink
Flip Starlark implementation of cc_toolchain and `apple_cc_toolch…
Browse files Browse the repository at this point in the history
…ain` rules.

PiperOrigin-RevId: 547510311
Change-Id: I52eae04c2d797803d56c330797191cceadf72901
  • Loading branch information
buildbreaker2021 authored and copybara-github committed Jul 12, 2023
1 parent c775c0d commit 50a1afb
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.rules.cpp;

import static com.google.devtools.build.lib.packages.BuildType.NODEP_LABEL;
import static com.google.devtools.build.lib.packages.Type.BOOLEAN;

import com.google.common.base.Preconditions;
Expand Down Expand Up @@ -67,7 +66,6 @@ public class CcToolchainAttributesProvider extends NativeInfo implements HasCcTo
private final NestedSet<Artifact> arFiles;
private final NestedSet<Artifact> linkerFiles;
private final NestedSet<Artifact> dwpFiles;
private final Label libcTopAttribute;
private final NestedSet<Artifact> libc;
private final TransitiveInfoCollection libcTop;
private final NestedSet<Artifact> targetLibc;
Expand Down Expand Up @@ -105,6 +103,8 @@ public class CcToolchainAttributesProvider extends NativeInfo implements HasCcTo
private final PackageSpecificationProvider allowlistForLayeringCheck;
private final PackageSpecificationProvider allowlistForLooseHeaderCheck;
private final StarlarkFunction ccToolchainBuildVariablesFunc;
private final String lateBoundLibc;
private final String lateBoundTargetLibc;

public CcToolchainAttributesProvider(
RuleContext ruleContext,
Expand Down Expand Up @@ -141,13 +141,14 @@ public CcToolchainAttributesProvider(
this.linkerFiles = getFiles(ruleContext, "linker_files");
this.dwpFiles = getFiles(ruleContext, "dwp_files");

this.libc = getOptionalFiles(ruleContext, CcToolchainRule.LIBC_TOP_ATTR);
this.libcTop = ruleContext.getPrerequisite(CcToolchainRule.LIBC_TOP_ATTR);
this.lateBoundLibc = getLateBoundLibc(ruleContext, "libc_top", ":libc_top");
this.lateBoundTargetLibc = getLateBoundLibc(ruleContext, "libc_top", ":target_libc_top");

this.targetLibc = getOptionalFiles(ruleContext, CcToolchainRule.TARGET_LIBC_TOP_ATTR);
this.targetLibcTop = ruleContext.getPrerequisite(CcToolchainRule.TARGET_LIBC_TOP_ATTR);
this.libc = getOptionalFiles(ruleContext, lateBoundLibc);
this.libcTop = ruleContext.getPrerequisite(lateBoundLibc);

this.libcTopAttribute = ruleContext.attributes().get("libc_top", BuildType.LABEL);
this.targetLibc = getOptionalFiles(ruleContext, lateBoundTargetLibc);
this.targetLibcTop = ruleContext.getPrerequisite(lateBoundTargetLibc);

this.fullInputsForCrosstool =
NestedSetBuilder.<Artifact>stableOrder()
Expand Down Expand Up @@ -180,8 +181,7 @@ public CcToolchainAttributesProvider(
this.propellerOptimize =
ruleContext.getPrerequisite(":propeller_optimize", PropellerOptimizeProvider.PROVIDER);
this.memprofProfileProvider =
ruleContext.getPrerequisite(
CcToolchainRule.MEMPROF_PROFILE_ATTR, MemProfProfileProvider.PROVIDER);
ruleContext.getPrerequisite(":memprof_profile", MemProfProfileProvider.PROVIDER);
this.moduleMap = ruleContext.getPrerequisite("module_map");
this.moduleMapArtifact = ruleContext.getPrerequisiteArtifact("module_map");
this.zipper = ruleContext.getPrerequisiteArtifact(":zipper");
Expand Down Expand Up @@ -217,7 +217,9 @@ public CcToolchainAttributesProvider(
// TODO(b/65835260): Remove this conditional once j2objc can learn the toolchain type.
if (ruleContext.attributes().has(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME)) {
this.toolchainType =
ruleContext.attributes().get(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME, NODEP_LABEL);
ruleContext
.attributes()
.get(CcToolchain.CC_TOOLCHAIN_TYPE_ATTRIBUTE_NAME, BuildType.LABEL);
} else {
this.toolchainType = null;
}
Expand All @@ -230,6 +232,19 @@ public CcToolchainAttributesProvider(
this.ccToolchainBuildVariablesFunc = ccToolchainBuildVariablesFunc;
}

// This is to avoid Starlark limitation of not being able to have complex logic in configuration
// field. The logic here was encapsulated in native cc_toolchain rule's :libc_top's and
// :target_libc_top's LateBoundDefault attributes.
// In case :libc_top or :target_libc_top were not specified from command line, i.e. grte_top was
// not set we will try to use public attributes instead.
private static String getLateBoundLibc(
RuleContext ruleContext, String attribute, String implicitAttribute) {
if (ruleContext.getPrerequisite(implicitAttribute) == null) {
return attribute;
}
return implicitAttribute;
}

@Override
public BuiltinProvider<CcToolchainAttributesProvider> getProvider() {
return PROVIDER;
Expand Down Expand Up @@ -543,10 +558,6 @@ public Label getTargetLibcTopLabel() {
return getTargetLibcTop() == null ? null : getTargetLibcTop().getLabel();
}

public Label getLibcTopAttribute() {
return libcTopAttribute;
}

public String getCompiler() {
return compiler;
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/starlark/builtins_bzl/common/exports.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ exported_rules = {
"proto_lang_toolchain": proto_lang_toolchain,
"+py_runtime": py_runtime,
"+cc_toolchain_alias": cc_toolchain_alias,
"-cc_toolchain": cc_toolchain,
"-apple_cc_toolchain": apple_cc_toolchain,
"+cc_toolchain": cc_toolchain,
"+apple_cc_toolchain": apple_cc_toolchain,
}

# A list of Starlark functions callable from native rules implementation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,10 +474,10 @@ public void testXFdoOptimizeNotProvider() throws Exception {
scratch.file(
"a/BUILD",
"cc_toolchain_alias(name = 'b')",
"fdo_profile(name='out.xfdo', profile='profile.xfdo')");
"genrule(name='profile.xfdo', outs = ['c.txt'], cmd = '')");
useConfiguration("-c", "opt", "--xbinary_fdo=//a:profile.xfdo");
assertThat(getConfiguredTarget("//a:b")).isNull();
assertContainsEvent("--fdo_profile/--xbinary_fdo input needs to be an fdo_profile rule");
assertContainsEvent("does not have mandatory providers: 'FdoProfileInfo'");
}

@Test
Expand Down

0 comments on commit 50a1afb

Please sign in to comment.