Skip to content

Commit

Permalink
Add a way for swift_interop_hint to suppress the module for targets…
Browse files Browse the repository at this point in the history
… that generate them by default, like `objc_library`.

PiperOrigin-RevId: 391087374
  • Loading branch information
allevato authored and swiple-rules-gardener committed Aug 16, 2021
1 parent a81c591 commit de0f604
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 9 deletions.
8 changes: 8 additions & 0 deletions swift/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,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",
Expand Down
13 changes: 13 additions & 0 deletions swift/internal/swift_clang_module_aspect.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,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
Expand All @@ -92,6 +96,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.
Expand Down Expand Up @@ -139,6 +144,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
Expand All @@ -163,6 +170,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,
)
Expand Down Expand Up @@ -599,6 +607,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)
Expand Down
42 changes: 42 additions & 0 deletions swift/internal/swift_interop_hint.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
),
Expand Down Expand Up @@ -136,6 +145,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,
)
1 change: 1 addition & 0 deletions test/fixtures/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ bzl_library(
name = "starlark_tests_bzls",
srcs = glob(["*.bzl"]),
visibility = ["//test:__pkg__"],
deps = ["//swift"],
)
25 changes: 25 additions & 0 deletions test/fixtures/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
43 changes: 42 additions & 1 deletion test/fixtures/interop_hints/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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__"],
Expand Down Expand Up @@ -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",
target = ":objc_library_suppressed_lib",
)

objc_library(
name = "objc_library_suppressed_lib",
hdrs = ["header1.h"],
aspect_hints = [":suppress_hint"],
tags = FIXTURE_TAGS,
)

forward_swift_info_from_swift_clang_module_aspect(
name = "objc_library_with_swift_dep_suppressed",
target = ":objc_library_with_swift_dep_suppressed_lib",
)

objc_library(
name = "objc_library_with_swift_dep_suppressed_lib",
hdrs = ["header1.h"],
aspect_hints = [":suppress_hint"],
tags = FIXTURE_TAGS,
deps = [":empty_lib"],
)

swift_library(
name = "empty_lib",
srcs = ["Empty.swift"],
tags = FIXTURE_TAGS,
)

swift_interop_hint(
name = "suppress_hint",
suppressed = True,
tags = FIXTURE_TAGS,
)
1 change: 1 addition & 0 deletions test/fixtures/interop_hints/Empty.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// Intentionally empty.
18 changes: 18 additions & 0 deletions test/interop_hints_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,24 @@ def interop_hints_test_suite():
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",
)

# 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",
)

native.test_suite(
name = name,
tags = [name],
Expand Down
54 changes: 46 additions & 8 deletions test/rules/provider_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,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):
Expand Down Expand Up @@ -322,12 +317,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)
Expand Down Expand Up @@ -363,6 +387,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 = """\
Expand All @@ -384,7 +422,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.
Expand All @@ -398,7 +436,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.
Expand Down

0 comments on commit de0f604

Please sign in to comment.