fix: correct issues with modulemap generation (#1360)

Fixes duplicate definition errors caused by custom/generated modulemap
files.

Author: luispadron

docs/faq.md

@@ -13,7 +13,6 @@
 * [After running `//:swift_update_pkgs`, I see a `.build` directory. What is it? Do I need it?](#after-running-swift_update_pkgs-i-see-a-build-directory-what-is-it-do-i-need-it)
 * [Does the Gazelle plugin run Swift package manager with every execution?](#does-the-gazelle-plugin-run-swift-package-manager-with-every-execution)
 * [Can I store the Swift dependency files in a sub-package (i.e., not in the root of the workspace)?](#can-i-store-the-swift-dependency-files-in-a-sub-package-ie-not-in-the-root-of-the-workspace)
-* [My project builds successfully with `bazel build ...`, but it does not build when using `rules_xcodeproj`. How can I fix this?](#my-project-builds-successfully-with-bazel-build--but-it-does-not-build-when-using-rules_xcodeproj-how-can-i-fix-this)
   * [Why does this happen?](#why-does-this-happen)
 * [How do I handle the error `Unable to resolve byName reference XXX in @swiftpkg_yyy.`?](#how-do-i-handle-the-error-unable-to-resolve-byname-reference-xxx-in-swiftpkg_yyy)
   * [How do I fix this issue?](#how-do-i-fix-this-issue)
@@ -94,21 +93,6 @@ the source files that exist in your project to generate the Bazel build files.
 Yes. The [vapor example] demonstrates storing the Swift dependency files in a sub-package called
 `swift`.
 
-## My project builds successfully with `bazel build ...`, but it does not build when using `rules_xcodeproj`. How can I fix this?
-
-tl;dr Add the following to your `.bazelrc`.
-
-```
-# Ensure that sandboxed is added to the spawn strategy list when building with
-# rules_xcodeproj.
-build:rules_xcodeproj --spawn_strategy=remote,worker,sandboxed,local
-```
-
-Alternatively, you can use the [--strategy_regexp] flag to target the relevant targets. For
-instance, if `Sources/BranchSDK/BNCContentDiscoveryManager.m` is not building properly, you can
-specify `--strategy_regexp="Compiling Sources/BranchSDK/.*=sandboxed"` to use the `sandboxed` strategy
-for that file. The regular expression matches on the _description_ for the action.
-
 ### Why does this happen?
 
 This can happen with some Swift packages (e.g. `firebase-ios-sdk`). [rules_xcodeproj removes the

examples/firebase_example/.bazelrc

@@ -7,8 +7,8 @@ import %workspace%/../../ci.bazelrc
 # Try to import a local.rc file; typically, written by CI
 try-import %workspace%/../../local.bazelrc
 
-# NOTE: Puposefully not specifying --copt='-std=c99' as it is applied to 
-# objc_library targets that contain Objective-C++ files. Tried using 
+# NOTE: Puposefully not specifying --copt='-std=c99' as it is applied to
+# objc_library targets that contain Objective-C++ files. Tried using
 # --per_file_copt to exclude .mm files, but did not have success.
 # https://stackoverflow.com/questions/40260242/how-to-set-c-standard-version-when-build-with-bazel/43388168#43388168
 #
@@ -20,16 +20,3 @@ build --cxxopt='-std=gnu++14'
 # Firebase SPM support requires `-ObjC` linker option.
 # https://github.com/firebase/firebase-ios-sdk/blob/master/SwiftPackageManager.md#requirements
 build --linkopt='-ObjC'
-
-# Use the sandbox for rules_xcodeproj-specific builds.  rules_xcodeproj removes
-# the sandboxed spawn strategy (--spawn_strategy=remote,worker,local) by
-# default to speed up builds. However, to support setting C/C++ language
-# standard flags per target
-# (https://github.com/cgrindel/rules_swift_package_manager/issues/1079), we
-# need to generate multiple {cc,objc}_library targets based upon the type of
-# source files (e.g. C, C++, ObjC, ObjC++). When builds occur outside of the
-# sandbox, we can experience "error: duplicate interface definition for class
-# 'XXX'" errors. Until we can address the underlying error, clients that use
-# rules_swift_package_manager with rules_xcodeproj may need to use sandbox
-# builds.
-common:rules_xcodeproj --spawn_strategy=remote,worker,sandboxed,local

examples/interesting_deps/BUILD.bazel

@@ -1,4 +1,5 @@
 load("@bazel_gazelle//:def.bzl", "gazelle", "gazelle_binary")
+load("@build_bazel_rules_apple//apple:macos.bzl", "macos_unit_test")
 load("@build_bazel_rules_swift//swift:swift.bzl", "swift_binary")
 load("@cgrindel_bazel_starlib//bzltidy:defs.bzl", "tidy")
 
@@ -46,6 +47,19 @@ swift_binary(
     ],
 )
 
+objc_library(
+    name = "objc_test.lib",
+    srcs = ["objc_test.m"],
+    enable_modules = True,
+    deps = ["@swiftpkg_ocmock//:OCMock"],
+)
+
+macos_unit_test(
+    name = "objc_test",
+    minimum_os_version = "10.15",
+    deps = [":objc_test.lib"],
+)
+
 sh_test(
     name = "simple_test",
     srcs = ["simple_test.sh"],

examples/interesting_deps/MODULE.bazel

@@ -23,6 +23,11 @@ bazel_dep(
     version = "2.2.4",
     repo_name = "build_bazel_rules_swift",
 )
+bazel_dep(
+    name = "rules_apple",
+    version = "3.6.0",
+    repo_name = "build_bazel_rules_apple",
+)
 
 bazel_dep(
     name = "bazel_skylib_gazelle_plugin",
@@ -70,6 +75,7 @@ use_repo(
     "swiftpkg_cocoalumberjack",
     "swiftpkg_geoswift",
     "swiftpkg_libwebp_xcode",
+    "swiftpkg_ocmock",
     "swiftpkg_opencombine",
     "swiftpkg_swift_log",
 )

examples/interesting_deps/Package.resolved

@@ -10,5 +10,6 @@ let package = Package(
         .package(url: "https://github.com/OpenCombine/OpenCombine", from: "0.14.0"),
         .package(url: "https://github.com/SDWebImage/libwebp-Xcode.git", from: "1.3.2"),
         .package(url: "https://github.com/apple/swift-log", from: "1.6.2"),
+        .package(url: "https://github.com/erikdoe/ocmock", from: "3.9.0"),
     ]
 )

examples/interesting_deps/Package.swift

@@ -2,7 +2,7 @@
 
 set -o errexit -o nounset -o pipefail
 
-# Use the Bazel binary specified by the integration test. Otherise, fall back
+# Use the Bazel binary specified by the integration test. Otherwise, fall back
 # to bazel.
 bazel="${BIT_BAZEL_BINARY:-bazel}"
 
@@ -12,5 +12,7 @@ bazel="${BIT_BAZEL_BINARY:-bazel}"
 # Test resolving the package via the `swift_package` repo.
 "${bazel}" run @swift_package//:resolve
 
-# Ensure that it builds and tests pass
-"${bazel}" test //...
+# Ensure that it builds in both sandbox and local strategies.
+# This tests issues with modulemap include/generation.
+"${bazel}" test //... --spawn_strategy=sandboxed,local
+"${bazel}" test //... --spawn_strategy=local

examples/interesting_deps/do_test

@@ -0,0 +1,16 @@
+#import <OCMock/OCMock.h>
+
+@import Foundation;
+@import XCTest;
+
+@interface ObjcTest : XCTestCase
+@end
+
+@implementation ObjcTest
+
+- (void)testMock {
+  OCMockObject *mock = [OCMockObject mockForClass:[NSObject class]];
+  XCTAssertNotNil(mock);
+}
+
+@end

examples/interesting_deps/objc_test.m

@@ -229,6 +229,7 @@ def _c_child_library(
         rule_kind,
         srcs,
         language_standard = None,
+        modulemap_path = None,
         res_copts = None):
     child_attrs = dicts.omit(attrs, ["data"])
     child_copts = list(attrs.get("copts", []))
@@ -238,6 +239,8 @@ def _c_child_library(
     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,
@@ -434,8 +437,8 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target):
         modulemap_target_name = pkginfo_targets.modulemap_label_name(bzl_target_name)
         noop_modulemap = clang_src_info.modulemap_path != None
         modulemap_attrs = {
-            "deps": bzl_selects.to_starlark(modulemap_deps),
-            "hdrs": clang_src_info.hdrs,
+            "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__"],
@@ -448,6 +451,23 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target):
             ),
         )
 
+        # 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,
+                },
+            ),
+        )
+
         if clang_src_info.organized_srcs.objc_srcs:
             child_name = "{}_objc".format(bzl_target_name)
             child_dep_names.append(child_name)
@@ -461,6 +481,7 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target):
                            clang_src_info.organized_srcs.objc_srcs +
                            clang_src_info.organized_srcs.other_srcs,
                     language_standard = pkg_ctx.pkg_info.c_language_standard,
+                    modulemap_path = clang_src_info.modulemap_path,
                     res_copts = res_copts,
                 ),
             )
@@ -477,18 +498,20 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target):
                            clang_src_info.organized_srcs.objcxx_srcs +
                            clang_src_info.organized_srcs.other_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,
         }
-        uber_attrs["module_name"] = target.c99name
         decls.append(
             build_decls.new(
                 objc_kinds.library,
@@ -502,6 +525,8 @@ def _clang_target_build_file(repository_ctx, pkg_ctx, target):
             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(

swiftpkg/internal/swiftpkg_build_files.bzl

@@ -621,6 +621,7 @@ swift_interop_hint(
             msg = "Objc target",
             name = "ObjcLibrary",
             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(
@@ -634,6 +635,7 @@ generate_modulemap(
 
 objc_library(
     name = "ObjcLibrary.rspm",
+    aspect_hints = ["ObjcLibrary.rspm_swift_hint"],
     deps = [":ObjcLibrary.rspm_objc"],
     enable_modules = True,
     hdrs = ["include/external.h"],
@@ -698,25 +700,33 @@ objc_library(
     textual_hdrs = ["src/foo.m"],
     visibility = ["//:__subpackages__"],
 )
+
+swift_interop_hint(
+    name = "ObjcLibrary.rspm_swift_hint",
+    module_map = "ObjcLibrary.rspm_modulemap",
+    module_name = "ObjcLibrary",
+)
 """,
         ),
         struct(
             msg = "Objc target with a modulemap",
             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 = ["@swiftpkg_mypackage//:ObjcLibraryDep.rspm_modulemap"],
-    hdrs = ["include/external.h"],
+    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"],
@@ -758,6 +768,7 @@ objc_library(
     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",
@@ -781,6 +792,12 @@ objc_library(
     textual_hdrs = ["src/foo.m"],
     visibility = ["//:__subpackages__"],
 )
+
+swift_interop_hint(
+    name = "ObjcLibraryWithModulemap.rspm_swift_hint",
+    module_map = "include/module.modulemap",
+    module_name = "ObjcLibraryWithModulemap",
+)
 """,
         ),
         struct(
@@ -931,6 +948,7 @@ swift_library(
             name = "ObjcLibraryWithResources",
             exp = """\
 load("@build_bazel_rules_apple//apple:resources.bzl", "apple_resource_bundle")
+load("@build_bazel_rules_swift//swift:swift.bzl", "swift_interop_hint")
 load("@rules_swift_package_manager//swiftpkg:build_defs.bzl", "generate_modulemap", "objc_resource_bundle_accessor_hdr", "objc_resource_bundle_accessor_impl", "resource_bundle_infoplist")
 
 apple_resource_bundle(
@@ -952,6 +970,7 @@ generate_modulemap(
 
 objc_library(
     name = "ObjcLibraryWithResources.rspm",
+    aspect_hints = ["ObjcLibraryWithResources.rspm_swift_hint"],
     data = [":ObjcLibraryWithResources.rspm_resource_bundle"],
     deps = [":ObjcLibraryWithResources.rspm_objc"],
     enable_modules = True,
@@ -1039,6 +1058,12 @@ resource_bundle_infoplist(
     name = "ObjcLibraryWithResources.rspm_resource_bundle_infoplist",
     region = "en",
 )
+
+swift_interop_hint(
+    name = "ObjcLibraryWithResources.rspm_swift_hint",
+    module_map = "ObjcLibraryWithResources.rspm_modulemap",
+    module_name = "ObjcLibraryWithResources",
+)
 """,
         ),
     ]