diff options
7 files changed, 48 insertions, 98 deletions
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 de55d33b12..92902bcac7 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<String, String> getLocalBuildVariables(RuleContext ruleContext) + protected Map<String, String> getBuildVariables(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 55b1190049..73060ef00f 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,33 +354,14 @@ public class CcToolchain implements RuleConfiguredTargetFactory { } /** - * Returns a map that should be templated into the crosstool as build variables + * 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 final Map<String, String> getBuildVariables(RuleContext ruleContext) - throws RuleErrorException { - CppConfiguration cppConfiguration = ruleContext.getFragment(CppConfiguration.class); - ImmutableMap.Builder<String, String> 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<String, String> getLocalBuildVariables(RuleContext ruleContext) + protected Map<String, String> getBuildVariables(RuleContext ruleContext) throws RuleErrorException { return ImmutableMap.<String, String>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 c24dc4b402..ae53fc1d46 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 @@ -63,7 +63,6 @@ public class CppActionConfigs { " tool {", " tool_path: '" + gccToolPath + "'", " }", - " implies: 'sysroot'", "}", "action_config {", " config_name: 'c++-compile'", @@ -71,15 +70,6 @@ public class CppActionConfigs { " tool {", " tool_path: '" + gccToolPath + "'", " }", - " implies: 'sysroot'", - "}", - "action_config {", - " config_name: 'c++-module-compile'", - " action_name: 'c++-module-compile'", - " tool {", - " tool_path: '" + gccToolPath + "'", - " }", - " implies: 'sysroot'", "}", "action_config {", " config_name: 'c++-link-executable'", @@ -89,7 +79,6 @@ public class CppActionConfigs { " }", " implies: 'symbol_counts'", " implies: 'strip_debug_symbols'", - " implies: 'sysroot'", " implies: 'linkstamps'", " implies: 'output_execpath_flags_executable'", " implies: 'runtime_library_search_directories'", @@ -111,7 +100,6 @@ public class CppActionConfigs { " implies: 'symbol_counts'", " implies: 'strip_debug_symbols'", " implies: 'shared_flag'", - " implies: 'sysroot'", " implies: 'linkstamps'", " implies: 'output_execpath_flags'", " implies: 'runtime_library_search_directories'", @@ -255,21 +243,6 @@ 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'", @@ -494,21 +467,6 @@ public class CppActionConfigs { " }", "}", "feature {", - " name: 'sysroot'", - " flag_set {", - " action: 'c-compile'", - " action: 'c++-compile'", - " action: 'c++-module-compile'", - " 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 8842f99b58..f658f6732e 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,17 +632,31 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // and libc versions, you must always choose compatible ones. runtimeSysroot = defaultSysroot; - unfilteredCompilerFlags = - new FlagList( - ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()), - convertOptionalOptions(toolchain.getOptionalUnfilteredCxxFlagList()), - ImmutableList.<String>of()); + String sysrootFlag; + if (sysroot != null) { + sysrootFlag = "--sysroot=" + sysroot; + } else { + sysrootFlag = null; + } + + ImmutableList.Builder<String> unfilteredCoptsBuilder = ImmutableList.builder(); + if (sysrootFlag != null) { + unfilteredCoptsBuilder.add(sysrootFlag); + } + unfilteredCoptsBuilder.addAll(toolchain.getUnfilteredCxxFlagList()); + unfilteredCompilerFlags = new FlagList( + unfilteredCoptsBuilder.build(), + convertOptionalOptions(toolchain.getOptionalUnfilteredCxxFlagList()), + ImmutableList.<String>of()); ImmutableList.Builder<String> 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<String> ltoindexoptsBuilder = ImmutableList.builder(); @@ -704,14 +718,10 @@ public class CppConfiguration extends BuildConfiguration.Fragment { for (CrosstoolConfig.MakeVariable variable : toolchain.getMakeVariableList()) { makeVariablesBuilder.put(variable.getName(), variable.getValue()); } - // TODO(b/36544671): Remove after crosstools have been updated to add sysroot to CC_FLAGS - if (sysroot != null) { + if (sysrootFlag != null) { String ccFlags = makeVariablesBuilder.get("CC_FLAGS"); - if (!ccFlags.contains(sysroot.getSafePathString())) { - String sysrootFlag = "--sysroot=" + sysroot.getSafePathString(); - ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag; - makeVariablesBuilder.put("CC_FLAGS", ccFlags); - } + ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag; + makeVariablesBuilder.put("CC_FLAGS", ccFlags); } this.additionalMakeVariables = ImmutableMap.copyOf(makeVariablesBuilder); } @@ -730,7 +740,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return result.build(); } - + private static boolean actionsAreConfigured(CToolchain toolchain) { return Iterables.any( toolchain.getActionConfigList(), @@ -905,6 +915,26 @@ 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 5d78885f5a..698353a105 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,7 +310,4 @@ 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 bf59a7e62e..1fd7a011ad 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("unfiltered-flag-A-1", "unfiltered-flag-A-2"), + Arrays.asList("--sysroot=some", "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 bb2c774704..004347cdc9 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 @@ -293,22 +293,6 @@ public class LinkBuildVariablesTest extends BuildViewTestCase { } @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", "cc_binary(name = 'foo', srcs = ['foo.cc'])"); |