From 20e3bb9a28f252f706d16426b01bd1f5aa0d5f39 Mon Sep 17 00:00:00 2001 From: Luis Padron Date: Tue, 26 Nov 2024 18:33:15 -0500 Subject: [PATCH] fix: Set module_map attr for targets with custom modulemaps --- docs/faq.md | 16 ---------- examples/firebase_example/.bazelrc | 17 ++-------- examples/interesting_deps/BUILD.bazel | 14 +++++++++ examples/interesting_deps/MODULE.bazel | 6 ++++ examples/interesting_deps/Package.resolved | 21 +++++++++---- examples/interesting_deps/Package.swift | 1 + examples/interesting_deps/do_test | 8 +++-- examples/interesting_deps/objc_test.m | 16 ++++++++++ swiftpkg/internal/swiftpkg_build_files.bzl | 31 +++++++++++++++++-- swiftpkg/tests/swiftpkg_build_files_tests.bzl | 29 +++++++++++++++-- 10 files changed, 114 insertions(+), 45 deletions(-) create mode 100644 examples/interesting_deps/objc_test.m diff --git a/docs/faq.md b/docs/faq.md index ef9a0b779..5d11cdaa2 100644 --- a/docs/faq.md +++ b/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 diff --git a/examples/firebase_example/.bazelrc b/examples/firebase_example/.bazelrc index 070b61fa2..4c7b3c554 100644 --- a/examples/firebase_example/.bazelrc +++ b/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 diff --git a/examples/interesting_deps/BUILD.bazel b/examples/interesting_deps/BUILD.bazel index 48b890f01..3b45f5926 100644 --- a/examples/interesting_deps/BUILD.bazel +++ b/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"], diff --git a/examples/interesting_deps/MODULE.bazel b/examples/interesting_deps/MODULE.bazel index 407873ac2..821123e02 100644 --- a/examples/interesting_deps/MODULE.bazel +++ b/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", ) diff --git a/examples/interesting_deps/Package.resolved b/examples/interesting_deps/Package.resolved index 4e560e61a..d498e7767 100644 --- a/examples/interesting_deps/Package.resolved +++ b/examples/interesting_deps/Package.resolved @@ -14,8 +14,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/GEOSwift/geos.git", "state" : { - "revision" : "f510e634c822116fca615064d889300dba40d761", - "version" : "8.1.0" + "revision" : "4d8af495e7507f48f1ae9104102debdd2043917b", + "version" : "9.0.0" } }, { @@ -23,8 +23,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/GEOSwift/GEOSwift", "state" : { - "revision" : "2d23ede8c82c067655d3a9f0221bc73f08cd399a", - "version" : "10.1.0" + "revision" : "e42c062c2feb0df61373ae8cd6031a823a0dc981", + "version" : "11.0.0" } }, { @@ -36,6 +36,15 @@ "version" : "1.3.2" } }, + { + "identity" : "ocmock", + "kind" : "remoteSourceControl", + "location" : "https://github.com/erikdoe/ocmock", + "state" : { + "revision" : "2c0bfd373289f4a7716db5d6db471640f91a6507", + "version" : "3.9.4" + } + }, { "identity" : "opencombine", "kind" : "remoteSourceControl", @@ -50,8 +59,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-log", "state" : { - "revision" : "e97a6fcb1ab07462881ac165fdbb37f067e205d5", - "version" : "1.5.4" + "revision" : "96a2f8a0fa41e9e09af4585e2724c4e825410b91", + "version" : "1.6.2" } } ], diff --git a/examples/interesting_deps/Package.swift b/examples/interesting_deps/Package.swift index 0dae0a77d..d1234a183 100644 --- a/examples/interesting_deps/Package.swift +++ b/examples/interesting_deps/Package.swift @@ -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"), ] ) diff --git a/examples/interesting_deps/do_test b/examples/interesting_deps/do_test index 903dbf5f4..4e14503d0 100755 --- a/examples/interesting_deps/do_test +++ b/examples/interesting_deps/do_test @@ -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 diff --git a/examples/interesting_deps/objc_test.m b/examples/interesting_deps/objc_test.m new file mode 100644 index 000000000..553005290 --- /dev/null +++ b/examples/interesting_deps/objc_test.m @@ -0,0 +1,16 @@ +#import + +@import Foundation; +@import XCTest; + +@interface ObjcTest : XCTestCase +@end + +@implementation ObjcTest + +- (void)testMock { + OCMockObject *mock = [OCMockObject mockForClass:[NSObject class]]; + XCTAssertNotNil(mock); +} + +@end diff --git a/swiftpkg/internal/swiftpkg_build_files.bzl b/swiftpkg/internal/swiftpkg_build_files.bzl index 3075f1889..d94ecc862 100644 --- a/swiftpkg/internal/swiftpkg_build_files.bzl +++ b/swiftpkg/internal/swiftpkg_build_files.bzl @@ -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( diff --git a/swiftpkg/tests/swiftpkg_build_files_tests.bzl b/swiftpkg/tests/swiftpkg_build_files_tests.bzl index 8a656e24b..53cbf878b 100644 --- a/swiftpkg/tests/swiftpkg_build_files_tests.bzl +++ b/swiftpkg/tests/swiftpkg_build_files_tests.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,18 +700,25 @@ 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__"], @@ -717,6 +726,7 @@ generate_modulemap( 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", +) """, ), ]