Automated g4 rollback of commit e7c730ba64d1c34ac7049f4165a33dc1329d0019.
*** Reason for rollback ***
breaks tests
*** Original change description ***
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: 153351147
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 de55d33..92902bc 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 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 3d5c922..9cb154f 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 @@
}
/**
- * 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 c70e3f3..ae53fc1 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
@@ -58,20 +58,6 @@
.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'",
" tool {",
@@ -86,34 +72,6 @@
" }",
"}",
"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 {",
@@ -285,21 +243,6 @@
" }",
"}",
"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'",
@@ -524,28 +467,6 @@
" }",
"}",
"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 03dd461..54e8d64 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 @@
// 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 @@
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 @@
return result.build();
}
-
+
private static boolean actionsAreConfigured(CToolchain toolchain) {
return Iterables.any(
toolchain.getActionConfigList(),
@@ -905,6 +915,26 @@
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 5d78885..698353a 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 @@
/** 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 cff877e..30b5af1 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 @@
"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 bb2c774..004347c 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 @@
}
@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'])");
