diff options
8 files changed, 205 insertions, 54 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java index 2d4aafa515..332f4c17dc 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java @@ -109,9 +109,9 @@ public class BuildConfiguration implements BuildEvent { /** * An interface for language-specific configurations. * - * <p>All implementations must be immutable and communicate this as clearly as possible - * (e.g. declare {@link ImmutableList} signatures on their interfaces vs. {@link List}). - * This is because fragment instances may be shared across configurations. + * <p>All implementations must be immutable and communicate this as clearly as possible (e.g. + * declare {@link ImmutableList} signatures on their interfaces vs. {@link List}). This is because + * fragment instances may be shared across configurations. */ public abstract static class Fragment { /** @@ -953,9 +953,30 @@ public class BuildConfiguration implements BuildEvent { ) public Label autoCpuEnvironmentGroup; - /** - * Values for --experimental_dynamic_configs. - */ + /** The source of make variables for this configuration. */ + public enum MakeVariableSource { + CONFIGURATION, + TOOLCHAIN + } + + /** Converter for --make_variables_source. */ + public static class MakeVariableSourceConverter extends EnumConverter<MakeVariableSource> { + public MakeVariableSourceConverter() { + super(MakeVariableSource.class, "Make variable source"); + } + } + + @Option( + name = "make_variables_source", + converter = MakeVariableSourceConverter.class, + defaultValue = "configuration", + metadataTags = {OptionMetadataTag.HIDDEN}, + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.UNKNOWN} + ) + public MakeVariableSource makeVariableSource; + + /** Values for --experimental_dynamic_configs. */ public enum ConfigsMode { /** Only include the configuration fragments each rule needs. */ ON, diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/ToolchainInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/ToolchainInfo.java index 185d1a188c..0768a8b624 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/platform/ToolchainInfo.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/ToolchainInfo.java @@ -16,6 +16,7 @@ package com.google.devtools.build.lib.analysis.platform; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMap.Builder; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.packages.NativeInfo; @@ -78,4 +79,7 @@ public class ToolchainInfo extends NativeInfo { public static ToolchainInfo create(Map<String, Object> toolchainData, Location loc) { return new ToolchainInfo(toolchainData, loc); } + + /** Add make variables to be exported to dependers. */ + public void addGlobalMakeVariables(Builder<String, String> globalMakeEnvBuilder) {} } diff --git a/src/main/java/com/google/devtools/build/lib/rules/ToolchainType.java b/src/main/java/com/google/devtools/build/lib/rules/ToolchainType.java index 46d2828f9b..6ddc531fa0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/ToolchainType.java +++ b/src/main/java/com/google/devtools/build/lib/rules/ToolchainType.java @@ -1,4 +1,4 @@ -// Copyright 2016 The Bazel Authors. All rights reserved. +// Copyright 2017 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. @@ -17,14 +17,18 @@ package com.google.devtools.build.lib.rules; import com.google.common.base.Function; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.PlatformConfiguration; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.TemplateVariableInfo; +import com.google.devtools.build.lib.analysis.ToolchainContext.ResolvedToolchainProviders; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options.MakeVariableSource; import com.google.devtools.build.lib.cmdline.Label; import java.util.TreeMap; @@ -65,14 +69,29 @@ public class ToolchainType implements RuleConfiguredTargetFactory { // This cannot be an ImmutableMap.Builder because that asserts when a key is duplicated TreeMap<String, String> makeVariables = new TreeMap<>(); - Class<? extends BuildConfiguration.Fragment> fragmentClass = - fragmentMap.get(ruleContext.getLabel()); - if (fragmentClass != null) { - BuildConfiguration.Fragment fragment = ruleContext.getFragment(fragmentClass); - ImmutableMap.Builder<String, String> fragmentBuilder = ImmutableMap.builder(); - fragment.addGlobalMakeVariables(fragmentBuilder); - makeVariables.putAll(fragmentBuilder.build()); + ImmutableMap.Builder<String, String> fragmentBuilder = ImmutableMap.builder(); + + // If this toolchain type is enabled, we can derive make variables from it. Otherwise, we + // derive make variables from the corresponding configuration fragment. + if (ruleContext.getConfiguration().getOptions().get(Options.class).makeVariableSource + == MakeVariableSource.TOOLCHAIN + && ruleContext + .getFragment(PlatformConfiguration.class) + .getEnabledToolchainTypes() + .contains(ruleContext.getLabel())) { + ResolvedToolchainProviders providers = + (ResolvedToolchainProviders) + ruleContext.getToolchainContext().getResolvedToolchainProviders(); + providers.getForToolchainType(ruleContext.getLabel()).addGlobalMakeVariables(fragmentBuilder); + } else { + Class<? extends BuildConfiguration.Fragment> fragmentClass = + fragmentMap.get(ruleContext.getLabel()); + if (fragmentClass != null) { + BuildConfiguration.Fragment fragment = ruleContext.getFragment(fragmentClass); + fragment.addGlobalMakeVariables(fragmentBuilder); + } } + makeVariables.putAll(fragmentBuilder.build()); ImmutableMap<String, String> hardcodedVariables = hardcodedVariableMap.get(ruleContext.getLabel()); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index bb3b4abe7d..860983a233 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -414,6 +414,7 @@ public class CcToolchain implements RuleConfiguredTargetFactory { cppConfiguration, toolchain, toolchainInfo, + cppConfiguration.getCrosstoolTopPathFragment(), crosstool, fullInputsForCrosstool(ruleContext, crosstoolMiddleman), compile, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index e916f7c157..980d817a93 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java @@ -13,10 +13,13 @@ // limitations under the License. package com.google.devtools.build.lib.rules.cpp; +import com.google.common.base.Function; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMap.Builder; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.platform.ToolchainInfo; import com.google.devtools.build.lib.cmdline.Label; @@ -26,11 +29,13 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.Variables; +import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; +import java.util.Map; import javax.annotation.Nullable; /** Information about a C++ compiler used by the <code>cc_*</code> rules. */ @@ -46,6 +51,7 @@ public final class CcToolchainProvider extends ToolchainInfo { null, null, null, + null, NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), @@ -76,6 +82,7 @@ public final class CcToolchainProvider extends ToolchainInfo { @Nullable private final CppConfiguration cppConfiguration; private final CToolchain toolchain; private final CppToolchainInfo toolchainInfo; + private final PathFragment crosstoolTopPathFragment; private final NestedSet<Artifact> crosstool; private final NestedSet<Artifact> crosstoolMiddleman; private final NestedSet<Artifact> compile; @@ -108,6 +115,7 @@ public final class CcToolchainProvider extends ToolchainInfo { @Nullable CppConfiguration cppConfiguration, CToolchain toolchain, CppToolchainInfo toolchainInfo, + PathFragment crosstoolTopPathFragment, NestedSet<Artifact> crosstool, NestedSet<Artifact> crosstoolMiddleman, NestedSet<Artifact> compile, @@ -138,6 +146,7 @@ public final class CcToolchainProvider extends ToolchainInfo { this.cppConfiguration = cppConfiguration; this.toolchain = toolchain; this.toolchainInfo = toolchainInfo; + this.crosstoolTopPathFragment = crosstoolTopPathFragment; this.crosstool = Preconditions.checkNotNull(crosstool); this.crosstoolMiddleman = Preconditions.checkNotNull(crosstoolMiddleman); this.compile = Preconditions.checkNotNull(compile); @@ -166,6 +175,76 @@ public final class CcToolchainProvider extends ToolchainInfo { this.sysroot = sysroot; } + /** Returns c++ Make variables. */ + public static Map<String, String> getCppBuildVariables( + Function<Tool, PathFragment> getToolPathFragment, + String targetLibc, + String compiler, + String targetCpu, + PathFragment crosstoolTopPathFragment, + String abiGlibcVersion, + String abi, + Map<String, String> additionalMakeVariables) { + ImmutableMap.Builder<String, String> result = ImmutableMap.builder(); + + // hardcoded CC->gcc setting for unit tests + result.put("CC", getToolPathFragment.apply(Tool.GCC).getPathString()); + + // Make variables provided by crosstool/gcc compiler suite. + result.put("AR", getToolPathFragment.apply(Tool.AR).getPathString()); + result.put("NM", getToolPathFragment.apply(Tool.NM).getPathString()); + result.put("LD", getToolPathFragment.apply(Tool.LD).getPathString()); + PathFragment objcopyTool = getToolPathFragment.apply(Tool.OBJCOPY); + if (objcopyTool != null) { + // objcopy is optional in Crosstool + result.put("OBJCOPY", objcopyTool.getPathString()); + } + result.put("STRIP", getToolPathFragment.apply(Tool.STRIP).getPathString()); + + PathFragment gcovtool = getToolPathFragment.apply(Tool.GCOVTOOL); + if (gcovtool != null) { + // gcov-tool is optional in Crosstool + result.put("GCOVTOOL", gcovtool.getPathString()); + } + + if (targetLibc.startsWith("glibc-")) { + result.put("GLIBC_VERSION", targetLibc.substring("glibc-".length())); + } else { + result.put("GLIBC_VERSION", targetLibc); + } + + result.put("C_COMPILER", compiler); + result.put("TARGET_CPU", targetCpu); + + // Deprecated variables + + // TODO(bazel-team): delete all of these. + result.put("CROSSTOOLTOP", crosstoolTopPathFragment.getPathString()); + + // TODO(kmensah): Remove when skylark dependencies can be updated to rely on + // CcToolchainProvider. + result.putAll(additionalMakeVariables); + + result.put("ABI_GLIBC_VERSION", abiGlibcVersion); + result.put("ABI", abi); + + return result.build(); + } + + @Override + public void addGlobalMakeVariables(Builder<String, String> globalMakeEnvBuilder) { + globalMakeEnvBuilder.putAll( + getCppBuildVariables( + this::getToolPathFragment, + getTargetLibc(), + getCompiler(), + getTargetCpu(), + crosstoolTopPathFragment, + getAbiGlibcVersion(), + getAbi(), + getAdditionalMakeVariables())); + } + @SkylarkCallable( name = "built_in_include_directories", doc = "Returns the list of built-in directories of the compiler.", @@ -477,6 +556,36 @@ public final class CcToolchainProvider extends ToolchainInfo { return toolchainInfo.getDynamicRuntimeLibsLabel(); } + /** Returns the compiler version string (e.g. "gcc-4.1.1"). */ + @SkylarkCallable(name = "compiler", structField = true, doc = "C++ compiler.") + public String getCompiler() { + return toolchainInfo.getCompiler(); + } + + /** Returns the libc version string (e.g. "glibc-2.2.2"). */ + @SkylarkCallable(name = "libc", structField = true, doc = "libc version string.") + public String getTargetLibc() { + return toolchainInfo.getTargetLibc(); + } + + /** Returns the target architecture using blaze-specific constants (e.g. "piii"). */ + @SkylarkCallable(name = "cpu", structField = true, doc = "Target CPU of the C++ toolchain.") + public String getTargetCpu() { + return toolchainInfo.getTargetCpu(); + } + + /** + * Returns a map of additional make variables for use by {@link BuildConfiguration}. These are to + * used to allow some build rules to avoid the limits on stack frame sizes and variable-length + * arrays. + * + * <p>The returned map must contain an entry for {@code STACK_FRAME_UNLIMITED}, though the entry + * may be an empty string. + */ + public ImmutableMap<String, String> getAdditionalMakeVariables() { + return toolchainInfo.getAdditionalMakeVariables(); + } + @SkylarkCallable( name = "unfiltered_compiler_options_do_not_use", doc = diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index f07e43ae4b..85a90b5ae0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -26,6 +26,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ListMultimap; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Options.MakeVariableSource; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; @@ -258,6 +259,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment { private final CompilationMode compilationMode; private final boolean useLLVMCoverageMap; + private final boolean shouldProvideMakeVariables; + /** * If true, the ConfiguredTarget is only used to get the necessary cross-referenced * CppCompilationContexts, but registering build actions is disabled. @@ -283,6 +286,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment { this.crosstoolTopPathFragment = crosstoolTop.getPackageIdentifier().getPathUnderExecRoot(); this.cpuTransformer = params.cpuTransformer; this.cppToolchainInfo = new CppToolchainInfo(toolchain, crosstoolTopPathFragment, crosstoolTop); + this.shouldProvideMakeVariables = + params.commonOptions.makeVariableSource == MakeVariableSource.CONFIGURATION; // With LLVM, ThinLTO is automatically used in place of LIPO. ThinLTO works fine with dynamic // linking (and in fact creates a lot more work when dynamic linking is off). @@ -1393,47 +1398,19 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return; } - // hardcoded CC->gcc setting for unit tests - globalMakeEnvBuilder.put("CC", getToolPathFragment(Tool.GCC).getPathString()); - - // Make variables provided by crosstool/gcc compiler suite. - globalMakeEnvBuilder.put("AR", getToolPathFragment(Tool.AR).getPathString()); - globalMakeEnvBuilder.put("NM", getToolPathFragment(Tool.NM).getPathString()); - globalMakeEnvBuilder.put("LD", getToolPathFragment(Tool.LD).getPathString()); - PathFragment objcopyTool = getToolPathFragment(Tool.OBJCOPY); - if (objcopyTool != null) { - // objcopy is optional in Crosstool - globalMakeEnvBuilder.put("OBJCOPY", objcopyTool.getPathString()); - } - globalMakeEnvBuilder.put("STRIP", getToolPathFragment(Tool.STRIP).getPathString()); - - PathFragment gcovtool = getToolPathFragment(Tool.GCOVTOOL); - if (gcovtool != null) { - // gcov-tool is optional in Crosstool - globalMakeEnvBuilder.put("GCOVTOOL", gcovtool.getPathString()); - } - - if (getTargetLibc().startsWith("glibc-")) { - globalMakeEnvBuilder.put("GLIBC_VERSION", - getTargetLibc().substring("glibc-".length())); - } else { - globalMakeEnvBuilder.put("GLIBC_VERSION", getTargetLibc()); + if (!shouldProvideMakeVariables) { + return; } - - globalMakeEnvBuilder.put("C_COMPILER", getCompiler()); - globalMakeEnvBuilder.put("TARGET_CPU", getTargetCpu()); - - // Deprecated variables - - // TODO(bazel-team): delete all of these. - globalMakeEnvBuilder.put("CROSSTOOLTOP", crosstoolTopPathFragment.getPathString()); - - // TODO(kmensah): Remove when skylark dependencies can be updated to rely on - // CcToolchainProvider. - globalMakeEnvBuilder.putAll(getAdditionalMakeVariables()); - - globalMakeEnvBuilder.put("ABI_GLIBC_VERSION", cppToolchainInfo.getAbiGlibcVersion()); - globalMakeEnvBuilder.put("ABI", cppToolchainInfo.getAbi()); + globalMakeEnvBuilder.putAll( + CcToolchainProvider.getCppBuildVariables( + this::getToolPathFragment, + getTargetLibc(), + getCompiler(), + getTargetCpu(), + crosstoolTopPathFragment, + cppToolchainInfo.getAbiGlibcVersion(), + cppToolchainInfo.getAbi(), + getAdditionalMakeVariables())); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java b/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java index 9b6cc4995c..e3e4fbad15 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/ToolchainTypeTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.TemplateVariableInfo; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.packages.util.MockPlatformSupport; import com.google.devtools.build.lib.testutil.TestConstants; import org.junit.Test; import org.junit.runner.RunWith; @@ -41,4 +42,21 @@ public class ToolchainTypeTest extends BuildViewTestCase { getConfiguredTarget(TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type"); assertThat(cc.get(TemplateVariableInfo.PROVIDER).getVariables()).doesNotContainKey("JAVABASE"); } + + @Test + public void testMakeVariablesFromToolchain() throws Exception { + MockPlatformSupport.addMockPiiiPlatform( + mockToolsConfig, analysisMock.ccSupport().getMockCrosstoolLabel()); + useConfiguration( + "--enabled_toolchain_types=" + + TestConstants.TOOLS_REPOSITORY + + "//tools/cpp:toolchain_type", + "--experimental_platforms=//mock_platform:mock-piii-platform", + "--extra_toolchains=//mock_platform:toolchain_cc-compiler-piii", + "--make_variables_source=toolchain"); + ConfiguredTarget cc = + getConfiguredTarget(TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type"); + assertThat(cc.get(TemplateVariableInfo.PROVIDER).getVariables()) + .containsEntry("TARGET_CPU", "piii"); + } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java index 87eb6837e3..a899f05b75 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java @@ -40,6 +40,7 @@ public class CcToolchainProviderTest { null, null, null, + null, NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), @@ -73,6 +74,7 @@ public class CcToolchainProviderTest { null, null, null, + null, NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), NestedSetBuilder.<Artifact>emptySet(Order.STABLE_ORDER), |