From e7c730ba64d1c34ac7049f4165a33dc1329d0019 Mon Sep 17 00:00:00 2001 From: hlopko Date: Tue, 11 Apr 2017 13:03:26 +0000 Subject: Extract --sysroot flag from bazel and move it into crosstool This is an encore of https://github.com/bazelbuild/bazel/commit/6127358c1799d8d83cebbd499edac89f869df41b . RELNOTES: None. PiperOrigin-RevId: 152803621 --- .../lib/rules/apple/cpp/AppleCcToolchain.java | 2 +- .../devtools/build/lib/rules/cpp/CcToolchain.java | 25 ++++++- .../build/lib/rules/cpp/CppActionConfigs.java | 79 ++++++++++++++++++++++ .../build/lib/rules/cpp/CppConfiguration.java | 56 ++++----------- .../build/lib/rules/cpp/CppRuleClasses.java | 3 + .../cpp/CrosstoolConfigurationLoaderTest.java | 2 +- .../lib/rules/cpp/LinkBuildVariablesTest.java | 16 +++++ 7 files changed, 135 insertions(+), 48 deletions(-) (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java index 92902bcac7..de55d33b12 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/apple/cpp/AppleCcToolchain.java @@ -53,7 +53,7 @@ public class AppleCcToolchain extends CcToolchain { public static final String APPLE_SDK_PLATFORM_VALUE_KEY = "apple_sdk_platform_value"; @Override - protected Map getBuildVariables(RuleContext ruleContext) + protected Map getLocalBuildVariables(RuleContext ruleContext) throws RuleErrorException { AppleConfiguration appleConfiguration = ruleContext.getFragment(AppleConfiguration.class); 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 9cb154f3db..3d5c9223ca 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 @@ -354,14 +354,33 @@ public class CcToolchain implements RuleConfiguredTargetFactory { } /** - * Returns a map that should be templated into the crosstool as build variables. Is meant to - * be overridden by subclasses of CcToolchain. + * Returns a map that should be templated into the crosstool as build variables * * @param ruleContext the rule context * @throws RuleErrorException if there are configuration errors making it impossible to resolve * certain build variables of this toolchain */ - protected Map getBuildVariables(RuleContext ruleContext) + protected final Map getBuildVariables(RuleContext ruleContext) + throws RuleErrorException { + CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); + ImmutableMap.Builder variables = ImmutableMap.builder(); + if (cppConfiguration.getSysroot() != null) { + variables.put( + CppRuleClasses.SYSROOT_VARIABLE, cppConfiguration.getSysroot().getSafePathString()); + } + variables.putAll(getLocalBuildVariables(ruleContext)); + return variables.build(); + } + + /** + * Returns a map that should be templated into the crosstool as build variables. Is meant to be + * overridden by subclasses of CcToolchain. + * + * @param ruleContext the rule context + * @throws RuleErrorException if there are configuration errors making it impossible to resolve + * certain build variables of this toolchain + */ + protected Map getLocalBuildVariables(RuleContext ruleContext) throws RuleErrorException { return ImmutableMap.of(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java index ae53fc1d46..c70e3f34e6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java @@ -57,6 +57,20 @@ public class CppActionConfigs { return Joiner.on("\n") .join( ImmutableList.of( + "action_config {", + " config_name: 'assemble'", + " action_name: 'assemble'", + " tool {", + " tool_path: '" + gccToolPath + "'", + " }", + "}", + "action_config {", + " config_name: 'preprocess-assemble'", + " action_name: 'preprocess-assemble'", + " tool {", + " tool_path: '" + gccToolPath + "'", + " }", + "}", "action_config {", " config_name: 'c-compile'", " action_name: 'c-compile'", @@ -72,6 +86,34 @@ public class CppActionConfigs { " }", "}", "action_config {", + " config_name: 'c++-header-parsing'", + " action_name: 'c++-header-parsing'", + " tool {", + " tool_path: '" + gccToolPath + "'", + " }", + "}", + "action_config {", + " config_name: 'c++-header-preprocessing'", + " action_name: 'c++-header-preprocessing'", + " tool {", + " tool_path: '" + gccToolPath + "'", + " }", + "}", + "action_config {", + " config_name: 'c++-module-compile'", + " action_name: 'c++-module-compile'", + " tool {", + " tool_path: '" + gccToolPath + "'", + " }", + "}", + "action_config {", + " config_name: 'c++-module-codegen'", + " action_name: 'c++-module-codegen'", + " tool {", + " tool_path: '" + gccToolPath + "'", + " }", + "}", + "action_config {", " config_name: 'c++-link-executable'", " action_name: 'c++-link-executable'", " tool {", @@ -243,6 +285,21 @@ public class CppActionConfigs { " }", "}", "feature {", + " name: 'preprocessor_defines'", + " flag_set {", + " action: 'preprocess-assemble'", + " action: 'c-compile'", + " action: 'c++-compile'", + " action: 'c++-header-parsing'", + " action: 'c++-header-preprocessing'", + " action: 'c++-module-compile'", + " action: 'clif-match'", + " flag_group {", + " flag: '-D%{preprocessor_defines}'", + " }", + " }", + "}", + "feature {", " name: 'runtime_library_search_directories',", " flag_set {", " expand_if_all_available: 'runtime_library_search_directories'", @@ -467,6 +524,28 @@ public class CppActionConfigs { " }", "}", "feature {", + " name: 'sysroot'", + " enabled: true,", + " flag_set {", + " action: 'assemble'", + " action: 'preprocess-assemble'", + " action: 'c++-header-parsing'", + " action: 'c++-header-preprocessing'", + " action: 'c-compile'", + " action: 'c++-compile'", + " action: 'c++-module-compile'", + " action: 'c++-module-codegen'", + " action: 'clif-match'", + " action: 'c++-link-interface-dynamic-library'", + " action: 'c++-link-dynamic-library'", + " action: 'c++-link-executable'", + " expand_if_all_available: 'sysroot'", + " flag_group {", + " flag: '--sysroot=%{sysroot}'", + " }", + " }", + "}", + "feature {", " name: 'fission_support'", " flag_set {", " action: 'c++-link-executable'", 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 54e8d6457c..03dd461e53 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 @@ -632,31 +632,17 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // and libc versions, you must always choose compatible ones. runtimeSysroot = defaultSysroot; - String sysrootFlag; - if (sysroot != null) { - sysrootFlag = "--sysroot=" + sysroot; - } else { - sysrootFlag = null; - } - - ImmutableList.Builder unfilteredCoptsBuilder = ImmutableList.builder(); - if (sysrootFlag != null) { - unfilteredCoptsBuilder.add(sysrootFlag); - } - unfilteredCoptsBuilder.addAll(toolchain.getUnfilteredCxxFlagList()); - unfilteredCompilerFlags = new FlagList( - unfilteredCoptsBuilder.build(), - convertOptionalOptions(toolchain.getOptionalUnfilteredCxxFlagList()), - ImmutableList.of()); + unfilteredCompilerFlags = + new FlagList( + ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()), + convertOptionalOptions(toolchain.getOptionalUnfilteredCxxFlagList()), + ImmutableList.of()); ImmutableList.Builder linkoptsBuilder = ImmutableList.builder(); linkoptsBuilder.addAll(cppOptions.linkoptList); if (cppOptions.experimentalOmitfp) { linkoptsBuilder.add("-Wl,--eh-frame-hdr"); } - if (sysrootFlag != null) { - linkoptsBuilder.add(sysrootFlag); - } this.linkOptions = linkoptsBuilder.build(); ImmutableList.Builder ltoindexoptsBuilder = ImmutableList.builder(); @@ -718,10 +704,14 @@ public class CppConfiguration extends BuildConfiguration.Fragment { for (CrosstoolConfig.MakeVariable variable : toolchain.getMakeVariableList()) { makeVariablesBuilder.put(variable.getName(), variable.getValue()); } - if (sysrootFlag != null) { + // TODO(b/36544671): Remove after crosstools have been updated to add sysroot to CC_FLAGS + if (sysroot != null) { String ccFlags = makeVariablesBuilder.get("CC_FLAGS"); - ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag; - makeVariablesBuilder.put("CC_FLAGS", ccFlags); + if (!ccFlags.contains(sysroot.getSafePathString())) { + String sysrootFlag = "--sysroot=" + sysroot.getSafePathString(); + ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag; + makeVariablesBuilder.put("CC_FLAGS", ccFlags); + } } this.additionalMakeVariables = ImmutableMap.copyOf(makeVariablesBuilder); } @@ -740,7 +730,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return result.build(); } - + private static boolean actionsAreConfigured(CToolchain toolchain) { return Iterables.any( toolchain.getActionConfigList(), @@ -915,26 +905,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { toolchainBuilder); } - if (!features.contains("preprocessor_defines")) { - TextFormat.merge( - "" - + "feature {" - + " name: 'preprocessor_defines'" - + " flag_set {" - + " action: 'preprocess-assemble'" - + " action: 'c-compile'" - + " action: 'c++-compile'" - + " action: 'c++-header-parsing'" - + " action: 'c++-header-preprocessing'" - + " action: 'c++-module-compile'" - + " action: 'clif-match'" - + " flag_group {" - + " flag: '-D%{preprocessor_defines}'" - + " }" - + " }" - + "}", - toolchainBuilder); - } if (!features.contains("include_paths")) { TextFormat.merge( "" diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 698353a105..5d78885f5a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -310,4 +310,7 @@ public class CppRuleClasses { /** A string constant for the match-clif feature. */ public static final String MATCH_CLIF = "match_clif"; + + /** A build variable for the location of the sysroot (relative to the exec path or absolute) */ + public static final String SYSROOT_VARIABLE = "sysroot"; } diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java index 30b5af1d95..cff877e2f0 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java @@ -514,7 +514,7 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { "cxx-flag-A-1", "cxx-flag-A-2", "cxx-fastbuild-flag-A-1", "cxx-fastbuild-flag-A-2"), toolchainA.getCxxOptions(NO_FEATURES)); assertEquals( - Arrays.asList("--sysroot=some", "unfiltered-flag-A-1", "unfiltered-flag-A-2"), + Arrays.asList("unfiltered-flag-A-1", "unfiltered-flag-A-2"), toolchainA.getUnfilteredCompilerOptions(NO_FEATURES)); assertEquals( Arrays.asList( 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 004347cdc9..bb2c774704 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 @@ -292,6 +292,22 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { .isEqualTo(isEnabled); } + @Test + public void testSysrootVariable() throws Exception { + AnalysisMock.get() + .ccSupport() + .setupCrosstool(mockToolsConfig, "builtin_sysroot: '/usr/local/custom-sysroot'"); + useConfiguration(); + + scratch.file("x/BUILD", "cc_binary(name = 'foo', srcs = ['a.cc'])"); + scratch.file("x/a.cc"); + + ConfiguredTarget testTarget = getConfiguredTarget("//x:foo"); + Variables testVariables = getLinkBuildVariables(testTarget, LinkTargetType.EXECUTABLE); + + assertThat(testVariables.isAvailable(CppRuleClasses.SYSROOT_VARIABLE)).isTrue(); + } + @Test public void testIsUsingFissionVariable() throws Exception { scratch.file("x/BUILD", -- cgit v1.2.3