From eb5f6e5f5e4eabba054557c521a0bef979528265 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Tue, 27 Jul 2021 10:54:17 -0700 Subject: [PATCH 01/16] Add the `swift_interop_hint` rule to be used with `aspect_hints`. The new `aspect_hints` attribute in Bazel, available on all rules, lets us attach arbitrary providers to targets that `swift_clang_module_aspect` can read. The `swift_interop_hint` rule uses this to provide the module name for whichever target references it in its `aspect_hints`. A canonical auto-deriving hint is also provided as part of the Swift build rules (in `.../swift:auto_module`), so that the default/recommended behavior of deriving a module name from the target label can be easily obtained without having to declare one's own `swift_module_hint` targets. This is a more principled approach to handling Swift interop than the current `tags` implementation (which will be removed in a future change), and lets us associate additional metadata easily in the future, including files (for example, custom module maps or APINotes). PiperOrigin-RevId: 387147846 (cherry picked from commit c42a37a025b51d48ee9efff0ec5f06ad29003b92) --- doc/BUILD | 1 + doc/README.md | 2 +- doc/doc.bzl | 2 + doc/rules.md | 81 ++++++++++++++ examples/xplatform/c_from_swift/BUILD | 19 +++- swift/BUILD | 9 ++ swift/internal/BUILD | 7 ++ swift/internal/swift_clang_module_aspect.bzl | 59 ++++++++++- swift/internal/swift_interop_hint.bzl | 106 +++++++++++++++++++ swift/swift.bzl | 5 + test/fixtures/private_deps/BUILD | 6 +- 11 files changed, 289 insertions(+), 8 deletions(-) create mode 100644 swift/internal/swift_interop_hint.bzl diff --git a/doc/BUILD b/doc/BUILD index 4e38d3cea..eca971523 100644 --- a/doc/BUILD +++ b/doc/BUILD @@ -26,6 +26,7 @@ _DOC_SRCS = { "universal_swift_compiler_plugin", "swift_feature_allowlist", "swift_import", + "swift_interop_hint", "swift_library", "swift_library_group", "swift_module_alias", diff --git a/doc/README.md b/doc/README.md index d0268b088..8fbfdf7f1 100644 --- a/doc/README.md +++ b/doc/README.md @@ -3,8 +3,8 @@ ## BUILD Rules * [swift_binary](rules.md#swift_binary) -* [swift_c_module](rules.md#swift_c_module) * [swift_import](rules.md#swift_import) +* [swift_interop_hint](rules.md#swift_interop_hint) * [swift_library](rules.md#swift_library) * [swift_module_alias](rules.md#swift_module_alias) * [swift_proto_library](rules.md#swift_proto_library) diff --git a/doc/doc.bzl b/doc/doc.bzl index 55a0966d5..f11845697 100644 --- a/doc/doc.bzl +++ b/doc/doc.bzl @@ -60,6 +60,7 @@ load( _swift_compiler_plugin = "swift_compiler_plugin", _swift_feature_allowlist = "swift_feature_allowlist", _swift_import = "swift_import", + _swift_interop_hint = "swift_interop_hint", _swift_library = "swift_library", _swift_library_group = "swift_library_group", _swift_module_alias = "swift_module_alias", @@ -95,6 +96,7 @@ swift_compiler_plugin = _swift_compiler_plugin universal_swift_compiler_plugin = _universal_swift_compiler_plugin swift_feature_allowlist = _swift_feature_allowlist swift_import = _swift_import +swift_interop_hint = _swift_interop_hint swift_library = _swift_library swift_library_group = _swift_library_group swift_module_alias = _swift_module_alias diff --git a/doc/rules.md b/doc/rules.md index 46b838e00..b461cf084 100644 --- a/doc/rules.md +++ b/doc/rules.md @@ -25,6 +25,7 @@ On this page: * [universal_swift_compiler_plugin](#universal_swift_compiler_plugin) * [swift_feature_allowlist](#swift_feature_allowlist) * [swift_import](#swift_import) + * [swift_interop_hint](#swift_interop_hint) * [swift_library](#swift_library) * [swift_library_group](#swift_library_group) * [swift_module_alias](#swift_module_alias) @@ -469,6 +470,86 @@ the `.private.swiftinterface` files are required in order to build any code that | swiftmodule | The `.swiftmodule` file provided to Swift targets that depend on this target. | Label | optional | `None` | + + +## swift_interop_hint + +
+swift_interop_hint(name, module_name)
+
+ +Defines an aspect hint that associates non-Swift BUILD targets with additional +information required for them to be imported by Swift. + +> [!NOTE] +> Bazel 6 users must set the `--experimental_enable_aspect_hints` flag to utilize +> this rule. In addition, downstream consumers of rules that utilize this rule +> must also set the flag. The flag is enabled by default in Bazel 7. + +Some build rules, such as `objc_library`, support interoperability with Swift +simply by depending on them; a module map is generated automatically. This is +for convenience, because the common case is that most `objc_library` targets +contain code that is compatible (i.e., capable of being imported) by Swift. + +For other rules, like `cc_library`, additional information must be provided to +indicate that a particular target is compatible with Swift. This is done using +the `aspect_hints` attribute and the `swift_interop_hint` rule. + +#### Using the automatically derived module name (recommended) + +If you want to import a non-Swift, non-Objective-C target into Swift using the +module name that is automatically derived from the BUILD label, there is no need +to declare an instance of `swift_interop_hint`. A canonical one that requests +module name derivation has been provided in +`@build_bazel_rules_swift//swift:auto_module`. Simply add it to the `aspect_hints` of +the target you wish to import: + +```build +# //my/project/BUILD +cc_library( + name = "somelib", + srcs = ["somelib.c"], + hdrs = ["somelib.h"], + aspect_hints = ["@build_bazel_rules_swift//swift:auto_module"], +) +``` + +When this `cc_library` is a dependency of a Swift target, a module map will be +generated for it. In this case, the module's name would be `my_project_somelib`. + +#### Using an explicit module name + +If you need to provide an explicit name for the module (for example, if it is +part of a third-party library that expects to be imported with a specific name), +then you can declare your own `swift_interop_hint` target to define the name: + +```build +# //my/project/BUILD +cc_library( + name = "somelib", + srcs = ["somelib.c"], + hdrs = ["somelib.h"], + aspect_hints = [":somelib_swift_interop"], +) + +swift_interop_hint( + name = "somelib_swift_interop", + module_name = "CSomeLib", +) +``` + +When this `cc_library` is a dependency of a Swift target, a module map will be +generated for it with the module name `CSomeLib`. + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| module_name | The name that will be used to import the hinted module into Swift.

If left unspecified, the module name will be computed based on the hinted target's build label, by stripping the leading `//` and replacing `/`, `:`, and other non-identifier characters with underscores. | String | optional | `""` | + + ## swift_library diff --git a/examples/xplatform/c_from_swift/BUILD b/examples/xplatform/c_from_swift/BUILD index 308d748a1..717b99d26 100644 --- a/examples/xplatform/c_from_swift/BUILD +++ b/examples/xplatform/c_from_swift/BUILD @@ -1,4 +1,9 @@ -load("//swift:swift.bzl", "swift_binary", "swift_library") +load( + "//swift:swift.bzl", + "swift_binary", + "swift_interop_hint", + "swift_library", +) licenses(["notice"]) @@ -10,16 +15,24 @@ cc_library( linkstatic = True, ) -# 2. ...but Swift can't import C++ yet, so we implement a wrapper API in C. +# 2. ...but Swift can't import C++ yet, so we implement a wrapper API in C. Use +# the `swift_interop_hint` rule to enable module map generation and provide the +# module name for these headers, since `cc_library` doesn't do enable this by +# default. cc_library( name = "c_counter", srcs = ["c_counter.cc"], hdrs = ["c_counter.h"], linkstatic = True, - tags = ["swift_module=CCounter"], + aspect_hints = [":c_counter_swift_hint"], deps = [":counter"], ) +swift_interop_hint( + name = "c_counter_swift_hint", + module_name = "CCounter", +) + # 3. The Swift library then depends on the `cc_library`. This causes a # Swift-compatible module map to be created for the `cc_library` so that the # Swift code can import it. diff --git a/swift/BUILD b/swift/BUILD index f4439ad8b..4d2c1c60e 100644 --- a/swift/BUILD +++ b/swift/BUILD @@ -5,6 +5,7 @@ load( "per_module_swiftcopt_flag", "repeatable_string_flag", ) +load(":swift.bzl", "swift_interop_hint") package(default_visibility = ["//visibility:public"]) @@ -57,6 +58,7 @@ bzl_library( "//swift/internal:swift_common", "//swift/internal:swift_feature_allowlist", "//swift/internal:swift_import", + "//swift/internal:swift_interop_hint", "//swift/internal:swift_library", "//swift/internal:swift_library_group", "//swift/internal:swift_module_alias", @@ -93,6 +95,13 @@ repeatable_string_flag( repeatable_string_flag( name = "exec_copt", build_setting_default = [], +) + +# An aspect hint that enables module map generation for a non-Swift, +# non-Objective-C target, deriving the module name automatically based on the +# hinted target's label. +swift_interop_hint( + name = "auto_module", visibility = ["//visibility:public"], ) diff --git a/swift/internal/BUILD b/swift/internal/BUILD index 31a669689..96d9ffec8 100644 --- a/swift/internal/BUILD +++ b/swift/internal/BUILD @@ -210,6 +210,13 @@ bzl_library( ], ) +bzl_library( + name = "swift_interop_hint", + srcs = ["swift_interop_hint.bzl"], + visibility = ["//swift:__subpackages__"], + deps = [":swift_common"], +) + bzl_library( name = "swift_library", srcs = ["swift_library.bzl"], diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 31bf73cea..c0576a37f 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -586,6 +586,61 @@ def _compilation_context_for_target(target): return compilation_context +def _find_swift_interop_info(target, aspect_ctx): + """Finds a `_SwiftInteropInfo` provider associated with the target. + + This function first looks at the target itself to determine if it propagated + a `_SwiftInteropInfo` provider directly (that is, its rule implementation + function called `swift_common.create_swift_interop_info`). If it did not, + then the target's `aspect_hints` attribute is checked for a reference to a + target that propagates `_SwiftInteropInfo` (such as `swift_interop_hint`). + + It is an error if `aspect_hints` contains two or more targets that propagate + `_SwiftInteropInfo`, or if the target directly propagates the provider and + there is also any target in `aspect_hints` that propagates it. + + Args: + target: The target to which the aspect is currently being applied. + aspect_ctx: The aspect's context. + + Returns: + The `_SwiftInteropInfo` associated with the target, if found; otherwise, + None. + """ + interop_target = None + interop_from_rule = False + + if _SwiftInteropInfo in target: + interop_target = target + interop_from_rule = True + + # We don't break this loop early when we find a matching hint, because we + # want to give an error message if there are two aspect hints that provide + # `_SwiftInteropInfo` (or if both the rule and an aspect hint do). + for hint in aspect_ctx.rule.attr.aspect_hints: + if _SwiftInteropInfo in hint: + if interop_target: + if interop_from_rule: + fail(("Conflicting Swift interop info from the target " + + "'{target}' ({rule} rule) and the aspect hint " + + "'{hint}'. Only one is allowed.").format( + hint = str(hint.label), + target = str(target.label), + rule = aspect_ctx.rule.kind, + )) + else: + fail(("Conflicting Swift interop info from aspect hints " + + "'{hint1}' and '{hint2}'. Only one is " + + "allowed.").format( + hint1 = str(interop_target.label), + hint2 = str(hint.label), + )) + interop_target = hint + + if interop_target: + return interop_target[_SwiftInteropInfo] + return None + def _swift_clang_module_aspect_impl(target, aspect_ctx): # Do nothing if the target already propagates `SwiftInfo`. if SwiftInfo in target: @@ -594,8 +649,8 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): requested_features = aspect_ctx.features unsupported_features = aspect_ctx.disabled_features - if _SwiftInteropInfo in target: - interop_info = target[_SwiftInteropInfo] + interop_info = _find_swift_interop_info(target, aspect_ctx) + if interop_info: module_map_file = interop_info.module_map module_name = ( interop_info.module_name or derive_module_name(target.label) diff --git a/swift/internal/swift_interop_hint.bzl b/swift/internal/swift_interop_hint.bzl new file mode 100644 index 000000000..42016cf93 --- /dev/null +++ b/swift/internal/swift_interop_hint.bzl @@ -0,0 +1,106 @@ +# Copyright 2021 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Implementation of the `swift_interop_hint` rule.""" + +load(":swift_common.bzl", "swift_common") + +def _swift_interop_hint_impl(ctx): + # TODO(b/194733180): For now, this rule only supports interop via an + # auto-derived module name or an explicit module name, but still using the + # auto-generated module name. Add support for manual module maps and other + # features, like APINotes, later. + return swift_common.create_swift_interop_info( + module_name = ctx.attr.module_name, + ) + +swift_interop_hint = rule( + attrs = { + "module_name": attr.string( + doc = """\ +The name that will be used to import the hinted module into Swift. + +If left unspecified, the module name will be computed based on the hinted +target's build label, by stripping the leading `//` and replacing `/`, `:`, and +other non-identifier characters with underscores. +""", + mandatory = False, + ), + }, + doc = """\ +Defines an aspect hint that associates non-Swift BUILD targets with additional +information required for them to be imported by Swift. + +> [!NOTE] +> Bazel 6 users must set the `--experimental_enable_aspect_hints` flag to utilize +> this rule. In addition, downstream consumers of rules that utilize this rule +> must also set the flag. The flag is enabled by default in Bazel 7. + +Some build rules, such as `objc_library`, support interoperability with Swift +simply by depending on them; a module map is generated automatically. This is +for convenience, because the common case is that most `objc_library` targets +contain code that is compatible (i.e., capable of being imported) by Swift. + +For other rules, like `cc_library`, additional information must be provided to +indicate that a particular target is compatible with Swift. This is done using +the `aspect_hints` attribute and the `swift_interop_hint` rule. + +#### Using the automatically derived module name (recommended) + +If you want to import a non-Swift, non-Objective-C target into Swift using the +module name that is automatically derived from the BUILD label, there is no need +to declare an instance of `swift_interop_hint`. A canonical one that requests +module name derivation has been provided in +`@build_bazel_rules_swift//swift:auto_module`. Simply add it to the `aspect_hints` of +the target you wish to import: + +```build +# //my/project/BUILD +cc_library( + name = "somelib", + srcs = ["somelib.c"], + hdrs = ["somelib.h"], + aspect_hints = ["@build_bazel_rules_swift//swift:auto_module"], +) +``` + +When this `cc_library` is a dependency of a Swift target, a module map will be +generated for it. In this case, the module's name would be `my_project_somelib`. + +#### Using an explicit module name + +If you need to provide an explicit name for the module (for example, if it is +part of a third-party library that expects to be imported with a specific name), +then you can declare your own `swift_interop_hint` target to define the name: + +```build +# //my/project/BUILD +cc_library( + name = "somelib", + srcs = ["somelib.c"], + hdrs = ["somelib.h"], + aspect_hints = [":somelib_swift_interop"], +) + +swift_interop_hint( + name = "somelib_swift_interop", + module_name = "CSomeLib", +) +``` + +When this `cc_library` is a dependency of a Swift target, a module map will be +generated for it with the module name `CSomeLib`. +""", + implementation = _swift_interop_hint_impl, +) diff --git a/swift/swift.bzl b/swift/swift.bzl index 2bab57cfd..308cd2b16 100644 --- a/swift/swift.bzl +++ b/swift/swift.bzl @@ -74,6 +74,10 @@ load( "@build_bazel_rules_swift//swift/internal:swift_import.bzl", _swift_import = "swift_import", ) +load( + "@build_bazel_rules_swift//swift/internal:swift_interop_hint.bzl", + _swift_interop_hint = "swift_interop_hint", +) load( "@build_bazel_rules_swift//swift/internal:swift_library.bzl", _swift_library = "swift_library", @@ -112,6 +116,7 @@ swift_compiler_plugin = _swift_compiler_plugin universal_swift_compiler_plugin = _universal_swift_compiler_plugin swift_feature_allowlist = _swift_feature_allowlist swift_import = _swift_import +swift_interop_hint = _swift_interop_hint swift_library = _swift_library swift_library_group = _swift_library_group swift_module_alias = _swift_module_alias diff --git a/test/fixtures/private_deps/BUILD b/test/fixtures/private_deps/BUILD index 1f07ad986..a8bb980ba 100644 --- a/test/fixtures/private_deps/BUILD +++ b/test/fixtures/private_deps/BUILD @@ -44,6 +44,7 @@ cc_library( name = "private_cc", srcs = ["private.c"], hdrs = ["private.h"], + aspect_hints = ["//swift:auto_module"], features = [ # A bit hacky, but by claiming we don't support PIC, we can get the # output libraries in `libraries_to_link.static_library` instead of @@ -52,19 +53,20 @@ cc_library( "-pic", "-supports_pic", ], - tags = FIXTURE_TAGS + ["swift_module"], + tags = FIXTURE_TAGS, ) cc_library( name = "public_cc", srcs = ["public.c"], hdrs = ["public.h"], + aspect_hints = ["//swift:auto_module"], features = [ # See the comment in the target above. "-pic", "-supports_pic", ], - tags = FIXTURE_TAGS + ["swift_module"], + tags = FIXTURE_TAGS, ) swift_library( From 708c6cd797449844679984e97b7d1ed6859c7a69 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Tue, 27 Jul 2021 14:20:32 -0700 Subject: [PATCH 02/16] Add support for custom `module_map`s in `swift_interop_hint`. This allows rules like `cc_library` to associate a custom module map if needed (however, this should be rare and used sparingly). It also eliminates the need for the `swift_c_module`, which will be removed. Added analysis tests around the propagation of the module map artifacts. PiperOrigin-RevId: 387195026 (cherry picked from commit 135636525364111b15bb89ab3ed0fa5d94c6ce8f) --- doc/rules.md | 29 ++++++- swift/internal/swift_interop_hint.bzl | 48 ++++++++++- test/BUILD | 3 + test/fixtures/interop_hints/BUILD | 82 +++++++++++++++++++ .../interop_hints/ImportModuleName.swift | 1 + .../interop_hints/ImportSubmodule.swift | 1 + test/fixtures/interop_hints/header1.h | 1 + test/fixtures/interop_hints/header2.h | 1 + test/fixtures/interop_hints/module.modulemap | 9 ++ test/interop_hints_tests.bzl | 72 ++++++++++++++++ 10 files changed, 242 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/interop_hints/BUILD create mode 100644 test/fixtures/interop_hints/ImportModuleName.swift create mode 100644 test/fixtures/interop_hints/ImportSubmodule.swift create mode 100644 test/fixtures/interop_hints/header1.h create mode 100644 test/fixtures/interop_hints/header2.h create mode 100644 test/fixtures/interop_hints/module.modulemap create mode 100644 test/interop_hints_tests.bzl diff --git a/doc/rules.md b/doc/rules.md index b461cf084..f03f7de23 100644 --- a/doc/rules.md +++ b/doc/rules.md @@ -475,7 +475,7 @@ the `.private.swiftinterface` files are required in order to build any code that ## swift_interop_hint 
-swift_interop_hint(name, module_name)
+swift_interop_hint(name, module_map, module_name)
Defines an aspect hint that associates non-Swift BUILD targets with additional @@ -541,12 +541,39 @@ swift_interop_hint( When this `cc_library` is a dependency of a Swift target, a module map will be generated for it with the module name `CSomeLib`. +#### Using a custom module map + +In rare cases, the automatically generated module map may not be suitable. For +example, a Swift module may depend on a C module that defines specific +submodules, and this is not handled by the Swift build rules. In this case, you +can provide the module map file using the `module_map` attribute. + +When setting the `module_map` attribute, `module_name` must also be set to the +name of the desired top-level module; it cannot be omitted. + +```build +# //my/project/BUILD +cc_library( + name = "somelib", + srcs = ["somelib.c"], + hdrs = ["somelib.h"], + aspect_hints = [":somelib_swift_interop"], +) + +swift_interop_hint( + name = "somelib_swift_interop", + module_map = "module.modulemap", + module_name = "CSomeLib", +) +``` + **ATTRIBUTES** | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | +| module_map | An optional custom `.modulemap` file that defines the Clang module for the headers in the target to which this hint is applied.

If this attribute is omitted, a module map will be automatically generated based on the headers in the hinted target.

If this attribute is provided, then `module_name` must also be provided and match the name of the desired top-level module in the `.modulemap` file. (A single `.modulemap` file may define multiple top-level modules.) | Label | optional | `None` | | module_name | The name that will be used to import the hinted module into Swift.

If left unspecified, the module name will be computed based on the hinted target's build label, by stripping the leading `//` and replacing `/`, `:`, and other non-identifier characters with underscores. | String | optional | `""` | diff --git a/swift/internal/swift_interop_hint.bzl b/swift/internal/swift_interop_hint.bzl index 42016cf93..72fddbc9a 100644 --- a/swift/internal/swift_interop_hint.bzl +++ b/swift/internal/swift_interop_hint.bzl @@ -17,16 +17,30 @@ load(":swift_common.bzl", "swift_common") def _swift_interop_hint_impl(ctx): - # TODO(b/194733180): For now, this rule only supports interop via an - # auto-derived module name or an explicit module name, but still using the - # auto-generated module name. Add support for manual module maps and other - # features, like APINotes, later. + # TODO(b/194733180): Take advantage of the richer API to add support for + # other features, like APINotes, later. return swift_common.create_swift_interop_info( + module_map = ctx.file.module_map, module_name = ctx.attr.module_name, ) swift_interop_hint = rule( attrs = { + "module_map": attr.label( + allow_single_file = True, + doc = """\ +An optional custom `.modulemap` file that defines the Clang module for the +headers in the target to which this hint is applied. + +If this attribute is omitted, a module map will be automatically generated based +on the headers in the hinted target. + +If this attribute is provided, then `module_name` must also be provided and +match the name of the desired top-level module in the `.modulemap` file. (A +single `.modulemap` file may define multiple top-level modules.) +""", + mandatory = False, + ), "module_name": attr.string( doc = """\ The name that will be used to import the hinted module into Swift. @@ -101,6 +115,32 @@ swift_interop_hint( When this `cc_library` is a dependency of a Swift target, a module map will be generated for it with the module name `CSomeLib`. + +#### Using a custom module map + +In rare cases, the automatically generated module map may not be suitable. For +example, a Swift module may depend on a C module that defines specific +submodules, and this is not handled by the Swift build rules. In this case, you +can provide the module map file using the `module_map` attribute. + +When setting the `module_map` attribute, `module_name` must also be set to the +name of the desired top-level module; it cannot be omitted. + +```build +# //my/project/BUILD +cc_library( + name = "somelib", + srcs = ["somelib.c"], + hdrs = ["somelib.h"], + aspect_hints = [":somelib_swift_interop"], +) + +swift_interop_hint( + name = "somelib_swift_interop", + module_map = "module.modulemap", + module_name = "CSomeLib", +) +``` """, implementation = _swift_interop_hint_impl, ) diff --git a/test/BUILD b/test/BUILD index 5b5dc2638..3acbd7e0e 100644 --- a/test/BUILD +++ b/test/BUILD @@ -8,6 +8,7 @@ load(":explicit_modules_test.bzl", "explicit_modules_test_suite") load(":features_tests.bzl", "features_test_suite") load(":generated_header_tests.bzl", "generated_header_test_suite") load(":imported_framework_tests.bzl", "imported_framework_test_suite") +load(":interop_hints_tests.bzl", "interop_hints_test_suite") load(":mainattr_tests.bzl", "mainattr_test_suite") load(":module_cache_settings_tests.bzl", "module_cache_settings_test_suite") load(":module_interface_tests.bzl", "module_interface_test_suite") @@ -44,6 +45,8 @@ output_file_map_test_suite(name = "output_file_map") module_interface_test_suite(name = "module_interface") +interop_hints_test_suite() + private_deps_test_suite(name = "private_deps") swift_through_non_swift_test_suite(name = "swift_through_non_swift") diff --git a/test/fixtures/interop_hints/BUILD b/test/fixtures/interop_hints/BUILD new file mode 100644 index 000000000..2b33cb36f --- /dev/null +++ b/test/fixtures/interop_hints/BUILD @@ -0,0 +1,82 @@ +load( + "//swift:swift.bzl", + "swift_interop_hint", + "swift_library", +) +load("//test/fixtures:common.bzl", "FIXTURE_TAGS") + +package( + default_visibility = ["//test:__subpackages__"], +) + +licenses(["notice"]) + +swift_library( + name = "import_module_name_swift", + srcs = ["ImportModuleName.swift"], + tags = FIXTURE_TAGS, + deps = [":cc_lib_custom_module_name"], +) + +cc_library( + name = "cc_lib_custom_module_name", + hdrs = [ + "header1.h", + "header2.h", + ], + aspect_hints = [":cc_lib_custom_module_name_hint"], + tags = FIXTURE_TAGS, +) + +swift_interop_hint( + name = "cc_lib_custom_module_name_hint", + module_name = "ModuleName", + tags = FIXTURE_TAGS, +) + +swift_library( + name = "import_submodule_swift", + srcs = ["ImportSubmodule.swift"], + tags = FIXTURE_TAGS, + deps = [":cc_lib_submodule"], +) + +cc_library( + name = "cc_lib_submodule", + hdrs = [ + "header1.h", + "header2.h", + ], + aspect_hints = [":cc_lib_submodule_hint"], + tags = FIXTURE_TAGS, +) + +swift_interop_hint( + name = "cc_lib_submodule_hint", + module_map = "module.modulemap", + module_name = "TopModule", + tags = FIXTURE_TAGS, +) + +swift_library( + name = "invalid_swift", + srcs = ["ImportSubmodule.swift"], + tags = FIXTURE_TAGS, + deps = [":cc_lib_invalid"], +) + +cc_library( + name = "cc_lib_invalid", + hdrs = [ + "header1.h", + "header2.h", + ], + aspect_hints = [":cc_lib_invalid_hint"], + tags = FIXTURE_TAGS, +) + +swift_interop_hint( + name = "cc_lib_invalid_hint", + module_map = "module.modulemap", + tags = FIXTURE_TAGS, +) diff --git a/test/fixtures/interop_hints/ImportModuleName.swift b/test/fixtures/interop_hints/ImportModuleName.swift new file mode 100644 index 000000000..f5e291cdd --- /dev/null +++ b/test/fixtures/interop_hints/ImportModuleName.swift @@ -0,0 +1 @@ +import ModuleName diff --git a/test/fixtures/interop_hints/ImportSubmodule.swift b/test/fixtures/interop_hints/ImportSubmodule.swift new file mode 100644 index 000000000..856f8cbb2 --- /dev/null +++ b/test/fixtures/interop_hints/ImportSubmodule.swift @@ -0,0 +1 @@ +import TopModule.Submodule diff --git a/test/fixtures/interop_hints/header1.h b/test/fixtures/interop_hints/header1.h new file mode 100644 index 000000000..bd9ec079d --- /dev/null +++ b/test/fixtures/interop_hints/header1.h @@ -0,0 +1 @@ +// Intentionally empty. diff --git a/test/fixtures/interop_hints/header2.h b/test/fixtures/interop_hints/header2.h new file mode 100644 index 000000000..bd9ec079d --- /dev/null +++ b/test/fixtures/interop_hints/header2.h @@ -0,0 +1 @@ +// Intentionally empty. diff --git a/test/fixtures/interop_hints/module.modulemap b/test/fixtures/interop_hints/module.modulemap new file mode 100644 index 000000000..5338c14d8 --- /dev/null +++ b/test/fixtures/interop_hints/module.modulemap @@ -0,0 +1,9 @@ +module TopModule { + header "header1.h" + export * + + module Submodule { + header "header2.h" + export * + } +} diff --git a/test/interop_hints_tests.bzl b/test/interop_hints_tests.bzl new file mode 100644 index 000000000..7a87e4252 --- /dev/null +++ b/test/interop_hints_tests.bzl @@ -0,0 +1,72 @@ +# Copyright 2021 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for `swift_interop_hint`.""" + +load( + "@build_bazel_rules_swift//test/rules:analysis_failure_test.bzl", + "analysis_failure_test", +) +load( + "@build_bazel_rules_swift//test/rules:provider_test.bzl", + "provider_test", +) + +def interop_hints_test_suite(name = "interop_hints"): + """Test suite for `swift_interop_hint`. + + Args: + name: The name prefix for all the nested tests + """ + + # Verify that a hint with only a custom module name causes the `cc_library` + # to propagate a `SwiftInfo` info with the expected auto-generated module + # map. + provider_test( + name = "{}_hint_with_custom_module_name_builds".format(name), + expected_files = [ + "test/fixtures/interop_hints/cc_lib_custom_module_name.swift.modulemap", + ], + field = "transitive_modules.clang.module_map!", + provider = "SwiftInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/interop_hints:import_module_name_swift", + ) + + # Verify that a hint with a custom module map file causes the `cc_library` + # to propagate a `SwiftInfo` info with that file. + provider_test( + name = "{}_hint_with_custom_module_map_builds".format(name), + expected_files = [ + "test/fixtures/interop_hints/module.modulemap", + ], + field = "transitive_modules.clang.module_map!", + provider = "SwiftInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/interop_hints:import_submodule_swift", + ) + + # Verify that the build fails if a hint provides `module_map` without + # `module_name`. + analysis_failure_test( + name = "{}_fails_when_module_map_provided_without_module_name".format(name), + expected_message = "'module_name' must be specified when 'module_map' is specified.", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/interop_hints:invalid_swift", + ) + + native.test_suite( + name = name, + tags = [name], + ) From a1ae533c2603e5ecb93c0328deda6b2aa02bb11c Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Wed, 28 Jul 2021 08:27:06 -0700 Subject: [PATCH 03/16] Don't drop `SwiftInfo` providers from the target's dependencies when using `swift_interop_hint`. PiperOrigin-RevId: 387355219 (cherry picked from commit 81c307408621e22bfa17120be7cf98a80f6f6454) --- swift/internal/swift_clang_module_aspect.bzl | 85 +++++++++++++------- 1 file changed, 54 insertions(+), 31 deletions(-) diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index c0576a37f..b4fea41dd 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -32,11 +32,7 @@ load( "create_module", "create_swift_info", ) -load( - ":utils.bzl", - "compilation_context_for_explicit_module_compilation", - "get_providers", -) +load(":utils.bzl", "compilation_context_for_explicit_module_compilation") _MULTIPLE_TARGET_ASPECT_ATTRS = [ "deps", @@ -586,6 +582,32 @@ def _compilation_context_for_target(target): return compilation_context +def _collect_swift_infos_from_deps(aspect_ctx): + """Collect `SwiftInfo` providers from dependencies. + + Args: + aspect_ctx: The aspect's context. + + Returns: + A list of `SwiftInfo` providers from dependencies of the target to which + the aspect was applied. + """ + swift_infos = [] + + attr = aspect_ctx.rule.attr + for attr_name in _MULTIPLE_TARGET_ASPECT_ATTRS: + swift_infos.extend([ + dep[SwiftInfo] + for dep in getattr(attr, attr_name, []) + if SwiftInfo in dep + ]) + for attr_name in _SINGLE_TARGET_ASPECT_ATTRS: + dep = getattr(attr, attr_name, None) + if dep and SwiftInfo in dep: + swift_infos.append(dep[SwiftInfo]) + + return swift_infos + def _find_swift_interop_info(target, aspect_ctx): """Finds a `_SwiftInteropInfo` provider associated with the target. @@ -604,15 +626,32 @@ def _find_swift_interop_info(target, aspect_ctx): aspect_ctx: The aspect's context. Returns: - The `_SwiftInteropInfo` associated with the target, if found; otherwise, - None. - """ - interop_target = None - interop_from_rule = False + A tuple containing two elements: + - The `_SwiftInteropInfo` associated with the target, if found; + otherwise, None. + - A list of additional `SwiftInfo` providers that are treated as + direct dependencies of the target, determined by reading attributes + from the target if it did not provide `_SwiftInteropInfo` directly. + """ if _SwiftInteropInfo in target: + # If the target's rule implementation directly provides + # `_SwiftInteropInfo`, then it is that rule's responsibility to collect + # and merge `SwiftInfo` providers from relevant dependencies. interop_target = target interop_from_rule = True + default_swift_infos = [] + else: + # If the target's rule implementation does not directly provide + # `_SwiftInteropInfo`, then we need to collect the `SwiftInfo` providers + # from the default dependencies and returns those. Note that if a custom + # rule is used as a hint and returns a `_SwiftInteropInfo` that contains + # `SwiftInfo` providers, then we would consider the providers from the + # default dependencies and the providers from the hint; they are merged + # after the call site of this function. + interop_target = None + interop_from_rule = False + default_swift_infos = _collect_swift_infos_from_deps(aspect_ctx) # We don't break this loop early when we find a matching hint, because we # want to give an error message if there are two aspect hints that provide @@ -638,8 +677,8 @@ def _find_swift_interop_info(target, aspect_ctx): interop_target = hint if interop_target: - return interop_target[_SwiftInteropInfo] - return None + return interop_target[_SwiftInteropInfo], default_swift_infos + return None, default_swift_infos def _swift_clang_module_aspect_impl(target, aspect_ctx): # Do nothing if the target already propagates `SwiftInfo`. @@ -649,31 +688,19 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): requested_features = aspect_ctx.features unsupported_features = aspect_ctx.disabled_features - interop_info = _find_swift_interop_info(target, aspect_ctx) + interop_info, swift_infos = _find_swift_interop_info(target, aspect_ctx) if interop_info: module_map_file = interop_info.module_map module_name = ( interop_info.module_name or derive_module_name(target.label) ) - swift_infos = interop_info.swift_infos + swift_infos.extend(interop_info.swift_infos) requested_features.extend(interop_info.requested_features) unsupported_features.extend(interop_info.unsupported_features) else: module_map_file = None module_name = None - # Collect `SwiftInfo` providers from dependencies, based on the - # attributes that this aspect traverses. - deps = [] - attr = aspect_ctx.rule.attr - for attr_name in _MULTIPLE_TARGET_ASPECT_ATTRS: - deps.extend(getattr(attr, attr_name, [])) - for attr_name in _SINGLE_TARGET_ASPECT_ATTRS: - dep = getattr(attr, attr_name, None) - if dep: - deps.append(dep) - swift_infos = get_providers(deps, SwiftInfo) - swift_toolchain = aspect_ctx.attr._toolchain_for_aspect[SwiftToolchainInfo] feature_configuration = configure_features( ctx = aspect_ctx, @@ -682,11 +709,7 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): unsupported_features = unsupported_features, ) - if ( - _SwiftInteropInfo in target or - apple_common.Objc in target or - CcInfo in target - ): + if interop_info or apple_common.Objc in target or CcInfo in target: return _handle_module( aspect_ctx = aspect_ctx, compilation_context = _compilation_context_for_target(target), From e480acaf61bd86ef15f03e4fb312c04ff417c594 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Wed, 28 Jul 2021 08:43:45 -0700 Subject: [PATCH 04/16] Delete the `swift_c_module` rule. Its functionality has been replaced by `swift_import_hint` and `aspect_hints`. PiperOrigin-RevId: 387358449 (cherry picked from commit 553f697f030a5bfe9dba3670b8fbafa602e3e8e8) --- doc/BUILD | 1 - doc/doc.bzl | 2 - doc/rules.md | 69 +------ examples/apple/swift_explicit_modules/BUILD | 18 -- .../apple/swift_explicit_modules/main.swift | 1 - examples/xplatform/c_from_swift/BUILD | 2 +- swift/internal/attrs.bzl | 3 +- swift/internal/swift_c_module.bzl | 179 ------------------ swift/swift.bzl | 5 - test/BUILD | 3 - test/explicit_modules_test.bzl | 105 ---------- test/fixtures/explicit_modules/BUILD | 42 ---- test/fixtures/explicit_modules/Empty.swift | 1 - 13 files changed, 11 insertions(+), 420 deletions(-) delete mode 100644 examples/apple/swift_explicit_modules/BUILD delete mode 100644 examples/apple/swift_explicit_modules/main.swift delete mode 100644 swift/internal/swift_c_module.bzl delete mode 100644 test/explicit_modules_test.bzl delete mode 100644 test/fixtures/explicit_modules/BUILD delete mode 100644 test/fixtures/explicit_modules/Empty.swift diff --git a/doc/BUILD b/doc/BUILD index eca971523..61998df11 100644 --- a/doc/BUILD +++ b/doc/BUILD @@ -21,7 +21,6 @@ _DOC_SRCS = { ], "rules": [ "swift_binary", - "swift_c_module", "swift_compiler_plugin", "universal_swift_compiler_plugin", "swift_feature_allowlist", diff --git a/doc/doc.bzl b/doc/doc.bzl index f11845697..6b51d7d2e 100644 --- a/doc/doc.bzl +++ b/doc/doc.bzl @@ -54,7 +54,6 @@ load( _deprecated_swift_grpc_library = "deprecated_swift_grpc_library", _deprecated_swift_proto_library = "deprecated_swift_proto_library", _swift_binary = "swift_binary", - _swift_c_module = "swift_c_module", # api _swift_common = "swift_common", _swift_compiler_plugin = "swift_compiler_plugin", @@ -91,7 +90,6 @@ SwiftUsageInfo = _SwiftUsageInfo deprecated_swift_grpc_library = _deprecated_swift_grpc_library deprecated_swift_proto_library = _deprecated_swift_proto_library swift_binary = _swift_binary -swift_c_module = _swift_c_module swift_compiler_plugin = _swift_compiler_plugin universal_swift_compiler_plugin = _universal_swift_compiler_plugin swift_feature_allowlist = _swift_feature_allowlist diff --git a/doc/rules.md b/doc/rules.md index f03f7de23..1f84b54ab 100644 --- a/doc/rules.md +++ b/doc/rules.md @@ -20,7 +20,6 @@ load("@build_bazel_rules_swift//proto:proto.bzl", "swift_proto_library") On this page: * [swift_binary](#swift_binary) - * [swift_c_module](#swift_c_module) * [swift_compiler_plugin](#swift_compiler_plugin) * [universal_swift_compiler_plugin](#universal_swift_compiler_plugin) * [swift_feature_allowlist](#swift_feature_allowlist) @@ -250,7 +249,7 @@ please use one of the platform-specific application rules in | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | | srcs | A list of `.swift` source files that will be compiled into the library. | List of labels | optional | `[]` | | data | The list of files needed by this target at runtime.

Files and targets named in the `data` attribute will appear in the `*.runfiles` area of this target, if it has one. This may include data files needed by a binary or library, or other programs needed by it. | List of labels | optional | `[]` | | copts | Additional compiler options that should be passed to `swiftc`. These strings are subject to `$(location ...)` and ["Make" variable](https://docs.bazel.build/versions/master/be/make-variables.html) expansion. | List of strings | optional | `[]` | @@ -264,56 +263,6 @@ please use one of the platform-specific application rules in | swiftc_inputs | Additional files that are referenced using `$(location ...)` in attributes that support location expansion. | List of labels | optional | `[]` | - - -## swift_c_module - -
-swift_c_module(name, deps, module_map, module_name, system_module_map)
-
- -Wraps one or more C targets in a new module map that allows it to be imported -into Swift to access its C interfaces. - -The `cc_library` rule in Bazel does not produce module maps that are compatible -with Swift. In order to make interop between Swift and C possible, users have -one of two options: - -1. **Use an auto-generated module map.** In this case, the `swift_c_module` - rule is not needed. If a `cc_library` is a direct dependency of a - `swift_{binary,library,test}` target, a module map will be automatically - generated for it and the module's name will be derived from the Bazel target - label (in the same fashion that module names for Swift targets are derived). - The module name can be overridden by setting the `swift_module` tag on the - `cc_library`; e.g., `tags = ["swift_module=MyModule"]`. - -2. **Use a custom module map.** For finer control over the headers that are - exported by the module, use the `swift_c_module` rule to provide a custom - module map that specifies the name of the module, its headers, and any other - module information. The `cc_library` targets that contain the headers that - you wish to expose to Swift should be listed in the `deps` of your - `swift_c_module` (and by listing multiple targets, you can export multiple - libraries under a single module if desired). Then, your - `swift_{binary,library,test}` targets should depend on the `swift_c_module` - target, not on the underlying `cc_library` target(s). - -NOTE: Swift at this time does not support interop directly with C++. Any headers -referenced by a module map that is imported into Swift must have only C features -visible, often by using preprocessor conditions like `#if __cplusplus` to hide -any C++ declarations. - -**ATTRIBUTES** - - -| Name | Description | Type | Mandatory | Default | -| :------------- | :------------- | :------------- | :------------- | :------------- | -| name | A unique name for this target. | Name | required | | -| deps | A list of C targets (or anything propagating `CcInfo`) that are dependencies of this target and whose headers may be referenced by the module map. | List of labels | optional | `[]` | -| module_map | The module map file that should be loaded to import the C library dependency into Swift. This is mutally exclusive with `system_module_map`. | Label | optional | `None` | -| module_name | The name of the top-level module in the module map that this target represents.

A single `module.modulemap` file can define multiple top-level modules. When building with implicit modules, the presence of that module map allows any of the modules defined in it to be imported. When building explicit modules, however, there is a one-to-one correspondence between top-level modules and BUILD targets and the module name must be known without reading the module map file, so it must be provided directly. Therefore, one may have multiple `swift_c_module` targets that reference the same `module.modulemap` file but with different module names and headers. | String | required | | -| system_module_map | The path to a system framework module map. This is mutually exclusive with `module_map`.

Variables `__BAZEL_XCODE_SDKROOT__` and `__BAZEL_XCODE_DEVELOPER_DIR__` will be substitued appropriately for, i.e. `/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk` and `/Applications/Xcode.app/Contents/Developer` respectively. | String | optional | `""` | - - ## swift_compiler_plugin @@ -388,7 +337,7 @@ swift_library( | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | | srcs | A list of `.swift` source files that will be compiled into the library. | List of labels | optional | `[]` | | data | The list of files needed by this target at runtime.

Files and targets named in the `data` attribute will appear in the `*.runfiles` area of this target, if it has one. This may include data files needed by a binary or library, or other programs needed by it. | List of labels | optional | `[]` | | copts | Additional compiler options that should be passed to `swiftc`. These strings are subject to `$(location ...)` and ["Make" variable](https://docs.bazel.build/versions/master/be/make-variables.html) expansion. | List of strings | optional | `[]` | @@ -461,7 +410,7 @@ the `.private.swiftinterface` files are required in order to build any code that | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | | data | The list of files needed by this target at runtime.

Files and targets named in the `data` attribute will appear in the `*.runfiles` area of this target, if it has one. This may include data files needed by a binary or library, or other programs needed by it. | List of labels | optional | `[]` | | archives | The list of `.a` files provided to Swift targets that depend on this target. | List of labels | optional | `[]` | | module_name | The name of the module represented by this target. | String | required | | @@ -595,7 +544,7 @@ Compiles and links Swift code into a static library and Swift module. | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | | srcs | A list of `.swift` source files that will be compiled into the library. | List of labels | required | | | data | The list of files needed by this target at runtime.

Files and targets named in the `data` attribute will appear in the `*.runfiles` area of this target, if it has one. This may include data files needed by a binary or library, or other programs needed by it. | List of labels | optional | `[]` | | always_include_developer_search_paths | If `True`, the developer framework search paths will be added to the compilation command. This enables a Swift module to access `XCTest` without having to mark the target as `testonly = True`. | Boolean | optional | `False` | @@ -609,7 +558,7 @@ Compiles and links Swift code into a static library and Swift module. | module_name | The name of the Swift module being built.

If left unspecified, the module name will be computed based on the target's build label, by stripping the leading `//` and replacing `/`, `:`, and other non-identifier characters with underscores. | String | optional | `""` | | package_name | The semantic package of the Swift target being built. Targets with the same package_name can access APIs using the 'package' access control modifier in Swift 5.9+. | String | optional | `""` | | plugins | A list of `swift_compiler_plugin` targets that should be loaded by the compiler when compiling this module and any modules that directly depend on it. | List of labels | optional | `[]` | -| private_deps | A list of targets that are implementation-only dependencies of the target being built. Libraries/linker flags from these dependencies will be propagated to dependent for linking, but artifacts/flags required for compilation (such as .swiftmodule files, C headers, and search paths) will not be propagated.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| private_deps | A list of targets that are implementation-only dependencies of the target being built. Libraries/linker flags from these dependencies will be propagated to dependent for linking, but artifacts/flags required for compilation (such as .swiftmodule files, C headers, and search paths) will not be propagated.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | | swiftc_inputs | Additional files that are referenced using `$(location ...)` in attributes that support location expansion. | List of labels | optional | `[]` | @@ -634,7 +583,7 @@ need to import the grouped libraries directly. | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| deps | A list of targets that should be included in the group.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| deps | A list of targets that should be included in the group.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | @@ -779,10 +728,10 @@ swift_proto_library( | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | | srcs | A list of `.swift` source files that will be compiled into the library. | List of labels | optional | `[]` | | data | The list of files needed by this target at runtime.

Files and targets named in the `data` attribute will appear in the `*.runfiles` area of this target, if it has one. This may include data files needed by a binary or library, or other programs needed by it. | List of labels | optional | `[]` | -| additional_compiler_deps | List of additional dependencies required by the generated Swift code at compile time, whose SwiftProtoInfo will be ignored.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| additional_compiler_deps | List of additional dependencies required by the generated Swift code at compile time, whose SwiftProtoInfo will be ignored.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | | additional_compiler_info | Dictionary of additional information passed to the compiler targets. See the documentation of the respective compiler rules for more information on which fields are accepted and how they are used. | Dictionary: String -> String | optional | `{}` | | always_include_developer_search_paths | If `True`, the developer framework search paths will be added to the compilation command. This enables a Swift module to access `XCTest` without having to mark the target as `testonly = True`. | Boolean | optional | `False` | | alwayslink | If true, any binary that depends (directly or indirectly) on this Swift module will link in all the object files for the files listed in `srcs`, even if some contain no symbols referenced by the binary. This is useful if your code isn't explicitly called by code in the binary; for example, if you rely on runtime checks for protocol conformances added in extensions in the library but do not directly reference any other symbols in the object file that adds that conformance. | Boolean | optional | `False` | @@ -938,7 +887,7 @@ bazel test //:Tests --test_filter=TestModuleName.TestClassName/testMethodName | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | -| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_c_module`, `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | +| deps | A list of targets that are dependencies of the target being built, which will be linked into that target.

If the Swift toolchain supports implementation-only imports (`private_deps` on `swift_library`), then targets in `deps` are treated as regular (non-implementation-only) imports that are propagated both to their direct and indirect (transitive) dependents.

Allowed kinds of dependencies are:

* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`)

* `cc_library` (or anything propagating `CcInfo`)

Additionally, on platforms that support Objective-C interop, `objc_library` targets (or anything propagating the `apple_common.Objc` provider) are allowed as dependencies. On platforms that do not support Objective-C interop (such as Linux), those dependencies will be **ignored.** | List of labels | optional | `[]` | | srcs | A list of `.swift` source files that will be compiled into the library. | List of labels | optional | `[]` | | data | The list of files needed by this target at runtime.

Files and targets named in the `data` attribute will appear in the `*.runfiles` area of this target, if it has one. This may include data files needed by a binary or library, or other programs needed by it. | List of labels | optional | `[]` | | copts | Additional compiler options that should be passed to `swiftc`. These strings are subject to `$(location ...)` and ["Make" variable](https://docs.bazel.build/versions/master/be/make-variables.html) expansion. | List of strings | optional | `[]` | diff --git a/examples/apple/swift_explicit_modules/BUILD b/examples/apple/swift_explicit_modules/BUILD deleted file mode 100644 index 380984838..000000000 --- a/examples/apple/swift_explicit_modules/BUILD +++ /dev/null @@ -1,18 +0,0 @@ -load("//swift:swift.bzl", "swift_binary", "swift_c_module") - -# This package is meant to demonstrate the use of explicit clang module compilation. - -# In order to build with features "swift.use_c_modules" and "swift.emit_c_module", it is necessary -# to add all of the framework dependencies explicitly. SwiftShims is the only one required for -# compiling a swift binary with no imports. -swift_c_module( - name = "shims", - module_name = "SwiftShims", - system_module_map = "__BAZEL_XCODE_SDKROOT__/usr/lib/swift/shims/module.modulemap", -) - -swift_binary( - name = "hello_world", - srcs = ["main.swift"], - deps = [":shims"], -) diff --git a/examples/apple/swift_explicit_modules/main.swift b/examples/apple/swift_explicit_modules/main.swift deleted file mode 100644 index 44159b395..000000000 --- a/examples/apple/swift_explicit_modules/main.swift +++ /dev/null @@ -1 +0,0 @@ -print("Hello world") diff --git a/examples/xplatform/c_from_swift/BUILD b/examples/xplatform/c_from_swift/BUILD index 717b99d26..41eabeff6 100644 --- a/examples/xplatform/c_from_swift/BUILD +++ b/examples/xplatform/c_from_swift/BUILD @@ -23,8 +23,8 @@ cc_library( name = "c_counter", srcs = ["c_counter.cc"], hdrs = ["c_counter.h"], - linkstatic = True, aspect_hints = [":c_counter_swift_hint"], + linkstatic = True, deps = [":counter"], ) diff --git a/swift/internal/attrs.bzl b/swift/internal/attrs.bzl index 42f8f13c4..e95b122b9 100644 --- a/swift/internal/attrs.bzl +++ b/swift/internal/attrs.bzl @@ -221,8 +221,7 @@ def swift_deps_attr(*, additional_deps_providers = [], doc, **kwargs): Allowed kinds of dependencies are: -* `swift_c_module`, `swift_import` and `swift_library` (or anything - propagating `SwiftInfo`) +* `swift_import` and `swift_library` (or anything propagating `SwiftInfo`) * `cc_library` (or anything propagating `CcInfo`) diff --git a/swift/internal/swift_c_module.bzl b/swift/internal/swift_c_module.bzl deleted file mode 100644 index cc84f18c3..000000000 --- a/swift/internal/swift_c_module.bzl +++ /dev/null @@ -1,179 +0,0 @@ -# Copyright 2018 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Implementation of the `swift_c_module` rule.""" - -load("@bazel_skylib//lib:dicts.bzl", "dicts") -load(":attrs.bzl", "swift_toolchain_attrs") -load(":feature_names.bzl", "SWIFT_FEATURE_SYSTEM_MODULE") -load(":providers.bzl", "SwiftInfo", "SwiftToolchainInfo") -load(":swift_common.bzl", "swift_common") -load(":utils.bzl", "merge_runfiles") - -def _swift_c_module_impl(ctx): - if ( - (ctx.file.module_map and ctx.attr.system_module_map) or - (not ctx.file.module_map and not ctx.attr.system_module_map) - ): - fail("Must specify one (and only one) of module_map and system_module_map.") - swift_toolchain = ctx.attr._toolchain[SwiftToolchainInfo] - module_map = ctx.file.module_map or ctx.attr.system_module_map - is_system = True if ctx.attr.system_module_map else False - - deps = ctx.attr.deps - cc_infos = [dep[CcInfo] for dep in deps] - swift_infos = [dep[SwiftInfo] for dep in deps if SwiftInfo in dep] - data_runfiles = [dep[DefaultInfo].data_runfiles for dep in deps] - default_runfiles = [dep[DefaultInfo].default_runfiles for dep in deps] - - if cc_infos: - cc_info = cc_common.merge_cc_infos(cc_infos = cc_infos) - compilation_context = cc_info.compilation_context - else: - compilation_context = cc_common.create_compilation_context() - cc_info = CcInfo(compilation_context = compilation_context) - - requested_features = ctx.features - if is_system: - requested_features.append(SWIFT_FEATURE_SYSTEM_MODULE) - - feature_configuration = swift_common.configure_features( - ctx = ctx, - requested_features = requested_features, - swift_toolchain = swift_toolchain, - ) - - precompiled_module = swift_common.precompile_clang_module( - actions = ctx.actions, - cc_compilation_context = compilation_context, - module_map_file = module_map, - module_name = ctx.attr.module_name, - target_name = ctx.attr.name, - swift_toolchain = swift_toolchain, - feature_configuration = feature_configuration, - swift_infos = swift_infos, - ) - - providers = [ - # We must repropagate the dependencies' DefaultInfos, otherwise we - # will lose runtime dependencies that the library expects to be - # there during a test (or a regular `bazel run`). - DefaultInfo( - data_runfiles = merge_runfiles(data_runfiles), - default_runfiles = merge_runfiles(default_runfiles), - files = depset([module_map]) if not is_system else None, - ), - swift_common.create_swift_info( - modules = [ - swift_common.create_module( - name = ctx.attr.module_name, - clang = swift_common.create_clang_module( - compilation_context = compilation_context, - module_map = module_map, - precompiled_module = precompiled_module, - ), - is_system = is_system, - ), - ], - swift_infos = swift_infos, - ), - cc_info, - ] - - return providers - -swift_c_module = rule( - attrs = dicts.add( - swift_toolchain_attrs(), - { - "module_map": attr.label( - allow_single_file = True, - doc = """\ -The module map file that should be loaded to import the C library dependency -into Swift. This is mutally exclusive with `system_module_map`. -""", - mandatory = False, - ), - "system_module_map": attr.string( - doc = """\ -The path to a system framework module map. This is mutually exclusive with `module_map`. - -Variables `__BAZEL_XCODE_SDKROOT__` and `__BAZEL_XCODE_DEVELOPER_DIR__` will be substitued -appropriately for, i.e. -`/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk` -and -`/Applications/Xcode.app/Contents/Developer` respectively. -""", - mandatory = False, - ), - "module_name": attr.string( - doc = """\ -The name of the top-level module in the module map that this target represents. - -A single `module.modulemap` file can define multiple top-level modules. When -building with implicit modules, the presence of that module map allows any of -the modules defined in it to be imported. When building explicit modules, -however, there is a one-to-one correspondence between top-level modules and -BUILD targets and the module name must be known without reading the module map -file, so it must be provided directly. Therefore, one may have multiple -`swift_c_module` targets that reference the same `module.modulemap` file but -with different module names and headers. -""", - mandatory = True, - ), - "deps": attr.label_list( - allow_empty = True, - doc = """\ -A list of C targets (or anything propagating `CcInfo`) that are dependencies of -this target and whose headers may be referenced by the module map. -""", - mandatory = False, - providers = [[CcInfo]], - ), - }, - ), - doc = """\ -Wraps one or more C targets in a new module map that allows it to be imported -into Swift to access its C interfaces. - -The `cc_library` rule in Bazel does not produce module maps that are compatible -with Swift. In order to make interop between Swift and C possible, users have -one of two options: - -1. **Use an auto-generated module map.** In this case, the `swift_c_module` - rule is not needed. If a `cc_library` is a direct dependency of a - `swift_{binary,library,test}` target, a module map will be automatically - generated for it and the module's name will be derived from the Bazel target - label (in the same fashion that module names for Swift targets are derived). - The module name can be overridden by setting the `swift_module` tag on the - `cc_library`; e.g., `tags = ["swift_module=MyModule"]`. - -2. **Use a custom module map.** For finer control over the headers that are - exported by the module, use the `swift_c_module` rule to provide a custom - module map that specifies the name of the module, its headers, and any other - module information. The `cc_library` targets that contain the headers that - you wish to expose to Swift should be listed in the `deps` of your - `swift_c_module` (and by listing multiple targets, you can export multiple - libraries under a single module if desired). Then, your - `swift_{binary,library,test}` targets should depend on the `swift_c_module` - target, not on the underlying `cc_library` target(s). - -NOTE: Swift at this time does not support interop directly with C++. Any headers -referenced by a module map that is imported into Swift must have only C features -visible, often by using preprocessor conditions like `#if __cplusplus` to hide -any C++ declarations. -""", - implementation = _swift_c_module_impl, - fragments = ["cpp"], -) diff --git a/swift/swift.bzl b/swift/swift.bzl index 308cd2b16..b392cd190 100644 --- a/swift/swift.bzl +++ b/swift/swift.bzl @@ -54,10 +54,6 @@ load( _swift_binary = "swift_binary", _swift_test = "swift_test", ) -load( - "@build_bazel_rules_swift//swift/internal:swift_c_module.bzl", - _swift_c_module = "swift_c_module", -) load( "@build_bazel_rules_swift//swift/internal:swift_clang_module_aspect.bzl", _swift_clang_module_aspect = "swift_clang_module_aspect", @@ -111,7 +107,6 @@ swift_common = _swift_common # Re-export rules. swift_binary = _swift_binary -swift_c_module = _swift_c_module swift_compiler_plugin = _swift_compiler_plugin universal_swift_compiler_plugin = _universal_swift_compiler_plugin swift_feature_allowlist = _swift_feature_allowlist diff --git a/test/BUILD b/test/BUILD index 3acbd7e0e..8e7bb92fd 100644 --- a/test/BUILD +++ b/test/BUILD @@ -4,7 +4,6 @@ load(":bzl_test.bzl", "bzl_test") load(":compiler_arguments_tests.bzl", "compiler_arguments_test_suite") load(":coverage_settings_tests.bzl", "coverage_settings_test_suite") load(":debug_settings_tests.bzl", "debug_settings_test_suite") -load(":explicit_modules_test.bzl", "explicit_modules_test_suite") load(":features_tests.bzl", "features_test_suite") load(":generated_header_tests.bzl", "generated_header_test_suite") load(":imported_framework_tests.bzl", "imported_framework_test_suite") @@ -57,8 +56,6 @@ pch_output_dir_test_suite(name = "pch_output_dir_settings") private_swiftinterface_test_suite(name = "private_swiftinterface") -explicit_modules_test_suite(name = "explicit_modules") - xctest_runner_test_suite(name = "xctest_runner") utils_test_suite(name = "utils") diff --git a/test/explicit_modules_test.bzl b/test/explicit_modules_test.bzl deleted file mode 100644 index ba5770565..000000000 --- a/test/explicit_modules_test.bzl +++ /dev/null @@ -1,105 +0,0 @@ -# Copyright 2022 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Tests for explicit module compilation command line flags""" - -load( - "@build_bazel_rules_swift//test/rules:action_command_line_test.bzl", - "make_action_command_line_test_rule", -) - -explicit_modules_action_command_line_test = make_action_command_line_test_rule( - config_settings = { - "//command_line_option:features": [ - "swift.use_c_modules", - "swift.emit_c_module", - "swift.supports_system_module_flag", - ], - }, -) - -implicit_modules_action_command_line_test = make_action_command_line_test_rule() - -def explicit_modules_test_suite(name): - """Test suite for explicit clang module compilation. - - Args: - name: the base name to be used in things created by this macro - """ - - # Verify that swift libraries compile with the specified module file from deps. - explicit_modules_action_command_line_test( - name = "{}_enabled_swift_side_test".format(name), - expected_argv = [ - "-fmodule-file=SwiftShims", - "-fno-implicit-module-maps", - "-fno-implicit-modules", - ], - mnemonic = "SwiftCompile", - tags = [name], - target_under_test = "@build_bazel_rules_swift//test/fixtures/explicit_modules:simple", - target_compatible_with = ["@platforms//os:macos"], - ) - - # Verify that the swift_c_module precompiles. - explicit_modules_action_command_line_test( - name = "{}_enabled_c_module_side".format(name), - expected_argv = [ - "-fsystem-module", - "-module-name SwiftShims", - "-emit-pcm", - "-fno-implicit-module-maps", - "-fno-implicit-modules", - ], - mnemonic = "SwiftPrecompileCModule", - tags = [name], - target_under_test = "@build_bazel_rules_swift//test/fixtures/explicit_modules:shims", - target_compatible_with = ["@platforms//os:macos"], - ) - - # Verify that a swift_c_module with dependencies precompiles. - explicit_modules_action_command_line_test( - name = "{}_enabled_c_module_deps".format(name), - expected_argv = [ - "-fsystem-module", - "-fmodule-file=_Builtin_stddef_max_align_t", - "-fmodule-map-file=__BAZEL_XCODE_DEVELOPER_DIR__/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/clang/include/module.modulemap", - "-module-name Darwin", - "-emit-pcm", - "-fno-implicit-module-maps", - "-fno-implicit-modules", - ], - mnemonic = "SwiftPrecompileCModule", - tags = [name], - target_under_test = "@build_bazel_rules_swift//test/fixtures/explicit_modules:Darwin", - target_compatible_with = ["@platforms//os:macos"], - ) - - # Verify that the default behavior isn't impacted. - implicit_modules_action_command_line_test( - name = "{}_disabled_test".format(name), - not_expected_argv = [ - "-fmodule-file=SwiftShims", - "-fno-implicit-module-maps", - "-fno-implicit-modules", - ], - mnemonic = "SwiftCompile", - tags = [name], - target_under_test = "@build_bazel_rules_swift//test/fixtures/explicit_modules:simple", - ) - - native.test_suite( - name = name, - tags = [name], - ) diff --git a/test/fixtures/explicit_modules/BUILD b/test/fixtures/explicit_modules/BUILD deleted file mode 100644 index 5ae032abf..000000000 --- a/test/fixtures/explicit_modules/BUILD +++ /dev/null @@ -1,42 +0,0 @@ -load("//swift:swift.bzl", "swift_c_module", "swift_library") -load("//test/fixtures:common.bzl", "FIXTURE_TAGS") - -package( - default_visibility = ["//test:__subpackages__"], -) - -licenses(["notice"]) - -############################################################################### -# Fixtures for testing explicit clang module compilation. - -swift_library( - name = "simple", - srcs = ["Empty.swift"], - tags = FIXTURE_TAGS, - deps = [":shims"], -) - -swift_c_module( - name = "shims", - module_name = "SwiftShims", - system_module_map = "__BAZEL_XCODE_SDKROOT__/usr/lib/swift/shims/module.modulemap", - tags = FIXTURE_TAGS, -) - -swift_c_module( - name = "Darwin", - module_name = "Darwin", - system_module_map = "__BAZEL_XCODE_SDKROOT__/usr/include/module.modulemap", - tags = FIXTURE_TAGS, - deps = [ - ":_Builtin_stddef_max_align_t", - ], -) - -swift_c_module( - name = "_Builtin_stddef_max_align_t", - module_name = "_Builtin_stddef_max_align_t", - system_module_map = "__BAZEL_XCODE_DEVELOPER_DIR__/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/clang/include/module.modulemap", - tags = FIXTURE_TAGS, -) diff --git a/test/fixtures/explicit_modules/Empty.swift b/test/fixtures/explicit_modules/Empty.swift deleted file mode 100644 index bd9ec079d..000000000 --- a/test/fixtures/explicit_modules/Empty.swift +++ /dev/null @@ -1 +0,0 @@ -// Intentionally empty. From 5856821dd3f9663790587d75a9ddc0f87dbf9623 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Mon, 2 Aug 2021 09:56:08 -0700 Subject: [PATCH 05/16] Replace the `swift_module` tag in the gRPC BUILD overlay with a `swift_interop_hint`. PiperOrigin-RevId: 388242317 (cherry picked from commit 6ded44ed4e70fcd6c110d49609890d9e06775d5d) --- .../BUILD.overlay | 8 +++- .../com_github_apple_swift_nio/BUILD.overlay | 43 ++++++++++++++++--- .../BUILD.overlay | 15 ++++++- .../com_github_grpc_grpc_swift/BUILD.overlay | 8 +++- 4 files changed, 64 insertions(+), 10 deletions(-) diff --git a/third_party/com_github_apple_swift_atomics/BUILD.overlay b/third_party/com_github_apple_swift_atomics/BUILD.overlay index 8a4c7f946..daf04e60d 100644 --- a/third_party/com_github_apple_swift_atomics/BUILD.overlay +++ b/third_party/com_github_apple_swift_atomics/BUILD.overlay @@ -1,5 +1,6 @@ load( "@build_bazel_rules_swift//swift:swift.bzl", + "swift_interop_hint", "swift_library", ) @@ -24,8 +25,13 @@ cc_library( hdrs = glob([ "Sources/_AtomicsShims/**/*.h", ]), + aspect_hints = [":shims_interop"], copts = [], includes = ["Sources/_AtomicsShims/include"], - tags = ["swift_module=_AtomicsShims"], visibility = ["//visibility:private"], ) + +swift_interop_hint( + name = "shims_interop", + module_name = "_AtomicsShims", +) diff --git a/third_party/com_github_apple_swift_nio/BUILD.overlay b/third_party/com_github_apple_swift_nio/BUILD.overlay index d45fbe275..ebd14f7a6 100644 --- a/third_party/com_github_apple_swift_nio/BUILD.overlay +++ b/third_party/com_github_apple_swift_nio/BUILD.overlay @@ -1,5 +1,6 @@ load( "@build_bazel_rules_swift//swift:swift.bzl", + "swift_interop_hint", "swift_library", ) @@ -11,9 +12,14 @@ cc_library( hdrs = glob([ "Sources/CNIOAtomics/**/*.h", ]), + aspect_hints = [":CNIOAtomics_interop"], copts = [], includes = ["Sources/CNIOAtomics/include"], - tags = ["swift_module=CNIOAtomics"], +) + +swift_interop_hint( + name = "CNIOAtomics_interop", + module_name = "CNIOAtomics", ) cc_library( @@ -24,11 +30,16 @@ cc_library( hdrs = glob([ "Sources/CNIODarwin/**/*.h", ]), + aspect_hints = [":CNIODarwin_interop"], defines = [ "__APPLE_USE_RFC_3542", ], includes = ["Sources/CNIODarwin/include"], - tags = ["swift_module=CNIODarwin"], +) + +swift_interop_hint( + name = "CNIODarwin_interop", + module_name = "CNIODarwin", ) cc_library( @@ -39,12 +50,17 @@ cc_library( hdrs = glob([ "Sources/CNIOLLHTTP/**/*.h", ]), + aspect_hints = [":CNIOLLHTTP_interop"], copts = [], defines = [ "LLHTTP_STRICT_MODE", ], includes = ["Sources/CNIOLLHTTP/include"], - tags = ["swift_module=CNIOLLHTTP"], +) + +swift_interop_hint( + name = "CNIOLLHTTP_interop", + module_name = "CNIOLLHTTP", ) cc_library( @@ -55,9 +71,14 @@ cc_library( hdrs = glob([ "Sources/CNIOLinux/**/*.h", ]), + aspect_hints = [":CNIOLinux_interop"], copts = [], includes = ["Sources/CNIOLinux/include"], - tags = ["swift_module=CNIOLinux"], +) + +swift_interop_hint( + name = "CNIOLinux_interop", + module_name = "CNIOLinux", ) cc_library( @@ -68,9 +89,14 @@ cc_library( hdrs = [ "Sources/CNIOSHA1/**/*.h", ], + aspect_hints = [":CNIOSHA1_interop"], copts = [], includes = ["Sources/CNIOSHA1/include"], - tags = ["swift_module=CNIOSHA1"], +) + +swift_interop_hint( + name = "CNIOSHA1_interop", + module_name = "CNIOSHA1", ) cc_library( @@ -81,9 +107,14 @@ cc_library( hdrs = glob([ "Sources/CNIOWindows/**/*.h", ]), + aspect_hints = [":CNIOWindows_interop"], copts = [], includes = ["Sources/CNIOWindows/include"], - tags = ["swift_module=CNIOWindows"], +) + +swift_interop_hint( + name = "CNIOWindows_interop", + module_name = "CNIOWindows", ) swift_library( diff --git a/third_party/com_github_apple_swift_nio_ssl/BUILD.overlay b/third_party/com_github_apple_swift_nio_ssl/BUILD.overlay index e29e8ca9a..2ac93ab88 100644 --- a/third_party/com_github_apple_swift_nio_ssl/BUILD.overlay +++ b/third_party/com_github_apple_swift_nio_ssl/BUILD.overlay @@ -1,5 +1,6 @@ load( "@build_bazel_rules_swift//swift:swift.bzl", + "swift_interop_hint", "swift_library", ) @@ -28,13 +29,18 @@ cc_library( hdrs = glob([ "Sources/CNIOBoringSSLShims/include/**/*.h", ]), + aspect_hints = [":CNIOBoringSSLShims_interop"], copts = [], includes = ["Sources/CNIOBoringSSLShims/include"], - tags = ["swift_module=CNIOBoringSSLShims"], visibility = ["//visibility:public"], deps = [":CNIOBoringSSL"], ) +swift_interop_hint( + name = "CNIOBoringSSLShims_interop", + module_name = "CNIOBoringSSLShims", +) + cc_library( name = "CNIOBoringSSL", srcs = glob([ @@ -47,9 +53,14 @@ cc_library( "Sources/CNIOBoringSSL/include/**/*.h", "Sources/CNIOBoringSSL/include/**/*.inc", ]), + aspect_hints = [":CNIOBoringSSL_interop"], copts = [], includes = ["Sources/CNIOBoringSSL/include"], - tags = ["swift_module=CNIOBoringSSL"], visibility = ["//visibility:public"], deps = [], ) + +swift_interop_hint( + name = "CNIOBoringSSL_interop", + module_name = "CNIOBoringSSL", +) diff --git a/third_party/com_github_grpc_grpc_swift/BUILD.overlay b/third_party/com_github_grpc_grpc_swift/BUILD.overlay index f59a5cc1a..38228b04f 100644 --- a/third_party/com_github_grpc_grpc_swift/BUILD.overlay +++ b/third_party/com_github_grpc_grpc_swift/BUILD.overlay @@ -1,6 +1,7 @@ load( "@build_bazel_rules_swift//swift:swift.bzl", "swift_binary", + "swift_interop_hint", "swift_library", ) @@ -12,9 +13,14 @@ cc_library( hdrs = glob([ "Sources/CGRPCZlib/**/*.h", ]), + aspect_hints = [":CGRPCZLIB_interop"], includes = ["Sources/CGRPCZlib/include"], linkopts = ["-lz"], - tags = ["swift_module=CGRPCZlib"], +) + +swift_interop_hint( + name = "CGRPCZLIB_interop", + module_name = "CGRPCZlib", ) swift_library( From 15013e88e9289a0a65272a6e443e5fa128f44967 Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 24 Jan 2022 11:00:52 -0800 Subject: [PATCH 06/16] Revert "Generate module maps for `cc_library` deps" This reverts commit 15e8c3dc1f098d383666fd8c5e86fce6b8840272. --- swift/internal/swift_clang_module_aspect.bzl | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index b4fea41dd..6e5a17e02 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -398,17 +398,6 @@ def _module_info_for_target( attr = aspect_ctx.rule.attr module_map_file = None - # TODO: Remove once we cherry-pick the `swift_interop_hint` rule - if not module_name and aspect_ctx.rule.kind == "cc_library": - # For all other targets, there is no mechanism to provide a custom - # module map, and we only generate one if the target is tagged. - module_name = _tagged_target_module_name( - label = target.label, - tags = attr.tags, - ) - if not module_name: - return None, None - if not module_name: if apple_common.Objc not in target: return None, None From 69fa0f0aaa96b62ce4d50b1c4a9b8af1d95c158f Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Thu, 5 Aug 2021 08:29:12 -0700 Subject: [PATCH 07/16] Remove support for interop using the `swift_module` tag. Interop for non-Obj-C rules should now exclusively use `aspect_hints` (see the documentation for `swift_interop_hint`). PiperOrigin-RevId: 388940287 (cherry picked from commit a67043f5cf920c909bb26c08d220980819b91573) --- swift/BUILD | 1 - swift/internal/BUILD | 10 ---- swift/internal/compiling.bzl | 6 +-- swift/internal/swift_clang_module_aspect.bzl | 54 +++----------------- 4 files changed, 9 insertions(+), 62 deletions(-) diff --git a/swift/BUILD b/swift/BUILD index 4d2c1c60e..332c5e4da 100644 --- a/swift/BUILD +++ b/swift/BUILD @@ -54,7 +54,6 @@ bzl_library( "//swift/deprecated_proto:deprecated_swift_proto_library", "//swift/internal:providers", "//swift/internal:swift_binary_test_rules", - "//swift/internal:swift_c_module", "//swift/internal:swift_common", "//swift/internal:swift_feature_allowlist", "//swift/internal:swift_import", diff --git a/swift/internal/BUILD b/swift/internal/BUILD index 96d9ffec8..7ff653f46 100644 --- a/swift/internal/BUILD +++ b/swift/internal/BUILD @@ -144,16 +144,6 @@ bzl_library( deps = ["@build_bazel_rules_swift//swift/internal:feature_names"], ) -bzl_library( - name = "swift_c_module", - srcs = ["swift_c_module.bzl"], - visibility = ["//swift:__subpackages__"], - deps = [ - ":swift_common", - ":utils", - ], -) - bzl_library( name = "swift_feature_allowlist", srcs = ["swift_feature_allowlist.bzl"], diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 1f285a40a..b6f45cac1 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -1561,9 +1561,9 @@ def _collect_clang_module_inputs( # on the file system to map them to the module that contains them.) # However, we may also need some of the transitive headers, if the # module has dependencies that aren't recognized as modules (e.g., - # `cc_library` targets without the `swift_module` tag) and the - # module's headers include those. This will likely over-estimate the - # needed inputs, but we can't do better without include scanning in + # `cc_library` targets without an aspect hint) and the module's + # headers include those. This will likely over-estimate the needed + # inputs, but we can't do better without include scanning in # Starlark. transitive_inputs.append(cc_compilation_context.headers) diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 6e5a17e02..3efe9df1b 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -166,44 +166,6 @@ def create_swift_interop_info( unsupported_features = unsupported_features, ) -def _tagged_target_module_name(label, tags): - """Returns the module name of a `swift_module`-tagged target. - - The `swift_module` tag may take one of two forms: - - * `swift_module`: By itself, this indicates that the target is compatible - with Swift and should be given a module name that is derived from its - target label. - * `swift_module=name`: The module should be given the name `name`. - - If the `swift_module` tag is not present, no module name is used or - computed. - - Since tags are unprocessed strings, nothing prevents the `swift_module` tag - from being listed multiple times on the same target with different values. - For this reason, the aspect uses the _last_ occurrence that it finds in the - list. - - Args: - label: The target label from which a module name should be derived, if - necessary. - tags: The list of tags from the `cc_library` target to which the aspect - is being applied. - - Returns: - If the `swift_module` tag was present, then the return value is the - explicit name if it was of the form `swift_module=name`, or the - label-derived name if the tag was not followed by a name. Otherwise, if - the tag is not present, `None` is returned. - """ - module_name = None - for tag in tags: - if tag == "swift_module": - module_name = derive_module_name(label) - elif tag.startswith("swift_module="): - _, _, module_name = tag.partition("=") - return module_name - def _generate_module_map( actions, compilation_context, @@ -732,16 +694,12 @@ artifacts, and so that Swift module artifacts are not lost when passing through a non-Swift target in the build graph (for example, a `swift_library` that depends on an `objc_library` that depends on a `swift_library`). -It also manages module map generation for `cc_library` targets that have the -`swift_module` tag. This tag may take one of two forms: - -* `swift_module`: By itself, this indicates that the target is compatible - with Swift and should be given a module name that is derived from its - target label. -* `swift_module=name`: The module should be given the name `name`. - -Note that the public headers of such `cc_library` targets must be parsable as C, -since Swift does not support C++ interop at this time. +It also manages module map generation for targets that call +`swift_common.create_swift_interop_info` and do not provide their own module +map, and for targets that use the `swift_interop_hint` aspect hint. Note that if +one of these approaches is used to interop with a target such as a `cc_library`, +the headers must be parsable as C, since Swift does not support C++ interop at +this time. Most users will not need to interact directly with this aspect, since it is automatically applied to the `deps` attribute of all `swift_binary`, From 4cf19d23dbaeef6268ccda6eec2a4242f0828e84 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Mon, 16 Aug 2021 10:58:32 -0700 Subject: [PATCH 08/16] Add a way for `swift_interop_hint` to suppress the module for targets that generate them by default, like `objc_library`. PiperOrigin-RevId: 391087374 (cherry picked from commit de0f60472b5943275960b9ef4037374587064dab) --- doc/api.md | 5 +- doc/rules.md | 36 ++++++++++++- swift/BUILD | 8 +++ swift/internal/swift_clang_module_aspect.bzl | 13 +++++ swift/internal/swift_interop_hint.bzl | 42 +++++++++++++++ test/fixtures/BUILD | 6 +++ test/fixtures/common.bzl | 25 +++++++++ test/fixtures/interop_hints/BUILD | 43 +++++++++++++++- test/fixtures/interop_hints/Empty.swift | 1 + test/interop_hints_tests.bzl | 20 ++++++++ test/rules/provider_test.bzl | 54 +++++++++++++++++--- 11 files changed, 241 insertions(+), 12 deletions(-) create mode 100644 test/fixtures/interop_hints/Empty.swift diff --git a/doc/api.md b/doc/api.md index d5d386a69..a5b1069af 100644 --- a/doc/api.md +++ b/doc/api.md @@ -421,8 +421,8 @@ A new `SwiftInfo` provider with the given values. ## swift_common.create_swift_interop_info 
-swift_common.create_swift_interop_info(module_map, module_name, requested_features, swift_infos,
-                                       unsupported_features)
+swift_common.create_swift_interop_info(module_map, module_name, requested_features, suppressed,
+                                       swift_infos, unsupported_features)
Returns a provider that lets a target expose C/Objective-C APIs to Swift. @@ -459,6 +459,7 @@ exclude dependencies if necessary. | module_map | A `File` representing an existing module map that should be used to represent the module, or `None` (the default) if the module map should be generated based on the headers in the target's compilation context. If this argument is provided, then `module_name` must also be provided. | `None` | | module_name | A string denoting the name of the module, or `None` (the default) if the name should be derived automatically from the target label. | `None` | | requested_features | A list of features (empty by default) that should be requested for the target, which are added to those supplied in the `features` attribute of the target. These features will be enabled unless they are otherwise marked as unsupported (either on the target or by the toolchain). This allows the rule implementation to have additional control over features that should be supported by default for all instances of that rule as if it were creating the feature configuration itself; for example, a rule can request that `swift.emit_c_module` always be enabled for its targets even if it is not explicitly enabled in the toolchain or on the target directly. | `[]` | +| suppressed | A `bool` indicating whether the module that the aspect would create for the target should instead be suppressed. | `False` | | swift_infos | A list of `SwiftInfo` providers from dependencies, which will be merged with the new `SwiftInfo` created by the aspect. | `[]` | | unsupported_features | A list of features (empty by default) that should be considered unsupported for the target, which are added to those supplied as negations in the `features` attribute. This allows the rule implementation to have additional control over features that should be disabled by default for all instances of that rule as if it were creating the feature configuration itself; for example, a rule that processes frameworks with headers that do not follow strict layering can request that `swift.strict_module` always be disabled for its targets even if it is enabled by default in the toolchain. | `[]` | diff --git a/doc/rules.md b/doc/rules.md index 1f84b54ab..31a922fdd 100644 --- a/doc/rules.md +++ b/doc/rules.md @@ -424,7 +424,7 @@ the `.private.swiftinterface` files are required in order to build any code that ## swift_interop_hint 
-swift_interop_hint(name, module_map, module_name)
+swift_interop_hint(name, module_map, module_name, suppressed)
Defines an aspect hint that associates non-Swift BUILD targets with additional @@ -516,6 +516,39 @@ swift_interop_hint( ) ``` +#### Suppressing a module + +As mentioned above, `objc_library` and other Objective-C targets generate +modules by default, without an explicit hint, for convenience. In some +situations, this behavior may not be desirable. For example, an `objc_library` +might contain only Objective-C++ code in its headers that would not be possible +to import into Swift at all. + +When building with implicit modules, this is not typically an issue because the +module map would only be used if Swift code tried to import it (although it does +create useless actions and compiler inputs during the build). When building with +explicit modules, however, Bazel needs to know which targets represent modules +that it can compile and which do not. + +In these cases, there is no need to declare an instance of `swift_interop_hint`. +A canonical one that suppresses module generation has been provided in +`@build_bazel_rules_swift//swift:no_module`. Simply add it to the `aspect_hints` of +the target whose module you wish to suppress: + +```build +# //my/project/BUILD +objc_library( + name = "somelib", + srcs = ["somelib.mm"], + hdrs = ["somelib.h"], + aspect_hints = ["@build_bazel_rules_swift//swift:no_module"], +) +``` + +When this `objc_library` is a dependency of a Swift target, no module map or +explicit module will be generated for it, nor will any Swift information from +its transitive dependencies be propagated. + **ATTRIBUTES** @@ -524,6 +557,7 @@ swift_interop_hint( | name | A unique name for this target. | Name | required | | | module_map | An optional custom `.modulemap` file that defines the Clang module for the headers in the target to which this hint is applied.

If this attribute is omitted, a module map will be automatically generated based on the headers in the hinted target.

If this attribute is provided, then `module_name` must also be provided and match the name of the desired top-level module in the `.modulemap` file. (A single `.modulemap` file may define multiple top-level modules.) | Label | optional | `None` | | module_name | The name that will be used to import the hinted module into Swift.

If left unspecified, the module name will be computed based on the hinted target's build label, by stripping the leading `//` and replacing `/`, `:`, and other non-identifier characters with underscores. | String | optional | `""` | +| suppressed | If `True`, the hinted target should suppress any module that it would otherwise generate. | Boolean | optional | `False` | diff --git a/swift/BUILD b/swift/BUILD index 332c5e4da..9aef22966 100644 --- a/swift/BUILD +++ b/swift/BUILD @@ -104,6 +104,14 @@ swift_interop_hint( visibility = ["//visibility:public"], ) +# An aspect hint that suppresses generation of a module for a non-Swift target +# that would otherwise generate one by default, like an `objc_library`. +swift_interop_hint( + name = "no_module", + suppressed = True, + visibility = ["//visibility:public"], +) + # User settable flag that specifies additional Swift copts on a per-swiftmodule basis. per_module_swiftcopt_flag( name = "per_module_swiftcopt", diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 3efe9df1b..9c26edb35 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -69,6 +69,10 @@ implementation to supply an additional set of fixed features that should always be enabled when the aspect processes that target; for example, a rule can request that `swift.emit_c_module` always be enabled for its targets even if it is not explicitly enabled in the toolchain or on the target directly. +""", + "suppressed": """\ +A `bool` indicating whether the module that the aspect would create for the +target should instead be suppressed. """, "swift_infos": """\ A list of `SwiftInfo` providers from dependencies of the target, which will be @@ -91,6 +95,7 @@ def create_swift_interop_info( module_map = None, module_name = None, requested_features = [], + suppressed = False, swift_infos = [], unsupported_features = []): """Returns a provider that lets a target expose C/Objective-C APIs to Swift. @@ -138,6 +143,8 @@ def create_swift_interop_info( `swift.emit_c_module` always be enabled for its targets even if it is not explicitly enabled in the toolchain or on the target directly. + suppressed: A `bool` indicating whether the module that the aspect would + create for the target should instead be suppressed. swift_infos: A list of `SwiftInfo` providers from dependencies, which will be merged with the new `SwiftInfo` created by the aspect. unsupported_features: A list of features (empty by default) that should @@ -162,6 +169,7 @@ def create_swift_interop_info( module_map = module_map, module_name = module_name, requested_features = requested_features, + suppressed = suppressed, swift_infos = swift_infos, unsupported_features = unsupported_features, ) @@ -641,6 +649,11 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): interop_info, swift_infos = _find_swift_interop_info(target, aspect_ctx) if interop_info: + # If the module should be suppressed, return immediately and propagate + # nothing (not even transitive dependencies). + if interop_info.suppressed: + return [] + module_map_file = interop_info.module_map module_name = ( interop_info.module_name or derive_module_name(target.label) diff --git a/swift/internal/swift_interop_hint.bzl b/swift/internal/swift_interop_hint.bzl index 72fddbc9a..e3760cccb 100644 --- a/swift/internal/swift_interop_hint.bzl +++ b/swift/internal/swift_interop_hint.bzl @@ -22,6 +22,7 @@ def _swift_interop_hint_impl(ctx): return swift_common.create_swift_interop_info( module_map = ctx.file.module_map, module_name = ctx.attr.module_name, + suppressed = ctx.attr.suppressed, ) swift_interop_hint = rule( @@ -48,6 +49,14 @@ The name that will be used to import the hinted module into Swift. If left unspecified, the module name will be computed based on the hinted target's build label, by stripping the leading `//` and replacing `/`, `:`, and other non-identifier characters with underscores. +""", + mandatory = False, + ), + "suppressed": attr.bool( + default = False, + doc = """\ +If `True`, the hinted target should suppress any module that it would otherwise +generate. """, mandatory = False, ), @@ -141,6 +150,39 @@ swift_interop_hint( module_name = "CSomeLib", ) ``` + +#### Suppressing a module + +As mentioned above, `objc_library` and other Objective-C targets generate +modules by default, without an explicit hint, for convenience. In some +situations, this behavior may not be desirable. For example, an `objc_library` +might contain only Objective-C++ code in its headers that would not be possible +to import into Swift at all. + +When building with implicit modules, this is not typically an issue because the +module map would only be used if Swift code tried to import it (although it does +create useless actions and compiler inputs during the build). When building with +explicit modules, however, Bazel needs to know which targets represent modules +that it can compile and which do not. + +In these cases, there is no need to declare an instance of `swift_interop_hint`. +A canonical one that suppresses module generation has been provided in +`@build_bazel_rules_swift//swift:no_module`. Simply add it to the `aspect_hints` of +the target whose module you wish to suppress: + +```build +# //my/project/BUILD +objc_library( + name = "somelib", + srcs = ["somelib.mm"], + hdrs = ["somelib.h"], + aspect_hints = ["@build_bazel_rules_swift//swift:no_module"], +) +``` + +When this `objc_library` is a dependency of a Swift target, no module map or +explicit module will be generated for it, nor will any Swift information from +its transitive dependencies be propagated. """, implementation = _swift_interop_hint_impl, ) diff --git a/test/fixtures/BUILD b/test/fixtures/BUILD index ae789254e..6c4e8d7e8 100644 --- a/test/fixtures/BUILD +++ b/test/fixtures/BUILD @@ -2,6 +2,12 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library") package(default_visibility = ["//test:__subpackages__"]) +bzl_library( + name = "starlark_tests_bzls", + srcs = glob(["*.bzl"]), + deps = ["//swift"], +) + bzl_library( name = "common", srcs = ["common.bzl"], diff --git a/test/fixtures/common.bzl b/test/fixtures/common.bzl index 7fdf657fa..472ada8ea 100644 --- a/test/fixtures/common.bzl +++ b/test/fixtures/common.bzl @@ -14,9 +14,34 @@ """Common build definitions used by test fixtures.""" +load( + "@build_bazel_rules_swift//swift:swift.bzl", + "SwiftInfo", + "swift_clang_module_aspect", +) + # Common tags that prevent the test fixtures from actually being built (i.e., # their actions executed) when running `bazel test` to do analysis testing. FIXTURE_TAGS = [ "manual", "notap", ] + +def _forward_swift_info_from_swift_clang_module_aspect_impl(ctx): + if SwiftInfo in ctx.attr.target: + return [ctx.attr.target[SwiftInfo]] + return [] + +forward_swift_info_from_swift_clang_module_aspect = rule( + attrs = { + "target": attr.label( + aspects = [swift_clang_module_aspect], + mandatory = True, + ), + }, + doc = """\ +Applies `swift_clang_module_aspect` to the given target and forwards the +`SwiftInfo` provider that it attaches to the target, if any. +""", + implementation = _forward_swift_info_from_swift_clang_module_aspect_impl, +) diff --git a/test/fixtures/interop_hints/BUILD b/test/fixtures/interop_hints/BUILD index 2b33cb36f..0f1bab1fa 100644 --- a/test/fixtures/interop_hints/BUILD +++ b/test/fixtures/interop_hints/BUILD @@ -3,7 +3,11 @@ load( "swift_interop_hint", "swift_library", ) -load("//test/fixtures:common.bzl", "FIXTURE_TAGS") +load( + "//test/fixtures:common.bzl", + "FIXTURE_TAGS", + "forward_swift_info_from_swift_clang_module_aspect", +) package( default_visibility = ["//test:__subpackages__"], @@ -80,3 +84,40 @@ swift_interop_hint( module_map = "module.modulemap", tags = FIXTURE_TAGS, ) + +forward_swift_info_from_swift_clang_module_aspect( + name = "objc_library_suppressed", + tags = FIXTURE_TAGS, + target = ":objc_library_suppressed_lib", + target_compatible_with = ["@platforms//os:macos"], +) + +objc_library( + name = "objc_library_suppressed_lib", + hdrs = ["header1.h"], + aspect_hints = ["//swift:no_module"], + tags = FIXTURE_TAGS, + target_compatible_with = ["@platforms//os:macos"], +) + +forward_swift_info_from_swift_clang_module_aspect( + name = "objc_library_with_swift_dep_suppressed", + tags = FIXTURE_TAGS, + target = ":objc_library_with_swift_dep_suppressed_lib", + target_compatible_with = ["@platforms//os:macos"], +) + +objc_library( + name = "objc_library_with_swift_dep_suppressed_lib", + hdrs = ["header1.h"], + aspect_hints = ["//swift:no_module"], + tags = FIXTURE_TAGS, + target_compatible_with = ["@platforms//os:macos"], + deps = [":empty_lib"], +) + +swift_library( + name = "empty_lib", + srcs = ["Empty.swift"], + tags = FIXTURE_TAGS, +) diff --git a/test/fixtures/interop_hints/Empty.swift b/test/fixtures/interop_hints/Empty.swift new file mode 100644 index 000000000..bd9ec079d --- /dev/null +++ b/test/fixtures/interop_hints/Empty.swift @@ -0,0 +1 @@ +// Intentionally empty. diff --git a/test/interop_hints_tests.bzl b/test/interop_hints_tests.bzl index 7a87e4252..cf8f31dc4 100644 --- a/test/interop_hints_tests.bzl +++ b/test/interop_hints_tests.bzl @@ -66,6 +66,26 @@ def interop_hints_test_suite(name = "interop_hints"): target_under_test = "@build_bazel_rules_swift//test/fixtures/interop_hints:invalid_swift", ) + # Verify that an `objc_library` hinted to suppress its module does not + # propagate a `SwiftInfo` provider at all. + provider_test( + name = "{}_objc_library_module_suppressed".format(name), + does_not_propagate_provider = "SwiftInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/interop_hints:objc_library_suppressed", + target_compatible_with = ["@platforms//os:macos"], + ) + + # Verify that an `objc_library` hinted to suppress its module does not + # propagate a `SwiftInfo` provider even if it has Swift dependencies. + provider_test( + name = "{}_objc_library_module_with_swift_dep_suppressed".format(name), + does_not_propagate_provider = "SwiftInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/interop_hints:objc_library_with_swift_dep_suppressed", + target_compatible_with = ["@platforms//os:macos"], + ) + native.test_suite( name = name, tags = [name], diff --git a/test/rules/provider_test.bzl b/test/rules/provider_test.bzl index c34b54546..5ce1d0d72 100644 --- a/test/rules/provider_test.bzl +++ b/test/rules/provider_test.bzl @@ -165,11 +165,6 @@ def _lookup_provider_by_name(env, target, provider_name): if provider in target: return target[provider] - - unittest.fail( - env, - "Target '{}' did not provide '{}'.".format(target.label, provider_name), - ) return None def _field_access_description(target, provider, field): @@ -330,12 +325,41 @@ def _provider_test_impl(ctx): if types.is_list(target_under_test): target_under_test = target_under_test[0] + provider_name = ctx.attr.does_not_propagate_provider + if provider_name: + provider = _lookup_provider_by_name( + env, + target_under_test, + provider_name, + ) + if provider: + unittest.fail( + env, + "Expected {} to not propagate '{}', but it did: {}".format( + target_under_test.label, + provider_name, + provider, + ), + ) + return analysistest.end(env) + provider_name = ctx.attr.provider + field = ctx.attr.field + if not provider_name or not field: + fail("Either 'does_not_propagate_provider' must be specified, or " + + "both 'provider' and 'field' must be specified.") + provider = _lookup_provider_by_name(env, target_under_test, provider_name) if not provider: + unittest.fail( + env, + "Target '{}' did not provide '{}'.".format( + target_under_test.label, + provider_name, + ), + ) return analysistest.end(env) - field = ctx.attr.field actual = _evaluate_field(env, provider, field) if actual == _EVALUATE_FIELD_FAILED: return analysistest.end(env) @@ -371,6 +395,20 @@ def make_provider_test_rule(config_settings = {}): return analysistest.make( _provider_test_impl, attrs = { + "does_not_propagate_provider": attr.string( + mandatory = False, + doc = """\ +The name of a provider that is expected to not be propagated by the target under +test. + +Currently, only the following providers are recognized: + +* `CcInfo` +* `DefaultInfo` +* `SwiftInfo` +* `apple_common.Objc` +""", + ), "expected_files": attr.string_list( mandatory = False, doc = """\ @@ -392,7 +430,7 @@ configuration details, such as output directories for generated files. """, ), "field": attr.string( - mandatory = True, + mandatory = False, doc = """\ The field name or dotted field path of the provider that should be tested. @@ -406,7 +444,7 @@ the next component. """, ), "provider": attr.string( - mandatory = True, + mandatory = False, doc = """\ The name of the provider expected to be propagated by the target under test, and on which the field will be checked. From ea194769d38faa391a1f6b63ce12fcb3fff8396d Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Tue, 21 Sep 2021 13:52:27 -0700 Subject: [PATCH 09/16] Add `exclude_hdrs` to `swift_interop_hint`. This attribute can be used to exclude a list of headers from the Swift-generated module map (via `exclude header` declarations) without removing them from the hinted target completely. This is often helpful in cases where some subset of headers are not Swift-compatible but still needed as part of the library for other reasons (e.g., they are private headers used by implementation source files, or still used by other non-Swift dependents). PiperOrigin-RevId: 398076709 (cherry picked from commit ef6662cec1fdb7731f2c11004a0831fc8b4faa33) --- doc/api.md | 5 +- doc/rules.md | 3 +- swift/internal/module_maps.bzl | 56 +++++++++++++++----- swift/internal/swift_clang_module_aspect.bzl | 33 +++++++++++- swift/internal/swift_interop_hint.bzl | 16 ++++++ 5 files changed, 96 insertions(+), 17 deletions(-) diff --git a/doc/api.md b/doc/api.md index a5b1069af..a614d5ee5 100644 --- a/doc/api.md +++ b/doc/api.md @@ -421,8 +421,8 @@ A new `SwiftInfo` provider with the given values. ## swift_common.create_swift_interop_info 
-swift_common.create_swift_interop_info(module_map, module_name, requested_features, suppressed,
-                                       swift_infos, unsupported_features)
+swift_common.create_swift_interop_info(exclude_headers, module_map, module_name, requested_features,
+                                       suppressed, swift_infos, unsupported_features)
Returns a provider that lets a target expose C/Objective-C APIs to Swift. @@ -456,6 +456,7 @@ exclude dependencies if necessary. | Name | Description | Default Value | | :------------- | :------------- | :------------- | +| exclude_headers | A `list` of `File`s representing headers that should be excluded from the module if the module map is generated. | `[]` | | module_map | A `File` representing an existing module map that should be used to represent the module, or `None` (the default) if the module map should be generated based on the headers in the target's compilation context. If this argument is provided, then `module_name` must also be provided. | `None` | | module_name | A string denoting the name of the module, or `None` (the default) if the name should be derived automatically from the target label. | `None` | | requested_features | A list of features (empty by default) that should be requested for the target, which are added to those supplied in the `features` attribute of the target. These features will be enabled unless they are otherwise marked as unsupported (either on the target or by the toolchain). This allows the rule implementation to have additional control over features that should be supported by default for all instances of that rule as if it were creating the feature configuration itself; for example, a rule can request that `swift.emit_c_module` always be enabled for its targets even if it is not explicitly enabled in the toolchain or on the target directly. | `[]` | diff --git a/doc/rules.md b/doc/rules.md index 31a922fdd..d98cd76e5 100644 --- a/doc/rules.md +++ b/doc/rules.md @@ -424,7 +424,7 @@ the `.private.swiftinterface` files are required in order to build any code that ## swift_interop_hint 
-swift_interop_hint(name, module_map, module_name, suppressed)
+swift_interop_hint(name, exclude_hdrs, module_map, module_name, suppressed)
Defines an aspect hint that associates non-Swift BUILD targets with additional @@ -555,6 +555,7 @@ its transitive dependencies be propagated. | Name | Description | Type | Mandatory | Default | | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this target. | Name | required | | +| exclude_hdrs | A list of header files that should be excluded from the Clang module generated for the target to which this hint is applied. This allows a target to exclude a subset of a library's headers specifically from the Swift module map without removing them from the library completely, which can be useful if some headers are not Swift-compatible but are still needed by other sources in the library or by non-Swift dependents.

This attribute may only be specified if a custom `module_map` is not provided. Setting both attributes is an error. | List of labels | optional | `[]` | | module_map | An optional custom `.modulemap` file that defines the Clang module for the headers in the target to which this hint is applied.

If this attribute is omitted, a module map will be automatically generated based on the headers in the hinted target.

If this attribute is provided, then `module_name` must also be provided and match the name of the desired top-level module in the `.modulemap` file. (A single `.modulemap` file may define multiple top-level modules.) | Label | optional | `None` | | module_name | The name that will be used to import the hinted module into Swift.

If left unspecified, the module name will be computed based on the hinted target's build label, by stripping the leading `//` and replacing `/`, `:`, and other non-identifier characters with underscores. | String | optional | `""` | | suppressed | If `True`, the hinted target should suppress any module that it would otherwise generate. | Boolean | optional | `False` | diff --git a/swift/internal/module_maps.bzl b/swift/internal/module_maps.bzl index 972ad2b0b..21ff4ce9d 100644 --- a/swift/internal/module_maps.bzl +++ b/swift/internal/module_maps.bzl @@ -14,6 +14,8 @@ """Logic for generating Clang module map files.""" +load("@bazel_skylib//lib:sets.bzl", "sets") + # TODO(#723): Remove these disables once https://github.com/bazelbuild/buildtools/issues/926 is fixed # buildifier: disable=return-value # buildifier: disable=function-docstring-return @@ -22,6 +24,7 @@ def write_module_map( module_map_file, module_name, dependent_module_names = [], + exclude_headers = [], exported_module_ids = [], public_headers = [], public_textual_headers = [], @@ -37,6 +40,8 @@ def write_module_map( module_name: The name of the module being generated. dependent_module_names: A `list` of names of Clang modules that are direct dependencies of the target whose module map is being written. + exclude_headers: A `list` of `File`s representing headers that should be + explicitly excluded from the module being written. exported_module_ids: A `list` of Clang wildcard module identifiers that will be re-exported as part of the API of the module being written. The values in this list should match `wildcard-module-id` as @@ -71,34 +76,55 @@ def write_module_map( relative_to_dir = module_map_file.dirname back_to_root_path = "../" * len(relative_to_dir.split("/")) + excluded_headers_set = sets.make(exclude_headers) + content = actions.args() content.set_param_file_format("multiline") - content.add(module_name, format = 'module "%s" {') + def _relativized_header_path(file): + return _header_path( + header_file = file, + relative_to_dir = relative_to_dir, + back_to_root_path = back_to_root_path, + ) - # Write an `export` declaration for each of the module identifiers that - # should be re-exported by this module. - content.add_all(exported_module_ids, format_each = " export %s") - content.add("") + def _relativized_header_paths_with_exclusion( + file_or_dir, + directory_expander): + return [ + _relativized_header_path(file) + for file in directory_expander.expand(file_or_dir) + if not sets.contains(excluded_headers_set, file) + ] def _relativized_header_paths(file_or_dir, directory_expander): return [ - _header_path( - header_file = file, - relative_to_dir = relative_to_dir, - back_to_root_path = back_to_root_path, - ) + _relativized_header_path(file) for file in directory_expander.expand(file_or_dir) ] - def _add_headers(*, headers, kind): + def _add_headers(*, allow_excluded_headers = False, headers, kind): + if allow_excluded_headers: + map_each = _relativized_header_paths + else: + map_each = _relativized_header_paths_with_exclusion + content.add_all( headers, allow_closure = True, format_each = ' {} "%s"'.format(kind), - map_each = _relativized_header_paths, + map_each = map_each, ) + content.add(module_name, format = 'module "%s" {') + + # Write an `export` declaration for each of the module identifiers that + # should be re-exported by this module. + content.add_all(exported_module_ids, format_each = " export %s") + content.add("") + + # When writing these headers, honor the `exclude_headers` list (i.e., remove + # any headers from these lists that also appear there). if umbrella_header: _add_headers(headers = [umbrella_header], kind = "umbrella header") else: @@ -110,6 +136,12 @@ def write_module_map( kind = "private textual header", ) + _add_headers( + allow_excluded_headers = True, + headers = exclude_headers, + kind = "exclude header", + ) + content.add("") # Write a `use` declaration for each of the module's dependencies. diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 9c26edb35..506f2f857 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -52,6 +52,11 @@ Contains minimal information required to allow `swift_clang_module_aspect` to manage the creation of a `SwiftInfo` provider for a C/Objective-C target. """, fields = { + "exclude_headers": """\ +A `list` of `File`s representing headers that should be excluded from the +module, if a module map is being automatically generated based on the headers in +the target's compilation context. +""", "module_map": """\ A `File` representing an existing module map that should be used to represent the module, or `None` if the module map should be generated based on the headers @@ -92,6 +97,7 @@ enabled by default in the toolchain. def create_swift_interop_info( *, + exclude_headers = [], module_map = None, module_name = None, requested_features = [], @@ -124,6 +130,8 @@ def create_swift_interop_info( exclude dependencies if necessary. Args: + exclude_headers: A `list` of `File`s representing headers that should be + excluded from the module if the module map is generated. module_map: A `File` representing an existing module map that should be used to represent the module, or `None` (the default) if the module map should be generated based on the headers in the target's @@ -162,10 +170,16 @@ def create_swift_interop_info( A provider whose type/layout is an implementation detail and should not be relied upon. """ - if module_map and not module_name: - fail("'module_name' must be specified when 'module_map' is specified.") + if module_map: + if not module_name: + fail("'module_name' must be specified when 'module_map' is " + + "specified.") + if exclude_headers: + fail("'exclude_headers' may not be specified when 'module_map' " + + "is specified.") return _SwiftInteropInfo( + exclude_headers = exclude_headers, module_map = module_map, module_name = module_name, requested_features = requested_features, @@ -178,6 +192,7 @@ def _generate_module_map( actions, compilation_context, dependent_module_names, + exclude_headers, feature_configuration, module_name, target, @@ -190,6 +205,8 @@ def _generate_module_map( headers for the module. dependent_module_names: A `list` of names of Clang modules that are direct dependencies of the target whose module map is being written. + exclude_headers: A `list` of `File`s representing header files to + exclude, if any, if we are generating the module map. feature_configuration: A Swift feature configuration. module_name: The name of the module. target: The target for which the module map is being generated. @@ -231,6 +248,7 @@ def _generate_module_map( write_module_map( actions = actions, dependent_module_names = sorted(dependent_module_names), + exclude_headers = sorted(exclude_headers, key = _path_sorting_key), exported_module_ids = ["*"], module_map_file = module_map_file, module_name = module_name, @@ -323,6 +341,7 @@ def _module_info_for_target( aspect_ctx, compilation_context, dependent_module_names, + exclude_headers, feature_configuration, module_name, umbrella_header): @@ -335,6 +354,8 @@ def _module_info_for_target( headers for the module. dependent_module_names: A `list` of names of Clang modules that are direct dependencies of the target whose module map is being written. + exclude_headers: A `list` of `File`s representing header files to + exclude, if any, if we are generating the module map. feature_configuration: A Swift feature configuration. module_name: The module name to prefer (if we're generating a module map from `_SwiftInteropInfo`), or None to derive it from other @@ -387,6 +408,7 @@ def _module_info_for_target( actions = aspect_ctx.actions, compilation_context = compilation_context, dependent_module_names = dependent_module_names, + exclude_headers = exclude_headers, feature_configuration = feature_configuration, module_name = module_name, target = target, @@ -397,6 +419,7 @@ def _module_info_for_target( def _handle_module( aspect_ctx, compilation_context, + exclude_headers, feature_configuration, module_map_file, module_name, @@ -409,6 +432,8 @@ def _handle_module( aspect_ctx: The aspect's context. compilation_context: The `CcCompilationContext` containing the target's headers. + exclude_headers: A `list` of `File`s representing header files to + exclude, if any, if we are generating the module map. feature_configuration: The current feature configuration. module_map_file: The `.modulemap` file that defines the module, or None if it should be inferred from other properties of the target (for @@ -454,6 +479,7 @@ def _handle_module( aspect_ctx = aspect_ctx, compilation_context = compilation_context, dependent_module_names = dependent_module_names, + exclude_headers = exclude_headers, feature_configuration = feature_configuration, module_name = module_name, umbrella_header = umbrella_header, @@ -654,6 +680,7 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): if interop_info.suppressed: return [] + exclude_headers = interop_info.exclude_headers module_map_file = interop_info.module_map module_name = ( interop_info.module_name or derive_module_name(target.label) @@ -662,6 +689,7 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): requested_features.extend(interop_info.requested_features) unsupported_features.extend(interop_info.unsupported_features) else: + exclude_headers = [] module_map_file = None module_name = None @@ -677,6 +705,7 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): return _handle_module( aspect_ctx = aspect_ctx, compilation_context = _compilation_context_for_target(target), + exclude_headers = exclude_headers, feature_configuration = feature_configuration, module_map_file = module_map_file, module_name = module_name, diff --git a/swift/internal/swift_interop_hint.bzl b/swift/internal/swift_interop_hint.bzl index e3760cccb..87571f221 100644 --- a/swift/internal/swift_interop_hint.bzl +++ b/swift/internal/swift_interop_hint.bzl @@ -20,6 +20,7 @@ def _swift_interop_hint_impl(ctx): # TODO(b/194733180): Take advantage of the richer API to add support for # other features, like APINotes, later. return swift_common.create_swift_interop_info( + exclude_headers = ctx.files.exclude_hdrs, module_map = ctx.file.module_map, module_name = ctx.attr.module_name, suppressed = ctx.attr.suppressed, @@ -27,6 +28,21 @@ def _swift_interop_hint_impl(ctx): swift_interop_hint = rule( attrs = { + "exclude_hdrs": attr.label_list( + allow_files = True, + doc = """\ +A list of header files that should be excluded from the Clang module generated +for the target to which this hint is applied. This allows a target to exclude +a subset of a library's headers specifically from the Swift module map without +removing them from the library completely, which can be useful if some headers +are not Swift-compatible but are still needed by other sources in the library or +by non-Swift dependents. + +This attribute may only be specified if a custom `module_map` is not provided. +Setting both attributes is an error. +""", + mandatory = False, + ), "module_map": attr.label( allow_single_file = True, doc = """\ From 495edff658081d8d3f19753c570d03acc538394d Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Mon, 29 Nov 2021 10:07:50 -0800 Subject: [PATCH 10/16] Correctly exclude both the actual and virtual header when using `swift_interop_hint.exclude_hdrs` on a `cc_library` that has `include_prefix` and/or `strip_include_prefix` set. PiperOrigin-RevId: 412917671 (cherry picked from commit 6b13232996f759a8a98d21e4b94d086c4b604d64) --- swift/internal/swift_clang_module_aspect.bzl | 66 +++++++++++-- test/BUILD | 3 + test/cc_library_tests.bzl | 52 ++++++++++ test/fixtures/cc_library/BUILD | 95 +++++++++++++++++++ .../ImportPrefixAndStripPrefix.swift | 3 + ...ortPrefixAndStripPrefixWithExclusion.swift | 3 + .../cc_library/ImportPrefixOnly.swift | 3 + .../cc_library/ImportStripPrefixOnly.swift | 3 + test/fixtures/cc_library/header.h | 8 ++ .../header_prefix_and_strip_prefix.h | 6 ++ test/fixtures/cc_library/header_prefix_only.h | 6 ++ .../cc_library/header_strip_prefix_only.h | 6 ++ test/fixtures/cc_library/header_with_error.h | 6 ++ 13 files changed, 251 insertions(+), 9 deletions(-) create mode 100644 test/cc_library_tests.bzl create mode 100644 test/fixtures/cc_library/BUILD create mode 100644 test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift create mode 100644 test/fixtures/cc_library/ImportPrefixAndStripPrefixWithExclusion.swift create mode 100644 test/fixtures/cc_library/ImportPrefixOnly.swift create mode 100644 test/fixtures/cc_library/ImportStripPrefixOnly.swift create mode 100644 test/fixtures/cc_library/header.h create mode 100644 test/fixtures/cc_library/header_prefix_and_strip_prefix.h create mode 100644 test/fixtures/cc_library/header_prefix_only.h create mode 100644 test/fixtures/cc_library/header_strip_prefix_only.h create mode 100644 test/fixtures/cc_library/header_with_error.h diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 506f2f857..424292293 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -14,6 +14,7 @@ """Propagates unified `SwiftInfo` providers for C/Objective-C targets.""" +load("@bazel_skylib//lib:sets.bzl", "sets") load(":attrs.bzl", "swift_toolchain_attrs") load(":compiling.bzl", "derive_module_name", "precompile_clang_module") load(":derived_files.bzl", "derived_files") @@ -188,7 +189,42 @@ def create_swift_interop_info( unsupported_features = unsupported_features, ) +def _compute_all_excluded_headers(*, exclude_headers, target): + """Returns the full set of headers to exclude for a target. + + This function specifically handles the `cc_library` logic around the + `include_prefix` and `strip_include_prefix` attributes, which cause Bazel to + create a virtual header (symlink) for every public header in the target. For + the generated module map to be created, we must exclude both the actual + header file and the symlink. + + Args: + exclude_headers: A list of `File`s representing headers that should be + excluded from the module. + target: The target to which the aspect is being applied. + + Returns: + A list containing the complete set of headers that should be excluded, + including any virtual header symlinks that match a real header in the + excluded headers list passed into the function. + """ + exclude_headers_set = sets.make(exclude_headers) + virtual_exclude_headers = [] + + for action in target.actions: + if action.mnemonic != "Symlink": + continue + + original_header = action.inputs.to_list()[0] + virtual_header = action.outputs.to_list()[0] + + if sets.contains(exclude_headers_set, original_header): + virtual_exclude_headers.append(virtual_header) + + return exclude_headers + virtual_exclude_headers + def _generate_module_map( + *, actions, compilation_context, dependent_module_names, @@ -234,17 +270,32 @@ def _generate_module_map( else: private_headers = compilation_context.direct_private_headers - module_map_file = derived_files.module_map( - actions = actions, - target_name = target.label.name, - ) - # Sort dependent module names and the headers to ensure a deterministic # order in the output file, in the event the compilation context would ever # change this on us. For files, use the execution path as the sorting key. def _path_sorting_key(file): return file.path + public_headers = sorted( + compilation_context.direct_public_headers, + key = _path_sorting_key, + ) + + module_map_file = derived_files.module_map( + actions = actions, + target_name = target.label.name, + ) + + if exclude_headers: + # If we're excluding headers from the module map, make sure to pick up + # any virtual header symlinks that might be created, for example, by a + # `cc_library` using the `include_prefix` and/or `strip_include_prefix` + # attributes. + exclude_headers = _compute_all_excluded_headers( + exclude_headers = exclude_headers, + target = target, + ) + write_module_map( actions = actions, dependent_module_names = sorted(dependent_module_names), @@ -253,10 +304,7 @@ def _generate_module_map( module_map_file = module_map_file, module_name = module_name, private_headers = sorted(private_headers, key = _path_sorting_key), - public_headers = sorted( - compilation_context.direct_public_headers, - key = _path_sorting_key, - ), + public_headers = public_headers, public_textual_headers = sorted( compilation_context.direct_textual_headers, key = _path_sorting_key, diff --git a/test/BUILD b/test/BUILD index 8e7bb92fd..3097529be 100644 --- a/test/BUILD +++ b/test/BUILD @@ -1,6 +1,7 @@ load("@bazel_skylib//:bzl_library.bzl", "bzl_library") load(":ast_file_tests.bzl", "ast_file_test_suite") load(":bzl_test.bzl", "bzl_test") +load(":cc_library_tests.bzl", "cc_library_test_suite") load(":compiler_arguments_tests.bzl", "compiler_arguments_test_suite") load(":coverage_settings_tests.bzl", "coverage_settings_test_suite") load(":debug_settings_tests.bzl", "debug_settings_test_suite") @@ -26,6 +27,8 @@ ast_file_test_suite(name = "ast_file") compiler_arguments_test_suite(name = "compiler_arguments") +cc_library_test_suite(name = "cc_library") + coverage_settings_test_suite(name = "coverage_settings") debug_settings_test_suite(name = "debug_settings") diff --git a/test/cc_library_tests.bzl b/test/cc_library_tests.bzl new file mode 100644 index 000000000..2ec50a33e --- /dev/null +++ b/test/cc_library_tests.bzl @@ -0,0 +1,52 @@ +# Copyright 2021 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for interoperability with `cc_library`-specific features.""" + +load( + "@bazel_skylib//rules:build_test.bzl", + "build_test", +) + +def cc_library_test_suite(name): + """Test suite for interoperability with `cc_library`-specific features.""" + + # Verify that Swift can import a `cc_library` that uses `include_prefix`, + # `strip_include_prefix`, or both. + build_test( + name = "{}_swift_imports_cc_library_with_include_prefix_manipulation".format(name), + targets = [ + "@build_bazel_rules_swift//test/fixtures/cc_library:import_prefix_and_strip_prefix", + "@build_bazel_rules_swift//test/fixtures/cc_library:import_prefix_only", + "@build_bazel_rules_swift//test/fixtures/cc_library:import_strip_prefix_only", + ], + tags = [name], + ) + + # Verify that `swift_interop_hint.exclude_hdrs` correctly excludes headers + # from a `cc_library` that uses `include_prefix` and/or + # `strip_include_prefix` (i.e., both the real header and the virtual header + # are excluded). + build_test( + name = "{}_swift_interop_hint_excludes_headers_with_include_prefix_manipulation".format(name), + targets = [ + "@build_bazel_rules_swift//test/fixtures/cc_library:import_prefix_and_strip_prefix_with_exclusion", + ], + tags = [name], + ) + + native.test_suite( + name = name, + tags = [name], + ) diff --git a/test/fixtures/cc_library/BUILD b/test/fixtures/cc_library/BUILD new file mode 100644 index 000000000..7e473a8e3 --- /dev/null +++ b/test/fixtures/cc_library/BUILD @@ -0,0 +1,95 @@ +load( + "//swift:swift.bzl", + "swift_interop_hint", + "swift_library", +) +load("//test/fixtures:common.bzl", "FIXTURE_TAGS") + +package( + default_visibility = ["//test:__subpackages__"], +) + +licenses(["notice"]) + +############################################################################### +# Fixtures for testing complex `cc_library` interop cases. + +cc_library( + name = "prefix_only", + hdrs = [ + "header.h", + "header_prefix_only.h", + ], + aspect_hints = ["//swift:auto_module"], + include_prefix = "some/prefix", + tags = FIXTURE_TAGS, +) + +cc_library( + name = "strip_prefix_only", + hdrs = [ + "header.h", + "header_strip_prefix_only.h", + ], + aspect_hints = ["//swift:auto_module"], + strip_include_prefix = "//test", + tags = FIXTURE_TAGS, +) + +cc_library( + name = "prefix_and_strip_prefix", + hdrs = [ + "header.h", + "header_prefix_and_strip_prefix.h", + ], + aspect_hints = ["//swift:auto_module"], + include_prefix = "some/prefix", + strip_include_prefix = "//test", + tags = FIXTURE_TAGS, +) + +cc_library( + name = "prefix_and_strip_prefix_with_exclusion", + hdrs = [ + "header.h", + "header_with_error.h", + ], + aspect_hints = [":prefix_and_strip_prefix_with_exclusion_swift_hint"], + include_prefix = "some/prefix", + strip_include_prefix = "//test", + tags = FIXTURE_TAGS, +) + +swift_interop_hint( + name = "prefix_and_strip_prefix_with_exclusion_swift_hint", + exclude_hdrs = ["header_with_error.h"], + tags = FIXTURE_TAGS, +) + +swift_library( + name = "import_prefix_only", + srcs = ["ImportPrefixOnly.swift"], + tags = FIXTURE_TAGS, + deps = [":prefix_only"], +) + +swift_library( + name = "import_strip_prefix_only", + srcs = ["ImportStripPrefixOnly.swift"], + tags = FIXTURE_TAGS, + deps = [":strip_prefix_only"], +) + +swift_library( + name = "import_prefix_and_strip_prefix", + srcs = ["ImportPrefixAndStripPrefix.swift"], + tags = FIXTURE_TAGS, + deps = [":prefix_and_strip_prefix"], +) + +swift_library( + name = "import_prefix_and_strip_prefix_with_exclusion", + srcs = ["ImportPrefixAndStripPrefixWithExclusion.swift"], + tags = FIXTURE_TAGS, + deps = [":prefix_and_strip_prefix_with_exclusion"], +) diff --git a/test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift b/test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift new file mode 100644 index 000000000..fa63322ff --- /dev/null +++ b/test/fixtures/cc_library/ImportPrefixAndStripPrefix.swift @@ -0,0 +1,3 @@ +import test_fixtures_cc_library_prefix_and_strip_prefix + +func f(_ x: some_structure) {} diff --git a/test/fixtures/cc_library/ImportPrefixAndStripPrefixWithExclusion.swift b/test/fixtures/cc_library/ImportPrefixAndStripPrefixWithExclusion.swift new file mode 100644 index 000000000..83e47256d --- /dev/null +++ b/test/fixtures/cc_library/ImportPrefixAndStripPrefixWithExclusion.swift @@ -0,0 +1,3 @@ +import test_fixtures_cc_library_prefix_and_strip_prefix_with_exclusion + +func f(_ x: some_structure) {} diff --git a/test/fixtures/cc_library/ImportPrefixOnly.swift b/test/fixtures/cc_library/ImportPrefixOnly.swift new file mode 100644 index 000000000..75b9bf61e --- /dev/null +++ b/test/fixtures/cc_library/ImportPrefixOnly.swift @@ -0,0 +1,3 @@ +import test_fixtures_cc_library_prefix_only + +func f(_ x: some_structure) {} diff --git a/test/fixtures/cc_library/ImportStripPrefixOnly.swift b/test/fixtures/cc_library/ImportStripPrefixOnly.swift new file mode 100644 index 000000000..5a9910a7f --- /dev/null +++ b/test/fixtures/cc_library/ImportStripPrefixOnly.swift @@ -0,0 +1,3 @@ +import test_fixtures_cc_library_strip_prefix_only + +func f(_ x: some_structure) {} diff --git a/test/fixtures/cc_library/header.h b/test/fixtures/cc_library/header.h new file mode 100644 index 000000000..0ce38ca4e --- /dev/null +++ b/test/fixtures/cc_library/header.h @@ -0,0 +1,8 @@ +#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_H_ +#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_H_ + +typedef struct { + int field; +} some_structure; + +#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_H_ diff --git a/test/fixtures/cc_library/header_prefix_and_strip_prefix.h b/test/fixtures/cc_library/header_prefix_and_strip_prefix.h new file mode 100644 index 000000000..4b8f42c46 --- /dev/null +++ b/test/fixtures/cc_library/header_prefix_and_strip_prefix.h @@ -0,0 +1,6 @@ +#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_AND_STRIP_PREFIX_H_ +#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_AND_STRIP_PREFIX_H_ + +#include "some/prefix/fixtures/cc_library/header.h" + +#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_AND_STRIP_PREFIX_H_ diff --git a/test/fixtures/cc_library/header_prefix_only.h b/test/fixtures/cc_library/header_prefix_only.h new file mode 100644 index 000000000..137e8038a --- /dev/null +++ b/test/fixtures/cc_library/header_prefix_only.h @@ -0,0 +1,6 @@ +#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_ONLY_H_ +#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_ONLY_H_ + +#include "some/prefix/header.h" + +#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_PREFIX_ONLY_H_ diff --git a/test/fixtures/cc_library/header_strip_prefix_only.h b/test/fixtures/cc_library/header_strip_prefix_only.h new file mode 100644 index 000000000..2f307994c --- /dev/null +++ b/test/fixtures/cc_library/header_strip_prefix_only.h @@ -0,0 +1,6 @@ +#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_STRIP_PREFIX_ONLY_H_ +#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_STRIP_PREFIX_ONLY_H_ + +#include "fixtures/cc_library/header.h" + +#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_STRIP_PREFIX_ONLY_H_ diff --git a/test/fixtures/cc_library/header_with_error.h b/test/fixtures/cc_library/header_with_error.h new file mode 100644 index 000000000..6a89e9677 --- /dev/null +++ b/test/fixtures/cc_library/header_with_error.h @@ -0,0 +1,6 @@ +#ifndef RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_WITH_ERROR_H_ +#define RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_WITH_ERROR_H_ + +#error This should never get processed. + +#endif // RULES_SWIFT_TEST_FIXTURES_CC_LIBRARY_HEADER_WITH_ERROR_H_ From 3ee31297d04de401119bb9eb97df24f60df1ddf0 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 14 Dec 2021 11:42:25 -0800 Subject: [PATCH 11/16] Update Swift build rules to support, and enable, explicit modules for J2ObjC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Targets processed by J2ObjC's aspects have a new compilation context created that treats the generated J2ObjC headers as textual headers, and the "umbrella" header as a regular header. The root path of the generated headers is added to the compilation context as an include path so the headers can be found by dependents. To handle `java_libary`'s `exports` attribute (this lists targets that should be treated as a direct dependency when depending on the `java_libary` in question), the `SwiftInfos` from dependencies are split into direct and indirect. These are then propagated as `direct_swift_infos` vs `swift_infos` respectively to provide the semantics of `exports`. The modular import header rewriter was updated to accommodate the fact that the J2ObjC "umbrella" header is no longer marked as being an umbrella header in the module map. It still needs to be rewritten for the same reason as before, but must now be detected by a portion of the file path (🤮). PiperOrigin-RevId: 416355827 (cherry picked from commit dd22a51c5a9bf31138b4213810d3bacd13459cc8) --- swift/internal/compiling.bzl | 1 + swift/internal/providers.bzl | 3 +- swift/internal/swift_clang_module_aspect.bzl | 169 +++++++++++++------ swift/internal/utils.bzl | 10 ++ 4 files changed, 134 insertions(+), 49 deletions(-) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index b6f45cac1..e6eb1cebb 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -1566,6 +1566,7 @@ def _collect_clang_module_inputs( # inputs, but we can't do better without include scanning in # Starlark. transitive_inputs.append(cc_compilation_context.headers) + transitive_inputs.append(depset(cc_compilation_context.direct_textual_headers)) # Some rules still use the `umbrella_header` field to propagate a header # that they don't also include in `cc_compilation_context.headers`, so we diff --git a/swift/internal/providers.bzl b/swift/internal/providers.bzl index 9b3128f80..362f621da 100644 --- a/swift/internal/providers.bzl +++ b/swift/internal/providers.bzl @@ -533,8 +533,9 @@ def create_swift_info( """ direct_modules = modules + [ - provider.modules + module for provider in direct_swift_infos + for module in provider.direct_modules ] transitive_modules = [ provider.transitive_modules diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 424292293..42ead7cdc 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -20,8 +20,11 @@ load(":compiling.bzl", "derive_module_name", "precompile_clang_module") load(":derived_files.bzl", "derived_files") load( ":feature_names.bzl", + "SWIFT_FEATURE_EMIT_C_MODULE", + "SWIFT_FEATURE_LAYERING_CHECK", "SWIFT_FEATURE_MODULE_MAP_HOME_IS_CWD", "SWIFT_FEATURE_MODULE_MAP_NO_PRIVATE_HEADERS", + "SWIFT_FEATURE_USE_C_MODULES", ) load(":features.bzl", "configure_features", "is_feature_enabled") load(":module_maps.bzl", "write_module_map") @@ -45,6 +48,14 @@ _MULTIPLE_TARGET_ASPECT_ATTRS = [ _SINGLE_TARGET_ASPECT_ATTRS = [ # TODO(b/151667396): Remove j2objc-specific attributes when possible. "_jre_lib", + "_j2objc_proto_toolchain", + "runtime", +] + +# TODO(b/151667396): Remove j2objc-specific attributes when possible. +_DIRECT_ASPECT_ATTRS = [ + "exports", + "_j2objc_proto_toolchain", ] _SwiftInteropInfo = provider( @@ -231,8 +242,7 @@ def _generate_module_map( exclude_headers, feature_configuration, module_name, - target, - umbrella_header): + target): """Generates the module map file for the given target. Args: @@ -246,9 +256,6 @@ def _generate_module_map( feature_configuration: A Swift feature configuration. module_name: The name of the module. target: The target for which the module map is being generated. - umbrella_header: A `File` representing an umbrella header that, if - present, will be written into the module map instead of the list of - headers in the compilation context. Returns: A `File` representing the generated module map. """ @@ -309,7 +316,6 @@ def _generate_module_map( compilation_context.direct_textual_headers, key = _path_sorting_key, ), - umbrella_header = umbrella_header, workspace_relative = workspace_relative, ) return module_map_file @@ -342,12 +348,13 @@ def _objc_library_module_info(aspect_ctx): return module_name, module_map_file # TODO(b/151667396): Remove j2objc-specific knowledge. -def _j2objc_umbrella_workaround(target): - """Tries to find and return the umbrella header for a J2ObjC target. +def _j2objc_compilation_context(target): + """Construct and return a compilation context for a J2ObjC target This is an unfortunate hack/workaround needed for J2ObjC, which needs to use an umbrella header that `#include`s, rather than `#import`s, the headers in - the module due to the way they're segmented. + the module due to the way they're segmented. Additionally, the headers + need a header search path set for them to be found. It's also somewhat ugly in the way that it has to find the umbrella header, which is tied to Bazel's built-in module map generation. Since there's not a @@ -362,27 +369,47 @@ def _j2objc_umbrella_workaround(target): target: The target to which the aspect is being applied. Returns: - A tuple containing two elements: - - * A `File` representing the umbrella header generated by the target, - or `None` if there was none. - * A `CcCompilationContext` containing the direct generated headers of - the J2ObjC target (including the umbrella header), or `None` if the - target did not generate an umbrella header. + A `CcCompilationContext` containing the direct generated headers of + the J2ObjC target (including the umbrella header), or `None` if the + target did not generate an umbrella header. """ for action in target.actions: if action.mnemonic != "UmbrellaHeader": continue umbrella_header = action.outputs.to_list()[0] - compilation_context = cc_common.create_compilation_context( - headers = depset( - target[apple_common.Objc].direct_headers + [umbrella_header], - ), + + if JavaInfo not in target: + # This is not a `java_library`, but a `proto_library` processed by J2ObjC. We need to + # treat the generated headers as textual, but do not need the special header search path + # logic like we do for `java_library` targets below. + return cc_common.create_compilation_context( + headers = depset([umbrella_header]), + direct_textual_headers = target[apple_common.Objc].direct_headers, + ) + + # Use the path of the first generated header to determine the base directory that + # J2ObjC wrote its generated header files to. + header_path = target[apple_common.Objc].direct_headers[0].path + + if "/_j2objc/src_jar_files/" in header_path: + # When source jars are used the headers are generated within a tree artifact. + # We can use the path to the tree artifact as the include search path. + include_path = header_path + else: + # J2ObjC generates headers within //_j2objc//. + # Compute that path to use as the include search path. + header_path_components = header_path.split("/") + j2objc_index = header_path_components.index("_j2objc") + include_path = "/".join(header_path_components[:j2objc_index + 2]) + + return cc_common.create_compilation_context( + headers = depset([umbrella_header]), + direct_textual_headers = target[apple_common.Objc].direct_headers, + includes = depset([include_path]), ) - return umbrella_header, compilation_context - return None, None + return None def _module_info_for_target( target, @@ -391,8 +418,7 @@ def _module_info_for_target( dependent_module_names, exclude_headers, feature_configuration, - module_name, - umbrella_header): + module_name): """Returns the module name and module map for the target. Args: @@ -408,9 +434,6 @@ def _module_info_for_target( module_name: The module name to prefer (if we're generating a module map from `_SwiftInteropInfo`), or None to derive it from other properties of the target. - umbrella_header: A `File` representing an umbrella header that, if - present, will be written into the module map instead of the list of - headers in the compilation context. Returns: A tuple containing the module name (a string) and module map file (a @@ -460,7 +483,6 @@ def _module_info_for_target( feature_configuration = feature_configuration, module_name = module_name, target = target, - umbrella_header = umbrella_header, ) return module_name, module_map_file @@ -471,6 +493,7 @@ def _handle_module( feature_configuration, module_map_file, module_name, + direct_swift_infos, swift_infos, swift_toolchain, target): @@ -488,6 +511,9 @@ def _handle_module( legacy support). module_name: The name of the module, or None if it should be inferred from other properties of the target (for legacy support). + direct_swift_infos: The `SwiftInfo` providers of the current target's + dependencies, which should be merged into the `SwiftInfo` provider + created and returned for this target. swift_infos: The `SwiftInfo` providers of the current target's dependencies, which should be merged into the `SwiftInfo` provider created and returned for this target. @@ -500,7 +526,7 @@ def _handle_module( attr = aspect_ctx.rule.attr all_swift_infos = ( - swift_infos + swift_toolchain.clang_implicit_deps_providers.swift_infos + direct_swift_infos + swift_infos + swift_toolchain.clang_implicit_deps_providers.swift_infos ) # Collect the names of Clang modules that the module being built directly @@ -516,10 +542,8 @@ def _handle_module( # support legacy rules. if not module_map_file: # TODO(b/151667396): Remove j2objc-specific knowledge. - umbrella_header, new_compilation_context = _j2objc_umbrella_workaround( - target = target, - ) - if umbrella_header: + new_compilation_context = _j2objc_compilation_context(target = target) + if new_compilation_context: compilation_context = new_compilation_context module_name, module_map_file = _module_info_for_target( @@ -530,21 +554,34 @@ def _handle_module( exclude_headers = exclude_headers, feature_configuration = feature_configuration, module_name = module_name, - umbrella_header = umbrella_header, ) if not module_map_file: if all_swift_infos: - return [create_swift_info(swift_infos = swift_infos)] + return [ + create_swift_info( + direct_swift_infos = direct_swift_infos, + swift_infos = swift_infos, + ), + ] else: return [] compilation_context_to_compile = ( compilation_context_for_explicit_module_compilation( compilation_contexts = [compilation_context], - deps = getattr(attr, "deps", []), + deps = [ + target + for attr_name in _MULTIPLE_TARGET_ASPECT_ATTRS + for target in getattr(attr, attr_name, []) + ] + [ + getattr(attr, attr_name) + for attr_name in _SINGLE_TARGET_ASPECT_ATTRS + if hasattr(attr, attr_name) + ], ) ) + precompiled_module = precompile_clang_module( actions = aspect_ctx.actions, cc_compilation_context = compilation_context_to_compile, @@ -568,6 +605,7 @@ def _handle_module( ), ), ], + direct_swift_infos = direct_swift_infos, swift_infos = swift_infos, ), ] @@ -622,24 +660,44 @@ def _collect_swift_infos_from_deps(aspect_ctx): aspect_ctx: The aspect's context. Returns: - A list of `SwiftInfo` providers from dependencies of the target to which - the aspect was applied. + A tuple of lists of `SwiftInfo` providers from dependencies of the target to which + the aspect was applied. The first list contains those from attributes that should be treated + as direct, while the second list contains those from all other attributes. """ + direct_swift_infos = [] swift_infos = [] attr = aspect_ctx.rule.attr for attr_name in _MULTIPLE_TARGET_ASPECT_ATTRS: - swift_infos.extend([ + infos = [ dep[SwiftInfo] for dep in getattr(attr, attr_name, []) if SwiftInfo in dep - ]) + ] + + if attr_name in _DIRECT_ASPECT_ATTRS: + direct_swift_infos.extend(infos) + else: + swift_infos.extend(infos) + for attr_name in _SINGLE_TARGET_ASPECT_ATTRS: dep = getattr(attr, attr_name, None) if dep and SwiftInfo in dep: - swift_infos.append(dep[SwiftInfo]) + if attr_name in _DIRECT_ASPECT_ATTRS: + direct_swift_infos.append(dep[SwiftInfo]) + else: + swift_infos.append(dep[SwiftInfo]) + + # TODO(b/151667396): Remove j2objc-specific knowledge. + if str(aspect_ctx.label) in ("@bazel_tools//tools/j2objc:j2objc_proto_toolchain", "//third_party/java/j2objc:proto_runtime"): + # The J2ObjC proto runtime headers are implicit dependencies of the generated J2ObjC code + # by being transitively reachable via the toolchain and runtime targets. The `SwiftInfos` of + # these targets need to be propagated as direct so that J2ObjC code using the runtime + # headers appears to have a direct dependency on them. + direct_swift_infos.extend(swift_infos) + swift_infos = [] - return swift_infos + return direct_swift_infos, swift_infos def _find_swift_interop_info(target, aspect_ctx): """Finds a `_SwiftInteropInfo` provider associated with the target. @@ -673,6 +731,7 @@ def _find_swift_interop_info(target, aspect_ctx): # and merge `SwiftInfo` providers from relevant dependencies. interop_target = target interop_from_rule = True + default_direct_swift_infos = [] default_swift_infos = [] else: # If the target's rule implementation does not directly provide @@ -684,7 +743,7 @@ def _find_swift_interop_info(target, aspect_ctx): # after the call site of this function. interop_target = None interop_from_rule = False - default_swift_infos = _collect_swift_infos_from_deps(aspect_ctx) + default_direct_swift_infos, default_swift_infos = _collect_swift_infos_from_deps(aspect_ctx) # We don't break this loop early when we find a matching hint, because we # want to give an error message if there are two aspect hints that provide @@ -710,8 +769,8 @@ def _find_swift_interop_info(target, aspect_ctx): interop_target = hint if interop_target: - return interop_target[_SwiftInteropInfo], default_swift_infos - return None, default_swift_infos + return interop_target[_SwiftInteropInfo], default_direct_swift_infos, default_swift_infos + return None, default_direct_swift_infos, default_swift_infos def _swift_clang_module_aspect_impl(target, aspect_ctx): # Do nothing if the target already propagates `SwiftInfo`. @@ -721,7 +780,7 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): requested_features = aspect_ctx.features unsupported_features = aspect_ctx.disabled_features - interop_info, swift_infos = _find_swift_interop_info(target, aspect_ctx) + interop_info, direct_swift_infos, swift_infos = _find_swift_interop_info(target, aspect_ctx) if interop_info: # If the module should be suppressed, return immediately and propagate # nothing (not even transitive dependencies). @@ -741,6 +800,16 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): module_map_file = None module_name = None + if hasattr(aspect_ctx.rule.attr, "_jre_lib"): + # TODO(b/151667396): Remove j2objc-specific knowledge. + # Force explicit modules on for targets processed by `j2objc_library`. + requested_features.extend([ + SWIFT_FEATURE_EMIT_C_MODULE, + SWIFT_FEATURE_USE_C_MODULES, + SWIFT_FEATURE_LAYERING_CHECK, + ]) + unsupported_features.append(SWIFT_FEATURE_MODULE_MAP_NO_PRIVATE_HEADERS) + swift_toolchain = aspect_ctx.attr._toolchain_for_aspect[SwiftToolchainInfo] feature_configuration = configure_features( ctx = aspect_ctx, @@ -757,6 +826,7 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): feature_configuration = feature_configuration, module_map_file = module_map_file, module_name = module_name, + direct_swift_infos = direct_swift_infos, swift_infos = swift_infos, swift_toolchain = swift_toolchain, target = target, @@ -764,8 +834,11 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): # If it's any other rule, just merge the `SwiftInfo` providers from its # deps. - if swift_infos: - return [create_swift_info(swift_infos = swift_infos)] + if direct_swift_infos or swift_infos: + return [create_swift_info( + direct_swift_infos = direct_swift_infos, + swift_infos = swift_infos, + )] return [] diff --git a/swift/internal/utils.bzl b/swift/internal/utils.bzl index 6e1f9c14a..c9a54bad1 100644 --- a/swift/internal/utils.bzl +++ b/swift/internal/utils.bzl @@ -91,6 +91,16 @@ def compilation_context_for_explicit_module_compilation( for dep in deps: if CcInfo in dep: all_compilation_contexts.append(dep[CcInfo].compilation_context) + elif SwiftInfo in dep: + # TODO(b/151667396): Remove j2objc-specific knowledge. + # J2ObjC doesn't expose CcInfo directly on the `java_library` targets it process, but we can find the + # compilation context that was synthesized by `swift_clang_module_aspect` within the `SwiftInfo` provider. + all_compilation_contexts.extend([ + module.clang.compilation_context + for module in dep[SwiftInfo].direct_modules + if module.clang + ]) + if apple_common.Objc in dep: all_compilation_contexts.append( cc_common.create_compilation_context( From 8095662fdfddf930989be08e332d0f80c28ea169 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Thu, 16 Dec 2021 09:06:15 -0800 Subject: [PATCH 12/16] Represent symlinks from `cc_inc_library` targets as textual headers so that layering checks are still satisfied correctly. PiperOrigin-RevId: 416824355 (cherry picked from commit 7c7ee72acf60ad434faac65ae56d6a9193641682) --- swift/internal/swift_clang_module_aspect.bzl | 32 +++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 42ead7cdc..3baea0c46 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -237,6 +237,7 @@ def _compute_all_excluded_headers(*, exclude_headers, target): def _generate_module_map( *, actions, + aspect_ctx, compilation_context, dependent_module_names, exclude_headers, @@ -247,6 +248,7 @@ def _generate_module_map( Args: actions: The object used to register actions. + aspect_ctx: The aspect context. compilation_context: The C++ compilation context that provides the headers for the module. dependent_module_names: A `list` of names of Clang modules that are @@ -283,10 +285,26 @@ def _generate_module_map( def _path_sorting_key(file): return file.path - public_headers = sorted( - compilation_context.direct_public_headers, - key = _path_sorting_key, - ) + # The headers in a `cc_inc_library` are actually symlinks to headers in its + # `deps`. This interferes with layering because the `cc_inc_library` won't + # depend directly on the libraries containing headers that the symlinked + # headers include. Generating the module map with the symlinks as textual + # headers instead of modular headers fixes this. + if aspect_ctx.rule.kind == "cc_inc_library": + public_headers = [] + textual_headers = sorted( + compilation_context.direct_public_headers, + key = _path_sorting_key, + ) + else: + public_headers = sorted( + compilation_context.direct_public_headers, + key = _path_sorting_key, + ) + textual_headers = sorted( + compilation_context.direct_textual_headers, + key = _path_sorting_key, + ) module_map_file = derived_files.module_map( actions = actions, @@ -312,10 +330,7 @@ def _generate_module_map( module_name = module_name, private_headers = sorted(private_headers, key = _path_sorting_key), public_headers = public_headers, - public_textual_headers = sorted( - compilation_context.direct_textual_headers, - key = _path_sorting_key, - ), + public_textual_headers = textual_headers, workspace_relative = workspace_relative, ) return module_map_file @@ -477,6 +492,7 @@ def _module_info_for_target( if not module_map_file: module_map_file = _generate_module_map( actions = aspect_ctx.actions, + aspect_ctx = aspect_ctx, compilation_context = compilation_context, dependent_module_names = dependent_module_names, exclude_headers = exclude_headers, From 6920972cab22240f158269c32c1009f957b3f377 Mon Sep 17 00:00:00 2001 From: Walter Lee Date: Tue, 21 Dec 2021 10:32:59 -0800 Subject: [PATCH 13/16] Migrate ObjcProvider's direct headers usage to CcInfo The direct_headers field in ObjcProvider is being deprecated and will be removed. The same information can be found in CcInfo. J2ObjcAspect has been modified so that the CcInfo is accessible via Starlark. PiperOrigin-RevId: 417648590 (cherry picked from commit 612725e5c027f5525842880d5250b72e56d6b73c) --- swift/internal/swift_clang_module_aspect.bzl | 40 ++++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index 3baea0c46..eb58d0efb 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -375,10 +375,7 @@ def _j2objc_compilation_context(target): which is tied to Bazel's built-in module map generation. Since there's not a direct umbrella header field in `ObjcProvider`, we scan the target's actions to find the one that writes it out. Then, we return it and a new compilation - context with the direct headers from the `ObjcProvider`, since the generated - headers are not accessible via `CcInfo`--Java targets to which the J2ObjC - aspect are applied do not propagate `CcInfo` directly, but a native Bazel - provider that wraps the `CcInfo`, and we have no access to it from Starlark. + context with the direct headers from the `CcInfo` of the J2ObjC aspect. Args: target: The target to which the aspect is being applied. @@ -393,6 +390,7 @@ def _j2objc_compilation_context(target): continue umbrella_header = action.outputs.to_list()[0] + headers = target[CcInfo].compilation_context.direct_headers if JavaInfo not in target: # This is not a `java_library`, but a `proto_library` processed by J2ObjC. We need to @@ -400,28 +398,30 @@ def _j2objc_compilation_context(target): # logic like we do for `java_library` targets below. return cc_common.create_compilation_context( headers = depset([umbrella_header]), - direct_textual_headers = target[apple_common.Objc].direct_headers, + direct_textual_headers = headers, ) - # Use the path of the first generated header to determine the base directory that - # J2ObjC wrote its generated header files to. - header_path = target[apple_common.Objc].direct_headers[0].path + include_paths = sets.make() + for header in headers: + header_path = header.path - if "/_j2objc/src_jar_files/" in header_path: - # When source jars are used the headers are generated within a tree artifact. - # We can use the path to the tree artifact as the include search path. - include_path = header_path - else: - # J2ObjC generates headers within //_j2objc//. - # Compute that path to use as the include search path. - header_path_components = header_path.split("/") - j2objc_index = header_path_components.index("_j2objc") - include_path = "/".join(header_path_components[:j2objc_index + 2]) + if "/_j2objc/src_jar_files/" in header_path: + # When source jars are used the headers are generated within a tree artifact. + # We can use the path to the tree artifact as the include search path. + include_path = header_path + else: + # J2ObjC generates headers within //_j2objc//. + # Compute that path to use as the include search path. + header_path_components = header_path.split("/") + j2objc_index = header_path_components.index("_j2objc") + include_path = "/".join(header_path_components[:j2objc_index + 2]) + + sets.insert(include_paths, include_path) return cc_common.create_compilation_context( headers = depset([umbrella_header]), - direct_textual_headers = target[apple_common.Objc].direct_headers, - includes = depset([include_path]), + direct_textual_headers = headers, + includes = depset(sets.to_list(include_paths)), ) return None From 0eadb3d7c518e3f59059dc4e30f023de6c1d5d49 Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Mon, 24 Jan 2022 08:30:10 -0800 Subject: [PATCH 14/16] Copy `strict_includes` into `SwiftInfo` provider's Clang module descriptor in `swift_clang_module_aspect`. This makes it so that the `apple_common.Objc` provider no longer needs to be handled separately for compilation by Swift build APIs. (It is still used for linking until that is migrated entirely onto `CcInfo`.) PiperOrigin-RevId: 423822059 (cherry picked from commit 8ce95959d19334ce53963c6cbce3ac6eb40cd28c) --- doc/api.md | 33 +--- swift/internal/compiling.bzl | 153 ++++++++----------- swift/internal/providers.bzl | 34 ++--- swift/internal/swift_clang_module_aspect.bzl | 27 +++- swift/internal/utils.bzl | 31 ++-- 5 files changed, 120 insertions(+), 158 deletions(-) diff --git a/doc/api.md b/doc/api.md index a614d5ee5..0d57fa156 100644 --- a/doc/api.md +++ b/doc/api.md @@ -103,7 +103,7 @@ Compiles a Swift module. | additional_inputs | A list of `File`s representing additional input files that need to be passed to the Swift compile action because they are referenced by compiler flags. | `[]` | | copts | A list of compiler flags that apply to the target being built. These flags, along with those from Bazel's Swift configuration fragment (i.e., `--swiftcopt` command line flags) are scanned to determine whether whole module optimization is being requested, which affects the nature of the output files. | `[]` | | defines | Symbols that should be defined by passing `-D` to the compiler. | `[]` | -| deps | Non-private dependencies of the target being compiled. These targets are used as dependencies of both the Swift module being compiled and the Clang module for the generated header. These targets must propagate one of the following providers: `CcInfo`, `SwiftInfo`, or `apple_common.Objc`. | `[]` | +| deps | Non-private dependencies of the target being compiled. These targets are used as dependencies of both the Swift module being compiled and the Clang module for the generated header. These targets must propagate `CcInfo` or `SwiftInfo`. | `[]` | | extra_swift_infos | Extra `SwiftInfo` providers that aren't contained by the `deps` of the target being compiled but are required for compilation. | `[]` | | feature_configuration | A feature configuration obtained from `swift_common.configure_features`. | none | | generated_header_name | The name of the Objective-C generated header that should be generated for this module. If omitted, no header will be generated. | `None` | @@ -112,7 +112,7 @@ Compiles a Swift module. | module_name | The name of the Swift module being compiled. This must be present and valid; use `swift_common.derive_module_name` to generate a default from the target's label if needed. | none | | package_name | The semantic package of the name of the Swift module being compiled. | none | | plugins | A list of `SwiftCompilerPluginInfo` providers that represent plugins that should be loaded by the compiler. | `[]` | -| private_deps | Private (implementation-only) dependencies of the target being compiled. These are only used as dependencies of the Swift module, not of the Clang module for the generated header. These targets must propagate one of the following providers: `CcInfo`, `SwiftInfo`, or `apple_common.Objc`. | `[]` | +| private_deps | Private (implementation-only) dependencies of the target being compiled. These are only used as dependencies of the Swift module, not of the Clang module for the generated header. These targets must propagate `CcInfo` or `SwiftInfo`. | `[]` | | srcs | The Swift source files to compile. | none | | swift_toolchain | The `SwiftToolchainInfo` provider of the toolchain. | none | | target_name | The name of the target for which the code is being compiled, which is used to determine unique file paths for the outputs. | none | @@ -215,31 +215,12 @@ An opaque value representing the feature configuration that can be ## swift_common.create_clang_module 
-swift_common.create_clang_module(compilation_context, module_map, precompiled_module)
+swift_common.create_clang_module(compilation_context, module_map, precompiled_module,
+                                 strict_includes)
Creates a value representing a Clang module used as a Swift dependency. -Note: The `compilation_context` argument of this function is primarily -intended to communicate information *to* the Swift build rules, not to -retrieve information *back out.* In most cases, it is better to depend on -the `CcInfo` provider propagated by a Swift target to collect transitive -C/Objective-C compilation information about that target. This is because the -context used when compiling the module itself may not be the same as the -context desired when depending on it. (For example, `apple_common.Objc` -supports "strict include paths" which are only propagated to direct -dependents.) - -One valid exception to the guidance above is retrieving the generated header -associated with a specific Swift module. Since the `CcInfo` provider -propagated by the library will have already merged them transitively (or, -in the case of a hypothetical custom rule that propagates multiple direct -modules, the `direct_public_headers` of the `CcInfo` would also have them -merged), it is acceptable to read the headers from the compilation context -of the module struct itself in order to associate them with the module that -generated them. - - **PARAMETERS** @@ -248,11 +229,13 @@ generated them. | compilation_context | A `CcCompilationContext` that contains the header files and other context (such as include paths, preprocessor defines, and so forth) needed to compile this module as an explicit module. | none | | module_map | The text module map file that defines this module. This argument may be specified as a `File` or as a `string`; in the latter case, it is assumed to be the path to a file that cannot be provided as an action input because it is outside the workspace (for example, the module map for a module from an Xcode SDK). | none | | precompiled_module | A `File` representing the precompiled module (`.pcm` file) if one was emitted for the module. This may be `None` if no explicit module was built for the module; in that case, targets that depend on the module will fall back to the text module map and headers. | `None` | +| strict_includes | A `depset` of strings representing additional Clang include paths that should be passed to the compiler when this module is a _direct_ dependency of the module being compiled. May be `None`. **This field only exists to support a specific legacy use case and should otherwise not be used, as it is fundamentally incompatible with Swift's import model.** | `None` | **RETURNS** -A `struct` containing the `compilation_context`, `module_map`, and - `precompiled_module` fields provided as arguments. +A `struct` containing the `compilation_context`, `module_map`, + `precompiled_module`, and `strict_includes` fields provided as + arguments. diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index e6eb1cebb..c05405eb9 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -1458,17 +1458,28 @@ def _pcm_developer_framework_paths_configurator(prerequisites, args): args, ) +def _clang_module_strict_includes(module_context): + """Returns the strict Clang include paths for a module context.""" + if not module_context.clang: + return None + strict_includes = module_context.clang.strict_includes + if not strict_includes: + return None + return strict_includes.to_list() + def _clang_search_paths_configurator(prerequisites, args): """Adds Clang search paths to the command line.""" args.add_all( - depset(transitive = [ - prerequisites.cc_compilation_context.includes, - # TODO(b/146575101): Replace with `objc_info.include` once this bug - # is fixed. See `_merge_target_providers` below for more details. - prerequisites.objc_include_paths_workaround, - ]), + prerequisites.cc_compilation_context.includes, + before_each = "-Xcc", + format_each = "-I%s", + ) + args.add_all( + prerequisites.transitive_modules, before_each = "-Xcc", format_each = "-I%s", + map_each = _clang_module_strict_includes, + uniquify = True, ) # Add Clang search paths for the workspace root and Bazel output roots. The @@ -1515,7 +1526,6 @@ def _collect_clang_module_inputs( cc_compilation_context, is_swift, modules, - objc_info, prefer_precompiled_modules): """Collects Clang module-related inputs to pass to an action. @@ -1529,8 +1539,6 @@ def _collect_clang_module_inputs( `swift_common.create_module`). The precompiled Clang modules or the textual module maps and headers of these modules (depending on the value of `prefer_precompiled_modules`) will be collected as inputs. - objc_info: The `apple_common.Objc` provider of the target being - compiled. prefer_precompiled_modules: If True, precompiled module artifacts should be preferred over textual module map files and headers for modules that have them. If False, textual module map files and headers @@ -1544,37 +1552,22 @@ def _collect_clang_module_inputs( direct_inputs = [] transitive_inputs = [] - if cc_compilation_context: - # The headers stored in the compilation context differ depending on the - # kind of action we're invoking: - if (is_swift and not prefer_precompiled_modules) or not is_swift: - # If this is a `SwiftCompile` with explicit modules disabled, the - # `headers` field is an already-computed set of the transitive - # headers of all the deps. (For an explicit module build, we skip it - # and will more selectively pick subsets for any individual modules - # that need to fallback to implicit modules in the loop below). - # - # If this is a `SwiftPrecompileCModule`, then by definition we're - # only here in a build with explicit modules enabled. We should only - # need the direct headers of the module being compiled and its - # direct dependencies (the latter because Clang needs them present - # on the file system to map them to the module that contains them.) - # However, we may also need some of the transitive headers, if the - # module has dependencies that aren't recognized as modules (e.g., - # `cc_library` targets without an aspect hint) and the module's - # headers include those. This will likely over-estimate the needed - # inputs, but we can't do better without include scanning in - # Starlark. - transitive_inputs.append(cc_compilation_context.headers) - transitive_inputs.append(depset(cc_compilation_context.direct_textual_headers)) - - # Some rules still use the `umbrella_header` field to propagate a header - # that they don't also include in `cc_compilation_context.headers`, so we - # also need to pull these in for the time being. - # TODO(b/142867898): This can be removed once the Swift rules start - # generating its own module map for these targets. - if objc_info: - transitive_inputs.append(objc_info.umbrella_header) + if cc_compilation_context and not is_swift: + # This is a `SwiftPrecompileCModule` action, so by definition we're + # only here in a build with explicit modules enabled. We should only + # need the direct headers of the module being compiled and its + # direct dependencies (the latter because Clang needs them present + # on the file system to map them to the module that contains them.) + # However, we may also need some of the transitive headers, if the + # module has dependencies that aren't recognized as modules (e.g., + # `cc_library` targets without an aspect hint) and the module's + # headers include those. This will likely over-estimate the needed + # inputs, but we can't do better without include scanning in + # Starlark. + transitive_inputs.append(cc_compilation_context.headers) + transitive_inputs.append( + depset(cc_compilation_context.direct_textual_headers), + ) for module in modules: clang_module = module.clang @@ -1585,22 +1578,24 @@ def _collect_clang_module_inputs( if not module.is_system and type(module_map) == "File": direct_inputs.append(module_map) - if prefer_precompiled_modules: - precompiled_module = clang_module.precompiled_module - if precompiled_module: - # For builds preferring explicit modules, use it if we have it - # and don't include any headers as inputs. - direct_inputs.append(precompiled_module) - else: - # If we don't have an explicit module, we need the transitive - # headers from the compilation context associated with the - # module. This will likely overestimate the headers that will - # actually be used in the action, but until we can use include - # scanning from Starlark, we can't compute a more precise input - # set. - transitive_inputs.append( - clang_module.compilation_context.headers, - ) + precompiled_module = clang_module.precompiled_module + + if prefer_precompiled_modules and precompiled_module: + # For builds preferring explicit modules, use it if we have it + # and don't include any headers as inputs. + direct_inputs.append(precompiled_module) + else: + # If we don't have an explicit module (or we're not using it), we + # need the transitive headers from the compilation context + # associated with the module. This will likely overestimate the + # headers that will actually be used in the action, but until we can + # use include scanning from Starlark, we can't compute a more + # precise input set. + compilation_context = clang_module.compilation_context + transitive_inputs.append(compilation_context.headers) + transitive_inputs.append( + depset(compilation_context.direct_textual_headers), + ) return swift_toolchain_config.config_result( inputs = direct_inputs, @@ -1685,7 +1680,6 @@ def _dependencies_clang_modulemaps_configurator(prerequisites, args): cc_compilation_context = prerequisites.cc_compilation_context, is_swift = prerequisites.is_swift, modules = modules, - objc_info = prerequisites.objc_info, prefer_precompiled_modules = False, ) @@ -1711,7 +1705,6 @@ def _dependencies_clang_modules_configurator(prerequisites, args): cc_compilation_context = prerequisites.cc_compilation_context, is_swift = prerequisites.is_swift, modules = modules, - objc_info = prerequisites.objc_info, prefer_precompiled_modules = True, ) @@ -2304,8 +2297,7 @@ def compile( deps: Non-private dependencies of the target being compiled. These targets are used as dependencies of both the Swift module being compiled and the Clang module for the generated header. These - targets must propagate one of the following providers: `CcInfo`, - `SwiftInfo`, or `apple_common.Objc`. + targets must propagate `CcInfo` or `SwiftInfo`. extra_swift_infos: Extra `SwiftInfo` providers that aren't contained by the `deps` of the target being compiled but are required for compilation. @@ -2329,8 +2321,7 @@ def compile( private_deps: Private (implementation-only) dependencies of the target being compiled. These are only used as dependencies of the Swift module, not of the Clang module for the generated header. These - targets must propagate one of the following providers: `CcInfo`, - `SwiftInfo`, or `apple_common.Objc`. + targets must propagate `CcInfo` or `SwiftInfo`. srcs: The Swift source files to compile. swift_toolchain: The `SwiftToolchainInfo` provider of the toolchain. target_name: The name of the target for which the code is being @@ -2444,9 +2435,9 @@ def compile( all_derived_outputs = [] # Merge the providers from our dependencies so that we have one each for - # `SwiftInfo`, `CcInfo`, and `apple_common.Objc`. Then we can pass these - # into the action prerequisites so that configurators have easy access to - # the full set of values and inputs through a single accessor. + # `SwiftInfo` and `CcInfo`. Then we can pass these into the action + # prerequisites so that configurators have easy access to the full set of + # values and inputs through a single accessor. merged_providers = _merge_targets_providers( implicit_deps_providers = swift_toolchain.implicit_deps_providers, targets = deps + private_deps, @@ -2556,9 +2547,6 @@ to use swift_common.compile(include_dev_srch_paths = ...) instead.\ is_swift = True, module_name = module_name, package_name = package_name, - objc_include_paths_workaround = ( - merged_providers.objc_include_paths_workaround - ), objc_info = merged_providers.objc_info, plugins = depset(used_plugins), source_files = srcs, @@ -2835,7 +2823,6 @@ def _precompile_clang_module( is_swift_generated_header = is_swift_generated_header, module_name = module_name, package_name = None, - objc_include_paths_workaround = depset(), objc_info = apple_common.new_objc_provider(), pcm_file = precompiled_module, source_files = [module_map_file], @@ -2875,8 +2862,7 @@ def _create_cc_compilation_context( deps: Non-private dependencies of the target being compiled. These targets are used as dependencies of both the Swift module being compiled and the Clang module for the generated header. These - targets must propagate one of the following providers: `CcInfo`, - `SwiftInfo`, or `apple_common.Objc`. + targets must propagate `CcInfo` or `SwiftInfo`. feature_configuration: A feature configuration obtained from `swift_common.configure_features`. public_hdrs: Public headers that should be propagated by the new @@ -3316,11 +3302,10 @@ def _declare_validated_generated_header(actions, generated_header_name): def _merge_targets_providers(implicit_deps_providers, targets): """Merges the compilation-related providers for the given targets. - This function merges the `CcInfo`, `SwiftInfo`, and `apple_common.Objc` - providers from the given targets into a single provider for each. These - providers are then meant to be passed as prerequisites to compilation - actions so that configurators can populate command lines and inputs based on - their data. + This function merges the `CcInfo` and `SwiftInfo` providers from the given + targets into a single provider for each. These providers are then meant to + be passed as prerequisites to compilation actions so that configurators can + populate command lines and inputs based on their data. Args: implicit_deps_providers: The implicit deps providers `struct` from the @@ -3331,10 +3316,6 @@ def _merge_targets_providers(implicit_deps_providers, targets): A `struct` containing the following fields: * `cc_info`: The merged `CcInfo` provider of the targets. - * `objc_include_paths_workaround`: A `depset` containing the include - paths from the given targets that should be passed to ClangImporter. - This is a workaround for some currently incorrect propagation - behavior that is being removed in the future. * `objc_info`: The merged `apple_common.Objc` provider of the targets. * `swift_info`: The merged `SwiftInfo` provider of the targets. """ @@ -3342,12 +3323,6 @@ def _merge_targets_providers(implicit_deps_providers, targets): objc_infos = list(implicit_deps_providers.objc_infos) swift_infos = list(implicit_deps_providers.swift_infos) - # TODO(b/146575101): This is only being used to preserve the current - # behavior of strict Objective-C include paths being propagated one more - # level than they should be. Once the remaining targets that depend on this - # behavior have been fixed, remove it. - objc_include_paths_workaround_depsets = [] - for target in targets: if CcInfo in target: cc_infos.append(target[CcInfo]) @@ -3355,15 +3330,9 @@ def _merge_targets_providers(implicit_deps_providers, targets): swift_infos.append(target[SwiftInfo]) if apple_common.Objc in target: objc_infos.append(target[apple_common.Objc]) - objc_include_paths_workaround_depsets.append( - target[apple_common.Objc].strict_include, - ) return struct( cc_info = cc_common.merge_cc_infos(cc_infos = cc_infos), - objc_include_paths_workaround = depset( - transitive = objc_include_paths_workaround_depsets, - ), objc_info = apple_common.new_objc_provider(providers = objc_infos), swift_info = create_swift_info(swift_infos = swift_infos), ) diff --git a/swift/internal/providers.bzl b/swift/internal/providers.bzl index 362f621da..aa703a5e6 100644 --- a/swift/internal/providers.bzl +++ b/swift/internal/providers.bzl @@ -394,28 +394,10 @@ def create_clang_module( *, compilation_context, module_map, - precompiled_module = None): + precompiled_module = None, + strict_includes = None): """Creates a value representing a Clang module used as a Swift dependency. - Note: The `compilation_context` argument of this function is primarily - intended to communicate information *to* the Swift build rules, not to - retrieve information *back out.* In most cases, it is better to depend on - the `CcInfo` provider propagated by a Swift target to collect transitive - C/Objective-C compilation information about that target. This is because the - context used when compiling the module itself may not be the same as the - context desired when depending on it. (For example, `apple_common.Objc` - supports "strict include paths" which are only propagated to direct - dependents.) - - One valid exception to the guidance above is retrieving the generated header - associated with a specific Swift module. Since the `CcInfo` provider - propagated by the library will have already merged them transitively (or, - in the case of a hypothetical custom rule that propagates multiple direct - modules, the `direct_public_headers` of the `CcInfo` would also have them - merged), it is acceptable to read the headers from the compilation context - of the module struct itself in order to associate them with the module that - generated them. - Args: compilation_context: A `CcCompilationContext` that contains the header files and other context (such as include paths, preprocessor @@ -431,15 +413,23 @@ def create_clang_module( explicit module was built for the module; in that case, targets that depend on the module will fall back to the text module map and headers. + strict_includes: A `depset` of strings representing additional Clang + include paths that should be passed to the compiler when this module + is a _direct_ dependency of the module being compiled. May be + `None`. **This field only exists to support a specific legacy use + case and should otherwise not be used, as it is fundamentally + incompatible with Swift's import model.** Returns: - A `struct` containing the `compilation_context`, `module_map`, and - `precompiled_module` fields provided as arguments. + A `struct` containing the `compilation_context`, `module_map`, + `precompiled_module`, and `strict_includes` fields provided as + arguments. """ return struct( compilation_context = compilation_context, module_map = module_map, precompiled_module = precompiled_module, + strict_includes = strict_includes, ) def create_swift_module( diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index eb58d0efb..aa6fa0010 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -504,7 +504,6 @@ def _module_info_for_target( def _handle_module( aspect_ctx, - compilation_context, exclude_headers, feature_configuration, module_map_file, @@ -517,8 +516,6 @@ def _handle_module( Args: aspect_ctx: The aspect's context. - compilation_context: The `CcCompilationContext` containing the target's - headers. exclude_headers: A `list` of `File`s representing header files to exclude, if any, if we are generating the module map. feature_configuration: The current feature configuration. @@ -545,6 +542,11 @@ def _handle_module( direct_swift_infos + swift_infos + swift_toolchain.clang_implicit_deps_providers.swift_infos ) + if CcInfo in target: + compilation_context = target[CcInfo].compilation_context + else: + compilation_context = None + # Collect the names of Clang modules that the module being built directly # depends on. dependent_module_names = [] @@ -583,9 +585,24 @@ def _handle_module( else: return [] + compilation_contexts_to_merge_for_compilation = [compilation_context] + + # Fold the `strict_includes` from `apple_common.Objc` into the Clang module + # descriptor in `SwiftInfo` so that the `Objc` provider doesn't have to be + # passed as a separate input to Swift build APIs. + if apple_common.Objc in target: + strict_includes = target[apple_common.Objc].strict_include + compilation_contexts_to_merge_for_compilation.append( + cc_common.create_compilation_context(includes = strict_includes), + ) + else: + strict_includes = None + compilation_context_to_compile = ( compilation_context_for_explicit_module_compilation( - compilation_contexts = [compilation_context], + compilation_contexts = ( + compilation_contexts_to_merge_for_compilation + ), deps = [ target for attr_name in _MULTIPLE_TARGET_ASPECT_ATTRS @@ -618,6 +635,7 @@ def _handle_module( compilation_context = compilation_context, module_map = module_map_file, precompiled_module = precompiled_module, + strict_includes = strict_includes, ), ), ], @@ -837,7 +855,6 @@ def _swift_clang_module_aspect_impl(target, aspect_ctx): if interop_info or apple_common.Objc in target or CcInfo in target: return _handle_module( aspect_ctx = aspect_ctx, - compilation_context = _compilation_context_for_target(target), exclude_headers = exclude_headers, feature_configuration = feature_configuration, module_map_file = module_map_file, diff --git a/swift/internal/utils.bzl b/swift/internal/utils.bzl index c9a54bad1..b01cab3a7 100644 --- a/swift/internal/utils.bzl +++ b/swift/internal/utils.bzl @@ -93,20 +93,23 @@ def compilation_context_for_explicit_module_compilation( all_compilation_contexts.append(dep[CcInfo].compilation_context) elif SwiftInfo in dep: # TODO(b/151667396): Remove j2objc-specific knowledge. - # J2ObjC doesn't expose CcInfo directly on the `java_library` targets it process, but we can find the - # compilation context that was synthesized by `swift_clang_module_aspect` within the `SwiftInfo` provider. - all_compilation_contexts.extend([ - module.clang.compilation_context - for module in dep[SwiftInfo].direct_modules - if module.clang - ]) - - if apple_common.Objc in dep: - all_compilation_contexts.append( - cc_common.create_compilation_context( - includes = dep[apple_common.Objc].strict_include, - ), - ) + # J2ObjC doesn't expose `CcInfo` directly on the `java_library` + # targets it processes, but we can find the compilation context that + # was synthesized by `swift_clang_module_aspect` within the + # `SwiftInfo` provider. + for module in dep[SwiftInfo].direct_modules: + clang = module.clang + if not clang: + continue + + if clang.compilation_context: + all_compilation_contexts.append(clang.compilation_context) + if clang.strict_includes: + all_compilation_contexts.append( + cc_common.create_compilation_context( + includes = clang.strict_includes, + ), + ) return cc_common.merge_compilation_contexts( compilation_contexts = all_compilation_contexts, From af1f5c6a071996faa2e493960698a0d442e40f80 Mon Sep 17 00:00:00 2001 From: Thomas Van Lenten Date: Tue, 22 Feb 2022 12:56:12 -0800 Subject: [PATCH 15/16] Remove unused variable & function. RELNOTES: None PiperOrigin-RevId: 430276243 (cherry picked from commit 01b9d327806c1b2bc401bd4d21cc42ac38598ee5) --- swift/internal/swift_clang_module_aspect.bzl | 40 ++------------------ 1 file changed, 4 insertions(+), 36 deletions(-) diff --git a/swift/internal/swift_clang_module_aspect.bzl b/swift/internal/swift_clang_module_aspect.bzl index aa6fa0010..2cb271c7c 100644 --- a/swift/internal/swift_clang_module_aspect.bzl +++ b/swift/internal/swift_clang_module_aspect.bzl @@ -36,7 +36,10 @@ load( "create_module", "create_swift_info", ) -load(":utils.bzl", "compilation_context_for_explicit_module_compilation") +load( + ":utils.bzl", + "compilation_context_for_explicit_module_compilation", +) _MULTIPLE_TARGET_ASPECT_ATTRS = [ "deps", @@ -472,7 +475,6 @@ def _module_info_for_target( ): return None, None - attr = aspect_ctx.rule.attr module_map_file = None if not module_name: @@ -653,40 +655,6 @@ def _handle_module( return providers -def _compilation_context_for_target(target): - """Gets the compilation context to use when compiling this target's module. - - This function also handles the special case of a target that propagates an - `apple_common.Objc` provider in addition to its `CcInfo` provider, where the - former contains strict include paths that must also be added when compiling - the module. - - Args: - target: The target to which the aspect is being applied. - - Returns: - A `CcCompilationContext` that contains the headers of the target being - compiled. - """ - if CcInfo not in target: - return None - - compilation_context = target[CcInfo].compilation_context - - if apple_common.Objc in target: - strict_includes = target[apple_common.Objc].strict_include - if strict_includes: - compilation_context = cc_common.merge_compilation_contexts( - compilation_contexts = [ - compilation_context, - cc_common.create_compilation_context( - includes = strict_includes, - ), - ], - ) - - return compilation_context - def _collect_swift_infos_from_deps(aspect_ctx): """Collect `SwiftInfo` providers from dependencies. From 318172faca6bddbc870ad1cff7daf0a664bc187e Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Wed, 2 Mar 2022 17:12:18 -0800 Subject: [PATCH 16/16] Enable `--experimental_enable_aspect_hints` for Bazel 6 CI --- .bazelrc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.bazelrc b/.bazelrc index ab04f51f7..4a132c30e 100644 --- a/.bazelrc +++ b/.bazelrc @@ -11,6 +11,12 @@ build --macos_minimum_os=13.0 # dependencies cause us to use a newer version build --check_direct_dependencies=off +# This is needed for Bazel 6 compatibility. +# It's enabled (and can't be disabled) in Bazel 7. +# TODO: Remove this once we drop Bazel 6 support. +# See also: https://github.com/bazelbuild/bazel/issues/14327 +build --experimental_enable_aspect_hints + # Make sure no warnings slip into the C++ tools we vendor build --features treat_warnings_as_errors