rules_swift_package_manager
rules_swift_package_manager copied to clipboard
Apps who depends on `swift-composable-architecture` build failed if `minimum_os_version >= 17.0`
I'm not sure whether it's a rules_swift_package_manager issue or rules_swift issue.
swift-composable-architecture has a dependency swift-perception.
swift-perception is a backport library for Observation. Since Observation requires iOS 17+, some API in swift-perception are marked obsoleted on iOS 17. eg. Bindable
@available(iOS, introduced: 13, obsoleted: 17)
@available(macOS, introduced: 10.15, obsoleted: 14)
@available(tvOS, introduced: 13, obsoleted: 17)
@available(watchOS, introduced: 6, obsoleted: 10)
@available(visionOS, unavailable)
@dynamicMemberLookup
@propertyWrapper
public struct Bindable<Value>
the TCA Sample build succeeds when minimum_os_version = "16.0".
However, if the line is changed to minimum_os_version = "17.0", errors like the following will occur.
error: emit-module command failed with exit code 1 (use -v to see invocation)
external/rules_swift_package_manager~override~swift_deps~swiftpkg_swift_perception/Sources/Perception/Bindable.swift:21:32: error: 'Bindable' is unavailable in iOS
public var projectedValue: Bindable<Value> {
^~~~~~~~
external/rules_swift_package_manager~override~swift_deps~swiftpkg_swift_perception/Sources/Perception/Bindable.swift:15:17: note: 'Bindable' was obsoleted in iOS 17
public struct Bindable<Value> {
^
external/rules_swift_package_manager~override~swift_deps~swiftpkg_swift_perception/Sources/Perception/Bindable.swift:36:63: warning: 'Perceptible' was deprecated in iOS 17: renamed to 'Observable'
public init(wrappedValue: Value) where Value: AnyObject & Perceptible {
^
external/rules_swift_package_manager~override~swift_deps~swiftpkg_swift_perception/Sources/Perception/Bindable.swift:36:63: note: use 'Observable' instead
public init(wrappedValue: Value) where Value: AnyObject & Perceptible {
^~~~~~~~~~~
Observable
While the minimum deployments versions of some TCA examples are also >= 17.0, and they can build successfully in Xcode & Swift Packages.
I have dug into this. However, I noticed that swift-perception has a macro target. We have not added support for that yet. I wonder if the macros make it work in Xcode. 🤔
But the generated BUILD file for swift-perception seems to be correct
swift_compiler_plugin(
name = "Sources_PerceptionMacros",
defines = ["SWIFT_PACKAGE"],
deps = [
"@swiftpkg_swift_syntax//:Sources_SwiftSyntaxMacros",
"@swiftpkg_swift_syntax//:Sources_SwiftCompilerPlugin",
],
module_name = "PerceptionMacros",
srcs = [
"Sources/PerceptionMacros/Availability.swift",
"Sources/PerceptionMacros/Extensions.swift",
"Sources/PerceptionMacros/PerceptibleMacro.swift",
"Sources/PerceptionMacros/Plugins.swift",
],
visibility = ["//visibility:public"],
)
TBH, I forgot that @jpsim added support for swift_compiler_plugin.
@jpsim @luispadron Do either of you have any thoughts or suggestions on this issue?
This seems correct to me? If the API is marked unavailable for iOS 17 and you use a min of 17, should we not expect the error?
Do those examples you mentioned using iOS 17 import Observation at all?
@luispadron They do not use Observation directly, but are using Store / StoreOf or ObservableState. TCA supports both swift-perception and Observation in its unified API.
Actually, if you create a new iOS 17 Xcode project, add the Swift file in TCA Example to the sources, and add swift-composable-architecture dependency using Swift Package, it can build successfully.
hmm yah I tested it out and it seems to work in Xcode with minimum iOS set to 17.2. I'm not sure here since I do not know much about TCA.
I'd double check the Swift compiler plugins are being used correctly (if they are whats making this possible).
I think next steps are to create a minimal repro of this, thinking a single swift_library with @available that we can file an issue to rules_swift for. I do not think we're doing anything wrong here in rules_swift_package_manager
I have found the cause.
Xcode and Swift Package Manager are able to build it because the Package.swift in swift-perception specifies the target platforms as follows:
platforms: [
.iOS(.v13),
.macOS(.v10_15),
.tvOS(.v13),
.watchOS(.v6),
],
It can be built for iOS 13, as Package.swift described, but it cannot be built for iOS 17.
However, we cannot determine the platforms in swift_library, the target platform depends on the upper-layer apps/frameworks/tests.
So, what do we think are the right next steps for this issue?
In my opinion, users would hope that as long as a dependency can run successfully in SPM, it can be converted to Bazel through this project. Therefore, I believe we need to find a way to support multiple platforms and specify the target platform version in Package.swift with one library.
swift_library cannot specify the target platform and version, so I think it is necessary to use XCFramework.
In rules_apple, there are rules support XCFramework, but seems not matching our needs.
apple_static_xcframework: It seems like what we need, but it doesn't support transitive swift_library dependencies.
apple_xcframework: It requires bundle_id, infoplists, etc.
Hi all. We're running into this issue as well.
More broadly, this seems like a very fundamental difference between the build settings Xcode uses for transitive SPM packages and the settings rules_swift/rules_swift_package_manage uses.
If you create a vanilla Xcode project whose "Minimum Deployment" is "iOS 17.0" and add TCA as a package dependency, then when you build for simulator, for example, the project's direct source files will be compiled with:
-target arm64-apple-ios17.0-simulator
-target-sdk-version 17.4
whereas the TCA sources will be compiled with:
-target arm64-apple-ios13.0-simulator
-target-sdk-version 17.4
But when you build that same project using rules_apple's ios_application with minimum_os_version = "17.0" then all sources, both in the project and all the SPM packages, will be compiled with:
-target arm64-apple-ios17.0-simulator
-target-sdk-version 17.4
And even if you use minimum_os_version = "16.0", you still get tons of compiler warnings about using deprecated APIs in the TCA packages when you build with bazel, but those warnings don't show up when building with Xcode because it compiles those with different -target versions—i.e. the ones specified in the platforms field of the package's Package.swift.
This seems like a fundamental and potentially important difference between rules_swift_package_manager and vanilla Xcode.
Could the copts field of swift_library be used to override the -target platform based on what's specified in the Package.swift? I guess you'd have to use a select to pick the right platform (i.e. simulator vs device)?
Or does this require a more fundamental change to swift_library itself? How doable would something like that be?
I asked about this on the #apple channel of the Bazel Slack workspace, and @brentleyjones had an excellent suggestion of using transitions to apply the correct minimum OS version to the package's swift_librarys. I'll try messing around with that and will report back if we have time to put up a PR.
Edit: he later commented that copts might actually be the way to go since transitions might introduce other bugs as they propagate to transitive dependencies.
Here's my progress update.
Based on the Swift Package Manager SupportedPlatform spec, we need to implement the following behavior:
- If a platform version is specified, then we need to use that version, overriding any
minimum_os_versionspecified by the toolchain from the top-levelios_application. - If no platform version is specified, then we need to use the oldest deployment target version that the installed SDK supports for that platform. Except for macOS, whose minimum deployment target version starts from 10.10.
Because of that second requirement to use the SDK-dependent oldest deployment target version, I think the solution to this needs to involve modifications to swift_library and friends. We can't just encode it into the generated repository for the package because the SDK that will be used could change depending on the Xcode version that's selected at runtime. In fact, the apple_support ruleset uses a placeholder value for the SDK directory and only replaces it with the runtime version of SDKROOT as part of a wrapper script. So ultimately, I'm not even sure exactly how to query this information and make it available during the analysis phase, e.g. as part of the toolchain. Perhaps it can be queried and saved as part of the Xcode configuration/discovery code alongside the default_ios_sdk_version, for example? (see [0] and [1])
Here's an example of how you'd query this from the command-line:
jq '.SupportedTargets.iphoneos.MinimumDeploymentTarget' \
$(xcrun --sdk iphoneos --show-sdk-path)/SDKSettings.json
In any case, let's assume that we'll eventually be able to solve this problem. For now (and for my team's purposes) we can simply hardcode a reasonable oldest support version.
Given this, I've put together a couple patches, one for rules_swift_package_manager and one for rules_swift to provide this functionality.
First, I propose adding a minimum_os_version attribute to swift_library and related rules. If we consider the TCA Swift package with the following platform versions defined in its Package.swift:
name: "swift-composable-architecture",
platforms: [
.iOS(.v13),
.macOS(.v10_15),
.tvOS(.v13),
.watchOS(.v6),
],
then ruels_swift_package_manager could generate the following swift_library:
swift_library(
name = "ComposableArchitecture.rspm",
minimum_os_version = select({
"@rules_swift_package_manager//config_settings/spm/platform:ios": "13.0",
"@rules_swift_package_manager//config_settings/spm/platform:macos": "10.15",
"@rules_swift_package_manager//config_settings/spm/platform:tvos": "13.0",
"@rules_swift_package_manager//config_settings/spm/platform:watchos": "6.0",
"//conditions:default": "oldest",
}),
...
)
Here are the two patches:
You'll see that in rules_swift I have hardcoded the oldest supported versions for macOS and iOS with a TODO that they should be obtained from the MinimumDeploymentTarget field of the platform SDK's SDKSettings file.
@brentleyjones said that he was going to look into this further when he's back from OOO next week.
In the meantime, I hope these patches help unblock anyone stuck on this issue!
Using them I was able to use minimum_os_version = "17.0" in our ios_application that uses TCA. It also nearly eliminated all the compiler warnings in the SPM packages. For us, the only remaining warnings are in TCA and will be fixed when https://github.com/pointfreeco/swift-composable-architecture/pull/2909 is released.
I'm planning on submitting https://github.com/bazelbuild/bazel/compare/bj/fix-capitalization-of-apple-platforms-xcode-sdk-and-os...brentleyjones:bazel:bj/apple_sdk_minimum_os to Bazel. This would allow us to use ctx.attr._xcode_config[apple_common.XcodeProperties] in rules_swift to determine the minimum supported OS for each platform.
@kersson Can you create a local Bazel build with that change, adjust your rules_swift patch, and verify that it works for you?
@brentleyjones here's an updated rules_swift patch that swaps out the hardcoded oldest supported version with a toolchain configurator that uses your new *_sdk_minimum_os attributes:
Everything still works wonderfully in our TCA ios_application using minimum_os_version = "17.0"!
I can put up a PR for each patch in rules_swift_package_manager and rules_swift, respectively. Let me know when you have a Bazel PR up so I can reference it. I'll also work on adding unit tests for all this.
I'll be on PTO for the next week, so I may not get to this until I get back.
@kersson: https://github.com/bazelbuild/bazel/pull/21991
@kersson see https://github.com/bazelbuild/bazel/pull/21991#issuecomment-2057445522.
It will be a bit before we have a more holistic solution. Once we can make the required changes I'll take your patches here and implement something that uses them (and add you as co-author if you don't mind).
@brentleyjones were we able to get an estimate of when they think those changes will be made/if they're being prioritized? I get wanting to do it the right way first if possible but this seems like a blocker for wider SPM support
The last I heard was about 5 more weeks.
Does anyone know what the next steps are to get this done?
- Depends on Bazel 8
- Need to do this and then merge that PR
- Then we need to create something like what is described in https://github.com/bazelbuild/bazel/pull/21991#issuecomment-2057445522
Does anyone have suggestions for reasonable short-term workarounds? I'm looking to set minimum_os_version = 17.0 for my project in the next week so I need a more short-term solution that what's outlined above.
Patching either rulesets or TCA is fine if it the patch scope can be relatively small.
Here are my updated patches that should apply to the latest rules_swift (2.1.1) and rules_swift_package_manager (0.39.0). We've been using them successfully. It's a pain to keep maintaining them, but it sounds like upstreaming them depends on a lot of stuff that won't be ready for a while 😞
rules_swift-set-platform-version.patch rules_swift_package_manager-set-platform-version.patch
Thanks @kersson, that's really helpful. I also needed this small change:
diff --git a/config_settings/spm/platform/platforms.bzl b/config_settings/spm/platform/platforms.bzl
index 3766536..6fc29df 100644
--- a/config_settings/spm/platform/platforms.bzl
+++ b/config_settings/spm/platform/platforms.bzl
@@ -67,6 +67,8 @@ def _label(name):
fail("Unsupported swift package manager platform: maccatalyst.")
if name == "driverkit":
name = "macos"
+ if name == "visionOS":
+ name = "visionos"
return "@rules_swift_package_manager//config_settings/spm/platform:{}".format(name)