From 3ee31297d04de401119bb9eb97df24f60df1ddf0 Mon Sep 17 00:00:00 2001 From: Googler Date: Tue, 14 Dec 2021 11:42:25 -0800 Subject: [PATCH] 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(