async-http-client
async-http-client copied to clipboard
swift-server
Fixed compilation on Mac Catalyst
Fix compilation on macCatalyst platform.
Motivation:
Support compilation on macCatalyst platform.
Modifications:
Updated availability guards in TLSConfiguration.
Result:
Fixes compilation on macCatalyst platform. Closes #304
note that SSLProtocol is depreciated
/**
* @enum SSLProtocol enumeration
* @abstract Enumerations for the set of supported TLS and DTLS protocol versions.
*
* @note This enumeration is deprecated. Use `tls_protocol_version_t` instead.
*/
public enum SSLProtocol : Int32 {
@available(iOS, introduced: 5.0, deprecated: 13.0)
@available(macCatalyst, introduced: 13.0, deprecated: 13.0)
// ...
}
Hmm, given your notes above I went back and checked: I have no trouble building the package for macCatalyst. The fact that the macCatalyst guards are redundant suggests that you shouldn't be either. Are you confident that your Xcode build is using the Xcode toolchain and not an OSS one?
I'm increasingly wondering if some of the platform checking for macCatalyst is not working right, as I'm seeing build warnings about redundant checking which don't make any sense either. Let's see if we can drill down to why this is happening.
For now we can address the warning about SSLProtocol. If you add @available(iOS, introduced: 5.0, deprecated: 13.0) to TLSVersion.sslProtocol then that will nicely resolve the build warnings for that.
Hmm, given your notes above I went back and checked: I have no trouble building the package for macCatalyst. The fact that the macCatalyst guards are redundant suggests that you shouldn't be either. Are you confident that your Xcode build is using the Xcode toolchain and not an OSS one?
How do you build it?
I have only Xcode installed, OSS Swift running typically in Docker;
I tested compilation using Xcode 11 and 12beta.
I have not generated XCode project opening the package manifest file Package.swift in Xcode.
The problem came up after importing AHC as Swift package to another iOS Xcode project (supporting macCatalyst platform).
I've been building directly by opening the Package.swift in Xcode using Xcode 12 beta.
I've been building directly by opening the Package.swift in Xcode using Xcode 12 beta.
Fails 100% time for me...
This is when opening async-http-client directly, not when using it as a dependency?
This is when opening async-http-client directly, not when using it as a dependency?
yes
Hmm, let me get hold of a machine that matches your setup and I'll try to replicate it.
Hmm, let me get hold of a machine that matches your setup and I'll try to replicate it.
Thank you, please let me know if/what to check/change on my end. Not sure how our environments could be different.
Ok, I have validated this definitely occurs. I'm going to reach out to some colleagues to see if I can understand this better. So far as I know the macCatalyst availability is strictly the same as the iOS availability, so I think this is just a bug in Xcode.
Ok, after some chatting with my colleagues this appears to be due to the awkward combination of the following: @available(macCatalyst 13) on the replacement declaration, with @unavailable on the fallback. Strictly macCatalyst version guards are redundant with iOS version guards (the macCatalyst version is assumed from the iOS version). However, the difference is that there is no macCatalyst version before 13. This has some weird effects: we see warnings about redundant version guards in this code, and we get that weird issue where we still have to sort out an else block that has no real fallback.
I propose we change the block to this:
if #available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) {
sec_protocol_options_set_min_tls_protocol_version(options.securityProtocolOptions, self.minimumTLSVersion.nwTLSProtocolVersion)
} else {
#if !targetEnvironment(macCatalyst)
sec_protocol_options_set_tls_min_version(options.securityProtocolOptions, self.minimumTLSVersion.sslProtocol)
#else
preconditionFailure("macCatalyst 13 is the first version of macCatalyst")
#endif
}
This code is pretty hideous, but it avoids the concern I have about that block scaling up to being too big. We can do the same in the other block. Want to update to that?
@Lukasa thank you for following up on that,
updated PR as suggested, let me know if you have more comments feel free to close this PR and submit a patch on your own if its faster that way;
BTW when compiling for Mac Catalyst there is still warning:
Unnecessary check for 'iOS'; enclosing scope ensures guard will always be true
Not sure if it makes sense to try to fix it (for example by putting the availability inside of targetEnvironment condition), IMO it would make the logic even less clear
BTW when compiling for Mac Catalyst there is still warning:
Unnecessary check for 'iOS'; enclosing scope ensures guard will always be true
Not sure if it makes sense to try to fix it (for example by putting the availability inside of targetEnvironment condition), IMO it would make the logic even less clear
Yeah, I'm aware of it: I've reported it as a bug upstream. For now I think we have to tolerate it.
There are probably no good routes out of this. Linux doesn't believe macCatalyst exists, so any reference to it will warn, but we have to refer to it in order to get a safe macCatalyst build because it has an explicit unavailable marker for the nonexistent macCatalyst 12-and-earlier version.
There are probably no good routes out of this. Linux doesn't believe macCatalyst exists, so any reference to it will warn, but we have to refer to it in order to get a safe macCatalyst build because it has an explicit unavailable marker for the nonexistent macCatalyst 12-and-earlier version.
how to define macCatalyst without using its name lol
one way to solve this would be to drop support for iOS12. is it feasible once iOS14 is officially released (which is days)?
that way AHC would support 2 stable iOS versions and Mac Catalyst
Ok, after some digging I've come up with a proposal.
If we wrap the #if targetEnvironment block in #if swift(>=5.3) this should not emit warnings: Swift 5.3 understands that macCatalyst is a real environment. So I think what we're going to do is say that macCatalyst is only supported on Swift 5.3, and wrap this in #if swift(>=5.3).
Does that sound acceptable to you @pokryfka?
@Lukasa
Thank you for following on that. Amazing weather here and trying to stay away from computer, I am sorry for late response.
So I think what we're going to do is say that macCatalyst is only supported on Swift 5.3,
To be honest supporting only iOS14/Sift5.3 is not ideal but an improvement nevertheless.
If we wrap the #if targetEnvironment block in #if swift(>=5.3) this should not emit warning
I am not sure I understood it correctly, just tried (ugly!):
if #available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) {
sec_protocol_options_set_min_tls_protocol_version(options.securityProtocolOptions, self.minimumTLSVersion.nwTLSProtocolVersion)
} else {
#if swift(>=5.3)
#if !targetEnvironment(macCatalyst)
sec_protocol_options_set_tls_min_version(options.securityProtocolOptions, self.minimumTLSVersion.sslProtocol)
#else
preconditionFailure("macCatalyst 13 is the first version of macCatalyst")
#endif
#endif
}
but it still fails on Linux with Swift 5.2:
docker-compose -f docker/docker-compose.yaml -f docker/docker-compose.1804.52.yaml -p async-http-client-swift52-prb run --rm test
/code/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift:72:44: error: unknown target environment for build configuration 'targetEnvironment'
#if !targetEnvironment(macCatalyst)
^
/code/Sources/AsyncHTTPClient/NIOTransportServices/TLSConfiguration.swift:72:44: note: did you mean 'simulator'?
#if !targetEnvironment(macCatalyst)
^~~~~~~~~~~
simulator