Change in bazel/bazel[master]: Add incompatible changes for legacy crosstool fields

1 view
Skip to first unread message

Copybara (Gerrit)

unread,
Aug 14, 2018, 3:51:01 AM8/14/18
to Marcel Hlopko

Copybara has uploaded this change for review.

View Change

Add incompatible changes for legacy crosstool fields

RELNOTES: None.
PiperOrigin-RevId: 208059848
Change-Id: I8220846a3f69d8ea82570c16fd74382d6b99e4d1
---
M site/docs/skylark/backward-compatibility.md
M src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
M src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
M src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
4 files changed, 98 insertions(+), 16 deletions(-)

diff --git a/site/docs/skylark/backward-compatibility.md b/site/docs/skylark/backward-compatibility.md
index 6c27fa6..2aeafba 100644
--- a/site/docs/skylark/backward-compatibility.md
+++ b/site/docs/skylark/backward-compatibility.md
@@ -46,6 +46,9 @@
* [Remove native http archive](#remove-native-http-archive)
* [New-style JavaInfo constructor](#new-style-java_info)
* [Disallow tools in action inputs](#disallow-tools-in-action-inputs)
+* [Disable legacy Skylark API for C++ toolchain](#disable-legacy-skylark-api-for-c++-toolchain)
+* [Disable legacy CROSSTOOL fields](#disable-legacy-crosstool-fields)
+


### Dictionary concatenation
@@ -398,3 +401,57 @@
* Default: `false`

<!-- Add new options here -->
+
+### Disable legacy Skylark API for C++ toolchain
+
+When the flag is set to true, the legacy API for getting C++ compilation and linking flags will be
+disabled. The only supported API will be
+[FeatureConfiguration-based](https://docs.google.com/document/d/1g91BWJITcYw_X-VxsDC0VgUn5E9g0kRBGoBSpoO41gA/edit#heading=h.lpocy537zjgc).
+
+So instead of:
+
+```
+executable = cc_toolchain.compiler_executable
+flags = cc_toolchain.compiler_options() +
+ cc_toolchain.unfiltered_compiler_options([])
+```
+
+use:
+
+```
+load(
+ "@bazel_tools//tools/build_defs/cc:action_names.bzl",
+ "CPP_COMPILE_ACTION_NAME")
+
+...
+
+feature_configuration = cc_common.configure_features(
+ cc_toolchain = cc_toolchain,
+ requested_features = ctx.features + ["foo"],
+ unsupported_features = ctx.disabled_features + ["bar"])
+variables = cc_common.create_compile_build_variables(
+ feature_configuration = feature_configuration,
+ cc_toolchain = cc_toolchain,
+ user_compile_flags = ["-w", "-Wno-error"])
+executable = cc_common.get_tool_for_action(
+ feature_configuration = feature_configuration,
+ action_name = CPP_COMPILE_ACTION_NAME,
+)
+flags = cc_common.get_memory_inefficient_command_line(
+ feature_configuration = feature_configuration,
+ action_name = CPP_COMPILE_ACTION_NAME,
+ variables = variables)
+```
+
+* Flag: `--incompatible_disable_skylark_api_for_legacy_crosstool_fields`
+* Default: `false`
+
+### Disable legacy CROSSTOOL fields
+
+When this flag is set to true, the legacy CROSSTOOL fields (such as
+`compiler_flag` or `linker_flag`) will be removed. Then the only way to express
+flags is using `action_config`s and `feature`s. See [documentation tracking
+issue](https://github.com/bazelbuild/bazel/issues/5847).
+
+* Flag: `--incompatible_disable_legacy_crosstool_fields`
+* Default: `false`
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 10b65c1..0005ecd 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
@@ -97,11 +97,11 @@
}

public boolean disableCompilationModeFlags() {
- return cppOptions.disableCompilationModeFlags;
+ return cppOptions.disableCompilationModeFlags || cppOptions.disableLegacyCrosstoolFields;
}

public boolean disableLinkingModeFlags() {
- return cppOptions.disableLinkingModeFlags;
+ return cppOptions.disableLinkingModeFlags || cppOptions.disableLegacyCrosstoolFields;
}

public boolean enableLinkoptsInUserLinkFlags() {
@@ -251,8 +251,8 @@
params.ccToolchainLabel,
params.ccToolchainConfigInfo,
cppOptions.disableLegacyCrosstoolFields,
- cppOptions.disableCompilationModeFlags,
- cppOptions.disableLinkingModeFlags);
+ cppOptions.disableCompilationModeFlags || cppOptions.disableLegacyCrosstoolFields,
+ cppOptions.disableLinkingModeFlags || cppOptions.disableLegacyCrosstoolFields);

CompilationMode compilationMode = params.commonOptions.compilationMode;

@@ -1207,26 +1207,27 @@
}

private void checkForToolchainSkylarkApiAvailability() throws EvalException {
- if (!cppOptions.enableLegacyToolchainSkylarkApi) {
+ if (!cppOptions.enableLegacyCppConfigurationApi || cppOptions.disableLegacyToolchainApi) {
throw new EvalException(null, "Information about the C++ toolchain API is not accessible "
+ "anymore through ctx.fragments.cpp . Use CcToolchainInfo instead.");
}
}

public void checkForLegacyCompilationApiAvailability() throws EvalException {
- if (cppOptions.disableLegacyCompilationApi) {
+ if (cppOptions.disableLegacyCompilationApi || cppOptions.disableLegacyToolchainApi) {
throw new EvalException(
null,
- "Skylark APIs accessing compilation flags has been removed. "
+ "Skylark APIs accessing legacy compilation flags has been removed. "
+ "Use the new API on cc_common.");
}
}

public void checkForLegacyLinkingApiAvailability() throws EvalException {
- if (cppOptions.disableLegacyLinkingApi) {
+ if (cppOptions.disableLegacyLinkingApi || cppOptions.disableLegacyToolchainApi) {
throw new EvalException(
null,
- "Skylark APIs accessing linking flags has been removed. Use the new API on cc_common.");
+ "Skylark APIs accessing legacy linking flags has been removed. "
+ + "Use the new API on cc_common.");
}
}

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 a5b9ebb..5178d79 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
@@ -778,10 +778,11 @@
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
- OptionMetadataTag.INCOMPATIBLE_CHANGE,
- OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES},
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
help = "Flag for disabling access to the C++ toolchain API through the ctx.fragments.cpp .")
- public boolean enableLegacyToolchainSkylarkApi;
+ public boolean enableLegacyCppConfigurationApi;

@Option(
name = "experimental_disable_legacy_cc_linking_api",
@@ -802,6 +803,15 @@
public boolean disableLegacyCompilationApi;

@Option(
+ name = "incompatible_disable_skylark_api_for_legacy_crosstool_fields",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ metadataTags = {OptionMetadataTag.EXPERIMENTAL},
+ help = "Flag for disabling access to the legacy C++ toolchain API.")
+ public boolean disableLegacyToolchainApi;
+
+ @Option(
name = "experimental_disable_linking_mode_flags",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
@@ -820,7 +830,7 @@
public boolean disableCompilationModeFlags;

@Option(
- name = "experimental_disable_legacy_crosstool_fields",
+ name = "incompatible_disable_legacy_crosstool_fields",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
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 9264059..9d2db93 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
@@ -251,7 +251,7 @@
assertThat(e)
.hasMessageThat()
.contains(
- "Skylark APIs accessing linking flags has been removed. "
+ "Skylark APIs accessing legacy linking flags has been removed. "
+ "Use the new API on cc_common.");
}

@@ -320,7 +320,7 @@
assertThat(e)
.hasMessageThat()
.contains(
- "Skylark APIs accessing compilation flags has been removed. "
+ "Skylark APIs accessing legacy compilation flags has been removed. "
+ "Use the new API on cc_common.");
}

@@ -350,6 +350,15 @@
.doesNotContain("-bar_from_compilation_mode");
assertThat(ccToolchainProvider.getLegacyMostlyStaticLinkFlags(CompilationMode.OPT))
.doesNotContain("-baz_from_compilation_mode");
+
+ useConfiguration("-c", "opt", "--incompatible_disable_legacy_crosstool_fields");
+ ccToolchainProvider = getCcToolchainProvider();
+ assertThat(ccToolchainProvider.getCompilerOptions())
+ .doesNotContain("-foo_from_compilation_mode");
+ assertThat(ccToolchainProvider.getLegacyCxxOptions())
+ .doesNotContain("-bar_from_compilation_mode");
+ assertThat(ccToolchainProvider.getLegacyMostlyStaticLinkFlags(CompilationMode.OPT))
+ .doesNotContain("-baz_from_compilation_mode");
}

private CcToolchainProvider getCcToolchainProvider() throws Exception {
@@ -376,6 +385,11 @@
ccToolchainProvider = getCcToolchainProvider();
assertThat(ccToolchainProvider.getLegacyMostlyStaticLinkFlags(CompilationMode.OPT))
.doesNotContain("-foo_from_linking_mode");
+
+ useConfiguration("--incompatible_disable_legacy_crosstool_fields");
+ ccToolchainProvider = getCcToolchainProvider();
+ assertThat(ccToolchainProvider.getLegacyMostlyStaticLinkFlags(CompilationMode.OPT))
+ .doesNotContain("-foo_from_linking_mode");
}

@Test
@@ -403,7 +417,7 @@
.contains("-foo_dynamic_library_linker_flag");
assertThat(ccToolchainProvider.getTestOnlyLinkOptions()).contains("-foo_test_only_linker_flag");

- useConfiguration("--experimental_disable_legacy_crosstool_fields");
+ useConfiguration("--incompatible_disable_legacy_crosstool_fields");
ccToolchainProvider = getCcToolchainProvider();
assertThat(ccToolchainProvider.getLegacyCompileOptions()).doesNotContain("-foo_compiler_flag");
assertThat(ccToolchainProvider.getLegacyCxxOptions()).doesNotContain("-foo_cxx_flag");

To view, visit change 70351. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Change-Id: I8220846a3f69d8ea82570c16fd74382d6b99e4d1
Gerrit-Change-Number: 70351
Gerrit-PatchSet: 1
Gerrit-Owner: Copybara
Gerrit-Reviewer: Marcel Hlopko <hlo...@google.com>
Gerrit-MessageType: newchange

Copybara (Gerrit)

unread,
Aug 14, 2018, 5:38:42 AM8/14/18
to Marcel Hlopko

Copybara uploaded patch set #2 to this change.

View Change

Add incompatible changes for legacy crosstool fields

Stage 2 from #5883.


RELNOTES: None.
PiperOrigin-RevId: 208059848
Change-Id: I8220846a3f69d8ea82570c16fd74382d6b99e4d1
---
M site/docs/skylark/backward-compatibility.md
M src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
M src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
M src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProviderTest.java
4 files changed, 105 insertions(+), 17 deletions(-)

To view, visit change 70351. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: bazel
Gerrit-Branch: master
Gerrit-Change-Id: I8220846a3f69d8ea82570c16fd74382d6b99e4d1
Gerrit-Change-Number: 70351
Gerrit-PatchSet: 2
Gerrit-Owner: Copybara
Gerrit-Reviewer: Marcel Hlopko <hlo...@google.com>
Gerrit-MessageType: newpatchset
Reply all
Reply to author
Forward
0 new messages