From 451808508cee98d60301011a7c1c0fc914e0bf1e Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Fri, 31 Jan 2020 07:32:33 -0800 Subject: [PATCH] Add a `generated_header` attribute to `swift_library`. Co-authored-by: Samuel Giddins Closes #350. RELNOTES: None. PiperOrigin-RevId: 292541776 --- swift/internal/api.bzl | 13 ++ swift/internal/compiling.bzl | 93 ++++++++---- swift/internal/derived_files.bzl | 26 ++-- swift/internal/swift_library.bzl | 1 + test/BUILD | 3 + test/fixtures/generated_header/BUILD | 35 +++++ test/fixtures/generated_header/Empty.swift | 1 + test/generated_header_tests.bzl | 161 +++++++++++++++++++++ test/rules/analysis_failure_test.bzl | 55 +++++++ test/rules/provider_test.bzl | 11 +- 10 files changed, 359 insertions(+), 40 deletions(-) create mode 100644 test/fixtures/generated_header/BUILD create mode 100644 test/fixtures/generated_header/Empty.swift create mode 100644 test/generated_header_tests.bzl create mode 100644 test/rules/analysis_failure_test.bzl diff --git a/swift/internal/api.bzl b/swift/internal/api.bzl index 2e0a61971..4ffd47ad3 100644 --- a/swift/internal/api.bzl +++ b/swift/internal/api.bzl @@ -309,6 +309,19 @@ directly reference any other symbols in the object file that adds that conformance. """, ), + "generated_header_name": attr.string( + doc = """\ +The name of the generated Objective-C interface header. This name must end with +a `.h` extension and cannot contain any path separators. + +If this attribute is not specified, then the default behavior is to name the +header `${target_name}-Swift.h`. + +This attribute is ignored if the toolchain does not support generating headers +or if the target has the `swift.no_generated_header` feature enabled. +""", + mandatory = False, + ), }, ) diff --git a/swift/internal/compiling.bzl b/swift/internal/compiling.bzl index 8a313c9ce..bd600fcb0 100644 --- a/swift/internal/compiling.bzl +++ b/swift/internal/compiling.bzl @@ -727,6 +727,7 @@ def compile( copts = [], defines = [], deps = [], + generated_header_name = None, genfiles_dir = None): """Compiles a Swift module. @@ -759,6 +760,9 @@ def compile( deps: Dependencies of the target being compiled. These targets must propagate one of the following providers: `CcInfo`, `SwiftInfo`, or `apple_common.Objc`. + generated_header_name: The name of the Objective-C generated header that + should be generated for this module. If omitted, the name + `${target_name}-Swift.h` will be used. genfiles_dir: The Bazel `*-genfiles` directory root. If provided, its path is added to ClangImporter's header search paths for compatibility with Bazel's C++ and Objective-C rules which support @@ -800,6 +804,7 @@ def compile( """ compile_outputs, other_outputs = _declare_compile_outputs( actions = actions, + generated_header_name = generated_header_name, feature_configuration = feature_configuration, module_name = module_name, srcs = srcs, @@ -912,6 +917,7 @@ def get_implicit_deps(feature_configuration, swift_toolchain): def _declare_compile_outputs( actions, + generated_header_name, feature_configuration, module_name, srcs, @@ -921,6 +927,8 @@ def _declare_compile_outputs( Args: actions: The object used to register actions. + generated_header_name: The desired name of the generated header for this + module, or `None` to use `${target_name}-Swift.h`. feature_configuration: A feature configuration obtained from `swift_common.configure_features`. module_name: The name of the Swift module being compiled. @@ -978,43 +986,48 @@ def _declare_compile_outputs( else: stats_directory = None - # If supported, generate a Swift header for this library so that it can be + # If supported, generate the Swift header for this library so that it can be # included by Objective-C code that depends on it. if not is_feature_enabled( feature_configuration = feature_configuration, feature_name = SWIFT_FEATURE_NO_GENERATED_HEADER, ): - generated_header = derived_files.objc_header( - actions = actions, - target_name = target_name, - ) - - # Create a module map for the generated header file. This ensures that - # inclusions of it are treated modularly, not textually. - # - # Caveat: Generated module maps are incompatible with the hack that some - # folks are using to support mixed Objective-C and Swift modules. This - # trap door lets them escape the module redefinition error, with the - # caveat that certain import scenarios could lead to incorrect behavior - # because a header can be imported textually instead of modularly. - if not is_feature_enabled( - feature_configuration = feature_configuration, - feature_name = SWIFT_FEATURE_NO_GENERATED_MODULE_MAP, - ): - generated_module_map = derived_files.module_map( + if generated_header_name: + generated_header = _declare_validated_generated_header( actions = actions, - target_name = target_name, + generated_header_name = generated_header_name, ) - _write_objc_header_module_map( + else: + generated_header = derived_files.default_generated_header( actions = actions, - module_name = module_name, - objc_header = generated_header, - output = generated_module_map, + target_name = target_name, ) - else: - generated_module_map = None else: generated_header = None + + # If not disabled, create a module map for the generated header file. This + # ensures that inclusions of it are treated modularly, not textually. + # + # Caveat: Generated module maps are incompatible with the hack that some + # folks are using to support mixed Objective-C and Swift modules. This + # trap door lets them escape the module redefinition error, with the + # caveat that certain import scenarios could lead to incorrect behavior + # because a header can be imported textually instead of modularly. + if generated_header and not is_feature_enabled( + feature_configuration = feature_configuration, + feature_name = SWIFT_FEATURE_NO_GENERATED_MODULE_MAP, + ): + generated_module_map = derived_files.module_map( + actions = actions, + target_name = target_name, + ) + _write_objc_header_module_map( + actions = actions, + module_name = module_name, + objc_header = generated_header, + output = generated_module_map, + ) + else: generated_module_map = None # Now, declare outputs like object files for which there may be one or many, @@ -1164,6 +1177,34 @@ def _declare_multiple_outputs_and_write_output_file_map( output_file_map = output_map_file, ) +def _declare_validated_generated_header(actions, generated_header_name): + """Validates and declares the explicitly named generated header. + + If the file does not have a `.h` extension or conatins path separators, the + build will fail. + + Args: + actions: The context's `actions` object. + generated_header_name: The desired name of the generated header. + + Returns: + A `File` that should be used as the output for the generated header. + """ + if "/" in generated_header_name: + fail( + "The generated header for a Swift module may not contain " + + "directory components (got '{}').".format(generated_header_name), + ) + + extension = paths.split_extension(generated_header_name)[1] + if extension != ".h": + fail( + "The generated header for a Swift module must have a '.h' " + + "extension (got '{}').".format(generated_header_name), + ) + + return actions.declare_file(generated_header_name) + def _merge_targets_providers(supports_objc_interop, targets): """Merges the compilation-related providers for the given targets. diff --git a/swift/internal/derived_files.bzl b/swift/internal/derived_files.bzl index 5da4b0a3d..439aa6fae 100644 --- a/swift/internal/derived_files.bzl +++ b/swift/internal/derived_files.bzl @@ -29,6 +29,18 @@ def _autolink_flags(actions, target_name): """ return actions.declare_file("{}.autolink".format(target_name)) +def _default_generated_header(actions, target_name): + """Declares the automatically-named generated header for a Swift target. + + Args: + actions: The context's actions object. + target_name: The name of the target being built. + + Returns: + The declared `File`. + """ + return actions.declare_file("{}-Swift.h".format(target_name)) + def _executable(actions, target_name): """Declares a file for the executable created by a binary or test rule. @@ -122,18 +134,6 @@ def _modulewrap_object(actions, target_name): """ return actions.declare_file("{}.modulewrap.o".format(target_name)) -def _objc_header(actions, target_name): - """Declares the generated header file exposing Swift APIs to Objective-C. - - Args: - actions: The context's actions object. - target_name: The name of the target being built. - - Returns: - The declared `File`. - """ - return actions.declare_file("{}-Swift.h".format(target_name)) - def _partial_swiftmodule(actions, target_name, src): """Declares a file for a partial Swift module created during compilation. @@ -291,12 +291,12 @@ def _xctest_runner_script(actions, target_name): derived_files = struct( autolink_flags = _autolink_flags, + default_generated_header = _default_generated_header, executable = _executable, indexstore_directory = _indexstore_directory, intermediate_object_file = _intermediate_object_file, module_map = _module_map, modulewrap_object = _modulewrap_object, - objc_header = _objc_header, partial_swiftmodule = _partial_swiftmodule, reexport_modules_src = _reexport_modules_src, static_archive = _static_archive, diff --git a/swift/internal/swift_library.bzl b/swift/internal/swift_library.bzl index c3761c4f0..78df113a0 100644 --- a/swift/internal/swift_library.bzl +++ b/swift/internal/swift_library.bzl @@ -157,6 +157,7 @@ def _swift_library_impl(ctx): defines = ctx.attr.defines, deps = deps + private_deps, feature_configuration = feature_configuration, + generated_header_name = ctx.attr.generated_header_name, genfiles_dir = ctx.genfiles_dir, module_name = module_name, srcs = srcs, diff --git a/test/BUILD b/test/BUILD index a5f2217f2..74acc0e63 100644 --- a/test/BUILD +++ b/test/BUILD @@ -1,7 +1,10 @@ +load(":generated_header_tests.bzl", "generated_header_test_suite") load(":private_deps_tests.bzl", "private_deps_test_suite") licenses(["notice"]) +generated_header_test_suite() + private_deps_test_suite() test_suite( diff --git a/test/fixtures/generated_header/BUILD b/test/fixtures/generated_header/BUILD new file mode 100644 index 000000000..1263edd94 --- /dev/null +++ b/test/fixtures/generated_header/BUILD @@ -0,0 +1,35 @@ +load("//swift:swift.bzl", "swift_library") +load("//test/fixtures:common.bzl", "FIXTURE_TAGS") + +package( + default_visibility = ["//test:__subpackages__"], +) + +licenses(["notice"]) + +swift_library( + name = "auto_header", + srcs = ["Empty.swift"], + tags = FIXTURE_TAGS, +) + +swift_library( + name = "explicit_header", + srcs = ["Empty.swift"], + generated_header_name = "SomeOtherName.h", + tags = FIXTURE_TAGS, +) + +swift_library( + name = "invalid_extension", + srcs = ["Empty.swift"], + generated_header_name = "Invalid.extension", + tags = FIXTURE_TAGS, +) + +swift_library( + name = "invalid_path_separator", + srcs = ["Empty.swift"], + generated_header_name = "Invalid/Separator.h", + tags = FIXTURE_TAGS, +) diff --git a/test/fixtures/generated_header/Empty.swift b/test/fixtures/generated_header/Empty.swift new file mode 100644 index 000000000..bd9ec079d --- /dev/null +++ b/test/fixtures/generated_header/Empty.swift @@ -0,0 +1 @@ +// Intentionally empty. diff --git a/test/generated_header_tests.bzl b/test/generated_header_tests.bzl new file mode 100644 index 000000000..939d3e983 --- /dev/null +++ b/test/generated_header_tests.bzl @@ -0,0 +1,161 @@ +# Copyright 2020 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_library.generated_header`.""" + +load( + "@build_bazel_rules_swift//test/rules:analysis_failure_test.bzl", + "make_analysis_failure_test_rule", +) +load( + "@build_bazel_rules_swift//test/rules:provider_test.bzl", + "make_provider_test_rule", +) + +# A configuration that forces header and module map generation, regardless of +# the toolchain's default feature set. +GENERATE_HEADER_AND_MODULE_MAP_CONFIG_SETTINGS = { + "//command_line_option:features": [ + "-swift.no_generated_header", + "-swift.no_generated_module_map", + ], +} + +# A configuration that disables header (and therefore module map) generation, +# regardless of the toolchain's default feature set. +NO_GENERATE_HEADER_CONFIG_SETTINGS = { + "//command_line_option:features": [ + "swift.no_generated_header", + ], +} + +generate_header_and_module_map_provider_test = make_provider_test_rule( + config_settings = GENERATE_HEADER_AND_MODULE_MAP_CONFIG_SETTINGS, +) + +generate_header_and_module_map_failure_test = make_analysis_failure_test_rule( + config_settings = GENERATE_HEADER_AND_MODULE_MAP_CONFIG_SETTINGS, +) + +no_generate_header_provider_test = make_provider_test_rule( + config_settings = NO_GENERATE_HEADER_CONFIG_SETTINGS, +) + +def generated_header_test_suite(): + """Test suite for `swift_library.generated_header`.""" + name = "generated_header" + + # Verify that the generated header by default gets an automatically + # generated name and is an output of the rule. + generate_header_and_module_map_provider_test( + name = "{}_automatically_named_header_is_rule_output".format(name), + expected_files = [ + "test/fixtures/generated_header/auto_header-Swift.h", + "*", + ], + field = "files", + provider = "DefaultInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/generated_header:auto_header", + ) + + # Verify that the generated header is propagated in `SwiftInfo`. + generate_header_and_module_map_provider_test( + name = "{}_automatically_named_header_is_propagated".format(name), + expected_files = [ + "test/fixtures/generated_header/auto_header-Swift.h", + ], + field = "transitive_generated_headers", + provider = "SwiftInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/generated_header:auto_header", + ) + + # Verify that the generated module map is propagated in `apple_common.Objc`. + # TODO(b/148604334): Enable this when it analyzes correctly on all platforms. + # generate_header_and_module_map_provider_test( + # name = "{}_automatically_named_header_modulemap_is_propagated".format(name), + # expected_files = [ + # "test/fixtures/generated_header/auto_header.modulemaps/module.modulemap", + # ], + # field = "direct_module_maps", + # provider = "apple_common.Objc", + # tags = [name], + # target_under_test = "@build_bazel_rules_swift//test/fixtures/generated_header:auto_header", + # ) + + # Verify that the explicit generated header is an output of the rule and + # that the automatically named one is *not*. + generate_header_and_module_map_provider_test( + name = "{}_explicit_header".format(name), + expected_files = [ + "test/fixtures/generated_header/SomeOtherName.h", + "-test/fixtures/generated_header/explicit_header-Swift.h", + "*", + ], + field = "files", + provider = "DefaultInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/generated_header:explicit_header", + ) + + # Verify that the build fails to analyze if an invalid extension is used. + generate_header_and_module_map_failure_test( + name = "{}_invalid_extension".format(name), + expected_message = "The generated header for a Swift module must have a '.h' extension", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/generated_header:invalid_extension", + ) + + # Verify that the build fails to analyze if a path separator is used. + generate_header_and_module_map_failure_test( + name = "{}_invalid_path_separator".format(name), + expected_message = "The generated header for a Swift module may not contain directory components", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/generated_header:invalid_path_separator", + ) + + # Verify that the header is not generated if the feature + # `swift.no_generated_header` set, when using an automatically named header. + no_generate_header_provider_test( + name = "{}_no_header".format(name), + expected_files = [ + "-test/fixtures/generated_header/auto_header-Swift.h", + "*", + ], + field = "files", + provider = "DefaultInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/generated_header:auto_header", + ) + + # Verify that the header is not generated if the feature + # `swift.no_generated_header` set, even when specifying an explicit header + # name. + no_generate_header_provider_test( + name = "{}_no_explicit_header".format(name), + expected_files = [ + "-test/fixtures/generated_header/SomeOtherName.h", + "*", + ], + field = "files", + provider = "DefaultInfo", + tags = [name], + target_under_test = "@build_bazel_rules_swift//test/fixtures/generated_header:explicit_header", + ) + + native.test_suite( + name = name, + tags = [name], + ) diff --git a/test/rules/analysis_failure_test.bzl b/test/rules/analysis_failure_test.bzl new file mode 100644 index 000000000..c5be5eea5 --- /dev/null +++ b/test/rules/analysis_failure_test.bzl @@ -0,0 +1,55 @@ +# Copyright 2020 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. + +"""Rules for testing analysis-time failures.""" + +load( + "@bazel_skylib//lib:unittest.bzl", + "analysistest", + "asserts", +) + +def _analysis_failure_test_impl(ctx): + env = analysistest.begin(ctx) + asserts.expect_failure(env, ctx.attr.expected_message) + return analysistest.end(env) + +def make_analysis_failure_test_rule(config_settings = {}): + """Returns a `analysis_failure_test`-like rule with custom config settings. + + Args: + config_settings: A dictionary of configuration settings and their values + that should be applied during tests. + + Returns: + A rule returned by `analysistest.make` that has the + `analysis_failure_test` interface and the given config settings. + """ + return analysistest.make( + _analysis_failure_test_impl, + attrs = { + "expected_message": attr.string( + mandatory = True, + doc = """\ +The expected failure message (passed in a call to `fail()` by the rule under +test). +""", + ), + }, + config_settings = config_settings, + expect_failure = True, + ) + +# A default instantiation of the rule when no custom config settings are needed. +analysis_failure_test = make_analysis_failure_test_rule() diff --git a/test/rules/provider_test.bzl b/test/rules/provider_test.bzl index 79c9ac1ae..eba66af58 100644 --- a/test/rules/provider_test.bzl +++ b/test/rules/provider_test.bzl @@ -135,8 +135,12 @@ def _lookup_provider_by_name(env, target, provider_name): provider = None if provider_name == "CcInfo": provider = CcInfo + elif provider_name == "DefaultInfo": + provider = DefaultInfo elif provider_name == "SwiftInfo": provider = SwiftInfo + elif provider_name == "apple_common.Objc": + provider = apple_common.Objc if not provider: unittest.fail( @@ -390,7 +394,12 @@ the next component. The name of the provider expected to be propagated by the target under test, and on which the field will be checked. -Currently, the only recognized providers are `CcInfo` and `SwiftInfo`. +Currently, only the following providers are recognized: + +* `CcInfo` +* `DefaultInfo` +* `SwiftInfo` +* `apple_common.Objc` """, ), },