Bug: duplicate targets in project when a rules_ios apple_framework depends on a package from rules_swift_package_manager

[jschear]

Description

I'm trying to migrate a project to depend on external swift dependencies via rules_swift_package_manager. When a rules_ios apple_framework depends on a target generated by rules_swift_package_manager, it appears that the target merging logic fails to unify the apple_framework's underlying bazel targets.

Reproduction steps

I have a reproducer using the rules_ios example in this branch: https://github.com/jschear/rules_xcodeproj/tree/js/rules_swift_package_manager_rules_ios_example

• cd examples/rules_ios and bazel run //:xcodeproj-incremental
• open the generated project and find the target list
• MixedAnswer is present as both a library and static framework, and MixedAnswer_objc is present as a library.

Expected behavior

• MixedAnswer is only present as a static framework (which happens on main)

rules_xcodeproj version

44b6f046d95b84933c1149fbf7f9d81fd4e32020

Xcode version

15.4

Bazel version

7.1.1

rules_apple version

3.2.1

rules_swift version

1.16.0

Additional information

No response

[thiagohmcruz]

cc @luispadron wondering if Cash hit anything similar while working on SPM stuff?

[luispadron]

We haven't gotten that far into testing SPM + rules_ios yet.

I'm out this week but can help look more next. Just to confirm, we can repro this with rules_iOS and rules_spm (not using the fork y'all have)?

[thiagohmcruz]

We haven't gotten that far into testing SPM + rules_ios yet.

I'm out this week but can help look more next. Just to confirm, we can repro this with rules_iOS and rules_spm (not using the fork y'all have)?

Correct, we can repro using the setup Jonathan shared above using the examples in the repo and the upstream rules.

[thiagohmcruz]

cc @brentleyjones I'd appreciate any insight here 🙇

[jschear]

I can repro the same issue when a rules_ios apple_framework depends directly on a swift_library, without involving rules_swift_package_manager at all: https://github.com/MobileNativeFoundation/rules_xcodeproj/compare/main...jschear:rules_xcodeproj:js/rules_ios_swift_library_merging. I can see that there are three mergeable_infos for the target (the underlying MixedAnswer_swift swift_library, the MixedAnswer_objc objc_library, and the new swift_library), and only targets with two mergeable_infos are merged: https://github.com/MobileNativeFoundation/rules_xcodeproj/blob/44b6f046d95b84933c1149fbf7f9d81fd4e32020/xcodeproj/internal/processed_targets/mergeable_infos.bzl#L83

Still ramping up on the codebase, but that's what I've figured out so far.

[brentleyjones]

This seems like a rules_ios bug then? The new swift_library should be a dep of MixedAnswer_swift and/or MixedAnswer_objc, but not the packaging rule or whatever.

On the rules_xcodeproj side I think we can solve this by making sure that if a dep is handled upstream, that we don't deal with it again for merging. I believe it used to do this, so something about generating and handling the mergeable_infos is causing the new swift_library to be handled multiple times and not filtered out.

[jschear]

Thanks for taking a look at this.

This seems like a rules_ios bug then? The new swift_library should be a dep of MixedAnswer_swift and/or MixedAnswer_objc, but not the packaging rule or whatever.

Very possibly a rules_ios bug. As far as I can tell the swift_library is not present in the deps of the packaging rule, so it's surprising to me that it's showing up in that mergeable_infos list -- my understanding of the code thus far is that only direct dependencies should be included.

bazel query --output=build //iOSApp/Source/CoreUtilsMixed/MixedAnswer:all:

apple_framework_packaging(
  name = "MixedAnswer",
  visibility = ["//iOSApp:__subpackages__"],
  generator_name = "MixedAnswer",
  generator_function = "apple_framework",
  generator_location = "iOSApp/Source/CoreUtilsMixed/MixedAnswer/BUILD:6:26",
  testonly = False,
  framework_name = "MixedAnswer",
  deps = ["//iOSApp/Source/CoreUtilsMixed/MixedAnswer:MixedAnswer_swift", "//iOSApp/Source/CoreUtilsMixed/MixedAnswer:MixedAnswer_objc"],
  private_deps = [],
  library_linkopts = [] + [],
  vfs = [],
  transitive_deps = ["@swiftpkg_swift_log//:Logging"],
  environment_plist = ...,
  bundle_id = "rules-xcodeproj.MixedAnswer",
  platforms = {"ios": "15.0"},
  platform_type = ...,
  minimum_os_version = ...,
)

It is in transitive_deps, but I don't believe anything in rules_xcodeproj checks that attribute.

On the rules_xcodeproj side I think we can solve this by making sure that if a dep is handled upstream, that we don't deal with it again for merging. I believe it used to do this, so something about generating and handling the mergeable_infos is causing the new swift_library to be handled multiple times and not filtered out.

By "handled upstream", do you mean included in the project as a target?

I'll dig into why the swift_library target is getting included as a mergeable_info.

[brentleyjones]

By "handled upstream", do you mean included in the project as a target?

I mean that MixedAnswer_swift is already looking at @swiftpkg_swift_log//:Logging, so it should provide some information of that so that MixedAnswer can ignore it.

[jschear]

Ah, rules_xcodeproj does actually consider transitive_deps of apple_framework_packaging when merging targets: https://github.com/MobileNativeFoundation/rules_xcodeproj/pull/2750

I don't have full context on why transitive_deps exists in the first place, but I'm going to check if removing it works.

[brentleyjones]

Hmm, seems like we shouldn't with the new merging logic.

[jschear]

Shouldn't remove it, or shouldn't consider transitive_deps? 😄

[brentleyjones]

Shouldn’t consider it.

[luispadron]

I think this might be related to #3094 which i just opened (forget about this one). I added a reproducible example in that issue though