Overlapping dependencies from proto compiler and rules_swift_package_manager

[ed-irl]

Hello, I have a project where I rely on Swift NIO directly within my library. I also have some code that uses swift GRPC.

The swift_proto_library rule contributes a bunch of dependencies (collectively called non_module_deps) that overlap with my rules_swift_package_manager dependencies. I've attached bazel query png's that show an example of where this happens in my project with the Swift Atomics dependency.

The result is a bunch of compilation errors about overlapping module names. For example:

bazel-out/ios_sim_arm64-fastbuild-ios-sim_arm64-min17.0-applebin_ios-ST-2f808fb2bd56/bin/external/rules_swift_package_manager~~swift_deps~swiftpkg_swift_nio/CNIOAtomics.rspm_modulemap/_/module.modulemap:1:8: error: redefinition of module 'CNIOAtomics'
module "CNIOAtomics" {
       ^
bazel-out/ios_sim_arm64-fastbuild-ios-sim_arm64-min17.0-applebin_ios-ST-2f808fb2bd56/bin/external/rules_swift~~non_module_deps~com_github_apple_swift_nio/CNIOAtomics_modulemap/_/module.modulemap:1:8: note: previously defined here
module "CNIOAtomics" {

rules_jvm_external has an Override generated targets feature where some external dependencies can be mapped to different labels. It would be helpful to have such an override mechanism available in rules_swift_package_manager.

[ed-irl]

Another note - in case other folks happen across this issue with the same problem.

I was able to overcome this issue by creating separate swift_proto_compiler targets in my project and passing these to swift_proto_library -

load("@rules_swift//proto:swift_proto_compiler.bzl", "swift_proto_compiler")
load(
    "@rules_swift//proto/compilers:swift_proto_compiler_macros.bzl",
    "GRPC_VARIANT_CLIENT",
    "GRPC_VARIANT_SERVER",
    "GRPC_VARIANT_TEST_CLIENT",
    "PROTO_PLUGIN_OPTIONS",
    "PROTO_PLUGIN_OPTION_ALLOWLIST",
)
load(
    ":macros.bzl",
    "make_grpc_swift_proto_compiler",
)

swift_proto_compiler(
    name = "swift_proto",
    plugin = "@rules_swift//tools/protoc_wrapper:ProtoCompilerPlugin",
    plugin_name = "swift",
    plugin_option_allowlist = PROTO_PLUGIN_OPTION_ALLOWLIST,
    plugin_options = PROTO_PLUGIN_OPTIONS,
    protoc = "//tools/protoc",
    suffixes = [".pb.swift"],
    visibility = ["//visibility:public"],
    deps = [
        "@swiftpkg_swift_protobuf//:SwiftProtobuf",
    ],
)

make_grpc_swift_proto_compiler(
    name = "swift_server_proto",
    variants = [GRPC_VARIANT_SERVER],
)

make_grpc_swift_proto_compiler(
    name = "swift_client_proto",
    variants = [GRPC_VARIANT_CLIENT],
)

load("@bazel_skylib//lib:dicts.bzl", "dicts")
load("@rules_swift//proto:swift_proto_compiler.bzl", "swift_proto_compiler")

# NOTE: The ProtoPathModuleMappings option is set internally for all plugins.
# This is used to inform the plugins which Swift module the generated code for each plugin is located in.
PROTO_PLUGIN_OPTION_ALLOWLIST = [
    "FileNaming",
    "Visibility",
]
PROTO_PLUGIN_OPTIONS = {
    "Visibility": "Public",
}
GRPC_VARIANT_SERVER = "Server"
GRPC_VARIANT_CLIENT = "Client"
GRPC_VARIANT_TEST_CLIENT = "TestClient"
GRPC_VARIANTS = [
    GRPC_VARIANT_SERVER,
    GRPC_VARIANT_CLIENT,
    GRPC_VARIANT_TEST_CLIENT,
]
GRPC_PLUGIN_OPTION_ALLOWLIST = PROTO_PLUGIN_OPTION_ALLOWLIST + [
    "KeepMethodCasing",
    "ExtraModuleImports",
    "GRPCModuleName",
    "SwiftProtobufModuleName",
] + GRPC_VARIANTS

# NOTE: As of Swift 5.6, the TestClient flavor is deprecated in grpc-swift.
# This is because they are not sendable and needed to be marked as unchecked sendable for async/await.
# We might just want to drop support for it during this migration.

def make_grpc_swift_proto_compiler(
        name,
        variants,
        plugin_options = PROTO_PLUGIN_OPTIONS):
    """Generates a GRPC swift_proto_compiler target for the given variants.

    Args:
        name: The name of the generated swift proto compiler target.
        variants: The list of variants the compiler should generate.
        plugin_options: Additional options to pass to the plugin.
    """

    # Merge the plugin options to include the variants:
    merged_plugin_options = dicts.add(
        plugin_options,
        {variant: "false" for variant in GRPC_VARIANTS},
    )
    for variant in variants:
        merged_plugin_options[variant] = "true"

    swift_proto_compiler(
        name = name,
        protoc = "//tools/protoc",
        plugin = "@swiftpkg_grpc_swift//:protoc-gen-grpc-swift",
        plugin_name = name.removesuffix("_proto"),
        plugin_option_allowlist = GRPC_PLUGIN_OPTION_ALLOWLIST,
        plugin_options = merged_plugin_options,
        suffixes = [".grpc.swift"],
        deps = [
            "@swiftpkg_swift_protobuf//:SwiftProtobuf",
            "@swiftpkg_grpc_swift//:GRPC",
        ],
        visibility = ["//visibility:public"],
    )

[cgrindel]

@AttilaTheFun Didn't you recently perform some work refactoring addressing this type of overlap from third-party dependencies?

[AttilaTheFun]

@ed-irl I created the new swift_proto_library rule with customizable swift_proto_compiler targets specifically to address this issue.

Previously, the old swift_proto_library implementation could only use the SwiftProtobuf and grpc-swift dependencies defined in the rules_swift repository. As a result, I would get duplicate symbol errors if there were differing versions of SwiftProtobuf and grpc-swift included by rules_swift and rules_swift_package_manager.

The new implementation allows you to define your own swift_proto_compiler targets using whatever plugins or dependencies you want -- they don't even need to be protoc based. I also updated this repository to add a directive to specify which compiler targets to add to the generated swift_proto_library targets.

@cgrindel I actually have an old PR that never merged demonstrating this feature: https://github.com/cgrindel/rules_swift_package_manager/pull/1019

I was not able to merge that previously, because on Ubuntu the Swift Atomics library crashes without the always link flag. I fixed this in the rules_swift repo here: https://github.com/bazelbuild/rules_swift/blob/master/third_party/com_github_apple_swift_atomics/BUILD.overlay#L17

In order to merge that PR, we need to add the alwayslink flag to the generated swift_library targets. I'm not sure if there is a good heuristic for when it is necessary. If there is a way to patch the generated build file with RSPM, I could fix it that way too.

[cgrindel]

In order to merge that PR, we need to add the alwayslink flag to the generated swift_library targets. I'm not sure if there is a good heuristic for when it is necessary. If there is a way to patch the generated build file with RSPM, I could fix it that way too.

A good heuristic does not come to mind. Perhaps we can add a bzlmod tag class that allows a client to configure attributes for Swift products.

That having been said, I am trying to understand why this case requires the flag. 🤔

[AttilaTheFun]

@cgrindel It was specifically the ManagedAtomic type in swift atomics that was failing on ubuntu.

I think the flag was not necessary on apple platforms because their linker has different default behavior.

The flag controls whether the linker should ignore object files that are unreferenced elsewhere in the program.

In Swift, you can extend types to conform to a protocol outside of their defining file / module. I believe in this case an extension was "falling off" and the program was crashing at runtime.

There was some discussion around whether this should just be the default for swift_library.

As for RSPM, I think a tag class that allows this to be configured on a per-dependency basis sounds good. If you add that, I'm happy to rebase the PR and get it working again.

https://stackoverflow.com/questions/48653517/what-does-bazel-alwayslink-true-mean

[cgrindel]

It is probably worth capturing the build options from SPM to see how it builds the packages in question. Usually issues with missing symbols have to do with the deployment of shared libraries. That does not seem relevant here. 🤔

[AttilaTheFun]

@cgrindel I did actually diff the flags when going down that rabbit hole.

IIRC the flags weren't meaningfully different (before I added the alwayslink flag), but Apple ships a modified LLVM toolchain with Xcode, so their linker has different default behavior.

On ubuntu we were using the vanilla LLVM and swiftc, so we needed the flag.