From c1b4912c4092dae9510be243b280ecb8c2c90284 Mon Sep 17 00:00:00 2001 From: hlopko Date: Fri, 28 Dec 2018 00:26:51 -0800 Subject: [PATCH] Add --incompatible_disable_legacy_crosstool_fields Incompatible flag issue: https://github.com/bazelbuild/bazel/issues/6861 Tracking issue: https://github.com/bazelbuild/bazel/issues/5883 RELNOTES: Added --incompatible_disable_legacy_crosstool_fields. See the migration notes at https://github.com/bazelbuild/bazel/issues/6861. PiperOrigin-RevId: 227104932 --- .../lib/rules/cpp/CcToolchainConfigInfo.java | 106 ++++++- .../rules/cpp/CcToolchainProviderHelper.java | 7 +- .../build/lib/rules/cpp/CppConfiguration.java | 12 +- .../build/lib/rules/cpp/CppOptions.java | 29 +- .../build/lib/rules/cpp/CppToolchainInfo.java | 56 ++-- .../cpp/CcLibraryConfiguredTargetTest.java | 6 +- .../rules/cpp/CcToolchainFeaturesTest.java | 283 +++++++++++------- .../rules/cpp/CcToolchainProviderTest.java | 63 ++-- .../lib/rules/cpp/CompileCommandLineTest.java | 18 +- .../lib/rules/cpp/CppLinkActionTest.java | 94 ++++-- .../cpp/CrosstoolConfigurationHelper.java | 6 +- .../lib/rules/cpp/LinkBuildVariablesTest.java | 84 +++++- .../rules/cpp/LinkBuildVariablesTestCase.java | 20 +- .../rules/objc/ObjcBuildVariablesTest.java | 44 ++- 14 files changed, 564 insertions(+), 264 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java index e2c38352f08319..54b9c45b789a78 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainConfigInfo.java @@ -14,9 +14,12 @@ package com.google.devtools.build.lib.rules.cpp; +import static com.google.devtools.build.lib.rules.cpp.CppConfiguration.getLegacyCrosstoolFieldErrorMessage; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.config.CompilationMode; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.packages.NativeInfo; @@ -181,8 +184,10 @@ public CcToolchainConfigInfo( this.proto = proto; } - public static CcToolchainConfigInfo fromToolchain(CToolchain toolchain) throws EvalException { - + public static CcToolchainConfigInfo fromToolchain(RuleContext ruleContext, CToolchain toolchain) + throws EvalException { + boolean disableLegacyCrosstoolFields = + ruleContext.getFragment(CppConfiguration.class).disableLegacyCrosstoolFields(); ImmutableList.Builder actionConfigBuilder = ImmutableList.builder(); for (CToolchain.ActionConfig actionConfig : toolchain.getActionConfigList()) { actionConfigBuilder.add(new ActionConfig(actionConfig)); @@ -214,6 +219,103 @@ public static CcToolchainConfigInfo fromToolchain(CToolchain toolchain) throws E ImmutableList.Builder dynamicLinkerFlags = ImmutableList.builder(); ImmutableList.Builder mostlyStaticLibrariesLinkerFlags = ImmutableList.builder(); + if (disableLegacyCrosstoolFields) { + if (toolchain.getCompilationModeFlagsCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("compilation_mode_flags")); + } + if (toolchain.getLinkingModeFlagsCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("linking_mode_flags")); + } + if (toolchain.getArFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("ar_flag")); + } + if (toolchain.getArThinArchivesFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("ar_thin_archives_flag")); + } + if (toolchain.getCompilerFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("compiler_flag")); + } + if (toolchain.getCxxFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("cxx_flag")); + } + if (toolchain.getDebianExtraRequiresCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("debian_extra_requires")); + } + if (toolchain.hasDefaultPythonTop()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("default_python_top")); + } + if (toolchain.hasDefaultPythonVersion()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("default_python_version")); + } + if (toolchain.getDynamicLibraryLinkerFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("dynamic_library_linker_flag")); + } + if (toolchain.getGccPluginCompilerFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("gcc_plugin_compiler_flag")); + } + if (toolchain.getGccPluginHeaderDirectoryCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("gcc_plugin_header_directory")); + } + if (toolchain.getLdEmbedFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("ld_embed_flag")); + } + if (toolchain.getLinkerFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("linker_flag")); + } + if (toolchain.getMaoPluginHeaderDirectoryCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("mao_plugin_header_directory")); + } + if (toolchain.hasNeedsPic()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("needsPic")); + } + if (toolchain.getObjcopyEmbedFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("objcopy_embed_flag")); + } + if (toolchain.hasPythonPreloadSwigdeps()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("python_preload_swigdeps")); + } + if (toolchain.hasStaticRuntimesFilegroup()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("static_runtimes_filegroup")); + } + if (toolchain.hasDynamicRuntimesFilegroup()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("dynamic_runtimes_filegroup")); + } + if (toolchain.hasSupportsDsym()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("supports_dsym")); + } + if (toolchain.hasSupportsEmbeddedRuntimes()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("supports_embedded_runtimes")); + } + if (toolchain.hasSupportsFission()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("supports_fission")); + } + if (toolchain.hasSupportsGoldLinker()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("supports_gold_linker")); + } + if (toolchain.hasSupportsIncrementalLinker()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("supports_incremental_linker")); + } + if (toolchain.hasSupportsInterfaceSharedObjects()) { + ruleContext.ruleError( + getLegacyCrosstoolFieldErrorMessage("supports_interface_shared_objects")); + } + if (toolchain.hasSupportsNormalizingAr()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("supports_normalizing_ar")); + } + if (toolchain.hasSupportsStartEndLib()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("supports_start_end_lib")); + } + if (toolchain.hasSupportsThinArchives()) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("supports_thin_archives")); + } + if (toolchain.getTestOnlyLinkerFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("test_only_linker_flag")); + } + if (toolchain.getUnfilteredCxxFlagCount() != 0) { + ruleContext.ruleError(getLegacyCrosstoolFieldErrorMessage("unfiltered_cxx_flag")); + } + } + for (CompilationModeFlags flag : toolchain.getCompilationModeFlagsList()) { switch (flag.getMode()) { case OPT: diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java index c8e9fd328c16f1..50607bc38e1ef5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderHelper.java @@ -722,8 +722,6 @@ private static CppToolchainInfo getCppToolchainInfo( ruleContext.getLabel(), configInfo, cppConfiguration.disableLegacyCrosstoolFields(), - cppConfiguration.disableCompilationModeFlags(), - cppConfiguration.disableLinkingModeFlags(), cppConfiguration.disableGenruleCcToolchainDependency()); } catch (EvalException e) { throw ruleContext.throwWithRuleError(e.getMessage()); @@ -748,13 +746,12 @@ private static CppToolchainInfo getCppToolchainInfo( toolchain = CppToolchainInfo.addLegacyFeatures( toolchain, CppToolchainInfo.getToolsDirectory(attributes.getCcToolchainLabel())); - CcToolchainConfigInfo ccToolchainConfigInfo = CcToolchainConfigInfo.fromToolchain(toolchain); + CcToolchainConfigInfo ccToolchainConfigInfo = + CcToolchainConfigInfo.fromToolchain(ruleContext, toolchain); return CppToolchainInfo.create( attributes.getCcToolchainLabel(), ccToolchainConfigInfo, cppConfiguration.disableLegacyCrosstoolFields(), - cppConfiguration.disableCompilationModeFlags(), - cppConfiguration.disableLinkingModeFlags(), cppConfiguration.disableGenruleCcToolchainDependency()); } catch (EvalException e) { throw ruleContext.throwWithRuleError(e.getMessage()); 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 7e66da91885dea..383ce7e7990b5d 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 @@ -548,12 +548,12 @@ public boolean disableLegacyCrosstoolFields() { return cppOptions.disableLegacyCrosstoolFields; } - public boolean disableCompilationModeFlags() { - return cppOptions.disableCompilationModeFlags; - } - - public boolean disableLinkingModeFlags() { - return cppOptions.disableLinkingModeFlags; + public static String getLegacyCrosstoolFieldErrorMessage(String field) { + Preconditions.checkNotNull(field); + return field + + " is disabled by --incompatible_disable_legacy_crosstool_fields, please " + + "migrate your CROSSTOOL (see https://github.com/bazelbuild/bazel/issues/6861 for " + + "migration instructions)."; } public boolean enableLinkoptsInUserLinkFlags() { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java index 22130d8c084eae..2c53688a795e38 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java @@ -685,31 +685,18 @@ public Label getFdoPrefetchHintsLabel() { public boolean useLLVMCoverageMapFormat; @Option( - name = "experimental_disable_linking_mode_flags", + name = "incompatible_disable_legacy_crosstool_fields", + oldName = "experimental_disable_legacy_crosstool_fields", defaultValue = "false", documentationCategory = OptionDocumentationCategory.TOOLCHAIN, effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - metadataTags = {OptionMetadataTag.EXPERIMENTAL}, - help = "If true, Bazel will not read crosstool flags from linking_mode_flags field.") - public boolean disableLinkingModeFlags; - - @Option( - name = "experimental_disable_compilation_mode_flags", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.TOOLCHAIN, - effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - metadataTags = {OptionMetadataTag.EXPERIMENTAL}, - help = "If true, Bazel will not read crosstool flags from compilation_mode_flags field.") - public boolean disableCompilationModeFlags; - - @Option( - name = "experimental_disable_legacy_crosstool_fields", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.TOOLCHAIN, - effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, - metadataTags = {OptionMetadataTag.EXPERIMENTAL}, + metadataTags = { + OptionMetadataTag.INCOMPATIBLE_CHANGE, + OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES + }, help = - "If true, Bazel will not read crosstool flags from legacy crosstool fields (see #5187).") + "If true, Bazel will not read crosstool flags from legacy crosstool fields " + + "(see https://github.com/bazelbuild/bazel/issues/6861 for migration instructions).") public boolean disableLegacyCrosstoolFields; @Option( diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java index 3181b1e4466462..c45605678a2079 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java @@ -115,8 +115,6 @@ public static CppToolchainInfo create( Label toolchainLabel, CcToolchainConfigInfo ccToolchainConfigInfo, boolean disableLegacyCrosstoolFields, - boolean disableCompilationModeFlags, - boolean disableLinkingModeFlags, boolean disableGenruleCcToolchainDependency) throws EvalException { ImmutableMap toolPaths = @@ -128,7 +126,7 @@ public static CppToolchainInfo create( ImmutableListMultimap.builder(); boolean haveDynamicMode = false; - if (!disableLinkingModeFlags) { + if (!disableLegacyCrosstoolFields) { // If a toolchain supports dynamic libraries at all, there must be at least one // of the following: // - a "DYNAMIC" section in linking_mode_flags (even if no flags are needed) @@ -153,7 +151,7 @@ public static CppToolchainInfo create( ImmutableListMultimap.Builder linkOptionsFromCompilationModeBuilder = ImmutableListMultimap.builder(); - if (!disableCompilationModeFlags) { + if (!disableLegacyCrosstoolFields) { cFlagsBuilder.putAll( importCompilationMode(CrosstoolConfig.CompilationMode.OPT), ccToolchainConfigInfo.getOptCompilationModeCompilerFlags()); @@ -212,19 +210,27 @@ public static CppToolchainInfo create( disableLegacyCrosstoolFields ? ImmutableList.of() : ccToolchainConfigInfo.getTestOnlyLinkerFlags(), - ccToolchainConfigInfo.getLdEmbedFlags(), - ccToolchainConfigInfo.getObjcopyEmbedFlags(), + disableLegacyCrosstoolFields + ? ImmutableList.of() + : ccToolchainConfigInfo.getLdEmbedFlags(), + disableLegacyCrosstoolFields + ? ImmutableList.of() + : ccToolchainConfigInfo.getObjcopyEmbedFlags(), toolchainLabel, - toolchainLabel.getRelativeWithRemapping( - !ccToolchainConfigInfo.getStaticRuntimesFilegroup().isEmpty() - ? ccToolchainConfigInfo.getStaticRuntimesFilegroup() - : "static-runtime-libs-" + ccToolchainConfigInfo.getTargetCpu(), - ImmutableMap.of()), - toolchainLabel.getRelativeWithRemapping( - !ccToolchainConfigInfo.getDynamicRuntimesFilegroup().isEmpty() - ? ccToolchainConfigInfo.getDynamicRuntimesFilegroup() - : "dynamic-runtime-libs-" + ccToolchainConfigInfo.getTargetCpu(), - ImmutableMap.of()), + disableLegacyCrosstoolFields + ? null + : toolchainLabel.getRelativeWithRemapping( + !ccToolchainConfigInfo.getStaticRuntimesFilegroup().isEmpty() + ? ccToolchainConfigInfo.getStaticRuntimesFilegroup() + : "static-runtime-libs-" + ccToolchainConfigInfo.getTargetCpu(), + ImmutableMap.of()), + disableLegacyCrosstoolFields + ? null + : toolchainLabel.getRelativeWithRemapping( + !ccToolchainConfigInfo.getDynamicRuntimesFilegroup().isEmpty() + ? ccToolchainConfigInfo.getDynamicRuntimesFilegroup() + : "dynamic-runtime-libs-" + ccToolchainConfigInfo.getTargetCpu(), + ImmutableMap.of()), "_solib_" + ccToolchainConfigInfo.getTargetCpu(), ccToolchainConfigInfo.getAbiVersion(), ccToolchainConfigInfo.getTargetSystemName(), @@ -240,13 +246,17 @@ public static CppToolchainInfo create( disableLegacyCrosstoolFields ? ImmutableList.of() : ccToolchainConfigInfo.getUnfilteredCxxFlags(), - ccToolchainConfigInfo.supportsFission(), - ccToolchainConfigInfo.supportsStartEndLib(), - ccToolchainConfigInfo.supportsEmbeddedRuntimes(), - haveDynamicMode || !ccToolchainConfigInfo.getDynamicLibraryLinkerFlags().isEmpty(), - ccToolchainConfigInfo.supportsInterfaceSharedLibraries(), - ccToolchainConfigInfo.supportsGoldLinker(), - ccToolchainConfigInfo.needsPic()); + disableLegacyCrosstoolFields ? false : ccToolchainConfigInfo.supportsFission(), + disableLegacyCrosstoolFields ? false : ccToolchainConfigInfo.supportsStartEndLib(), + disableLegacyCrosstoolFields ? false : ccToolchainConfigInfo.supportsEmbeddedRuntimes(), + disableLegacyCrosstoolFields + ? false + : haveDynamicMode || !ccToolchainConfigInfo.getDynamicLibraryLinkerFlags().isEmpty(), + disableLegacyCrosstoolFields + ? false + : ccToolchainConfigInfo.supportsInterfaceSharedLibraries(), + disableLegacyCrosstoolFields ? false : ccToolchainConfigInfo.supportsGoldLinker(), + disableLegacyCrosstoolFields ? false : ccToolchainConfigInfo.needsPic()); } catch (LabelSyntaxException e) { // All of the above label.getRelativeWithRemapping() calls are valid labels, and the // crosstool_top was already checked earlier in the process. diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java index 25bb98a3f5cf7a..5f5ccf95a128d1 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java @@ -222,10 +222,12 @@ public void testFilesToBuild() throws Exception { public void testFilesToBuildWithoutDSO() throws Exception { CrosstoolConfig.CrosstoolRelease.Builder release = CrosstoolConfig.CrosstoolRelease.newBuilder() .mergeFrom(CrosstoolConfigurationHelper.simpleCompleteToolchainProto()); - release.getToolchainBuilder(0) + release + .getToolchainBuilder(0) .setTargetCpu("k8") .setCompiler("compiler") - .clearLinkingModeFlags(); + // To remove "supports_dynamic_linker" feature + .clearFeature(); scratch.file("crosstool/BUILD", "cc_toolchain_suite(", diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java index ea233031ce38a2..c9f7baf7c0da60 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeaturesTest.java @@ -26,6 +26,8 @@ import com.google.common.collect.Multimap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ActionConfig; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.ExpansionException; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; @@ -40,7 +42,6 @@ import com.google.devtools.build.lib.skyframe.serialization.ObjectCodecs; import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester; import com.google.devtools.build.lib.syntax.EvalException; -import com.google.devtools.build.lib.testutil.FoundationTestCase; import com.google.devtools.build.lib.testutil.TestUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -51,13 +52,22 @@ import java.util.List; import java.util.Map; import java.util.Set; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; /** Tests for toolchain features. */ @RunWith(JUnit4.class) -public class CcToolchainFeaturesTest extends FoundationTestCase { +public class CcToolchainFeaturesTest extends BuildViewTestCase { + + private RuleContext ruleContext; + + @Before + public void setup() throws Exception { + scratch.file("foo/BUILD", "cc_library(name = 'foo')"); + ruleContext = getRuleContext(getConfiguredTarget("//foo:foo")); + } /** * Creates a {@code Variables} configuration from a list of key/value pairs. @@ -86,19 +96,19 @@ private CcToolchainVariables createVariables(String... entries) { return variables.build(); } - /** - * Creates a CcToolchainFeatures from features described in the given toolchain fragment. - */ - public static CcToolchainFeatures buildFeatures(String... toolchain) throws Exception { + /** Creates a CcToolchainFeatures from features described in the given toolchain fragment. */ + public static CcToolchainFeatures buildFeatures(RuleContext ruleContext, String... toolchain) + throws Exception { CToolchain.Builder toolchainBuilder = CToolchain.newBuilder(); TextFormat.merge(Joiner.on("").join(toolchain), toolchainBuilder); return new CcToolchainFeatures( - CcToolchainConfigInfo.fromToolchain(toolchainBuilder.buildPartial()), + CcToolchainConfigInfo.fromToolchain(ruleContext, toolchainBuilder.buildPartial()), PathFragment.create("crosstool/")); } /** Creates a CcToolchainFeatures from given features and action configs. */ public static CcToolchainFeatures buildFeatures( + RuleContext ruleContext, ImmutableList features, ImmutableList actionConfigs) throws Exception { @@ -106,16 +116,16 @@ public static CcToolchainFeatures buildFeatures( toolchainBuilder.addAllFeature(features); toolchainBuilder.addAllActionConfig(actionConfigs); return new CcToolchainFeatures( - CcToolchainConfigInfo.fromToolchain(toolchainBuilder.buildPartial()), + CcToolchainConfigInfo.fromToolchain(ruleContext, toolchainBuilder.buildPartial()), PathFragment.create("crosstool/")); } /** Creates an empty CcToolchainFeatures. */ - public static CcToolchainFeatures buildEmptyFeatures(String... toolchain) throws Exception { + public CcToolchainFeatures buildEmptyFeatures(String... toolchain) throws Exception { CToolchain.Builder toolchainBuilder = CToolchain.newBuilder(); TextFormat.merge(Joiner.on("").join(toolchain), toolchainBuilder); return new CcToolchainFeatures( - CcToolchainConfigInfo.fromToolchain(toolchainBuilder.buildPartial()), + CcToolchainConfigInfo.fromToolchain(ruleContext, toolchainBuilder.buildPartial()), PathFragment.EMPTY_FRAGMENT); } @@ -147,11 +157,13 @@ private Artifact scratchArtifact(String s) { public void testFeatureConfigurationCodec() throws Exception { FeatureConfiguration emptyConfiguration = buildEmptyFeatures("").getFeatureConfiguration(ImmutableSet.of()); + RuleContext ruleContext = getRuleContext(getConfiguredTarget("//foo:foo")); FeatureConfiguration emptyFeatures = - buildFeatures("feature {name: 'a'}", "feature {name: 'b'}") + buildFeatures(ruleContext, "feature {name: 'a'}", "feature {name: 'b'}") .getFeatureConfiguration(ImmutableSet.of("a", "b")); FeatureConfiguration featuresWithFlags = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -173,6 +185,7 @@ public void testFeatureConfigurationCodec() throws Exception { .getFeatureConfiguration(ImmutableSet.of("a", "b")); FeatureConfiguration featureWithEnvSet = buildFeatures( + ruleContext, "feature {", " name: 'a'", " env_set {", @@ -200,15 +213,19 @@ public void testCrosstoolProtoCanBeSerialized() throws Exception { @Test public void testUnconditionalFeature() throws Exception { - assertThat(buildFeatures("").getFeatureConfiguration(ImmutableSet.of("a")).isEnabled("a")) + RuleContext ruleContext = getRuleContext(getConfiguredTarget("//foo:foo")); + assertThat( + buildFeatures(ruleContext, "") + .getFeatureConfiguration(ImmutableSet.of("a")) + .isEnabled("a")) .isFalse(); assertThat( - buildFeatures("feature { name: 'a' }") + buildFeatures(ruleContext, "feature { name: 'a' }") .getFeatureConfiguration(ImmutableSet.of("b")) .isEnabled("a")) .isFalse(); assertThat( - buildFeatures("feature { name: 'a' }") + buildFeatures(ruleContext, "feature { name: 'a' }") .getFeatureConfiguration(ImmutableSet.of("a")) .isEnabled("a")) .isTrue(); @@ -217,7 +234,7 @@ public void testUnconditionalFeature() throws Exception { @Test public void testUnsupportedAction() throws Exception { FeatureConfiguration configuration = - buildFeatures("").getFeatureConfiguration(ImmutableSet.of()); + buildFeatures(ruleContext, "").getFeatureConfiguration(ImmutableSet.of()); assertThat(configuration.getCommandLine("invalid-action", createVariables())).isEmpty(); } @@ -225,6 +242,7 @@ public void testUnsupportedAction() throws Exception { public void testFlagOrderEqualsSpecOrder() throws Exception { FeatureConfiguration configuration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -257,6 +275,7 @@ public void testFlagOrderEqualsSpecOrder() throws Exception { public void testEnvVars() throws Exception { FeatureConfiguration configuration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " env_set {", @@ -332,6 +351,7 @@ private List getCommandLineForFlagGroups(String groups, CcToolchainVaria throws Exception { FeatureConfiguration configuration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -898,6 +918,7 @@ public void testFlagTreeVariableExpansion() throws Exception { try { buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -916,21 +937,25 @@ public void testFlagTreeVariableExpansion() throws Exception { @Test public void testImplies() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' implies: 'b' implies: 'c' }", - "feature { name: 'b' }", - "feature { name: 'c' implies: 'd' }", - "feature { name: 'd' }", - "feature { name: 'e' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' implies: 'b' implies: 'c' }", + "feature { name: 'b' }", + "feature { name: 'c' implies: 'd' }", + "feature { name: 'd' }", + "feature { name: 'e' }"); assertThat(getEnabledFeatures(features, "a")).containsExactly("a", "b", "c", "d"); } @Test public void testRequires() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' requires: { feature: 'b' } }", - "feature { name: 'b' requires: { feature: 'c' } }", - "feature { name: 'c' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' requires: { feature: 'b' } }", + "feature { name: 'b' requires: { feature: 'c' } }", + "feature { name: 'c' }"); assertThat(getEnabledFeatures(features, "a")).isEmpty(); assertThat(getEnabledFeatures(features, "a", "b")).isEmpty(); assertThat(getEnabledFeatures(features, "a", "c")).containsExactly("c"); @@ -939,40 +964,52 @@ public void testRequires() throws Exception { @Test public void testDisabledRequirementChain() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' }", - "feature { name: 'b' requires: { feature: 'c' } implies: 'a' }", - "feature { name: 'c' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' }", + "feature { name: 'b' requires: { feature: 'c' } implies: 'a' }", + "feature { name: 'c' }"); assertThat(getEnabledFeatures(features, "b")).isEmpty(); - features = buildFeatures( - "feature { name: 'a' }", - "feature { name: 'b' requires: { feature: 'a' } implies: 'c' }", - "feature { name: 'c' }", - "feature { name: 'd' requires: { feature: 'c' } implies: 'e' }", - "feature { name: 'e' }"); + features = + buildFeatures( + ruleContext, + "feature { name: 'a' }", + "feature { name: 'b' requires: { feature: 'a' } implies: 'c' }", + "feature { name: 'c' }", + "feature { name: 'd' requires: { feature: 'c' } implies: 'e' }", + "feature { name: 'e' }"); assertThat(getEnabledFeatures(features, "b", "d")).isEmpty(); } @Test public void testEnabledRequirementChain() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: '0' implies: 'a' }", - "feature { name: 'a' }", - "feature { name: 'b' requires: { feature: 'a' } implies: 'c' }", - "feature { name: 'c' }", - "feature { name: 'd' requires: { feature: 'c' } implies: 'e' }", - "feature { name: 'e' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: '0' implies: 'a' }", + "feature { name: 'a' }", + "feature { name: 'b' requires: { feature: 'a' } implies: 'c' }", + "feature { name: 'c' }", + "feature { name: 'd' requires: { feature: 'c' } implies: 'e' }", + "feature { name: 'e' }"); assertThat(getEnabledFeatures(features, "0", "b", "d")).containsExactly( "0", "a", "b", "c", "d", "e"); } @Test public void testLogicInRequirements() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' requires: { feature: 'b' feature: 'c' } requires: { feature: 'd' } }", - "feature { name: 'b' }", - "feature { name: 'c' }", - "feature { name: 'd' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature {", + " name: 'a'", + " requires: { feature: 'b' feature: 'c' }", + " requires: { feature: 'd' }", + "}", + "feature { name: 'b' }", + "feature { name: 'c' }", + "feature { name: 'd' }"); assertThat(getEnabledFeatures(features, "a", "b", "c")).containsExactly("a", "b", "c"); assertThat(getEnabledFeatures(features, "a", "b")).containsExactly("b"); assertThat(getEnabledFeatures(features, "a", "c")).containsExactly("c"); @@ -981,31 +1018,37 @@ public void testLogicInRequirements() throws Exception { @Test public void testImpliesImpliesRequires() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' implies: 'b' }", - "feature { name: 'b' requires: { feature: 'c' } }", - "feature { name: 'c' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' implies: 'b' }", + "feature { name: 'b' requires: { feature: 'c' } }", + "feature { name: 'c' }"); assertThat(getEnabledFeatures(features, "a")).isEmpty(); } @Test public void testMultipleImplies() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' implies: 'b' implies: 'c' implies: 'd' }", - "feature { name: 'b' }", - "feature { name: 'c' requires: { feature: 'e' } }", - "feature { name: 'd' }", - "feature { name: 'e' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' implies: 'b' implies: 'c' implies: 'd' }", + "feature { name: 'b' }", + "feature { name: 'c' requires: { feature: 'e' } }", + "feature { name: 'd' }", + "feature { name: 'e' }"); assertThat(getEnabledFeatures(features, "a")).isEmpty(); assertThat(getEnabledFeatures(features, "a", "e")).containsExactly("a", "b", "c", "d", "e"); } @Test public void testDisabledFeaturesDoNotEnableImplications() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' implies: 'b' requires: { feature: 'c' } }", - "feature { name: 'b' }", - "feature { name: 'c' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' implies: 'b' requires: { feature: 'c' } }", + "feature { name: 'b' }", + "feature { name: 'c' }"); assertThat(getEnabledFeatures(features, "a")).isEmpty(); } @@ -1013,6 +1056,7 @@ public void testDisabledFeaturesDoNotEnableImplications() throws Exception { public void testFeatureNameCollision() throws Exception { try { buildFeatures( + ruleContext, "feature { name: '<<>>' }", "feature { name: '<<>>' }"); fail("Expected EvalException"); @@ -1024,7 +1068,7 @@ public void testFeatureNameCollision() throws Exception { @Test public void testReferenceToUndefinedFeature() throws Exception { try { - buildFeatures("feature { name: 'a' implies: '<<>>' }"); + buildFeatures(ruleContext, "feature { name: 'a' implies: '<<>>' }"); fail("Expected EvalException"); } catch (EvalException e) { assertThat(e).hasMessageThat().contains("<<>>"); @@ -1033,22 +1077,26 @@ public void testReferenceToUndefinedFeature() throws Exception { @Test public void testImpliesWithCycle() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' implies: 'b' }", - "feature { name: 'b' implies: 'a' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' implies: 'b' }", + "feature { name: 'b' implies: 'a' }"); assertThat(getEnabledFeatures(features, "a")).containsExactly("a", "b"); assertThat(getEnabledFeatures(features, "b")).containsExactly("a", "b"); } @Test public void testMultipleImpliesCycle() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' implies: 'b' implies: 'c' implies: 'd' }", - "feature { name: 'b' }", - "feature { name: 'c' requires: { feature: 'e' } }", - "feature { name: 'd' requires: { feature: 'f' } }", - "feature { name: 'e' requires: { feature: 'c' } }", - "feature { name: 'f' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' implies: 'b' implies: 'c' implies: 'd' }", + "feature { name: 'b' }", + "feature { name: 'c' requires: { feature: 'e' } }", + "feature { name: 'd' requires: { feature: 'f' } }", + "feature { name: 'e' requires: { feature: 'c' } }", + "feature { name: 'f' }"); assertThat(getEnabledFeatures(features, "a", "e")).isEmpty(); assertThat(getEnabledFeatures(features, "a", "e", "f")).containsExactly( "a", "b", "c", "d", "e", "f"); @@ -1056,11 +1104,13 @@ public void testMultipleImpliesCycle() throws Exception { @Test public void testRequiresWithCycle() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' requires: { feature: 'b' } }", - "feature { name: 'b' requires: { feature: 'a' } }", - "feature { name: 'c' implies: 'a' }", - "feature { name: 'd' implies: 'b' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' requires: { feature: 'b' } }", + "feature { name: 'b' requires: { feature: 'a' } }", + "feature { name: 'c' implies: 'a' }", + "feature { name: 'd' implies: 'b' }"); assertThat(getEnabledFeatures(features, "c")).isEmpty(); assertThat(getEnabledFeatures(features, "d")).isEmpty(); assertThat(getEnabledFeatures(features, "c", "d")).containsExactly("a", "b", "c", "d"); @@ -1068,21 +1118,25 @@ public void testRequiresWithCycle() throws Exception { @Test public void testImpliedByOneEnabledAndOneDisabledFeature() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' }", - "feature { name: 'b' requires: { feature: 'a' } implies: 'd' }", - "feature { name: 'c' implies: 'd' }", - "feature { name: 'd' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' }", + "feature { name: 'b' requires: { feature: 'a' } implies: 'd' }", + "feature { name: 'c' implies: 'd' }", + "feature { name: 'd' }"); assertThat(getEnabledFeatures(features, "b", "c")).containsExactly("c", "d"); } @Test public void testRequiresOneEnabledAndOneUnsupportedFeature() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature { name: 'a' requires: { feature: 'b' } requires: { feature: 'c' } }", - "feature { name: 'b' }", - "feature { name: 'c' requires: { feature: 'd' } }", - "feature { name: 'd' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature { name: 'a' requires: { feature: 'b' } requires: { feature: 'c' } }", + "feature { name: 'b' }", + "feature { name: 'c' requires: { feature: 'd' } }", + "feature { name: 'd' }"); assertThat(getEnabledFeatures(features, "a", "b", "c")).containsExactly("a", "b"); } @@ -1090,6 +1144,7 @@ public void testRequiresOneEnabledAndOneUnsupportedFeature() throws Exception { public void testFlagSetWithMissingVariableIsNotExpanded() throws Exception { FeatureConfiguration configuration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -1112,6 +1167,7 @@ public void testFlagSetWithMissingVariableIsNotExpanded() throws Exception { public void testOnlyFlagSetsWithAllVariablesPresentAreExpanded() throws Exception { FeatureConfiguration configuration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -1140,6 +1196,7 @@ public void testOnlyFlagSetsWithAllVariablesPresentAreExpanded() throws Exceptio public void testOnlyInnerFlagSetIsIteratedWithSequenceVariable() throws Exception { FeatureConfiguration configuration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -1171,6 +1228,7 @@ CppActionNames.CPP_COMPILE, createVariables("v", "1", "v", "2"))) public void testFlagSetsAreIteratedIndividuallyForSequenceVariables() throws Exception { FeatureConfiguration configuration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -1200,18 +1258,20 @@ CppActionNames.CPP_COMPILE, createVariables("v", "1", "v", "2", "w", "3"))) @Test public void testConfiguration() throws Exception { - CcToolchainFeatures features = buildFeatures( - "feature {", - " name: 'a'", - " flag_set {", - " action: 'c++-compile'", - " flag_group {", - " flag: '-f'", - " flag: '%{v}'", - " }", - " }", - "}", - "feature { name: 'b' implies: 'a' }"); + CcToolchainFeatures features = + buildFeatures( + ruleContext, + "feature {", + " name: 'a'", + " flag_set {", + " action: 'c++-compile'", + " flag_group {", + " flag: '-f'", + " flag: '%{v}'", + " }", + " }", + "}", + "feature { name: 'b' implies: 'a' }"); assertThat(getEnabledFeatures(features, "b")).containsExactly("a", "b"); assertThat( features @@ -1232,14 +1292,16 @@ public void testConfiguration() throws Exception { @Test public void testDefaultFeatures() throws Exception { CcToolchainFeatures features = - buildFeatures("feature { name: 'a' }", "feature { name: 'b' enabled: true }"); + buildFeatures(ruleContext, "feature { name: 'a' }", "feature { name: 'b' enabled: true }"); assertThat(features.getDefaultFeaturesAndActionConfigs()).containsExactly("b"); } @Test public void testDefaultActionConfigs() throws Exception { CcToolchainFeatures features = - buildFeatures("action_config { config_name: 'a' action_name: 'a'}", + buildFeatures( + ruleContext, + "action_config { config_name: 'a' action_name: 'a'}", "action_config { config_name: 'b' action_name: 'b' enabled: true }"); assertThat(features.getDefaultFeaturesAndActionConfigs()).containsExactly("b"); } @@ -1248,6 +1310,7 @@ public void testDefaultActionConfigs() throws Exception { public void testWithFeature_OneSetOneFeature() throws Exception { CcToolchainFeatures features = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -1275,6 +1338,7 @@ public void testWithFeature_OneSetOneFeature() throws Exception { public void testWithFeature_OneSetMultipleFeatures() throws Exception { CcToolchainFeatures features = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -1308,6 +1372,7 @@ public void testWithFeature_OneSetMultipleFeatures() throws Exception { public void testWithFeature_MulipleSetsMultipleFeatures() throws Exception { CcToolchainFeatures features = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -1344,6 +1409,7 @@ public void testWithFeature_MulipleSetsMultipleFeatures() throws Exception { public void testWithFeature_NotFeature() throws Exception { CcToolchainFeatures features = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -1390,6 +1456,7 @@ public void testWithFeature_NotFeature() throws Exception { public void testActivateActionConfigFromFeature() throws Exception { CcToolchainFeatures toolchainFeatures = buildFeatures( + ruleContext, "action_config {", " config_name: 'action-a'", " action_name: 'action-a'", @@ -1413,6 +1480,7 @@ public void testActivateActionConfigFromFeature() throws Exception { public void testFeatureCanRequireActionConfig() throws Exception { CcToolchainFeatures toolchainFeatures = buildFeatures( + ruleContext, "action_config {", " config_name: 'action-a'", " action_name: 'action-a'", @@ -1439,6 +1507,7 @@ public void testFeatureCanRequireActionConfig() throws Exception { public void testSimpleActionTool() throws Exception { FeatureConfiguration configuration = buildFeatures( + ruleContext, "action_config {", " config_name: 'action-a'", " action_name: 'action-a'", @@ -1458,6 +1527,7 @@ public void testSimpleActionTool() throws Exception { public void testActionToolFromFeatureSet() throws Exception { CcToolchainFeatures toolchainFeatures = buildFeatures( + ruleContext, "action_config {", " config_name: 'action-a'", " action_name: 'action-a'", @@ -1538,6 +1608,7 @@ public void testActionToolFromFeatureSet() throws Exception { public void testErrorForNoMatchingTool() throws Exception { CcToolchainFeatures toolchainFeatures = buildFeatures( + ruleContext, "action_config {", " config_name: 'action-a'", " action_name: 'action-a'", @@ -1571,6 +1642,7 @@ public void testErrorForNoMatchingTool() throws Exception { public void testActivateActionConfigDirectly() throws Exception { CcToolchainFeatures toolchainFeatures = buildFeatures( + ruleContext, "action_config {", " config_name: 'action-a'", " action_name: 'action-a'", @@ -1590,6 +1662,7 @@ public void testActivateActionConfigDirectly() throws Exception { public void testActionConfigCanActivateFeature() throws Exception { CcToolchainFeatures toolchainFeatures = buildFeatures( + ruleContext, "action_config {", " config_name: 'action-a'", " action_name: 'action-a'", @@ -1613,6 +1686,7 @@ public void testActionConfigCanActivateFeature() throws Exception { public void testInvalidActionConfigurationDuplicateActionConfigs() throws Exception { try { buildFeatures( + ruleContext, "action_config {", " config_name: 'action-a'", " action_name: 'action-1'", @@ -1633,6 +1707,7 @@ public void testInvalidActionConfigurationDuplicateActionConfigs() throws Except public void testInvalidActionConfigurationMultipleActionConfigsForAction() throws Exception { try { buildFeatures( + ruleContext, "action_config {", " config_name: 'name-a'", " action_name: 'action-a'", @@ -1651,6 +1726,7 @@ public void testInvalidActionConfigurationMultipleActionConfigsForAction() throw public void testFlagsFromActionConfig() throws Exception { FeatureConfiguration featureConfiguration = buildFeatures( + ruleContext, "action_config {", " config_name: 'c++-compile'", " action_name: 'c++-compile'", @@ -1668,6 +1744,7 @@ public void testFlagsFromActionConfig() throws Exception { public void testErrorForFlagFromActionConfigWithSpecifiedAction() throws Exception { try { buildFeatures( + ruleContext, "action_config {", " config_name: 'c++-compile'", " action_name: 'c++-compile'", @@ -1721,6 +1798,7 @@ public void testLibraryToLinkValue() { public void testProvidesCollision() throws Exception { try { buildFeatures( + ruleContext, "feature {", " name: 'a'", " provides: 'provides_string'", @@ -1740,6 +1818,7 @@ public void testProvidesCollision() throws Exception { public void testErrorForNoMatchingArtifactNamePatternCategory() throws Exception { try { buildFeatures( + ruleContext, "artifact_name_pattern {", "category_name: 'NONEXISTENT_CATEGORY'", "prefix: 'foo'", @@ -1757,6 +1836,7 @@ public void testErrorForNoMatchingArtifactPatternForCategory() throws Exception try { CcToolchainFeatures toolchainFeatures = buildFeatures( + ruleContext, "artifact_name_pattern {", "category_name: 'static_library'", "prefix: 'foo'", @@ -1777,6 +1857,7 @@ public void testErrorForNoMatchingArtifactPatternForCategory() throws Exception public void testGetArtifactNameExtensionForCategory() throws Exception { CcToolchainFeatures toolchainFeatures = buildFeatures( + ruleContext, "artifact_name_pattern {", " category_name: 'object_file'", " prefix: ''", 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 d028ada1bfd957..257ff53aad9a2e 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 @@ -16,7 +16,6 @@ import static com.google.common.truth.Truth.assertThat; -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.RuleContext; @@ -83,6 +82,7 @@ public void testSkylarkCallables() throws Exception { @Test public void testDisablingCompilationModeFlags() throws Exception { + reporter.removeHandler(failFastHandler); AnalysisMock.get() .ccSupport() .setupCrosstool( @@ -92,7 +92,7 @@ public void testDisablingCompilationModeFlags() throws Exception { "compilation_mode_flags { mode: OPT linker_flag: '-baz_from_compilation_mode' }"); scratch.file("a/BUILD", "cc_library(name='a', srcs=['a.cc'])"); - useConfiguration("-c", "opt"); + useConfiguration("-c", "opt", "--noincompatible_disable_legacy_crosstool_fields"); CcToolchainProvider ccToolchainProvider = getCcToolchainProvider(); assertThat(ccToolchainProvider.getLegacyCompileOptionsWithCopts()) .contains("-foo_from_compilation_mode"); @@ -100,12 +100,11 @@ public void testDisablingCompilationModeFlags() throws Exception { assertThat(ccToolchainProvider.getLegacyMostlyStaticLinkFlags(CompilationMode.OPT)) .contains("-baz_from_compilation_mode"); - useConfiguration("-c", "opt", "--experimental_disable_compilation_mode_flags"); - ccToolchainProvider = getCcToolchainProvider(); - assertThat(ccToolchainProvider.getLegacyCxxOptions()) - .doesNotContain("-bar_from_compilation_mode"); - assertThat(ccToolchainProvider.getLegacyMostlyStaticLinkFlags(CompilationMode.OPT)) - .doesNotContain("-baz_from_compilation_mode"); + useConfiguration("-c", "opt", "--incompatible_disable_legacy_crosstool_fields"); + getConfiguredTarget("//a"); + assertContainsEvent( + "compilation_mode_flags is disabled by " + + "--incompatible_disable_legacy_crosstool_fields, please migrate your CROSSTOOL"); } private CcToolchainProvider getCcToolchainProvider() throws Exception { @@ -116,6 +115,7 @@ private CcToolchainProvider getCcToolchainProvider() throws Exception { @Test public void testDisablingLinkingModeFlags() throws Exception { + reporter.removeHandler(failFastHandler); AnalysisMock.get() .ccSupport() .setupCrosstool( @@ -123,53 +123,36 @@ public void testDisablingLinkingModeFlags() throws Exception { "linking_mode_flags { mode: MOSTLY_STATIC linker_flag: '-foo_from_linking_mode' }"); scratch.file("a/BUILD", "cc_library(name='a', srcs=['a.cc'])"); - useConfiguration(); + useConfiguration("--noincompatible_disable_legacy_crosstool_fields"); CcToolchainProvider ccToolchainProvider = getCcToolchainProvider(); assertThat(ccToolchainProvider.getLegacyMostlyStaticLinkFlags(CompilationMode.OPT)) .contains("-foo_from_linking_mode"); - useConfiguration("--experimental_disable_linking_mode_flags"); - ccToolchainProvider = getCcToolchainProvider(); - assertThat(ccToolchainProvider.getLegacyMostlyStaticLinkFlags(CompilationMode.OPT)) - .doesNotContain("-foo_from_linking_mode"); + useConfiguration("--incompatible_disable_legacy_crosstool_fields"); + getConfiguredTarget("//a"); + assertContainsEvent( + "linking_mode_flags is disabled by " + + "--incompatible_disable_legacy_crosstool_fields, please migrate your CROSSTOOL"); } @Test public void testDisablingLegacyCrosstoolFields() throws Exception { + reporter.removeHandler(failFastHandler); AnalysisMock.get() .ccSupport() - .setupCrosstool( - mockToolsConfig, - "compiler_flag: '-foo_compiler_flag'", - "cxx_flag: '-foo_cxx_flag'", - "unfiltered_cxx_flag: '-foo_unfiltered_cxx_flag'", - "linker_flag: '-foo_linker_flag'", - "dynamic_library_linker_flag: '-foo_dynamic_library_linker_flag'", - "test_only_linker_flag: '-foo_test_only_linker_flag'"); + .setupCrosstool(mockToolsConfig, "compiler_flag: '-foo_compiler_flag'"); scratch.file("a/BUILD", "cc_library(name='a', srcs=['a.cc'])"); - useConfiguration(); + useConfiguration("--noincompatible_disable_legacy_crosstool_fields"); CcToolchainProvider ccToolchainProvider = getCcToolchainProvider(); assertThat(ccToolchainProvider.getLegacyCompileOptions()).contains("-foo_compiler_flag"); - assertThat(ccToolchainProvider.getLegacyCxxOptions()).contains("-foo_cxx_flag"); - assertThat(ccToolchainProvider.getUnfilteredCompilerOptions()) - .contains("-foo_unfiltered_cxx_flag"); - assertThat(ccToolchainProvider.getLegacyLinkOptions()).contains("-foo_linker_flag"); - assertThat(ccToolchainProvider.getSharedLibraryLinkOptions(/* flags= */ ImmutableList.of())) - .contains("-foo_dynamic_library_linker_flag"); - assertThat(ccToolchainProvider.getTestOnlyLinkOptions()).contains("-foo_test_only_linker_flag"); - useConfiguration("--experimental_disable_legacy_crosstool_fields"); - ccToolchainProvider = getCcToolchainProvider(); - assertThat(ccToolchainProvider.getLegacyCompileOptions()).doesNotContain("-foo_compiler_flag"); - assertThat(ccToolchainProvider.getLegacyCxxOptions()).doesNotContain("-foo_cxx_flag"); - assertThat(ccToolchainProvider.getUnfilteredCompilerOptions()) - .doesNotContain("-foo_unfiltered_cxx_flag"); - assertThat(ccToolchainProvider.getLegacyLinkOptions()).doesNotContain("-foo_linker_flag"); - assertThat(ccToolchainProvider.getSharedLibraryLinkOptions(/* flags= */ ImmutableList.of())) - .doesNotContain("-foo_dynamic_library_linker_flag"); - assertThat(ccToolchainProvider.getTestOnlyLinkOptions()) - .doesNotContain("-foo_test_only_linker_flag"); + useConfiguration("--incompatible_disable_legacy_crosstool_fields"); + getConfiguredTarget("//a"); + + assertContainsEvent( + "compiler_flag is disabled by --incompatible_disable_legacy_crosstool_fields, please " + + "migrate your CROSSTOOL"); } /* diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java index 37f32f13167a0a..58e700dce0f512 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLineTest.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; @@ -26,6 +27,7 @@ import java.io.IOException; import java.util.List; import java.util.regex.Pattern; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -39,6 +41,14 @@ @RunWith(JUnit4.class) public class CompileCommandLineTest extends BuildViewTestCase { + private RuleContext ruleContext; + + @Before + public void initializeRuleContext() throws Exception { + scratch.file("foo/BUILD", "cc_library(name = 'foo')"); + ruleContext = getRuleContext(getConfiguredTarget("//foo:foo")); + } + private Artifact scratchArtifact(String s) { Path execRoot = outputBase.getRelative("exec"); Path outputRoot = execRoot.getRelative("root"); @@ -50,9 +60,9 @@ private Artifact scratchArtifact(String s) { } } - private static FeatureConfiguration getMockFeatureConfiguration(String... crosstool) - throws Exception { - return CcToolchainFeaturesTest.buildFeatures(crosstool) + private static FeatureConfiguration getMockFeatureConfiguration( + RuleContext ruleContext, String... crosstool) throws Exception { + return CcToolchainFeaturesTest.buildFeatures(ruleContext, crosstool) .getFeatureConfiguration( ImmutableSet.of( CppActionNames.ASSEMBLE, @@ -70,6 +80,7 @@ public void testFeatureConfigurationCommandLineIsUsed() throws Exception { makeCompileCommandLineBuilder() .setFeatureConfiguration( getMockFeatureConfiguration( + ruleContext, "", "action_config {", " config_name: 'c++-compile'", @@ -111,6 +122,7 @@ private List getCompileCommandLineWithCoptsFilter(String featureName) th makeCompileCommandLineBuilder() .setFeatureConfiguration( getMockFeatureConfiguration( + ruleContext, "", "action_config {", " config_name: 'c++-compile'", diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java index 5d2b4b13224f5d..a240fd55c736d1 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionTest.java @@ -69,6 +69,7 @@ /** Tests for {@link CppLinkAction}. */ @RunWith(JUnit4.class) public class CppLinkActionTest extends BuildViewTestCase { + private RuleContext createDummyRuleContext() throws Exception { return view.getRuleContextForTesting( reporter, @@ -92,7 +93,8 @@ public Artifact getDerivedArtifact(PathFragment rootRelativePath, ArtifactRoot r masterConfig); } - private final FeatureConfiguration getMockFeatureConfiguration() throws Exception { + private final FeatureConfiguration getMockFeatureConfiguration(RuleContext ruleContext) + throws Exception { ImmutableList features = new ImmutableList.Builder() .addAll( @@ -113,7 +115,7 @@ private final FeatureConfiguration getMockFeatureConfiguration() throws Exceptio "strip_tool", /* supportsInterfaceSharedLibraries= */ false); - return CcToolchainFeaturesTest.buildFeatures(features, actionConfigs) + return CcToolchainFeaturesTest.buildFeatures(ruleContext, features, actionConfigs) .getFeatureConfiguration( ImmutableSet.of( Link.LinkTargetType.EXECUTABLE.getActionName(), @@ -124,8 +126,11 @@ private final FeatureConfiguration getMockFeatureConfiguration() throws Exceptio @Test public void testToolchainFeatureFlags() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + FeatureConfiguration featureConfiguration = CcToolchainFeaturesTest.buildFeatures( + ruleContext, MockCcSupport.EMPTY_EXECUTABLE_ACTION_CONFIG, "feature {", " name: 'a'", @@ -139,6 +144,7 @@ public void testToolchainFeatureFlags() throws Exception { CppLinkAction linkAction = createLinkBuilder( + ruleContext, Link.LinkTargetType.EXECUTABLE, "dummyRuleContext/out", ImmutableList.of(), @@ -150,8 +156,11 @@ public void testToolchainFeatureFlags() throws Exception { @Test public void testExecutionRequirementsFromCrosstool() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + FeatureConfiguration featureConfiguration = CcToolchainFeaturesTest.buildFeatures( + ruleContext, "action_config {", " config_name: '" + LinkTargetType.EXECUTABLE.getActionName() + "'", " action_name: '" + LinkTargetType.EXECUTABLE.getActionName() + "'", @@ -164,6 +173,7 @@ public void testExecutionRequirementsFromCrosstool() throws Exception { CppLinkAction linkAction = createLinkBuilder( + ruleContext, LinkTargetType.EXECUTABLE, "dummyRuleContext/out", ImmutableList.of(), @@ -329,8 +339,11 @@ public void testCompilesDynamicModeBinarySourcesWithoutFeatureIntoDynamicLibrary @Test public void testToolchainFeatureEnv() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + FeatureConfiguration featureConfiguration = CcToolchainFeaturesTest.buildFeatures( + ruleContext, MockCcSupport.EMPTY_EXECUTABLE_ACTION_CONFIG, "feature {", " name: 'a'", @@ -344,6 +357,7 @@ public void testToolchainFeatureEnv() throws Exception { CppLinkAction linkAction = createLinkBuilder( + ruleContext, Link.LinkTargetType.EXECUTABLE, "dummyRuleContext/out", ImmutableList.of(), @@ -372,7 +386,7 @@ public void testComputeKeyNonStatic() throws Exception { final PathFragment dynamicOutputPath = PathFragment.create("dummyRuleContext/output/path.so"); final Artifact staticOutputFile = getBinArtifactWithNoOwner(exeOutputPath.getPathString()); final Artifact dynamicOutputFile = getBinArtifactWithNoOwner(dynamicOutputPath.getPathString()); - final FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(); + final FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(ruleContext); ActionTester.runTest( NonStaticAttributes.class, @@ -430,7 +444,7 @@ public void testComputeKeyStatic() throws Exception { final PathFragment dynamicOutputPath = PathFragment.create("dummyRuleContext/output/path.so"); final Artifact staticOutputFile = getBinArtifactWithNoOwner(staticOutputPath.getPathString()); final Artifact dynamicOutputFile = getBinArtifactWithNoOwner(dynamicOutputPath.getPathString()); - final FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(); + final FeatureConfiguration featureConfiguration = getMockFeatureConfiguration(ruleContext); ActionTester.runTest( StaticKeyAttributes.class, @@ -517,6 +531,8 @@ public void testLargeLocalLinkResourceEstimate() throws Exception { } private void assertLinkSizeAccuracy(int inputs) throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + ImmutableList.Builder objects = ImmutableList.builder(); for (int i = 0; i < inputs; i++) { objects.add(getOutputArtifact("object" + i + ".o")); @@ -524,11 +540,12 @@ private void assertLinkSizeAccuracy(int inputs) throws Exception { CppLinkAction linkAction = createLinkBuilder( + ruleContext, Link.LinkTargetType.EXECUTABLE, "dummyRuleContext/binary2", objects.build(), ImmutableList.of(), - getMockFeatureConfiguration()) + getMockFeatureConfiguration(ruleContext)) .setFake(true) .build(); @@ -553,13 +570,13 @@ private void assertLinkSizeAccuracy(int inputs) throws Exception { } private CppLinkActionBuilder createLinkBuilder( + RuleContext ruleContext, Link.LinkTargetType type, String outputPath, Iterable nonLibraryInputs, ImmutableList libraryInputs, FeatureConfiguration featureConfiguration) throws Exception { - RuleContext ruleContext = createDummyRuleContext(); CcToolchainProvider toolchain = CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext); CppLinkActionBuilder builder = @@ -582,14 +599,16 @@ private CppLinkActionBuilder createLinkBuilder( return builder; } - private CppLinkActionBuilder createLinkBuilder(Link.LinkTargetType type) throws Exception { + private CppLinkActionBuilder createLinkBuilder(RuleContext ruleContext, Link.LinkTargetType type) + throws Exception { PathFragment output = PathFragment.create("dummyRuleContext/output/path.a"); return createLinkBuilder( + ruleContext, type, output.getPathString(), ImmutableList.of(), ImmutableList.of(), - getMockFeatureConfiguration()); + getMockFeatureConfiguration(ruleContext)); } public Artifact getOutputArtifact(String relpath) { @@ -621,8 +640,10 @@ private static void assertError(String expectedSubstring, CppLinkActionBuilder b @Test public void testInterfaceOutputWithoutBuildingDynamicLibraryIsError() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.EXECUTABLE) + createLinkBuilder(ruleContext, LinkTargetType.EXECUTABLE) .setInterfaceOutput(scratchArtifact("FakeInterfaceOutput")); assertError("Interface output can only be used with non-fake DYNAMIC_LIBRARY targets", builder); @@ -630,8 +651,11 @@ public void testInterfaceOutputWithoutBuildingDynamicLibraryIsError() throws Exc @Test public void testInterfaceOutputForDynamicLibrary() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + FeatureConfiguration featureConfiguration = CcToolchainFeaturesTest.buildFeatures( + ruleContext, "supports_interface_shared_objects: true ", "feature {", " name: 'build_interface_libraries'", @@ -674,6 +698,7 @@ public void testInterfaceOutputForDynamicLibrary() throws Exception { LinkTargetType.NODEPS_DYNAMIC_LIBRARY.getActionName())); CppLinkActionBuilder builder = createLinkBuilder( + ruleContext, LinkTargetType.NODEPS_DYNAMIC_LIBRARY, "foo.so", ImmutableList.of(), @@ -694,8 +719,10 @@ public void testInterfaceOutputForDynamicLibrary() throws Exception { @Test public void testStaticLinkWithDynamicIsError() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) + createLinkBuilder(ruleContext, LinkTargetType.STATIC_LIBRARY) .setLinkingMode(Link.LinkingMode.DYNAMIC) .setLibraryIdentifier("foo"); @@ -704,8 +731,10 @@ public void testStaticLinkWithDynamicIsError() throws Exception { @Test public void testStaticLinkWithSymbolsCountOutputIsError() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) + createLinkBuilder(ruleContext, LinkTargetType.STATIC_LIBRARY) .setLinkingMode(LinkingMode.STATIC) .setLibraryIdentifier("foo") .setSymbolCountsOutput(scratchArtifact("dummySymbolCounts")); @@ -715,8 +744,10 @@ public void testStaticLinkWithSymbolsCountOutputIsError() throws Exception { @Test public void testStaticLinkWithNativeDepsIsError() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) + createLinkBuilder(ruleContext, LinkTargetType.STATIC_LIBRARY) .setLinkingMode(LinkingMode.STATIC) .setLibraryIdentifier("foo") .setNativeDeps(true); @@ -726,8 +757,10 @@ public void testStaticLinkWithNativeDepsIsError() throws Exception { @Test public void testStaticLinkWithWholeArchiveIsError() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) + createLinkBuilder(ruleContext, LinkTargetType.STATIC_LIBRARY) .setLinkingMode(LinkingMode.STATIC) .setLibraryIdentifier("foo") .setWholeArchive(true); @@ -756,6 +789,8 @@ private void verifyArguments( @Test public void testLinksTreeArtifactLibraries() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory"); TreeFileArtifact library0 = ActionInputHelper.treeFileArtifact(testTreeArtifact, "library0.o"); @@ -773,7 +808,7 @@ public void expand(Artifact artifact, Collection output) { }; CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) + createLinkBuilder(ruleContext, LinkTargetType.STATIC_LIBRARY) .setLibraryIdentifier("foo") .addObjectFiles(ImmutableList.of(testTreeArtifact)); @@ -801,6 +836,7 @@ public void testLinksTreeArtifactLibraryForDeps() throws Exception { // This test only makes sense if start/end lib archives are supported. analysisMock.ccSupport().setupCrosstool(mockToolsConfig, "supports_start_end_lib: true"); useConfiguration("--start_end_lib"); + RuleContext ruleContext = createDummyRuleContext(); SpecialArtifact testTreeArtifact = createTreeArtifact("library_directory"); @@ -821,7 +857,7 @@ public void expand(Artifact artifact, Collection output) { Artifact archiveFile = scratchArtifact("library.a"); CppLinkActionBuilder builder = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) + createLinkBuilder(ruleContext, LinkTargetType.STATIC_LIBRARY) .setLibraryIdentifier("foo") .addLibrary( LinkerInputs.newInputLibrary( @@ -855,6 +891,8 @@ public void expand(Artifact artifact, Collection output) { @Test public void testStaticLinking() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + ImmutableList targetTypesToTest = ImmutableList.of( LinkTargetType.STATIC_LIBRARY, @@ -884,11 +922,12 @@ public void testStaticLinking() throws Exception { CppLinkActionBuilder builder = createLinkBuilder( + ruleContext, linkType, output.getExecPathString(), ImmutableList.of(), ImmutableList.of(), - getMockFeatureConfiguration()) + getMockFeatureConfiguration(ruleContext)) .setLibraryIdentifier("foo") .addObjectFiles(ImmutableList.of(testTreeArtifact)) .addObjectFile(objectFile) @@ -908,13 +947,16 @@ public void testStaticLinking() throws Exception { /** Tests that -pie is removed when -shared is also present (http://b/5611891#). */ @Test public void testPieOptionDisabledForSharedLibraries() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + CppLinkAction linkAction = createLinkBuilder( + ruleContext, LinkTargetType.DYNAMIC_LIBRARY, "dummyRuleContext/out.so", ImmutableList.of(), ImmutableList.of(), - getMockFeatureConfiguration()) + getMockFeatureConfiguration(ruleContext)) .setLinkingMode(Link.LinkingMode.STATIC) .addLinkopts(ImmutableList.of("-pie", "-other", "-pie")) .setLibraryIdentifier("foo") @@ -928,13 +970,16 @@ public void testPieOptionDisabledForSharedLibraries() throws Exception { /** Tests that -pie is removed when -shared is also present (http://b/5611891#). */ @Test public void testPieOptionKeptForExecutables() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + CppLinkAction linkAction = createLinkBuilder( + ruleContext, LinkTargetType.EXECUTABLE, "dummyRuleContext/out", ImmutableList.of(), ImmutableList.of(), - getMockFeatureConfiguration()) + getMockFeatureConfiguration(ruleContext)) .setLinkingMode(Link.LinkingMode.STATIC) .addLinkopts(ImmutableList.of("-pie", "-other", "-pie")) .build(); @@ -946,6 +991,8 @@ public void testPieOptionKeptForExecutables() throws Exception { @Test public void testLinkoptsComeAfterLinkerInputs() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + String solibPrefix = "_solib_" + CrosstoolConfigurationHelper.defaultCpu(); Iterable linkerInputs = LinkerInputs.opaqueLibrariesToLink( @@ -958,11 +1005,12 @@ public void testLinkoptsComeAfterLinkerInputs() throws Exception { CppLinkAction linkAction = createLinkBuilder( + ruleContext, LinkTargetType.EXECUTABLE, "dummyRuleContext/out", ImmutableList.of(), ImmutableList.copyOf(linkerInputs), - getMockFeatureConfiguration()) + getMockFeatureConfiguration(ruleContext)) .addLinkopts(ImmutableList.of("FakeLinkopt1", "FakeLinkopt2")) .build(); @@ -977,8 +1025,10 @@ public void testLinkoptsComeAfterLinkerInputs() throws Exception { @Test public void testLinkoptsAreOmittedForStaticLibrary() throws Exception { + RuleContext ruleContext = createDummyRuleContext(); + CppLinkAction linkAction = - createLinkBuilder(LinkTargetType.STATIC_LIBRARY) + createLinkBuilder(ruleContext, LinkTargetType.STATIC_LIBRARY) .addLinkopt("FakeLinkopt1") .setLibraryIdentifier("foo") .build(); @@ -988,7 +1038,9 @@ public void testLinkoptsAreOmittedForStaticLibrary() throws Exception { @Test public void testSplitExecutableLinkCommand() throws Exception { - CppLinkAction linkAction = createLinkBuilder(LinkTargetType.EXECUTABLE).build(); + RuleContext ruleContext = createDummyRuleContext(); + + CppLinkAction linkAction = createLinkBuilder(ruleContext, LinkTargetType.EXECUTABLE).build(); Pair, List> result = linkAction.getLinkCommandLine().splitCommandline(); String linkCommandLine = Joiner.on(" ").join(result.first); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java index ca775228d020a3..5c43c3f7286c79 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationHelper.java @@ -94,8 +94,10 @@ public static CrosstoolConfig.CrosstoolRelease simpleCompleteToolchainProto() { .setAbiVersion("gcc-3.4") .setAbiLibcVersion("2.3.2") // add a submessage that implies support for '.so' files - .addLinkingModeFlags(CrosstoolConfig.LinkingModeFlags.newBuilder() - .setMode(CrosstoolConfig.LinkingMode.DYNAMIC)) + .addFeature( + CrosstoolConfig.CToolchain.Feature.newBuilder() + .setName(CppRuleClasses.SUPPORTS_DYNAMIC_LINKER) + .setEnabled(true)) .addCxxBuiltinIncludeDirectory("/include/directory"); builder.addToolchain(toolchainBuilder); return builder.build(); diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java index cffef0494cca94..5823360314d85e 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTest.java @@ -21,12 +21,15 @@ import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.util.AnalysisMock; import com.google.devtools.build.lib.packages.util.MockCcSupport; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.LibraryToLinkValue; import com.google.devtools.build.lib.rules.cpp.CcToolchainVariables.VariableValue; import com.google.devtools.build.lib.rules.cpp.Link.LinkTargetType; +import java.io.IOException; import java.util.List; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -35,6 +38,15 @@ @RunWith(JUnit4.class) public class LinkBuildVariablesTest extends LinkBuildVariablesTestCase { + @Before + public void createFooFooCcLibraryForRuleContext() throws IOException { + scratch.file("foo/BUILD", "cc_library(name = 'foo')"); + } + + private RuleContext getRuleContext() throws Exception { + return getRuleContext(getConfiguredTarget("//foo:foo")); + } + @Test public void testForcePicBuildVariable() throws Exception { useConfiguration("--force_pic"); @@ -44,7 +56,8 @@ public void testForcePicBuildVariable() throws Exception { ConfiguredTarget target = getConfiguredTarget("//x:bin"); CcToolchainVariables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); String variableValue = - getVariableValue(variables, LinkBuildVariables.FORCE_PIC.getVariableName()); + getVariableValue( + getRuleContext(), variables, LinkBuildVariables.FORCE_PIC.getVariableName()); assertThat(variableValue).contains(""); } @@ -90,7 +103,9 @@ public void testLibrarySearchDirectoriesAreExported() throws Exception { CcToolchainVariables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); List variableValue = getSequenceVariableValue( - variables, LinkBuildVariables.LIBRARY_SEARCH_DIRECTORIES.getVariableName()); + getRuleContext(), + variables, + LinkBuildVariables.LIBRARY_SEARCH_DIRECTORIES.getVariableName()); assertThat(Iterables.getOnlyElement(variableValue)).contains("some-dir"); } @@ -105,7 +120,8 @@ public void testLinkerParamFileIsExported() throws Exception { ConfiguredTarget target = getConfiguredTarget("//x:bin"); CcToolchainVariables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); String variableValue = - getVariableValue(variables, LinkBuildVariables.LINKER_PARAM_FILE.getVariableName()); + getVariableValue( + getRuleContext(), variables, LinkBuildVariables.LINKER_PARAM_FILE.getVariableName()); assertThat(variableValue).matches(".*bin/x/bin" + "-2.params$"); } @@ -126,14 +142,25 @@ public void testInterfaceLibraryBuildingVariablesWhenGenerationPossible() throws getLinkBuildVariables(target, LinkTargetType.NODEPS_DYNAMIC_LIBRARY); String interfaceLibraryBuilder = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_BUILDER.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_BUILDER.getVariableName()); String interfaceLibraryInput = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_INPUT.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_INPUT.getVariableName()); String interfaceLibraryOutput = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_OUTPUT.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_OUTPUT.getVariableName()); String generateInterfaceLibrary = getVariableValue( - variables, LinkBuildVariables.GENERATE_INTERFACE_LIBRARY.getVariableName()); + getRuleContext(), + variables, + LinkBuildVariables.GENERATE_INTERFACE_LIBRARY.getVariableName()); assertThat(generateInterfaceLibrary).isEqualTo("yes"); assertThat(interfaceLibraryInput).endsWith("libfoo.so"); @@ -145,6 +172,7 @@ public void testInterfaceLibraryBuildingVariablesWhenGenerationPossible() throws public void testNoIfsoBuildingWhenWhenThinLtoIndexing() throws Exception { // Make sure the interface shared object generation is enabled in the configuration // (which it is not by default for some windows toolchains) + invalidatePackages(true); AnalysisMock.get() .ccSupport() .setupCrosstool( @@ -174,14 +202,25 @@ public void testNoIfsoBuildingWhenWhenThinLtoIndexing() throws Exception { CcToolchainVariables variables = indexAction.getLinkCommandLine().getBuildVariables(); String interfaceLibraryBuilder = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_BUILDER.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_BUILDER.getVariableName()); String interfaceLibraryInput = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_INPUT.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_INPUT.getVariableName()); String interfaceLibraryOutput = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_OUTPUT.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_OUTPUT.getVariableName()); String generateInterfaceLibrary = getVariableValue( - variables, LinkBuildVariables.GENERATE_INTERFACE_LIBRARY.getVariableName()); + getRuleContext(), + variables, + LinkBuildVariables.GENERATE_INTERFACE_LIBRARY.getVariableName()); assertThat(generateInterfaceLibrary).isEqualTo("no"); assertThat(interfaceLibraryInput).endsWith("ignored"); @@ -205,14 +244,25 @@ public void testInterfaceLibraryBuildingVariablesWhenGenerationNotAllowed() thro CcToolchainVariables variables = getLinkBuildVariables(target, LinkTargetType.STATIC_LIBRARY); String interfaceLibraryBuilder = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_BUILDER.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_BUILDER.getVariableName()); String interfaceLibraryInput = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_INPUT.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_INPUT.getVariableName()); String interfaceLibraryOutput = - getVariableValue(variables, LinkBuildVariables.INTERFACE_LIBRARY_OUTPUT.getVariableName()); + getVariableValue( + getRuleContext(), + variables, + LinkBuildVariables.INTERFACE_LIBRARY_OUTPUT.getVariableName()); String generateInterfaceLibrary = getVariableValue( - variables, LinkBuildVariables.GENERATE_INTERFACE_LIBRARY.getVariableName()); + getRuleContext(), + variables, + LinkBuildVariables.GENERATE_INTERFACE_LIBRARY.getVariableName()); assertThat(generateInterfaceLibrary).isEqualTo("no"); assertThat(interfaceLibraryInput).endsWith("ignored"); @@ -231,7 +281,9 @@ public void testOutputExecpath() throws Exception { CcToolchainVariables variables = getLinkBuildVariables(target, LinkTargetType.NODEPS_DYNAMIC_LIBRARY); - assertThat(getVariableValue(variables, LinkBuildVariables.OUTPUT_EXECPATH.getVariableName())) + assertThat( + getVariableValue( + getRuleContext(), variables, LinkBuildVariables.OUTPUT_EXECPATH.getVariableName())) .endsWith("x/libfoo.so"); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTestCase.java index b2e0da7563ead9..e0a21024889459 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/LinkBuildVariablesTestCase.java @@ -19,6 +19,7 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.vfs.PathFragment; @@ -65,22 +66,22 @@ protected CcToolchainVariables getLinkBuildVariables( return getCppLinkAction(target, type).getLinkCommandLine().getBuildVariables(); } - /** - * Creates a CcToolchainFeatures from features described in the given toolchain fragment. - */ - public static CcToolchainFeatures buildFeatures(String... toolchain) throws Exception { + /** Creates a CcToolchainFeatures from features described in the given toolchain fragment. */ + public static CcToolchainFeatures buildFeatures(RuleContext ruleContext, String... toolchain) + throws Exception { CToolchain.Builder toolchainBuilder = CToolchain.newBuilder(); TextFormat.merge(Joiner.on("").join(toolchain), toolchainBuilder); return new CcToolchainFeatures( - CcToolchainConfigInfo.fromToolchain(toolchainBuilder.buildPartial()), + CcToolchainConfigInfo.fromToolchain(ruleContext, toolchainBuilder.buildPartial()), /* ccToolchainPath= */ PathFragment.EMPTY_FRAGMENT); } /** Returns the value of a given sequence variable in context of the given Variables instance. */ - protected List getSequenceVariableValue(CcToolchainVariables variables, String variable) - throws Exception { + protected static List getSequenceVariableValue( + RuleContext ruleContext, CcToolchainVariables variables, String variable) throws Exception { FeatureConfiguration mockFeatureConfiguration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", @@ -96,10 +97,11 @@ protected List getSequenceVariableValue(CcToolchainVariables variables, } /** Returns the value of a given string variable in context of the given Variables instance. */ - protected String getVariableValue(CcToolchainVariables variables, String variable) - throws Exception { + protected static String getVariableValue( + RuleContext ruleContext, CcToolchainVariables variables, String variable) throws Exception { FeatureConfiguration mockFeatureConfiguration = buildFeatures( + ruleContext, "feature {", " name: 'a'", " flag_set {", diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java index ec0ca56b2ef96b..021146e1b461b9 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcBuildVariablesTest.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.actions.CommandAction; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.util.MockObjcSupport; import com.google.devtools.build.lib.rules.apple.AppleCommandLineOptions; @@ -33,6 +34,7 @@ import com.google.devtools.build.lib.rules.cpp.LinkBuildVariablesTestCase; import com.google.devtools.build.lib.testutil.TestConstants; import java.io.IOException; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -44,6 +46,15 @@ @RunWith(JUnit4.class) public class ObjcBuildVariablesTest extends LinkBuildVariablesTestCase { + @Before + public void createFooFooCcLibraryForRuleContext() throws IOException { + scratch.file("foo/BUILD", "cc_library(name = 'foo')"); + } + + private RuleContext getRuleContext() throws Exception { + return getRuleContext(getConfiguredTarget("//foo:foo")); + } + @Override public void initializeMockClient() throws IOException { super.initializeMockClient(); @@ -81,15 +92,18 @@ public void testAppleBuildVariablesIos() throws Exception { ConfiguredTarget target = getConfiguredTarget("//x:bin"); CcToolchainVariables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); assertThat( - getVariableValue(variables, AppleCcToolchain.XCODE_VERISON_OVERRIDE_VALUE_KEY)) + getVariableValue( + getRuleContext(), variables, AppleCcToolchain.XCODE_VERISON_OVERRIDE_VALUE_KEY)) .contains("5.8"); assertThat( getVariableValue( - variables, AppleCcToolchain.APPLE_SDK_VERSION_OVERRIDE_VALUE_KEY)) + getRuleContext(), variables, AppleCcToolchain.APPLE_SDK_VERSION_OVERRIDE_VALUE_KEY)) .contains("8.4"); - assertThat(getVariableValue(variables, AppleCcToolchain.APPLE_SDK_PLATFORM_VALUE_KEY)) + assertThat( + getVariableValue( + getRuleContext(), variables, AppleCcToolchain.APPLE_SDK_PLATFORM_VALUE_KEY)) .contains("iPhoneSimulator"); - assertThat(getVariableValue(variables, AppleCcToolchain.VERSION_MIN_KEY)) + assertThat(getVariableValue(getRuleContext(), variables, AppleCcToolchain.VERSION_MIN_KEY)) .contains("12.345"); } @@ -130,15 +144,18 @@ public void testAppleBuildVariablesWatchos() throws Exception { CcToolchainVariables variables = ccArchiveAction.getLinkCommandLine().getBuildVariables(); assertThat( - getVariableValue(variables, AppleCcToolchain.XCODE_VERISON_OVERRIDE_VALUE_KEY)) + getVariableValue( + getRuleContext(), variables, AppleCcToolchain.XCODE_VERISON_OVERRIDE_VALUE_KEY)) .contains("5.8"); assertThat( getVariableValue( - variables, AppleCcToolchain.APPLE_SDK_VERSION_OVERRIDE_VALUE_KEY)) + getRuleContext(), variables, AppleCcToolchain.APPLE_SDK_VERSION_OVERRIDE_VALUE_KEY)) .contains("2.0"); - assertThat(getVariableValue(variables, AppleCcToolchain.APPLE_SDK_PLATFORM_VALUE_KEY)) + assertThat( + getVariableValue( + getRuleContext(), variables, AppleCcToolchain.APPLE_SDK_PLATFORM_VALUE_KEY)) .contains("WatchOS"); - assertThat(getVariableValue(variables, AppleCcToolchain.VERSION_MIN_KEY)) + assertThat(getVariableValue(getRuleContext(), variables, AppleCcToolchain.VERSION_MIN_KEY)) .contains(dummyMinimumOsValue); } @@ -177,7 +194,7 @@ public void testAppleBuildVariablesMacos() throws Exception { CppLinkAction ccArchiveAction = (CppLinkAction) getGeneratingAction(archive); CcToolchainVariables variables = ccArchiveAction.getLinkCommandLine().getBuildVariables(); - assertThat(getVariableValue(variables, AppleCcToolchain.VERSION_MIN_KEY)) + assertThat(getVariableValue(getRuleContext(), variables, AppleCcToolchain.VERSION_MIN_KEY)) .contains(dummyMinimumOsValue); } @@ -197,13 +214,14 @@ public void testDefaultBuildVariablesIos() throws Exception { ConfiguredTarget target = getConfiguredTarget("//x:bin"); CcToolchainVariables variables = getLinkBuildVariables(target, Link.LinkTargetType.EXECUTABLE); assertThat( - getVariableValue(variables, AppleCcToolchain.XCODE_VERISON_OVERRIDE_VALUE_KEY)) + getVariableValue( + getRuleContext(), variables, AppleCcToolchain.XCODE_VERISON_OVERRIDE_VALUE_KEY)) .contains(MockObjcSupport.DEFAULT_XCODE_VERSION); assertThat( getVariableValue( - variables, AppleCcToolchain.APPLE_SDK_VERSION_OVERRIDE_VALUE_KEY)) + getRuleContext(), variables, AppleCcToolchain.APPLE_SDK_VERSION_OVERRIDE_VALUE_KEY)) .contains(MockObjcSupport.DEFAULT_IOS_SDK_VERSION); - assertThat(getVariableValue(variables, AppleCcToolchain.VERSION_MIN_KEY)) + assertThat(getVariableValue(getRuleContext(), variables, AppleCcToolchain.VERSION_MIN_KEY)) .contains(AppleCommandLineOptions.DEFAULT_IOS_SDK_VERSION); } @@ -242,7 +260,7 @@ public void testMinimumOsAttributeBuildVariable() throws Exception { CppLinkAction ccArchiveAction = (CppLinkAction) getGeneratingAction(archive); CcToolchainVariables variables = ccArchiveAction.getLinkCommandLine().getBuildVariables(); - assertThat(getVariableValue(variables, AppleCcToolchain.VERSION_MIN_KEY)) + assertThat(getVariableValue(getRuleContext(), variables, AppleCcToolchain.VERSION_MIN_KEY)) .contains(dummyMinimumOsValue); } }