apple / swift-nio

Event-driven network application framework for high performance protocol servers & clients, non-blocking.
https://swiftpackageindex.com/apple/swift-nio/documentation
Apache License 2.0
7.92k stars 641 forks source link

Replace CNIOSHA1 with SwiftCrypto #1377

Open weissi opened 4 years ago

weissi commented 4 years ago

I'd be fantastic to remove the CNIOSHA1 module and replace it with Crypto.Insecure.SHA1 (which sadly is mandated by the WebSocket spec). Unfortunately, SwiftNIO 2 supports Swift 5.0+ and SwiftCrypto requires Swift 5.1.

0xTim commented 4 years ago

@weissi @Lukasa so why does this require a major version bump?

I looked through one of the old posts on the forums about SemVer and Swift and saw no discussion about language version changes. IMO changing the language version is a minor release and not a major release for these reasons:

Alamofire are also currently considering a minor release to bump the minimum Swift version.

Completely understand if it's a team policy or similar, it's your code after all! 😄 But would be good to know the reasons.

Lukasa commented 4 years ago

The risk isn't the raise in the Swift version number, but the OS constraints added by Swift Crypto. This is shown in SR-11489, but the core problem is that to depend on Swift Crypto we need to raise the floor of our minimum deployment targets. This immediately breaks all downstreams, which have to raise their floors as well. SwiftPM does not bring the minimum OS deployment targets into the dependency resolution phase, so it will happily choose a NIO version whose minimum deployment target is not suitable for the leaf package and then fail that build.

0xTim commented 4 years ago

Ahhh got you, that makes sense. Thanks!