Add a Boolean library_evolution attribute to swift_library

Historically, library evolution has been supported by a global flag that Apple framework rules transition on in order to build all dependencies with library evolution enabled. In retrospect, this is wrong; we shouldn't be automatically applying library evolution to the whole subgraph in this way. Instead, the owner of the framework should explicitly specify library evolution on the client-facing modules that make up their SDK. The goal here is to ensure that framework owners carefully audit their public `deps` vs. implementation-only `private_deps` so that the `.swiftinterface` doesn't attempt to import anything that it shouldn't, especially with Swift 5.7 validating emitted interfaces after compilation by default. This change only adds the new attribute; the flag is still honored while we have clients and rules_apple depending on it. PiperOrigin-RevId: 486750877 (cherry picked from commit b9175e6) Cherry-pick notes: With this change the private swiftinterface is enabled by `ctx.attr.library_evolution` as well. It can be disabled with `features = ["-swift.emit_private_swiftinterface”]`. Signed-off-by: Brentley Jones <github@brentleyjones.com>
bazelbuild · Oct 3, 2024 · 8c518e3 · 8c518e3
1 parent d8cc3ea
commit 8c518e3
Show file tree

Hide file tree

Showing 10 changed files with 171 additions and 90 deletions.
diff --git a/swift/internal/attrs.bzl b/swift/internal/attrs.bzl
@@ -290,6 +290,20 @@ def swift_library_rule_attrs(
         ),
         swift_config_attrs(),
         {
+            "library_evolution": attr.bool(
+                default = False,
+                doc = """\
+Indicates whether the Swift code should be compiled with library evolution mode
+enabled.
+
+This attribute should be used to compile a module that will be distributed as
+part of a client-facing (non-implementation-only) module in a library or
+framework that will be distributed for use outside of the Bazel build graph.
+Setting this to true will compile the module with the `-library-evolution` flag
+and emit a `.swiftinterface` file as one of the compilation outputs.
+""",
+                mandatory = False,
+            ),
             "alwayslink": attr.bool(
                 default = False,
                 doc = """\

diff --git a/swift/swift_library.bzl b/swift/swift_library.bzl
@@ -120,11 +120,20 @@ def _swift_library_impl(ctx):
     copts.extend(module_copts)
 
     extra_features = []
-    if ctx.attr._config_emit_swiftinterface[BuildSettingInfo].value:
+
+    # TODO(b/239957001): Remove the global flag.
+    if (
+        ctx.attr.library_evolution or
+        ctx.attr._config_emit_swiftinterface[BuildSettingInfo].value
+    ):
         extra_features.append(SWIFT_FEATURE_ENABLE_LIBRARY_EVOLUTION)
         extra_features.append(SWIFT_FEATURE_EMIT_SWIFTINTERFACE)
 
-    if ctx.attr._config_emit_private_swiftinterface[BuildSettingInfo].value:
+    # TODO(b/239957001): Remove the global flag.
+    if (
+        ctx.attr.library_evolution or
+        ctx.attr._config_emit_private_swiftinterface[BuildSettingInfo].value
+    ):
         extra_features.append(SWIFT_FEATURE_ENABLE_LIBRARY_EVOLUTION)
         extra_features.append(SWIFT_FEATURE_EMIT_PRIVATE_SWIFTINTERFACE)
 

diff --git a/test/fixtures/module_interface/BUILD b/test/fixtures/module_interface/BUILD
@@ -1,11 +1,6 @@
 load("//swift:swift_binary.bzl", "swift_binary")
 load("//swift:swift_import.bzl", "swift_import")
-load("//swift:swift_library.bzl", "swift_library")
 load("//test/fixtures:common.bzl", "FIXTURE_TAGS")
-load(
-    "//test/rules:swift_library_artifact_collector.bzl",
-    "swift_library_artifact_collector",
-)
 
 package(
     default_testonly = True,
@@ -23,32 +18,11 @@ swift_binary(
 
 swift_import(
     name = "toy_module",
-    archives = [":toy_outputs/libToyModule.a"],
+    archives = [
+        "//test/fixtures/module_interface/library:toy_outputs/libToyModule.a",
+    ],
     module_name = "ToyModule",
-    swiftdoc = ":toy_outputs/ToyModule.swiftdoc",
-    swiftinterface = ":toy_outputs/ToyModule.swiftinterface",
-    tags = FIXTURE_TAGS,
-)
-
-# Checking in pre-built artifacts like a `.swiftinterface` and static libraries
-# would require different artifacts for every platform the test might run on.
-# Instead, build it on-demand but forward the outputs using the "artifact
-# collector" rule below to make them act as if they were pre-built outputs when
-# referenced by the `swift_import` rule.
-
-swift_library(
-    name = "toy_module_library",
-    srcs = ["ToyModule.swift"],
-    module_name = "ToyModule",
-    tags = FIXTURE_TAGS,
-)
-
-swift_library_artifact_collector(
-    name = "toy_module_artifact_collector",
-    static_library = "toy_outputs/libToyModule.a",
-    swiftdoc = "toy_outputs/ToyModule.swiftdoc",
-    swiftinterface = "toy_outputs/ToyModule.swiftinterface",
+    swiftdoc = "//test/fixtures/module_interface/library:toy_outputs/ToyModule.swiftdoc",
+    swiftinterface = "//test/fixtures/module_interface/library:toy_outputs/ToyModule.swiftinterface",
     tags = FIXTURE_TAGS,
-    target = ":toy_module_library",
-    target_compatible_with = ["@platforms//os:macos"],
 )
diff --git a/test/fixtures/module_interface/library/BUILD b/test/fixtures/module_interface/library/BUILD
@@ -0,0 +1,50 @@
+load("//swift:swift_library.bzl", "swift_library")
+load(
+    "//test/fixtures:common.bzl",
+    "FIXTURE_TAGS",
+)
+load(
+    "//test/rules:swift_library_artifact_collector.bzl",
+    "swift_library_artifact_collector",
+)
+
+package(
+    default_testonly = True,
+    default_visibility = ["//test:__subpackages__"],
+)
+
+licenses(["notice"])
+
+# Checking in pre-built artifacts like a `.swiftinterface` and static libraries
+# would require different artifacts for every platform the test might run on.
+# Instead, build it on-demand but forward the outputs using the "artifact
+# collector" rule below to make them act as if they were pre-built outputs when
+# referenced by the `swift_import` rule.
+#
+# These must be in a separate package than the `swift_import` target because
+# that rule propagates its pre-built inputs in `DefaultInfo`.
+
+swift_library(
+    name = "toy_module_library",
+    srcs = ["ToyModule.swift"],
+    library_evolution = True,
+    module_name = "ToyModule",
+    tags = FIXTURE_TAGS,
+)
+
+swift_library_artifact_collector(
+    name = "toy_module_artifact_collector",
+    static_library = "toy_outputs/libToyModule.a",
+    swiftdoc = "toy_outputs/ToyModule.swiftdoc",
+    swiftinterface = "toy_outputs/ToyModule.swiftinterface",
+    tags = FIXTURE_TAGS,
+    target = ":toy_module_library",
+)
+
+swift_library(
+    name = "toy_module_library_without_library_evolution",
+    srcs = ["ToyModule.swift"],
+    library_evolution = False,
+    module_name = "ToyModuleNoEvolution",
+    tags = FIXTURE_TAGS,
+)
diff --git a/...fixtures/module_interface/ToyModule.swift → .../module_interface/library/ToyModule.swift b/...fixtures/module_interface/ToyModule.swift → .../module_interface/library/ToyModule.swift
diff --git a/test/fixtures/private_swiftinterface/BUILD b/test/fixtures/private_swiftinterface/BUILD
@@ -20,12 +20,7 @@ check them in directly.
 
 load("//swift:swift_binary.bzl", "swift_binary")
 load("//swift:swift_import.bzl", "swift_import")
-load("//swift:swift_library.bzl", "swift_library")
 load("//test/fixtures:common.bzl", "FIXTURE_TAGS")
-load(
-    "//test/rules:swift_library_artifact_collector.bzl",
-    "swift_library_artifact_collector",
-)
 
 package(
     default_testonly = True,
@@ -43,30 +38,12 @@ swift_binary(
 
 swift_import(
     name = "private_swiftinterface_import",
-    archives = [":private_swiftinterface_outputs/libPrivateSwiftInterface.a"],
+    archives = ["//test/fixtures/private_swiftinterface/library:private_swiftinterface_outputs/libPrivateSwiftInterface.a"],
     module_name = "PrivateSwiftInterface",
-    swiftdoc = ":private_swiftinterface_outputs/PrivateSwiftInterface.swiftdoc",
+    swiftdoc = "//test/fixtures/private_swiftinterface/library:private_swiftinterface_outputs/PrivateSwiftInterface.swiftdoc",
     # Using the private interface allows using both the normal and private interfaces of a module.
     # Only one swiftinterface is allowed per module, so we can't use both at the same time.
     # This tests that using the private interface allows a dependent module to use an `@_spi` symbol.
-    swiftinterface = ":private_swiftinterface_outputs/PrivateSwiftInterface.private.swiftinterface",
-    tags = FIXTURE_TAGS,
-)
-
-swift_library(
-    name = "private_swiftinterface",
-    srcs = ["Lib.swift"],
-    module_name = "PrivateSwiftInterface",
-    tags = FIXTURE_TAGS,
-)
-
-swift_library_artifact_collector(
-    name = "private_swiftinterface_artifact_collector",
-    private_swiftinterface = "private_swiftinterface_outputs/PrivateSwiftInterface.private.swiftinterface",
-    static_library = "private_swiftinterface_outputs/libPrivateSwiftInterface.a",
-    swiftdoc = "private_swiftinterface_outputs/PrivateSwiftInterface.swiftdoc",
-    swiftinterface = "private_swiftinterface_outputs/PrivateSwiftInterface.swiftinterface",
+    swiftinterface = "//test/fixtures/private_swiftinterface/library:private_swiftinterface_outputs/PrivateSwiftInterface.private.swiftinterface",
     tags = FIXTURE_TAGS,
-    target = ":private_swiftinterface",
-    target_compatible_with = ["@platforms//os:macos"],
 )
diff --git a/test/fixtures/private_swiftinterface/library/BUILD b/test/fixtures/private_swiftinterface/library/BUILD
@@ -0,0 +1,53 @@
+load("//swift:swift_library.bzl", "swift_library")
+load(
+    "//test/fixtures:common.bzl",
+    "FIXTURE_TAGS",
+)
+load(
+    "//test/rules:swift_library_artifact_collector.bzl",
+    "swift_library_artifact_collector",
+)
+
+package(
+    default_testonly = True,
+    default_visibility = ["//test:__subpackages__"],
+)
+
+licenses(["notice"])
+
+
+# Checking in pre-built artifacts like a `.swiftinterface` and static libraries
+# would require different artifacts for every platform the test might run on.
+# Instead, build it on-demand but forward the outputs using the "artifact
+# collector" rule below to make them act as if they were pre-built outputs when
+# referenced by the `swift_import` rule.
+#
+# These must be in a separate package than the `swift_import` target because
+# that rule propagates its pre-built inputs in `DefaultInfo`.
+
+swift_library(
+    name = "private_swiftinterface_library",
+    srcs = ["Lib.swift"],
+    library_evolution = True,
+    module_name = "PrivateSwiftInterface",
+    tags = FIXTURE_TAGS,
+)
+
+swift_library_artifact_collector(
+    name = "private_swiftinterface_artifact_collector",
+    private_swiftinterface = "private_swiftinterface_outputs/PrivateSwiftInterface.private.swiftinterface",
+    static_library = "private_swiftinterface_outputs/libPrivateSwiftInterface.a",
+    swiftdoc = "private_swiftinterface_outputs/PrivateSwiftInterface.swiftdoc",
+    swiftinterface = "private_swiftinterface_outputs/PrivateSwiftInterface.swiftinterface",
+    tags = FIXTURE_TAGS,
+    target = ":private_swiftinterface_library",
+    target_compatible_with = ["@platforms//os:macos"],
+)
+
+swift_library(
+    name = "private_swiftinterface_without_library_evolution",
+    srcs = ["Lib.swift"],
+    library_evolution = False,
+    module_name = "PrivateSwiftInterface",
+    tags = FIXTURE_TAGS,
+)
diff --git a/...fixtures/private_swiftinterface/Lib.swift → .../private_swiftinterface/library/Lib.swift b/...fixtures/private_swiftinterface/Lib.swift → .../private_swiftinterface/library/Lib.swift
diff --git a/test/module_interface_tests.bzl b/test/module_interface_tests.bzl
@@ -19,9 +19,10 @@ load(
     "build_test",
 )
 load(
-    "@build_bazel_rules_swift//test/rules:action_command_line_test.bzl",
+    "//test/rules:action_command_line_test.bzl",
     "make_action_command_line_test_rule",
 )
+load("//test/rules:provider_test.bzl", "provider_test")
 
 explicit_swift_module_map_test = make_action_command_line_test_rule(
     config_settings = {
@@ -53,11 +54,39 @@ def module_interface_test_suite(name, tags = []):
     build_test(
         name = "{}_swift_binary_imports_swiftinterface".format(name),
         targets = [
-            "@build_bazel_rules_swift//test/fixtures/module_interface:client",
+            "//test/fixtures/module_interface:client",
         ],
         tags = all_tags,
     )
 
+    # Verify that `.swiftinterface` file is emitted when the `library_evolution`
+    # attribute is true.
+    provider_test(
+        name = "{}_swift_library_with_evolution_emits_swiftinterface".format(name),
+        expected_files = [
+            "test/fixtures/module_interface/library/ToyModule.swiftinterface",
+            "*",
+        ],
+        field = "files",
+        provider = "DefaultInfo",
+        tags = all_tags,
+        target_under_test = "//test/fixtures/module_interface/library:toy_module_library",
+    )
+
+    # Verify that `.swiftinterface` file is not emitted when the
+    # `library_evolution` attribute is false.
+    provider_test(
+        name = "{}_swift_library_without_evolution_emits_no_swiftinterface".format(name),
+        expected_files = [
+            "-test/fixtures/module_interface/library/ToyModuleNoEvolution.swiftinterface",
+            "*",
+        ],
+        field = "files",
+        provider = "DefaultInfo",
+        tags = all_tags,
+        target_under_test = "//test/fixtures/module_interface/library:toy_module_library_without_library_evolution",
+    )
+
     explicit_swift_module_map_test(
         name = "{}_explicit_swift_module_map_test".format(name),
         tags = all_tags,
@@ -68,7 +97,7 @@ def module_interface_test_suite(name, tags = []):
             "-Xfrontend",
         ],
         mnemonic = "SwiftCompileModuleInterface",
-        target_under_test = "@build_bazel_rules_swift//test/fixtures/module_interface:toy_module",
+        target_under_test = "//test/fixtures/module_interface:toy_module",
     )
 
     vfsoverlay_test(
@@ -84,7 +113,7 @@ def module_interface_test_suite(name, tags = []):
             "-Xfrontend",
         ],
         mnemonic = "SwiftCompileModuleInterface",
-        target_under_test = "@build_bazel_rules_swift//test/fixtures/module_interface:toy_module",
+        target_under_test = "//test/fixtures/module_interface:toy_module",
     )
 
     native.test_suite(

diff --git a/test/rules/swift_library_artifact_collector.bzl b/test/rules/swift_library_artifact_collector.bzl
@@ -20,25 +20,6 @@ check them in directly.
 
 load("//swift:providers.bzl", "SwiftInfo")
 
-def _swiftinterface_transition_impl(_settings, attr):
-    return {
-        # If the `.private.swiftinterface` file is requested, apply the setting that causes
-        # the rule to generate it.
-        "@build_bazel_rules_swift//swift:emit_private_swiftinterface": attr.private_swiftinterface != None,
-        # If the `.swiftinterface` file is requested, apply the setting that causes
-        # the rule to generate it.
-        "@build_bazel_rules_swift//swift:emit_swiftinterface": attr.swiftinterface != None,
-    }
-
-_swiftinterface_transition = transition(
-    implementation = _swiftinterface_transition_impl,
-    inputs = [],
-    outputs = [
-        "@build_bazel_rules_swift//swift:emit_private_swiftinterface",
-        "@build_bazel_rules_swift//swift:emit_swiftinterface",
-    ],
-)
-
 def _copy_file(actions, source, destination):
     """Copies the source file to the destination file.
 
@@ -63,7 +44,7 @@ cp "$1" "$2"
     )
 
 def _swift_library_artifact_collector_impl(ctx):
-    target = ctx.attr.target[0]
+    target = ctx.attr.target
     swift_info = target[SwiftInfo]
 
     if ctx.outputs.static_library:
@@ -108,13 +89,7 @@ swift_library_artifact_collector = rule(
         "swiftdoc": attr.output(mandatory = False),
         "swiftinterface": attr.output(mandatory = False),
         "swiftmodule": attr.output(mandatory = False),
-        "target": attr.label(
-            cfg = _swiftinterface_transition,
-            providers = [[CcInfo, SwiftInfo]],
-        ),
-        "_allowlist_function_transition": attr.label(
-            default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
-        ),
+        "target": attr.label(providers = [[CcInfo, SwiftInfo]]),
     },
     implementation = _swift_library_artifact_collector_impl,
 )