cgrindel / rules_swift_package_manager

Collection of utilities and Bazel rules to aid in the development and maintenance of Swift repositories using Bazel.
Apache License 2.0
72 stars 26 forks source link

Bug: Package names not being applied correctly? #1212

Closed jpsim closed 1 week ago

jpsim commented 2 weeks ago

I just tried updating to the latest versions of swift-composable-architecture and swift-navigation and am encountering this compiler error:

Sources/ComposableArchitecture/Effects/Cancellation.swift:191:50: error: ambiguous use of '_rethrowGet()'
189 |       return try await task.cancellableValue
190 |     } catch {
191 |       return try Result<T, Error>.failure(error)._rethrowGet()
    |                                                  `- error: ambiguous use of '_rethrowGet()'
192 |     }
193 |   }
    :
263 |   }
264 | 
265 |   func _rethrowGet() rethrows -> Output {
    |        `- note: found this candidate 
266 |     return try get()
267 |   }

Sources/SwiftNavigation/Internal/ErrorMechanism.swift:13:16: note: found this candidate in module 'SwiftNavigation'
11 |   }
12 | 
13 |   package func _rethrowGet() rethrows -> Output {
   |                `- note: found this candidate in module 'SwiftNavigation'
14 |     return try get()
15 |   }

This is odd because they have different package names, so a package ACL declaration from swift-navigation should not be visible to swift-composable-architecture.

jpsim commented 2 weeks ago

For now I can work around this issue by applying a small patch to swift-navigation:

--- a/Sources/SwiftNavigation/Internal/ErrorMechanism.swift
+++ b/Sources/SwiftNavigation/Internal/ErrorMechanism.swift
@@ -10,6 +10,7 @@ extension _ErrorMechanism {
     fatalError()
   }

+  @_disfavoredOverload
   package func _rethrowGet() rethrows -> Output {
     return try get()
   }
cgrindel commented 2 weeks ago

This is strange. Does this error occur when building a swift_xxx target that has a dependency on both swift-composable-architecture and swift-navigation? I presume that it does not happen when you build the targets in these external dependencies.

jpsim commented 2 weeks ago

Yes this is happening when building a swift_library target that has a dependency on swift-composable-architecture (which itself depends on swift-navigation). Here's a minimal repro:

swift_library(
    name = "tca_ios_lib",
    srcs = ["empty_file.swift"],
    deps = ["@swiftpkg_swift_composable_architecture//:ComposableArchitecture"],
)

ios_build_test(
    name = "tca_ios_build_test",
    minimum_os_version = "16.0",
    targets = ["tca_ios_lib"],
)

Then run bazel build tca_ios_build_test.

jpsim commented 2 weeks ago

Easiest way to repro might be in the rules_swift_package_manager repo itself:

$ git clone https://github.com/cgrindel/rules_swift_package_manager.git
$ cd rules_swift_package_manager/examples/tca_example
$ swift package update
$ rm -rf .build
$ ./do_test
...
ERROR: /private/var/tmp/_bazel_jp/dbda88051fb6e0fa343b8f5d77008601/external/rules_swift_package_manager~~swift_deps~swiftpkg_swift_composable_architecture/BUILD.bazel:77:14: Compiling Swift module @@rules_swift_package_manager~~swift_deps~swiftpkg_swift_composable_architecture//:ComposableArchitecture.rspm failed: (Exit 1): worker failed: error executing SwiftCompile command (from target @@rules_swift_package_manager~~swift_deps~swiftpkg_swift_composable_architecture//:ComposableArchitecture.rspm) 
  (cd /private/var/tmp/_bazel_jp/dbda88051fb6e0fa343b8f5d77008601/execroot/_main && \
  exec env - \
    APPLE_SDK_PLATFORM=iPhoneSimulator \
    APPLE_SDK_VERSION_OVERRIDE=17.5 \
    PATH=/bin:/usr/bin:/usr/local/bin \
    XCODE_VERSION_OVERRIDE=15.4.0.15F31d \
  bazel-out/darwin_arm64-opt-exec-ST-d57f47055a04/bin/external/rules_swift~/tools/worker/worker swiftc @bazel-out/ios_sim_arm64-fastbuild-ios-sim_arm64-min16.0-applebin_ios-ST-9c0a51e7fdb8/bin/external/rules_swift_package_manager~~swift_deps~swiftpkg_swift_composable_architecture/ComposableArchitecture.swiftmodule-0.params)
# Configuration: 5a7d7fb3d855741cece7c346c94ac1304d3b71b290d9348eb4f332a71b46c654
# Execution platform: @@platforms//host:host
external/rules_swift_package_manager~~swift_deps~swiftpkg_swift_composable_architecture/Sources/ComposableArchitecture/Effects/Cancellation.swift:220:50: error: ambiguous use of '_rethrowGet()'
      return try Result<T, Error>.failure(error)._rethrowGet()
                                                 ^
/private/var/tmp/_bazel_jp/dbda88051fb6e0fa343b8f5d77008601/execroot/_main/external/rules_swift_package_manager~~swift_deps~swiftpkg_swift_navigation/Sources/SwiftNavigation/Internal/ErrorMechanism.swift:13:16: note: found this candidate
  package func _rethrowGet() rethrows -> Output {
               ^
external/rules_swift_package_manager~~swift_deps~swiftpkg_swift_composable_architecture/Sources/ComposableArchitecture/Effects/Cancellation.swift:265:8: note: found this candidate
  func _rethrowGet() rethrows -> Output {
       ^
error: fatalError
jpsim commented 2 weeks ago

I'm interested in helping fix this, but would need some pointers for how find the (implicit?) generated BUILD files, how I can extract the package name as defined in the Package.swift file and where I could set it in the generated swift_library rule.

cgrindel commented 1 week ago

@brentleyjones @luispadron Do you have any thoughts on how to address incompatible extensions in two different Swift packages?

@jpsim I presume that this would fail in the same way if you are building with SPM or Xcode. 🤔

luispadron commented 1 week ago

Is the solution here to just use package_name like SPM does and ensure the names are different so package ACL works as expected.

jpsim commented 1 week ago

Is the solution here to just use package_name like SPM does and ensure the names are different so package ACL works as expected.

Yes I believe this is the solution, I would like a pointer for where I could set this within rspm :)

luispadron commented 1 week ago

Looks like we set it here currently to the repository name: https://github.com/cgrindel/rules_swift_package_manager/blob/main/swiftpkg/internal/swiftpkg_build_files.bzl#L62-L64

jpsim commented 1 week ago

Thanks @luispadron, indeed that was the culprit. This fixes it: https://github.com/cgrindel/rules_swift_package_manager/pull/1215