bazelbuild / rules_apple

Bazel rules to build apps for Apple platforms.
Apache License 2.0
512 stars 269 forks source link

Using `apple_dynamic_framework_import` creates implicit dependency on `swiftc` #2387

Closed maxbelanger closed 9 months ago

maxbelanger commented 9 months ago

In our setup, we share a WORKSPACE with a number of other targets. We noticed that when we started to use an apple_dynamic_framework_import rule in our macOS application build, a requirement was created for the workspace to have rules_swift:

[...] every rule of type apple_dynamic_framework_import implicitly depends upon the target '@build_bazel_rules_swift_local_config//:toolchain', but this target could not be found because of: no such package '@build_bazel_rules_swift_local_config//': The repository '@build_bazel_rules_swift_local_config' could not be resolved: Repository '@build_bazel_rules_swift_local_config' is not defined

Our initial attempt to fix this was to actually configure rules_swift in our global WORKSPACE, but then this creates a requirement that all Bazel users have swiftc on the path, which isn't true of many CI runners (this was from a query, FWIW):

every rule of type apple_dynamic_framework_import implicitly depends upon the target '@build_bazel_rules_swift_local_config//:toolchain', but this target could not be found because of: no such package '@build_bazel_rules_swift_local_config//': No 'swiftc' executable found in $PATH

We ended up fixing this temporarily by commenting out the references to rules_swift in the definition of apple_dynamic_framework_import, but that's not a great long-term solution:

diff --git a/apple/internal/apple_framework_import.bzl b/apple/internal/apple_framework_import.bzl
index 9a1fcf04..6ab2874d 100644
--- a/apple/internal/apple_framework_import.bzl
+++ b/apple/internal/apple_framework_import.bzl
@@ -490,7 +490,8 @@ apple_dynamic_framework_import = rule(
     fragments = ["cpp"],
     attrs = dicts.add(
         rule_attrs.common_tool_attrs(),
-        swift_common.toolchain_attrs(toolchain_attr_name = "_swift_toolchain"),
+        # DROPBOX: prevent implicit dependency on `rules_swift`
+        # swift_common.toolchain_attrs(toolchain_attr_name = "_swift_toolchain"),
         {
             "framework_imports": attr.label_list(
                 allow_empty = False,
@@ -502,7 +503,8 @@ on this target.
 """,
             ),
             "deps": attr.label_list(
-                aspects = [swift_clang_module_aspect],
+                # DROPBOX: prevent implicit dependency on `rules_swift`
+                # aspects = [swift_clang_module_aspect],
                 doc = """
 A list of targets that are dependencies of the target being built, which will be linked into that
 target.
keith commented 9 months ago

I agree that it would be nice to not have this if you "know" you don't need it, but is there another problem from having the implicit dependency on rules_swift?

maxbelanger commented 9 months ago

The main issue we found (so far) is the requirement for swiftc to be on the PATH.

keith commented 9 months ago

are you building without xcode?

maxbelanger commented 9 months ago

We're using Xcode. The problem is that we have a number of CI jobs that run on e.g. Linux (no Xcode) and run various bazel query commands (they're not actually building anything). These all fail now, complaining of a missing swiftc, which seems odd to me.

keith commented 9 months ago

ah yea, I assume https://github.com/bazelbuild/rules_swift/blob/4ae4598024b1ff5c308a40df522bfca93e2de673/swift/internal/swift_autoconfiguration.bzl#L265-L267 is the error you see. I'm not sure if it's possible for us to make that lazier. It might be possible for you to register a no-op toolchain on Linux only instead, still not ideal

maxbelanger commented 9 months ago

Would using toolchains fix this, perhaps? I noticed a TODO about that in https://github.com/bazelbuild/rules_swift/blob/4ae4598024b1ff5c308a40df522bfca93e2de673/swift/internal/swift_autoconfiguration.bzl#L425-L427

keith commented 9 months ago

Yea that's a good question, my impression is that the toolchain repository rule will still be evaluated immediately "just in case", but I'm not sure

maxbelanger commented 9 months ago

We found a better way to work around this than patching rules_apple: we added our own repository rule named build_bazel_rules_swift_local_config that defines "dummy" Swift toolchains. That seems to fix the issue, and allows analysis to proceed. This is similar to what is described in https://engineering.mercari.com/en/blog/entry/20211210-bazel-remote-execution-for-ios-builds-with-apple-silicon/ .

The proper solution to this does look like rules_swift actually implementing toolchains, as in https://github.com/bazelbuild/rules_swift/issues/3.