From 1a81546be0f816415cfed48ea4413e3a295d46ff Mon Sep 17 00:00:00 2001 From: Brentley Jones Date: Mon, 9 Dec 2024 13:12:04 -0600 Subject: [PATCH] fix: fix modulemaps to behave like SPM MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * If a modulemap needs to be generated (i.e. one isn’t provided by the target), we always generate it instead of letting rules_swift generate it for Swift targets; this ensures we have consistent and non-duplicate modulemaps * Generated modulemaps are now nested in a similar way to rules_swift, preventing them from being picked up accidentally * We no longer generate a modulemap for Swift targets, and instead use the `swift.propagate_generated_module_map` feature * We no longer create another target for the Objective-C resource module accessor; this aligns with how SPM does it and removes another target that could try to generate a modulemap * The parent target for clang targets is now only an aggregator (i.e. it doesn’t have headers of its own), preventing it from complicating the build graph * The parent target for clang targets includes the generated modulemap as a child dep, removing the need to generate a noop modulemap * When detecting a custom modulemap, we only consider the public includes paths, preventing accidentally picking up errant modulemaps Signed-off-by: Brentley Jones --- docs/internal_rules_and_macros_overview.md | 3 +- swiftpkg/internal/clang_files.bzl | 28 +- swiftpkg/internal/generate_modulemap.bzl | 31 +- swiftpkg/internal/pkginfo_target_deps.bzl | 36 +- swiftpkg/internal/pkginfo_targets.bzl | 13 - swiftpkg/internal/swiftpkg_build_files.bzl | 349 +++++++----------- swiftpkg/tests/clang_files_tests.bzl | 9 +- swiftpkg/tests/swiftpkg_build_files_tests.bzl | 179 +++------ 8 files changed, 224 insertions(+), 424 deletions(-) diff --git a/docs/internal_rules_and_macros_overview.md b/docs/internal_rules_and_macros_overview.md index 6c692b769..2301f9ffe 100644 --- a/docs/internal_rules_and_macros_overview.md +++ b/docs/internal_rules_and_macros_overview.md @@ -17,7 +17,7 @@ On this page: ## generate_modulemap 
-generate_modulemap(name, deps, hdrs, module_name, noop)
+generate_modulemap(name, deps, hdrs, module_name)
Generate a modulemap for an Objective-C module. @@ -31,7 +31,6 @@ Generate a modulemap for an Objective-C module. | deps | The module maps that this module uses. | List of labels | optional | `[]` | | hdrs | The public headers for this module. | List of labels | required | | | module_name | The name of the module. | String | optional | `""` | -| noop | Designates whether a modulemap should be generated. If `False`, a modulemap is generated. If `True`, a modulemap file is not generated and the returned providers are empty. | Boolean | optional | `False` | diff --git a/swiftpkg/internal/clang_files.bzl b/swiftpkg/internal/clang_files.bzl index f96757b14..1fe0790c1 100644 --- a/swiftpkg/internal/clang_files.bzl +++ b/swiftpkg/internal/clang_files.bzl @@ -42,13 +42,13 @@ def _is_hdr(path): _root, ext = paths.split_extension(path) return lists.contains(_HEADER_EXTS, ext) -def _is_include_hdr(path, public_includes = None): +def _is_include_hdr(path, public_includes = []): """Determines whether the path is a public header. Args: path: A path `string` value. public_includes: Optional. A `sequence` of path `string` values that - are used to identify public header files. + are used to identify public header files. Returns: A `bool` indicating whether the path is a public header. @@ -56,8 +56,7 @@ def _is_include_hdr(path, public_includes = None): if not _is_hdr(path): return False - public_includes = [] if public_includes == None else public_includes - if len(public_includes) > 0: + if public_includes: for include_path in public_includes: if include_path[-1] != "/": include_path += "/" @@ -94,18 +93,30 @@ def _find_magical_public_hdr_dir(path): return None -def _is_public_modulemap(path): +def _is_public_modulemap(path, public_includes = []): """Determines whether the specified path is to a public `module.modulemap` file. Args: path: A path `string`. + public_includes: Optional. A `sequence` of path `string` values that + are used to identify public header files. Returns: A `bool` indicating whether the path is a public `module.modulemap` file. """ basename = paths.basename(path) - return basename == "module.modulemap" + if basename != "module.modulemap": + return False + + if public_includes: + for public_include in public_includes: + if path.startswith(public_include): + return True + elif _find_magical_public_hdr_dir(path) != None: + return True + + return False def _get_hdr_paths_from_modulemap(repository_ctx, modulemap_path, module_name): """Retrieves the list of headers declared in the specified modulemap file \ @@ -249,7 +260,10 @@ def _collect_files( sets.insert(srcs_set, path) elif lists.contains(_SRC_EXTS, ext): sets.insert(srcs_set, path) - elif ext == ".modulemap" and _is_public_modulemap(path): + elif ext == ".modulemap" and _is_public_modulemap( + orig_path, + public_includes = public_includes, + ): if modulemap != None: fail("Found multiple modulemap files. {first} {second}".format( first = modulemap, diff --git a/swiftpkg/internal/generate_modulemap.bzl b/swiftpkg/internal/generate_modulemap.bzl index bddb3f05b..b9669e836 100644 --- a/swiftpkg/internal/generate_modulemap.bzl +++ b/swiftpkg/internal/generate_modulemap.bzl @@ -20,33 +20,13 @@ ModuleMapInfo = provider( def _generate_modulemap_impl(ctx): module_name = ctx.attr.module_name - # Decide whether we should generate the modulemap. If we do not generate - # the modulemap, we still need to return expected providers. - if ctx.attr.noop: - return [ - DefaultInfo(files = depset([])), - ModuleMapInfo( - module_name = module_name, - modulemap_file = None, - ), - apple_common.new_objc_provider( - module_map = depset([]), - ), - CcInfo( - compilation_context = cc_common.create_compilation_context( - headers = depset([]), - includes = depset([]), - ), - ), - ] - uses = [ dep[ModuleMapInfo].module_name for dep in ctx.attr.deps if ModuleMapInfo in dep ] - out_filename = "{}/module.modulemap".format(module_name) + out_filename = "{}_modulemap/_/module.modulemap".format(ctx.attr.name) modulemap_file = ctx.actions.declare_file(out_filename) hdrs = [ @@ -85,6 +65,7 @@ def _generate_modulemap_impl(ctx): CcInfo( compilation_context = cc_common.create_compilation_context( headers = depset(provider_hdr), + direct_public_headers = provider_hdr, includes = depset([modulemap_file.dirname]), ), ), @@ -106,14 +87,6 @@ generate_modulemap = rule( "module_name": attr.string( doc = "The name of the module.", ), - "noop": attr.bool( - doc = """\ -Designates whether a modulemap should be generated. If `False`, a modulemap is \ -generated. If `True`, a modulemap file is not generated and the returned \ -providers are empty.\ -""", - default = False, - ), }, doc = "Generate a modulemap for an Objective-C module.", ) diff --git a/swiftpkg/internal/pkginfo_target_deps.bzl b/swiftpkg/internal/pkginfo_target_deps.bzl index 2a29a4119..11737e844 100644 --- a/swiftpkg/internal/pkginfo_target_deps.bzl +++ b/swiftpkg/internal/pkginfo_target_deps.bzl @@ -4,30 +4,12 @@ load("@cgrindel_bazel_starlib//bzllib:defs.bzl", "bazel_labels", "lists") load(":bazel_repo_names.bzl", "bazel_repo_names") load(":bzl_selects.bzl", "bzl_selects") load(":pkginfo_dependencies.bzl", "pkginfo_dependencies") -load(":pkginfo_targets.bzl", "pkginfo_targets") # This value is used to group Bazel select conditions _target_dep_kind = "_target_dep" -def _src_type_for_target(target): - # Check Objc first. It will have a clang_src_info and an objc_src_info. - if target.swift_src_info: - return src_types.swift - elif target.objc_src_info: - return src_types.objc - elif target.clang_src_info: - return src_types.clang - return src_types.unknown - -def _modulemap_label_for_target(repo_name, target): - return bazel_labels.new( - name = pkginfo_targets.modulemap_label_name(target.label.name), - repository_name = repo_name, - package = target.label.package, - ) - def _labels_for_target(repo_name, target): - labels = [ + return [ bazel_labels.new( name = target.label.name, repository_name = repo_name, @@ -35,22 +17,6 @@ def _labels_for_target(repo_name, target): ), ] - src_type = _src_type_for_target(target) - if src_type == src_types.objc: - # If the dep is an objc, return the real Objective-C target, not the Swift - # module alias. This is part of a workaround for Objective-C modules not - # being able to `@import` modules from other Objective-C modules. - # See `swiftpkg_build_files.bzl` for more information. - labels.append(_modulemap_label_for_target(repo_name, target)) - - elif (src_type == src_types.swift and - target.swift_src_info.has_objc_directive): - # If an Objc module wants to @import a Swift module, it will need the - # modulemap target. - labels.append(_modulemap_label_for_target(repo_name, target)) - - return labels - def _resolve_by_name(pkg_ctx, name): repo_name = bazel_repo_names.normalize(pkg_ctx.repo_name) diff --git a/swiftpkg/internal/pkginfo_targets.bzl b/swiftpkg/internal/pkginfo_targets.bzl index 4bee70733..40faa9f7b 100644 --- a/swiftpkg/internal/pkginfo_targets.bzl +++ b/swiftpkg/internal/pkginfo_targets.bzl @@ -7,7 +7,6 @@ _modulemap_suffix = "_modulemap" _resource_bundle_suffix = "_resource_bundle" _objc_resource_bundle_accessor_hdr_suffix = "_objc_resource_bundle_accessor_hdr" _objc_resource_bundle_accessor_impl_suffix = "_objc_resource_bundle_accessor_impl" -_objc_resource_bundle_accessor_library_suffix = "_objc_resource_bundle_accessor_library" _resource_bundle_accessor_suffix = "_resource_bundle_accessor" _resource_bundle_infoplist_suffix = "_resource_bundle_infoplist" _swift_hint_suffix = "_swift_hint" @@ -197,17 +196,6 @@ def _objc_resource_bundle_accessor_impl_label_name(target_name): """ return target_name + _objc_resource_bundle_accessor_impl_suffix -def _objc_resource_bundle_accessor_library_label_name(target_name): - """Returns the name of the related `objc_library` target. - - Args: - target_name: The publicly advertised name for the Swift target. - - Returns: - The name of the `objc_library` as a `string`. - """ - return target_name + _objc_resource_bundle_accessor_library_suffix - def _resource_bundle_accessor_label_name(target_name): """Returns the name of the related `resource_bundle_accessor` target. @@ -287,7 +275,6 @@ def make_pkginfo_targets(bazel_labels): modulemap_label_name = _modulemap_label_name, objc_resource_bundle_accessor_hdr_label_name = _objc_resource_bundle_accessor_hdr_label_name, objc_resource_bundle_accessor_impl_label_name = _objc_resource_bundle_accessor_impl_label_name, - objc_resource_bundle_accessor_library_label_name = _objc_resource_bundle_accessor_library_label_name, resource_bundle_accessor_label_name = _resource_bundle_accessor_label_name, resource_bundle_infoplist_label_name = _resource_bundle_infoplist_label_name, resource_bundle_label_name = _resource_bundle_label_name, diff --git a/swiftpkg/internal/swiftpkg_build_files.bzl b/swiftpkg/internal/swiftpkg_build_files.bzl index df66523aa..a2f0a8f85 100644 --- a/swiftpkg/internal/swiftpkg_build_files.bzl +++ b/swiftpkg/internal/swiftpkg_build_files.bzl @@ -1,6 +1,5 @@ """Module for creating Bazel declarations to build a Swift package.""" -load("@bazel_skylib//lib:dicts.bzl", "dicts") load("@bazel_skylib//lib:paths.bzl", "paths") load("@cgrindel_bazel_starlib//bzllib:defs.bzl", "bazel_labels", "lists") load(":artifact_infos.bzl", "artifact_types", "link_types") @@ -14,8 +13,6 @@ load(":pkginfo_targets.bzl", "pkginfo_targets") load(":pkginfos.bzl", "build_setting_kinds", "module_types", "pkginfos", "target_types") load(":starlark_codegen.bzl", scg = "starlark_codegen") -_STRING_TYPE = type("") - # MARK: - Target Entry Point def _new_for_target(repository_ctx, pkg_ctx, target, artifact_infos = []): @@ -119,6 +116,7 @@ def _swift_target_build_file(pkg_ctx, target): # Objective-C code. If so, generate the bridge header file. if target.swift_src_info.has_objc_directive and is_library_target: attrs["generates_header"] = True + attrs["features"] = ["swift.propagate_generated_module_map"] if target.swift_settings != None: if len(target.swift_settings.defines) > 0: @@ -174,10 +172,6 @@ def _swift_target_build_file(pkg_ctx, target): decls = decls, )) - # Generate a modulemap for the Swift module. - if attrs.get("generates_header", False): - all_build_files.append(_generate_modulemap_for_swift_target(target, deps)) - return build_files.merge(*all_build_files) def _swift_library_from_target(target, attrs): @@ -229,18 +223,18 @@ def _c_child_library( rule_kind, srcs, language_standard = None, - modulemap_path = None, res_copts = None): - child_attrs = dicts.omit(attrs, ["data"]) + child_attrs = dict(attrs) + + child_attrs["srcs"] = lists.flatten([srcs, attrs.get("srcs", [])]) + child_copts = list(attrs.get("copts", [])) if res_copts: child_copts.extend(res_copts) - child_attrs["srcs"] = lists.flatten([srcs, attrs.get("srcs", [])]) if language_standard: child_copts.append("-std={}".format(language_standard)) child_attrs["copts"] = child_copts - if modulemap_path: - child_attrs["module_map"] = modulemap_path + return build_decls.new( rule_kind, name, @@ -264,45 +258,16 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target): # The SWIFT_PACKAGE define is a magical value that SPM uses when it # builds clang libraries that will be used as Swift modules. "-DSWIFT_PACKAGE=1", + # Module name + "-fmodule-name={}".format(target.c99name), ] # Do not add the srcs from the clang_src_info, yet. We will do that at the # end of this function where we will create separate targets based upon the # type of source file. srcs = [] - data = [] deps = [] - res_copts = [] - clang_apple_res_bundle_info = None - if target.resources: - clang_apple_res_bundle_info = _apple_resource_bundle_for_clang( - pkg_ctx, - target, - ) - all_build_files.append(clang_apple_res_bundle_info.build_file) - data.append(":{}".format( - clang_apple_res_bundle_info.bundle_label_name, - )) - if clang_apple_res_bundle_info.objc_accessor_hdr_label_name: - srcs.extend([ - ":{}".format( - clang_apple_res_bundle_info.objc_accessor_hdr_label_name, - ), - # NOTE: We add the implementation as a dependency on a target - # that compiles the implementation. - ]) - deps.append(":" + clang_apple_res_bundle_info.objc_accessor_library_label_name) - - # SPM provides a SWIFTPM_MODULE_BUNDLE macro to access the bundle for - # ObjC code. The header file contains the macro definition. It needs - # to be available in every Objc source file. So, we specify the - # -include flag specifying the header path. - # https://github.com/apple/swift-package-manager/blob/8387798811c6cc43761c5e1b48df2d3412dc5de4/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift#L390 - res_copts.append("-include$(location :{})".format( - clang_apple_res_bundle_info.objc_accessor_hdr_label_name, - )) - local_includes = [ bzl_selects.new(value = p, kind = _condition_kinds.private_includes) for p in clang_src_info.private_includes @@ -385,16 +350,83 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target): attrs["linkopts"] = linkopts if deps: attrs["deps"] = deps - if data: - attrs["data"] = data # Generate cc_xxx and objc_xxx targets. bzl_target_name = target.label.name decls = [] child_dep_names = [] + load_stmts = [] + + # Objective-C targets don't generate a modulemap for non-Swift target by + # default. We also disable the rules_swift generation of modulemaps to + # keep parity between consuming from Objective-C and Swift. Because of this + # we need to generate our own modulemap for Objective-C targets. We only do + # this if there isn't already a custom modulemap provided, matching the + # behavior of SPM (https://github.com/swiftlang/swift-package-manager/blob/4073657e12dc7a9699c08c691acdc087d66eb453/Sources/Build/BuildDescription/ClangModuleBuildDescription.swift#L175-L193). + # + # See `generate_modulemap.bzl` for details on the modulemap generation. + # See `//swiftpkg/tests/generate_modulemap_tests` package for a usage + # example. + if clang_src_info.modulemap_path: + hint_module_map = clang_src_info.modulemap_path + elif clang_src_info.hdrs: + modulemap_target_name = pkginfo_targets.modulemap_label_name( + bzl_target_name, + ) + load_stmts.append(swiftpkg_generate_modulemap_load_stmt) + decls.append( + build_decls.new( + kind = swiftpkg_kinds.generate_modulemap, + name = modulemap_target_name, + attrs = { + # We can't get a full picture of transitive dependencies + # like rules_swift can (we would need to re-implement the + # clang aspect). `use` declarations are only needed when + # `-fmodules-decluse` is specified, so we are probably fine. + # + # Ideally long term the modulemap code in rules_swift can be + # added to `objc_library` as well so we can stop generating + # modulemaps entierly. + "deps": [], + "hdrs": clang_src_info.hdrs, + "module_name": target.c99name, + "visibility": ["//:__subpackages__"], + }, + ), + ) + + # By including the modulemap as a dep of the parent target it gets + # propagated to all consumers automatically. + child_dep_names.append(modulemap_target_name) + + hint_module_map = modulemap_target_name + else: + hint_module_map = None + + # Create an interop hint so rules_swift can propagate transitive + # modulemaps correctly. Without this we won't get a modulemap generated + # for xcframework imports. + # `module_map` attr of `objc_library` is being removed. + aspect_hint_target_name = pkginfo_targets.swift_hint_label_name( + bzl_target_name, + ) + load_stmts.append(swift_interop_hint_load_stmt) + decls.append( + build_decls.new( + kind = swift_kinds.interop_hint, + name = aspect_hint_target_name, + attrs = { + "module_map": hint_module_map, + "module_name": target.c99name, + }, + ), + ) + attrs["aspect_hints"] = [aspect_hint_target_name] if target.objc_src_info != None: + rule_kind = objc_kinds.library + # Enable clang module support. # https://bazel.build/reference/be/objective-c#objc_library.enable_modules attrs["enable_modules"] = True @@ -413,62 +445,41 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target): if sdk_framework_bzl_selects: attrs["sdk_frameworks"] = sdk_framework_bzl_selects - # There is a known issue with Objective-C library targets not - # supporting the `@import` of modules defined in other Objective-C - # targets. As a workaround, we will define two targets. One is the - # `objc_library` target. The other is a `generate_modulemap` - # target. This second target generates a `module.modulemap` file and - # provides information about that generated file to `objc_library` - # targets, if `noop` is `False`. If `noop` is `True`, the target - # generates nothing and returns "empty" providers. - # - # Why not skip adding the `generate_modulemap` if `noop` is `True`? - # The logic that assigns dependencies for other targets has no way to - # know whether the modulemap target exists. Hence, we ensure that it - # always exists but does nothing. - # - # See `pkginfo_target_deps.bzl` for the logic that resolves the - # dependency labels. - # See `generate_modulemap.bzl` for details on the modulemap generation. - # See `//swiftpkg/tests/generate_modulemap_tests` package for a usage - # example. - modulemap_deps = _collect_modulemap_deps(attrs.get("deps", [])) - load_stmts = [swiftpkg_generate_modulemap_load_stmt] - modulemap_target_name = pkginfo_targets.modulemap_label_name(bzl_target_name) - noop_modulemap = clang_src_info.modulemap_path != None - modulemap_attrs = { - "deps": [] if noop_modulemap else bzl_selects.to_starlark(modulemap_deps), - "hdrs": [] if noop_modulemap else clang_src_info.hdrs, - "module_name": target.c99name, - "noop": noop_modulemap, - "visibility": ["//:__subpackages__"], - } - decls.append( - build_decls.new( - kind = swiftpkg_kinds.generate_modulemap, - name = modulemap_target_name, - attrs = modulemap_attrs, - ), - ) - - # Create an interop hint for any custom or generated modulemap. - # `module_map` attr of `objc_library` is being removed. - aspect_hint_target_name = pkginfo_targets.swift_hint_label_name( - bzl_target_name, - ) - load_stmts.append(swift_interop_hint_load_stmt) - decls.append( - build_decls.new( - kind = swift_kinds.interop_hint, - name = aspect_hint_target_name, - attrs = { - "module_map": clang_src_info.modulemap_path if clang_src_info.modulemap_path else modulemap_target_name, - "module_name": target.c99name, - }, - ), - ) + res_copts = [] + res_objc_srcs = [] + res_objcxx_srcs = [] + clang_apple_res_bundle_info = None + if target.resources: + clang_apple_res_bundle_info = _apple_resource_bundle_for_clang( + pkg_ctx, + target, + ) + all_build_files.append(clang_apple_res_bundle_info.build_file) + attrs["data"] = [":{}".format( + clang_apple_res_bundle_info.bundle_label_name, + )] + if clang_apple_res_bundle_info.objc_accessor_hdr_label_name: + res_objcxx_srcs = [ + ":{}".format( + clang_apple_res_bundle_info.objc_accessor_hdr_label_name, + ), + ] + res_objc_srcs = res_objcxx_srcs + [ + ":{}".format( + clang_apple_res_bundle_info.objc_accessor_impl_label_name, + ), + ] + + # SPM provides a SWIFTPM_MODULE_BUNDLE macro to access the bundle for + # ObjC code. The header file contains the macro definition. It needs + # to be available in every Objc source file. So, we specify the + # -include flag specifying the header path. + # https://github.com/apple/swift-package-manager/blob/8387798811c6cc43761c5e1b48df2d3412dc5de4/Sources/Build/BuildDescription/ClangTargetBuildDescription.swift#L390 + res_copts.append("-include$(location :{})".format( + clang_apple_res_bundle_info.objc_accessor_hdr_label_name, + )) - if clang_src_info.organized_srcs.objc_srcs: + if clang_src_info.organized_srcs.objc_srcs or res_objc_srcs: child_name = "{}_objc".format(bzl_target_name) child_dep_names.append(child_name) decls.append( @@ -476,12 +487,12 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target): repository_ctx, name = child_name, attrs = attrs, - rule_kind = objc_kinds.library, + rule_kind = rule_kind, srcs = clang_src_info.organized_srcs.c_srcs + clang_src_info.organized_srcs.objc_srcs + - clang_src_info.organized_srcs.other_srcs, + clang_src_info.organized_srcs.other_srcs + + res_objc_srcs, language_standard = pkg_ctx.pkg_info.c_language_standard, - modulemap_path = clang_src_info.modulemap_path, res_copts = res_copts, ), ) @@ -493,49 +504,17 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target): repository_ctx, name = child_name, attrs = attrs, - rule_kind = objc_kinds.library, + rule_kind = rule_kind, srcs = clang_src_info.organized_srcs.cxx_srcs + clang_src_info.organized_srcs.objcxx_srcs + - clang_src_info.organized_srcs.other_srcs, + clang_src_info.organized_srcs.other_srcs + + res_objcxx_srcs, language_standard = pkg_ctx.pkg_info.cxx_language_standard, - modulemap_path = clang_src_info.modulemap_path, res_copts = res_copts, ), ) - - # Add the objc_library that brings all of the child targets together. - uber_attrs = dicts.omit(attrs, ["srcs", "copts"]) | { - "aspect_hints": [aspect_hint_target_name], - "deps": [ - ":{}".format(dname) - for dname in child_dep_names - ], - "module_name": target.c99name, - } - decls.append( - build_decls.new( - objc_kinds.library, - bzl_target_name, - attrs = _starlarkify_clang_attrs(repository_ctx, uber_attrs), - ), - ) - else: - aspect_hint_target_name = pkginfo_targets.swift_hint_label_name( - bzl_target_name, - ) - aspect_hint_attrs = {"module_name": target.c99name} - if clang_src_info.modulemap_path: - aspect_hint_attrs["module_map"] = clang_src_info.modulemap_path - - load_stmts = [swift_interop_hint_load_stmt] - decls.append( - build_decls.new( - kind = swift_kinds.interop_hint, - name = aspect_hint_target_name, - attrs = aspect_hint_attrs, - ), - ) + rule_kind = clang_kinds.library if clang_src_info.organized_srcs.c_srcs: child_name = "{}_c".format(bzl_target_name) @@ -545,7 +524,7 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target): repository_ctx, name = child_name, attrs = attrs, - rule_kind = clang_kinds.library, + rule_kind = rule_kind, srcs = clang_src_info.organized_srcs.c_srcs + clang_src_info.organized_srcs.other_srcs, language_standard = pkg_ctx.pkg_info.c_language_standard, @@ -559,7 +538,7 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target): repository_ctx, name = child_name, attrs = attrs, - rule_kind = clang_kinds.library, + rule_kind = rule_kind, srcs = clang_src_info.organized_srcs.cxx_srcs + clang_src_info.organized_srcs.other_srcs, language_standard = pkg_ctx.pkg_info.cxx_language_standard, @@ -574,27 +553,27 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target): repository_ctx, name = child_name, attrs = attrs, - rule_kind = clang_kinds.library, + rule_kind = rule_kind, srcs = clang_src_info.organized_srcs.assembly_srcs + clang_src_info.organized_srcs.other_srcs, ), ) - # Add the cc_library that brings all of the child targets together. - uber_attrs = dicts.omit(attrs, ["srcs", "copts"]) | { - "aspect_hints": [":{}".format(aspect_hint_target_name)], - "deps": [ - ":{}".format(dname) - for dname in child_dep_names - ], - } - decls.append( - build_decls.new( - clang_kinds.library, - bzl_target_name, - attrs = _starlarkify_clang_attrs(repository_ctx, uber_attrs), - ), - ) + # Add the {cc,objc}_library that brings all of the child targets together. + parent_attrs = { + "deps": [ + ":{}".format(dname) + for dname in child_dep_names + ], + "visibility": ["//:__subpackages__"], + } + decls.append( + build_decls.new( + rule_kind, + bzl_target_name, + attrs = _starlarkify_clang_attrs(repository_ctx, parent_attrs), + ), + ) all_build_files.append(build_files.new( load_stmts = load_stmts, @@ -800,7 +779,6 @@ def _apple_resource_bundle_for_clang(pkg_ctx, target): all_build_files = [apple_res_bundle_info.build_file] objc_accessor_hdr_label_name = None objc_accessor_impl_label_name = None - objc_accessor_library_label_name = None if target.objc_src_info: bzl_target_name = pkginfo_targets.bazel_label_name(target) objc_accessor_hdr_label_name = pkginfo_targets.objc_resource_bundle_accessor_hdr_label_name( @@ -809,9 +787,6 @@ def _apple_resource_bundle_for_clang(pkg_ctx, target): objc_accessor_impl_label_name = pkginfo_targets.objc_resource_bundle_accessor_impl_label_name( bzl_target_name, ) - objc_accessor_library_label_name = pkginfo_targets.objc_resource_bundle_accessor_library_label_name( - bzl_target_name, - ) all_build_files.append( build_files.new( load_stmts = [ @@ -835,66 +810,16 @@ def _apple_resource_bundle_for_clang(pkg_ctx, target): "module_name": target.c99name, }, ), - build_decls.new( - kind = objc_kinds.library, - name = objc_accessor_library_label_name, - attrs = { - "hdrs": [":" + objc_accessor_hdr_label_name], - "srcs": [":" + objc_accessor_impl_label_name], - }, - ), ], ), ) return struct( bundle_label_name = apple_res_bundle_info.bundle_label_name, objc_accessor_hdr_label_name = objc_accessor_hdr_label_name, - objc_accessor_library_label_name = objc_accessor_library_label_name, + objc_accessor_impl_label_name = objc_accessor_impl_label_name, build_file = build_files.merge(*all_build_files), ) -# MARK: - Modulemap Generation - -def _collect_modulemap_deps(deps): - modulemap_deps = [] - for dep in deps: - if type(dep) == _STRING_TYPE: - continue - mm_values = [ - v - for v in dep.value - if pkginfo_targets.is_modulemap_label(v) - ] - if len(mm_values) == 0: - continue - mm_dep = bzl_selects.new( - value = mm_values, - kind = dep.kind, - condition = dep.condition, - ) - modulemap_deps.append(mm_dep) - return modulemap_deps - -def _generate_modulemap_for_swift_target(target, deps): - load_stmts = [swiftpkg_generate_modulemap_load_stmt] - bzl_target_name = pkginfo_targets.bazel_label_name(target) - modulemap_target_name = pkginfo_targets.modulemap_label_name(bzl_target_name) - modulemap_deps = _collect_modulemap_deps(deps) - attrs = { - "deps": bzl_selects.to_starlark(modulemap_deps), - "hdrs": [":{}".format(bzl_target_name)], - "module_name": target.c99name, - "visibility": ["//:__subpackages__"], - } - decls = [ - build_decls.new( - kind = swiftpkg_kinds.generate_modulemap, - name = modulemap_target_name, - attrs = attrs, - ), - ] - return build_files.new(load_stmts = load_stmts, decls = decls) - # MARK: - Products Entry Point def _new_for_products(pkg_ctx): diff --git a/swiftpkg/tests/clang_files_tests.bzl b/swiftpkg/tests/clang_files_tests.bzl index 0c5e84ae7..cb7ba261b 100644 --- a/swiftpkg/tests/clang_files_tests.bzl +++ b/swiftpkg/tests/clang_files_tests.bzl @@ -46,7 +46,14 @@ is_include_hdr_test = unittest.make(_is_include_hdr_test) def _is_public_modulemap_test(ctx): env = unittest.begin(ctx) - asserts.true(env, clang_files.is_public_modulemap("foo/bar/module.modulemap")) + asserts.true( + env, + clang_files.is_public_modulemap( + "foo/bar/module.modulemap", + public_includes = ["foo/bar"], + ), + ) + asserts.false(env, clang_files.is_public_modulemap("foo/bar/module.modulemap")) asserts.false(env, clang_files.is_public_modulemap("foo/bar/chicken.modulemap")) return unittest.end(env) diff --git a/swiftpkg/tests/swiftpkg_build_files_tests.bzl b/swiftpkg/tests/swiftpkg_build_files_tests.bzl index 53cbf878b..3080c196c 100644 --- a/swiftpkg/tests/swiftpkg_build_files_tests.bzl +++ b/swiftpkg/tests/swiftpkg_build_files_tests.bzl @@ -576,24 +576,26 @@ swift_binary( name = "ClangLibrary", exp = """\ load("@build_bazel_rules_swift//swift:swift.bzl", "swift_interop_hint") +load("@rules_swift_package_manager//swiftpkg:build_defs.bzl", "generate_modulemap") cc_library( name = "ClangLibrary.rspm", - aspect_hints = [":ClangLibrary.rspm_swift_hint"], - deps = [":ClangLibrary.rspm_cxx"], - hdrs = ["include/external.h"], - includes = ["include"], - textual_hdrs = ["src/foo.cc"], + deps = [ + ":ClangLibrary.rspm_modulemap", + ":ClangLibrary.rspm_cxx", + ], visibility = ["//:__subpackages__"], ) cc_library( name = "ClangLibrary.rspm_cxx", + aspect_hints = ["ClangLibrary.rspm_swift_hint"], copts = [ "-fblocks", "-fobjc-arc", "-fPIC", "-DSWIFT_PACKAGE=1", + "-fmodule-name=ClangLibrary", "-DPLATFORM_POSIX=1", "-Iexternal/bzlmodmangled~swiftpkg_mypackage/src", "-Iexternal/bzlmodmangled~swiftpkg_mypackage", @@ -611,8 +613,17 @@ cc_library( visibility = ["//:__subpackages__"], ) +generate_modulemap( + name = "ClangLibrary.rspm_modulemap", + deps = [], + hdrs = ["include/external.h"], + module_name = "ClangLibrary", + visibility = ["//:__subpackages__"], +) + swift_interop_hint( name = "ClangLibrary.rspm_swift_hint", + module_map = "ClangLibrary.rspm_modulemap", module_name = "ClangLibrary", ) """, @@ -626,54 +637,33 @@ load("@rules_swift_package_manager//swiftpkg:build_defs.bzl", "generate_modulema generate_modulemap( name = "ObjcLibrary.rspm_modulemap", - deps = ["@swiftpkg_mypackage//:ObjcLibraryDep.rspm_modulemap"], + deps = [], hdrs = ["include/external.h"], module_name = "ObjcLibrary", - noop = False, visibility = ["//:__subpackages__"], ) objc_library( name = "ObjcLibrary.rspm", - aspect_hints = ["ObjcLibrary.rspm_swift_hint"], - deps = [":ObjcLibrary.rspm_objc"], - enable_modules = True, - hdrs = ["include/external.h"], - includes = ["include"], - module_name = "ObjcLibrary", - sdk_frameworks = select({ - "@rules_swift_package_manager//config_settings/spm/platform:ios": [ - "Foundation", - "UIKit", - ], - "@rules_swift_package_manager//config_settings/spm/platform:macos": ["Foundation"], - "@rules_swift_package_manager//config_settings/spm/platform:tvos": [ - "Foundation", - "UIKit", - ], - "@rules_swift_package_manager//config_settings/spm/platform:watchos": [ - "Foundation", - "UIKit", - ], - "//conditions:default": [], - }), - textual_hdrs = ["src/foo.m"], + deps = [ + ":ObjcLibrary.rspm_modulemap", + ":ObjcLibrary.rspm_objc", + ], visibility = ["//:__subpackages__"], ) objc_library( name = "ObjcLibrary.rspm_objc", + aspect_hints = ["ObjcLibrary.rspm_swift_hint"], copts = [ "-fblocks", "-fobjc-arc", "-fPIC", "-DSWIFT_PACKAGE=1", + "-fmodule-name=ObjcLibrary", "-Iexternal/bzlmodmangled~swiftpkg_mypackage/src", ], - deps = [ - "@swiftpkg_mypackage//:ObjcLibraryDep.rspm", - "@swiftpkg_mypackage//:ObjcLibraryDep.rspm_modulemap", - ], + deps = ["@swiftpkg_mypackage//:ObjcLibraryDep.rspm"], enable_modules = True, hdrs = ["include/external.h"], includes = ["include"], @@ -713,62 +703,28 @@ swift_interop_hint( name = "ObjcLibraryWithModulemap", exp = """\ load("@build_bazel_rules_swift//swift:swift.bzl", "swift_interop_hint") -load("@rules_swift_package_manager//swiftpkg:build_defs.bzl", "generate_modulemap") - -generate_modulemap( - name = "ObjcLibraryWithModulemap.rspm_modulemap", - deps = [], - hdrs = [], - module_name = "ObjcLibraryWithModulemap", - noop = True, - visibility = ["//:__subpackages__"], -) objc_library( name = "ObjcLibraryWithModulemap.rspm", - aspect_hints = ["ObjcLibraryWithModulemap.rspm_swift_hint"], deps = [":ObjcLibraryWithModulemap.rspm_objc"], - enable_modules = True, - hdrs = ["include/external.h"], - includes = ["include"], - module_name = "ObjcLibraryWithModulemap", - sdk_frameworks = select({ - "@rules_swift_package_manager//config_settings/spm/platform:ios": [ - "Foundation", - "UIKit", - ], - "@rules_swift_package_manager//config_settings/spm/platform:macos": ["Foundation"], - "@rules_swift_package_manager//config_settings/spm/platform:tvos": [ - "Foundation", - "UIKit", - ], - "@rules_swift_package_manager//config_settings/spm/platform:watchos": [ - "Foundation", - "UIKit", - ], - "//conditions:default": [], - }), - textual_hdrs = ["src/foo.m"], visibility = ["//:__subpackages__"], ) objc_library( name = "ObjcLibraryWithModulemap.rspm_objc", + aspect_hints = ["ObjcLibraryWithModulemap.rspm_swift_hint"], copts = [ "-fblocks", "-fobjc-arc", "-fPIC", "-DSWIFT_PACKAGE=1", + "-fmodule-name=ObjcLibraryWithModulemap", "-Iexternal/bzlmodmangled~swiftpkg_mypackage/src", ], - deps = [ - "@swiftpkg_mypackage//:ObjcLibraryDep.rspm", - "@swiftpkg_mypackage//:ObjcLibraryDep.rspm_modulemap", - ], + deps = ["@swiftpkg_mypackage//:ObjcLibraryDep.rspm"], enable_modules = True, hdrs = ["include/external.h"], includes = ["include"], - module_map = "include/module.modulemap", sdk_frameworks = select({ "@rules_swift_package_manager//config_settings/spm/platform:ios": [ "Foundation", @@ -828,24 +784,26 @@ swift_library( name = "ClangLibraryWithConditionalDep", exp = """\ load("@build_bazel_rules_swift//swift:swift.bzl", "swift_interop_hint") +load("@rules_swift_package_manager//swiftpkg:build_defs.bzl", "generate_modulemap") cc_library( name = "ClangLibraryWithConditionalDep.rspm", - aspect_hints = [":ClangLibraryWithConditionalDep.rspm_swift_hint"], - deps = [":ClangLibraryWithConditionalDep.rspm_cxx"], - hdrs = ["include/external.h"], - includes = ["include"], - textual_hdrs = ["src/foo.cc"], + deps = [ + ":ClangLibraryWithConditionalDep.rspm_modulemap", + ":ClangLibraryWithConditionalDep.rspm_cxx", + ], visibility = ["//:__subpackages__"], ) cc_library( name = "ClangLibraryWithConditionalDep.rspm_cxx", + aspect_hints = ["ClangLibraryWithConditionalDep.rspm_swift_hint"], copts = [ "-fblocks", "-fobjc-arc", "-fPIC", "-DSWIFT_PACKAGE=1", + "-fmodule-name=ClangLibraryWithConditionalDep", "-Iexternal/bzlmodmangled~swiftpkg_mypackage/src", ], deps = select({ @@ -863,8 +821,17 @@ cc_library( visibility = ["//:__subpackages__"], ) +generate_modulemap( + name = "ClangLibraryWithConditionalDep.rspm_modulemap", + deps = [], + hdrs = ["include/external.h"], + module_name = "ClangLibraryWithConditionalDep", + visibility = ["//:__subpackages__"], +) + swift_interop_hint( name = "ClangLibraryWithConditionalDep.rspm_swift_hint", + module_map = "ClangLibraryWithConditionalDep.rspm_modulemap", module_name = "ClangLibraryWithConditionalDep", ) """, @@ -874,24 +841,13 @@ swift_interop_hint( name = "SwiftForObjcTarget", exp = """\ load("@build_bazel_rules_swift//swift:swift.bzl", "swift_library") -load("@rules_swift_package_manager//swiftpkg:build_defs.bzl", "generate_modulemap") - -generate_modulemap( - name = "SwiftForObjcTarget.rspm_modulemap", - deps = ["@swiftpkg_mypackage//:ObjcLibraryDep.rspm_modulemap"], - hdrs = [":SwiftForObjcTarget.rspm"], - module_name = "SwiftForObjcTarget", - visibility = ["//:__subpackages__"], -) swift_library( name = "SwiftForObjcTarget.rspm", always_include_developer_search_paths = True, copts = ["-DSWIFT_PACKAGE"], - deps = [ - "@swiftpkg_mypackage//:ObjcLibraryDep.rspm", - "@swiftpkg_mypackage//:ObjcLibraryDep.rspm_modulemap", - ], + deps = ["@swiftpkg_mypackage//:ObjcLibraryDep.rspm"], + features = ["swift.propagate_generated_module_map"], generates_header = True, module_name = "SwiftForObjcTarget", package_name = "MyPackage", @@ -964,50 +920,31 @@ generate_modulemap( deps = [], hdrs = ["include/external.h"], module_name = "ObjcLibraryWithResources", - noop = False, visibility = ["//:__subpackages__"], ) objc_library( name = "ObjcLibraryWithResources.rspm", - aspect_hints = ["ObjcLibraryWithResources.rspm_swift_hint"], - data = [":ObjcLibraryWithResources.rspm_resource_bundle"], - deps = [":ObjcLibraryWithResources.rspm_objc"], - enable_modules = True, - hdrs = ["include/external.h"], - includes = ["include"], - module_name = "ObjcLibraryWithResources", - sdk_frameworks = select({ - "@rules_swift_package_manager//config_settings/spm/platform:ios": [ - "Foundation", - "UIKit", - ], - "@rules_swift_package_manager//config_settings/spm/platform:macos": ["Foundation"], - "@rules_swift_package_manager//config_settings/spm/platform:tvos": [ - "Foundation", - "UIKit", - ], - "@rules_swift_package_manager//config_settings/spm/platform:watchos": [ - "Foundation", - "UIKit", - ], - "//conditions:default": [], - }), - textual_hdrs = ["src/foo.m"], + deps = [ + ":ObjcLibraryWithResources.rspm_modulemap", + ":ObjcLibraryWithResources.rspm_objc", + ], visibility = ["//:__subpackages__"], ) objc_library( name = "ObjcLibraryWithResources.rspm_objc", + aspect_hints = ["ObjcLibraryWithResources.rspm_swift_hint"], copts = [ "-fblocks", "-fobjc-arc", "-fPIC", "-DSWIFT_PACKAGE=1", + "-fmodule-name=ObjcLibraryWithResources", "-Iexternal/bzlmodmangled~swiftpkg_mypackage/src", "-include$(location :ObjcLibraryWithResources.rspm_objc_resource_bundle_accessor_hdr)", ], - deps = [":ObjcLibraryWithResources.rspm_objc_resource_bundle_accessor_library"], + data = [":ObjcLibraryWithResources.rspm_resource_bundle"], enable_modules = True, hdrs = ["include/external.h"], includes = ["include"], @@ -1031,17 +968,12 @@ objc_library( "src/foo.m", "src/foo.h", ":ObjcLibraryWithResources.rspm_objc_resource_bundle_accessor_hdr", + ":ObjcLibraryWithResources.rspm_objc_resource_bundle_accessor_impl", ], textual_hdrs = ["src/foo.m"], visibility = ["//:__subpackages__"], ) -objc_library( - name = "ObjcLibraryWithResources.rspm_objc_resource_bundle_accessor_library", - hdrs = [":ObjcLibraryWithResources.rspm_objc_resource_bundle_accessor_hdr"], - srcs = [":ObjcLibraryWithResources.rspm_objc_resource_bundle_accessor_impl"], -) - objc_resource_bundle_accessor_hdr( name = "ObjcLibraryWithResources.rspm_objc_resource_bundle_accessor_hdr", module_name = "ObjcLibraryWithResources", @@ -1134,10 +1066,7 @@ load("@build_bazel_rules_swift//swift:swift.bzl", "swift_library_group") swift_library_group( name = "ObjcLibraryWithModulemap", - deps = [ - "@swiftpkg_mypackage//:ObjcLibraryWithModulemap.rspm", - "@swiftpkg_mypackage//:ObjcLibraryWithModulemap.rspm_modulemap", - ], + deps = ["@swiftpkg_mypackage//:ObjcLibraryWithModulemap.rspm"], visibility = ["//visibility:public"], ) """,