Also move default_compile_flags to the top of the crosstool
Currently we move legacy_compile_flags to the top of the crosstool, and the same behavior is needed for migrated crosstools in case of default_compile_flags.
https://github.com/bazelbuild/bazel/issues/6861
https://github.com/bazelbuild/bazel/issues/5883
RELNOTES: None.
PiperOrigin-RevId: 229339668
diff --git a/site/docs/crosstool-reference.md b/site/docs/crosstool-reference.md
index 59e60ed..5ae1cd0 100644
--- a/site/docs/crosstool-reference.md
+++ b/site/docs/crosstool-reference.md
@@ -1043,4 +1043,75 @@
be enabled, `--force_pic` cannot be used.
</td>
</tr>
+ <tr>
+ <td>
+ <strong><code>static_linking_mode | dynamic_linking_mode</code></strong>
+ </td>
+ <td>Enabled by default based on linking mode.</td>
+ </tr>
+ <tr>
+ <td><strong><code>no_legacy_features</code></strong>
+ </td>
+ <td>
+ Prevents Bazel from adding legacy features to
+ the CROSSTOOL configuration when present. See the complete list of
+ features below.
+ </td>
+ </tr>
</table>
+
+#### Legacy features patching logic
+
+<p>
+ Bazel does following changes to the features and their order to stay backwards
+ compatible:
+
+ <ul>
+ <li>Moves <code>legacy_compile_flags</code> feature to the top of the toolchain</li>
+ <li>Moves <code>default_compile_flags</code> feature to the top of the toolchain</li>
+ <li>Adds <code>dependency_file</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>pic</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>per_object_debug_info</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>preprocessor_defines</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>includes</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>include_paths</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>fdo_instrument</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>fdo_optimize</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>fdo_prefetch_hints</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>autofdo</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>build_interface_libraries</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>dynamic_library_linker_tool</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>symbol_counts</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>shared_flag</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>linkstamps</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>output_execpath_flags</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>runtime_library_search_directories</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>library_search_directories</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>archiver_flags</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>libraries_to_link</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>force_pic_flags</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>user_link_flags</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>legacy_link_flags</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>static_libgcc</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>fission_support</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>strip_debug_symbols</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>coverage</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>llvm_coverage_map_format</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>gcc_coverage_map_format</code> (if not present) feature to the top of the toolchain</li>
+ <li>Adds <code>fully_static_link</code> (if not present) feature to the bottom of the toolchain</li>
+ <li>Adds <code>user_compile_flags</code> (if not present) feature to the bottom of the toolchain</li>
+ <li>Adds <code>sysroot</code> (if not present) feature to the bottom of the toolchain</li>
+ <li>Adds <code>unfiltered_compile_flags</code> (if not present) feature to the bottom of the toolchain</li>
+ <li>Adds <code>linker_param_file</code> (if not present) feature to the bottom of the toolchain</li>
+ <li>Adds <code>compiler_input_flags</code> (if not present) feature to the bottom of the toolchain</li>
+ <li>Adds <code>compiler_output_flags</code> (if not present) feature to the bottom of the toolchain</li>
+ </ul>
+</p>
+
+This is a long list of features. The plan is to get rid of them once
+[Crosstool in Starlark](https://github.com/bazelbuild/bazel/issues/5380) is
+done. For the curious reader see the implementation in
+[CppActionConfigs](https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/cpp/CppActionConfigs.java?q=cppactionconfigs&ss=bazel),
+and for production toolchains consider adding `no_legacy_features` to make
+the toolchain more standalone.
+
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
index 717cd0f..301d3d7 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -887,11 +887,24 @@
// compile command line. 'legacy_compile_flags' feature contains all these flags, and so it
// needs to appear before other features from {@link CppActionConfigs}.
if (featureNames.contains(CppRuleClasses.LEGACY_COMPILE_FLAGS)) {
- legacyFeaturesBuilder.add(
+ Feature legacyCompileFlags =
featureList.stream()
.filter(feature -> feature.getName().equals(CppRuleClasses.LEGACY_COMPILE_FLAGS))
.findFirst()
- .get());
+ .get();
+ if (legacyCompileFlags != null) {
+ legacyFeaturesBuilder.add(legacyCompileFlags);
+ }
+ }
+ if (featureNames.contains(CppRuleClasses.DEFAULT_COMPILE_FLAGS)) {
+ Feature defaultCompileFlags =
+ featureList.stream()
+ .filter(feature -> feature.getName().equals(CppRuleClasses.DEFAULT_COMPILE_FLAGS))
+ .findFirst()
+ .get();
+ if (defaultCompileFlags != null) {
+ legacyFeaturesBuilder.add(defaultCompileFlags);
+ }
}
CppPlatform platform = targetLibc.equals("macos") ? CppPlatform.MAC : CppPlatform.LINUX;
@@ -907,6 +920,7 @@
legacyFeaturesBuilder.addAll(
featureList.stream()
.filter(feature -> !feature.getName().equals(CppRuleClasses.LEGACY_COMPILE_FLAGS))
+ .filter(feature -> !feature.getName().equals(CppRuleClasses.DEFAULT_COMPILE_FLAGS))
.collect(ImmutableList.toImmutableList()));
for (CToolchain.Feature feature :
CppActionConfigs.getFeaturesToAppearLastInFeaturesList(featureNames)) {
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 d806d6b..ee53df9 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
@@ -224,6 +224,13 @@
public static final String LEGACY_COMPILE_FLAGS = "legacy_compile_flags";
/**
+ * A string constant for the default_compile_flags feature. If this feature is present in the
+ * toolchain, and the toolchain doesn't specify no_legacy_features, bazel will move
+ * default_compile_flags before other features from {@link CppActionConfigs}.
+ */
+ public static final String DEFAULT_COMPILE_FLAGS = "default_compile_flags";
+
+ /**
* A string constant for the feature that makes us build per-object debug info files.
*/
public static final String PER_OBJECT_DEBUG_INFO = "per_object_debug_info";
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 226e46a..c19831d 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
@@ -405,22 +405,32 @@
}
}
- // TODO(b/30109612): Remove fragile legacyCompileFlags shuffle once there are no legacy
- // crosstools.
- // Existing projects depend on flags from legacy toolchain fields appearing first on the
- // compile command line. 'legacy_compile_flags' feature contains all these flags, and so it
- // needs to appear before other features from {@link CppActionConfigs}.
- CToolchain.Feature legacyCompileFlagsFeature =
- toolchain
- .getFeatureList()
- .stream()
+ // TODO(b/30109612): Remove fragile legacyCompileFlags shuffle once there are no legacy
+ // crosstools.
+ // Existing projects depend on flags from legacy toolchain fields appearing first on the
+ // compile command line. 'legacy_compile_flags' feature contains all these flags, and so it
+ // needs to appear before other features from {@link CppActionConfigs}.
+ if (featureNames.contains(CppRuleClasses.LEGACY_COMPILE_FLAGS)) {
+ CToolchain.Feature legacyCompileFlags =
+ toolchain.getFeatureList().stream()
.filter(feature -> feature.getName().equals(CppRuleClasses.LEGACY_COMPILE_FLAGS))
.findFirst()
- .orElse(null);
- if (legacyCompileFlagsFeature != null) {
- toolchainBuilder.addFeature(legacyCompileFlagsFeature);
- toolchain = removeLegacyCompileFlagsFeatureFromToolchain(toolchain);
+ .get();
+ if (legacyCompileFlags != null) {
+ toolchainBuilder.addFeature(legacyCompileFlags);
}
+ }
+ if (featureNames.contains(CppRuleClasses.DEFAULT_COMPILE_FLAGS)) {
+ CToolchain.Feature defaultCompileFlags =
+ toolchain.getFeatureList().stream()
+ .filter(feature -> feature.getName().equals(CppRuleClasses.DEFAULT_COMPILE_FLAGS))
+ .findFirst()
+ .get();
+ if (defaultCompileFlags != null) {
+ toolchainBuilder.addFeature(defaultCompileFlags);
+ }
+ }
+ toolchain = removeSpecialFeatureFromToolchain(toolchain);
CppPlatform platform =
toolchain.getTargetLibc().equals("macosx") ? CppPlatform.MAC : CppPlatform.LINUX;
@@ -452,16 +462,15 @@
return toolchainBuilder.build();
}
- private static CToolchain removeLegacyCompileFlagsFeatureFromToolchain(CToolchain toolchain) {
+ private static CToolchain removeSpecialFeatureFromToolchain(CToolchain toolchain) {
FieldDescriptor featuresFieldDescriptor = CToolchain.getDescriptor().findFieldByName("feature");
return toolchain
.toBuilder()
.setField(
featuresFieldDescriptor,
- toolchain
- .getFeatureList()
- .stream()
+ toolchain.getFeatureList().stream()
.filter(feature -> !feature.getName().equals(CppRuleClasses.LEGACY_COMPILE_FLAGS))
+ .filter(feature -> !feature.getName().equals(CppRuleClasses.DEFAULT_COMPILE_FLAGS))
.collect(ImmutableList.toImmutableList()))
.build();
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
index aa79138..38d12c4 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/SkylarkCcCommonTest.java
@@ -4267,6 +4267,7 @@
" feature(name = 'custom_feature'),",
" feature(name = 'legacy_compile_flags'),",
" feature(name = 'fdo_optimize'),",
+ " feature(name = 'default_compile_flags'),",
" ],",
" action_configs = [action_config(action_name = 'custom-action')],",
" artifact_name_patterns = [artifact_name_pattern(",
@@ -4307,10 +4308,11 @@
.collect(ImmutableSet.toImmutableSet());
// fdo_optimize should not be re-added to the list of features by legacy behavior
assertThat(featureNames).containsNoDuplicates();
- // legacy_compile_flags should appear first in the list of features
- assertThat(featureNames.get(0)).isEqualTo("legacy_compile_flags");
+ // legacy_compile_flags should appear first in the list of features, followed by
+ // default_compile_flags.
assertThat(featureNames)
- .containsAllOf("legacy_compile_flags", "custom_feature", "fdo_optimize")
+ .containsAllOf(
+ "legacy_compile_flags", "default_compile_flags", "custom_feature", "fdo_optimize")
.inOrder();
// assemble is one of the action_configs added as a legacy behavior, therefore it needs to be
// prepended to the action configs defined by the user.
