Skip to content

Commit

Permalink
Add incompatible flags for removing legacy cc_toolchain skylark apis
Browse files Browse the repository at this point in the history
Progress on #4571.

RELNOTES: Added `--incompatible_disable_legacy_flags_cc_toolchain_api` to
deprecate legacy `cc_toolchain` Skylark API for legacy CROSSTOOL fields.
PiperOrigin-RevId: 214764514
  • Loading branch information
hlopko authored and Copybara-Service committed Sep 27, 2018
1 parent e936435 commit 7622016
Show file tree
Hide file tree
Showing 4 changed files with 215 additions and 27 deletions.
162 changes: 161 additions & 1 deletion site/docs/skylark/backward-compatibility.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ guarded behind flags in the current release:
flags](#disable-depsets-in-c-toolchain-api-in-user-flags)
* [Disallow using CROSSTOOL to select the cc_toolchain label](#disallow-using-crosstool-to-select-the-cc_toolchain-label)
* [Disallow using C++ Specific Make Variables from the configuration](#disallow-using-c-specific-make-variables-from-the-configuration)
* [Disable legacy C++ configuration API](#disable-legacy-c-configuration-api)
* [Disable legacy C++ toolchain API](#disable-legacy-c-toolchain-api)


### Dictionary concatenation
Expand Down Expand Up @@ -662,5 +664,163 @@ my_rule = rule(
* Default: `false`
* Introduced in: `0.18.0`

<!-- Add new options here -->
### Disable legacy C++ configuration API

This turns off legacy Starlark access to cc toolchain information via the
`ctx.fragments.cpp` fragment. Instead of declaring dependency on the `ctx.fragments.cpp` using the
`fragments` attribute declare a dependency on the `@bazel_tools//tools/cpp:current_cc_toolchain`
via implicit attribute named `_cc_toolchain` (see example below). Use `find_cpp_toolchain` from
`@bazel_tools//tools/cpp:toolchain_utils.bzl` to get the current C++ toolchain in the rule
implementation.

```python
# Before
def _impl(ctx):
...
ctx.fragments.cpp.compiler_options()

foo = rule(
implementation = _impl,
fragments = ["cpp"],
...
)

# After
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")

def _impl(ctx):
...
cc_toolchain = find_cpp_toolchain(ctx)
cc_toolchain.compiler_options()

foo = rule(
implementation = _impl,
attrs = {
"_cc_toolchain": attr.label(
default=Label("@bazel_tools//tools/cpp:current_cc_toolchain")
),
},
)
```

List of all legacy fields and their corresponding `cc_toolchain` alternative:

|`ctx.fragments.cpp` | `cc_toolchain` |
|---|---|
| `ar_executable` | `ar_executable()` |
| `built_in_include_directories` | `built_in_include_directories` |
| `c_options` | `c_options()` |
| `compiler` | `compiler` |
| `compiler_executable` | `compiler_executable()` |
| `compiler_options(unused_arg)` | `compiler_options()` |
| `cpu` | `cpu` |
| `cxx_options(unused_arg)` | `cxx_options()` |
| `dynamic_link_options(unused_arg, bool)` | `dynamic_link_options(bool)` |
| `fully_static_link_options(unused_arg, True)` | `fully_static_link_options(True)` |
| `ld_executable` | `ld_executable()` |
| `link_options` | `link_options_do_not_use` |
| `mostly_static_link_options(unused_arg, bool)` | `mostly_static_link_options(bool)` |
| `nm_executable` | `nm_executable()` |
| `objcopy_executable` | `objcopy_executable()` |
| `objdump_executable` | `objdump_executable()` |
| `preprocessor_executable` | `preprocessor_executable()` |
| `strip_executable` | `strip_executable()` |
| `sysroot` | `sysroot` |
| `target_gnu_system_name` | `target_gnu_system_name` |
| `unfiltered_compiler_options(unused_arg)` | `unfiltered_compiler_options(unused_arg)` |

* Flag: `--incompatible_disable_legacy_cpp_toolchain_skylark_api`
* Default: `false`
* Introduced in: `0.18.0`

### Disable legacy C++ toolchain API

We have deprecated the `cc_toolchain` Starlark API returning legacy CROSSTOOL fields:

* ar_executable
* c_options
* compiler_executable
* compiler_options
* cxx_options
* dynamic_link_options
* fully_static_link_options
* ld_executable
* link_options
* mostly_static_link_options
* nm_executable
* objcopy_executable
* objdump_executable
* preprocessor_executable
* strip_executable
* unfiltered_compiler_options

Use the new API from [cc_common](https://docs.bazel.build/versions/master/skylark/lib/cc_common.html)

```python
# Before:
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")

def _impl(ctx):
cc_toolchain = find_cc_toolchain(ctx)
compiler_options = (
cc_toolchain.compiler_options() +
cc_toolchain.unfiltered_compiler_options([]) +
["-w", "-Wno-error"]
)
link_options = (
["-shared", "-static-libgcc"] +
cc_toolchain.mostly_static_link_options(True) +
["-Wl,-whole-archive"] +
[l.path for l in libs] +
["-Wl,-no-whole-archive"] +
cc_toolchain.link_options_do_not_use
)

# After
load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain")
load(
"@bazel_tools//tools/build_defs/cc:action_names.bzl",
"CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME",
"C_COMPILE_ACTION_NAME",
)

def _impl(ctx):
cc_toolchain = find_cc_toolchain(ctx)
feature_configuration = cc_common.configure_features(
cc_toolchain = cc_toolchain,
requested_features = ctx.features,
unsupported_features = ctx.disabled_features,
)
compile_variables = cc_common.create_compile_variables(
feature_configuration = feature_configuration,
cc_toolchain = cc_toolchain,
user_compile_flags = depset(["-w", "-Wno-error"]),
)
compiler_options = cc_common.get_memory_inefficient_command_line(
feature_configuration = feature_configuration,
action_name = C_COMPILE_ACTION_NAME,
variables = compile_variables,
)

link_variables = cc_common.create_link_variables(
feature_configuration = feature_configuration,
cc_toolchain = cc_toolchain,
is_linking_dynamic_library = True,
user_link_flags =
["-static-libgcc"] +
["-Wl,-whole-archive"] +
[lib.path for lib in libs] +
["-Wl,-no-whole-archive"],
)
link_flags = cc_common.get_memory_inefficient_command_line(
feature_configuration = feature_configuration,
action_name = CPP_LINK_DYNAMIC_LIBRARY_ACTION_NAME,
variables = link_variables,
)
```

* Flag: `--incompatible_disable_legacy_flags_cc_toolchain_api`
* Default: `false`
* Introduced in: `0.19.0`

<!-- Add new options here -->
Original file line number Diff line number Diff line change
Expand Up @@ -1212,25 +1212,34 @@ private void checkForToolchainSkylarkApiAvailability() throws EvalException {
null,
"Information about the C++ toolchain API is not accessible "
+ "anymore through ctx.fragments.cpp "
+ "(See --incompatible_disable_legacy_cpp_toolchain_skylark_api). "
+ "(see --incompatible_disable_legacy_cpp_toolchain_skylark_api on "
+ "http://docs.bazel.build/versions/master/skylark/backward-compatibility.html"
+ "#disable-legacy-c-configuration-api for migration notes). "
+ "Use CcToolchainInfo instead.");
}
}

public void checkForLegacyCompilationApiAvailability() throws EvalException {
if (cppOptions.disableLegacyCompilationApi) {
if (cppOptions.disableLegacyCompilationApi || cppOptions.disableLegacyFlagsCcToolchainApi) {
throw new EvalException(
null,
"Skylark APIs accessing compilation flags has been removed. "
+ "Use the new API on cc_common.");
+ "Use the new API on cc_common (see "
+ "--incompatible_disable_legacy_flags_cc_toolchain_api on"
+ "https://docs.bazel.build/versions/master/skylark/backward-compatibility.html"
+ "#disable-legacy-c-toolchain-api for migration notes).");
}
}

public void checkForLegacyLinkingApiAvailability() throws EvalException {
if (cppOptions.disableLegacyLinkingApi) {
if (cppOptions.disableLegacyLinkingApi || cppOptions.disableLegacyFlagsCcToolchainApi) {
throw new EvalException(
null,
"Skylark APIs accessing linking flags has been removed. Use the new API on cc_common.");
"Skylark APIs accessing linking flags has been removed. "
+ "Use the new API on cc_common (see "
+ "--incompatible_disable_legacy_flags_cc_toolchain_api on"
+ "https://docs.bazel.build/versions/master/skylark/backward-compatibility.html"
+ "#disable-legacy-c-toolchain-api for migration notes).");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,7 @@ public Label getFdoPrefetchHintsLabel() {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help = "Flag for disabling access to the C++ toolchain API through the ctx.fragments.cpp .")
help = "Flag for disabling access to the C++ toolchain API through the ctx.fragments.cpp.")
public boolean disableLegacyToolchainSkylarkApi;

@Option(
Expand All @@ -797,6 +797,20 @@ public Label getFdoPrefetchHintsLabel() {
help = "If true, C++ Skylark API exposing compilation flags will be disabled.")
public boolean disableLegacyCompilationApi;

@Option(
name = "incompatible_disable_legacy_flags_cc_toolchain_api",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"Flag for disabling the legacy cc_toolchain Skylark API for accessing legacy "
+ "CROSSTOOL fields.")
public boolean disableLegacyFlagsCcToolchainApi;

@Option(
name = "experimental_disable_linking_mode_flags",
defaultValue = "false",
Expand All @@ -822,7 +836,7 @@ public Label getFdoPrefetchHintsLabel() {
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
help =
"If true, Bazel will not read crosstool flags from legacy crosstool fields (see #5187.")
"If true, Bazel will not read crosstool flags from legacy crosstool fields (see #5187).")
public boolean disableLegacyCrosstoolFields;

@Option(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.devtools.build.lib.rules.cpp;

import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
Expand Down Expand Up @@ -145,7 +144,13 @@ public void testDisablingLinkOptionsFromConfiguration() throws Exception {

private void testDisablingLinkingApiMethod(String method) throws Exception {
useConfiguration("--experimental_disable_legacy_cc_linking_api");
scratch.file(
testDisablingLinkingApiMethodWithConfiguration(method);
useConfiguration("--incompatible_disable_legacy_flags_cc_toolchain_api");
testDisablingLinkingApiMethodWithConfiguration(method);
}

private void testDisablingLinkingApiMethodWithConfiguration(String method) throws Exception {
scratch.overwriteFile(
"test/rule.bzl",
"def _impl(ctx):",
" provider = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]",
Expand All @@ -158,18 +163,15 @@ private void testDisablingLinkingApiMethod(String method) throws Exception {
" fragments = [ 'cpp' ],",
" attrs = {'_cc_toolchain': attr.label(default=Label('//test:toolchain')) }",
")");
scratch.file(
scratch.overwriteFile(
"test/BUILD",
"load(':rule.bzl', 'my_rule')",
"cc_toolchain_alias(name = 'toolchain')",
"my_rule(name = 'target')");
AssertionError e =
assertThrows(AssertionError.class, () -> getConfiguredTarget("//test:target"));
assertThat(e)
.hasMessageThat()
.contains(
"Skylark APIs accessing linking flags has been removed. "
+ "Use the new API on cc_common.");
invalidatePackages();
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test:target");
assertContainsEvent("Skylark APIs accessing linking flags has been removed.");
}

@Test
Expand Down Expand Up @@ -214,7 +216,13 @@ public void testDisablingUnfilteredOptionsFromConfiguration() throws Exception {

private void testDisablingCompilationApiMethod(String method) throws Exception {
useConfiguration("--experimental_disable_legacy_cc_compilation_api");
scratch.file(
testDisablingCompilationApiMethodWithConfiguration(method);
useConfiguration("--incompatible_disable_legacy_flags_cc_toolchain_api");
testDisablingCompilationApiMethodWithConfiguration(method);
}

private void testDisablingCompilationApiMethodWithConfiguration(String method) throws Exception {
scratch.overwriteFile(
"test/rule.bzl",
"def _impl(ctx):",
" provider = ctx.attr._cc_toolchain[cc_common.CcToolchainInfo]",
Expand All @@ -227,18 +235,15 @@ private void testDisablingCompilationApiMethod(String method) throws Exception {
" fragments = [ 'cpp' ],",
" attrs = {'_cc_toolchain': attr.label(default=Label('//test:toolchain')) }",
")");
scratch.file(
scratch.overwriteFile(
"test/BUILD",
"load(':rule.bzl', 'my_rule')",
"cc_toolchain_alias(name = 'toolchain')",
"my_rule(name = 'target')");
AssertionError e =
assertThrows(AssertionError.class, () -> getConfiguredTarget("//test:target"));
assertThat(e)
.hasMessageThat()
.contains(
"Skylark APIs accessing compilation flags has been removed. "
+ "Use the new API on cc_common.");
invalidatePackages();
reporter.removeHandler(failFastHandler);
getConfiguredTarget("//test:target");
assertContainsEvent("Skylark APIs accessing compilation flags has been removed.");
}

@Test
Expand Down

0 comments on commit 7622016

Please sign in to comment.