fix: fix modulemaps to behave like SPM

* 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 <[email protected]>
cgrindel · Dec 10, 2024 · 1a81546 · 1a81546
1 parent 0317282
commit 1a81546
Show file tree

Hide file tree

Showing 8 changed files with 224 additions and 424 deletions.
diff --git 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
 
 <pre>
-generate_modulemap(<a href="#generate_modulemap-name">name</a>, <a href="#generate_modulemap-deps">deps</a>, <a href="#generate_modulemap-hdrs">hdrs</a>, <a href="#generate_modulemap-module_name">module_name</a>, <a href="#generate_modulemap-noop">noop</a>)
+generate_modulemap(<a href="#generate_modulemap-name">name</a>, <a href="#generate_modulemap-deps">deps</a>, <a href="#generate_modulemap-hdrs">hdrs</a>, <a href="#generate_modulemap-module_name">module_name</a>)
 </pre>
 
 Generate a modulemap for an Objective-C module.
@@ -31,7 +31,6 @@ Generate a modulemap for an Objective-C module.
 | <a id="generate_modulemap-deps"></a>deps |  The module maps that this module uses.   | <a href="https://bazel.build/concepts/labels">List of labels</a> | optional |  `[]`  |
 | <a id="generate_modulemap-hdrs"></a>hdrs |  The public headers for this module.   | <a href="https://bazel.build/concepts/labels">List of labels</a> | required |  |
 | <a id="generate_modulemap-module_name"></a>module_name |  The name of the module.   | String | optional |  `""`  |
-| <a id="generate_modulemap-noop"></a>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`  |
 
 
 <a id="resource_bundle_accessor"></a>

diff --git a/swiftpkg/internal/clang_files.bzl b/swiftpkg/internal/clang_files.bzl
@@ -42,22 +42,21 @@ 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.
     """
     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
@@ -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
@@ -4,53 +4,19 @@ 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,
             package = target.label.package,
         ),
     ]
 
-    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
@@ -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,