Rxswift: Build fails on Linux with Swift 5.0

[keith]

On Linux with Swift 5.0 release and RxSwift at cce95dd704bc08cd3d69c087a05a6fc3118e2722, doing a swift build fails:

/home/keith/dev/RxSwift/Sources/RxSwift/Platform.Linux.swift:26:27: error: cannot assign to property: 'threadDictionary' setter is inaccessible
            currentThread.threadDictionary = threadDictionary
            ~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~

[keith]

Here's a possible fix for this https://github.com/ReactiveX/RxSwift/pull/1912

[kzaher]

Can you also fix our CI to point to 5.0 please? Our CI should have caught this. I hve no idea why my local one didn’t catch this. I was using swift:latest in docker.

[dduan]

Okay, I've uncovered an unfortunate roadblock to Swift 5 on Linux. The patch to open-source Foundation has another backward-incompatible side-effect in addition to the compilation problem: Thread.threadDictionary now rejects values it can't convert to NSObject. Why does that matter? Well, one of the values that ends up in there is of the type RxMutableBox<Queue<ScheduledItemType>> and it is very not convertible to NSObject. So code that leads down this path just crashes on Linux + Swift 5 now with this error:

Could not cast value of type 'RxSwift.RxMutableBox<RxSwift.Queue<RxSwift.ScheduledItemType>>' (0x55f49961f930) to 'Foundation.NSObject' (0x7f8bcf936c48).

[freak4pc]

Would it be possible making RxMutableClass subclass from NSObject on Linux specifically ?

[freak4pc]

@keith @dduan

I'm not seeing any build issue on Linux aside for using RunLoopMode instead of RunLoop.Mode (PR for this here: #1915)

I've build using Swift version 5.0 (swift-5.0-RELEASE) on Ubuntu 18.04 on Docker.

@dduan I guess what you're suggesting happens is a runtime issue? Do you have some reproducible code to try with ?

[dduan]

@freak4pc Sorry if wasn't clear. It is a runtime issue. You can reproduce it on the develop branch in a Swift 5 docker with the following code

Observable.from([0]).subscribe()

The underlying behavior of NSMutableDictionary can be reproduced in Swift 5 docker thusly (with SwiftPM + XCTest):

import Foundation
import XCTest

final class RxMutableBox<T> {
    var value : T
    init (_ value: T) {
        self.value = value
    }
}

final class BoxTests: XCTestCase {
    func testBoxing() {
        let value = RxMutableBox(1)
        let dict = NSMutableDictionary()
        dict["a"] = value
    }
}

[dduan]

It's reported here because the cause is the same as the original compile-time issue Keith encountered.

[kzaher]

Maybe we can make RxMutableBox inherit from NSObject on Linux?

I usually don't run Linux VM, I just use Docker on OSX. The weird thing is that previously swift:latest pointed to latest released version of Swift. This doesn't happen now.

Swiftenv also doesn't have Swift 5.0 enabled. I don't understand why do they have issues and how can we add Swift 5.0 on our CI. Thoughts?

[freak4pc]

@kzaher I've added a PR that solves the issue for me on Docker. I think I'd want to add a test in Anomalies to make sure it doesn't happen again.

@dduan can you test this for your issue?

https://github.com/ReactiveX/RxSwift/pull/1917

[freak4pc]

I'm not 100% sure on Swift 5 CI, I'll think about this a bit.

[kzaher]

@freak4pc Regarding #1917. I think we can keep them inside the same file. It is probably not worth to split them in this case. We've used different files to handle different APIs on Linux and OSX, but this is more of a hack to get this working :) Probably adding a comment in the same file why we are doing this would also make sense.

Yeah, have no idea how to add Swift 5.0 to our CI now :(

[freak4pc]

About #1917, Sure. No problems.

[freak4pc]

I addressed your feedback on that PR. I think I won't add a test since it would be more helpful to fix the CI. The entire existing test suite would fail on Linux without this patch, anyways.

[kzaher]

@freak4pc I've removed swiftenv and done manual install on Linux, so now we have 5.0, 4.2 and 4.0 on Linux. One dependency less 💪.

The issue is that your fix somehow doesn't work 😢 .
https://travis-ci.org/ReactiveX/RxSwift/jobs/516547195

It seems to me that swift(>=4.2) fails on Linux? Wtf?

[freak4pc]

@freak4pc I've removed swiftenv and done manual install on Linux, so now we have 5.0, 4.2 and 4.0 on Linux. One dependency less 💪.

Awesome! 👏

It seems to me that swift(>=4.2) fails on Linux? Wtf?

Wow, that's so weird. It does seem, though, that you're using Xcode 9.3 on that test - which doesn't have Swift 5 at all. Since you're using xcodebuild I don't think the fact swift CLI is 5 necessarily matters here.

[freak4pc]

I think that for that specific test you'd want to use Xcode 10.2 as it's the only version that has the Swift 5 compiler. And if you want to build using Swift CLI - you'll have to use swift build / swift test, as far as I know, at least.

[kzaher]

@freak4pc Linux doesn't have xcodebuild. We aren't even using xcodebuild, but swift build. I'm not sure what are you suggesting.

[freak4pc]

@kzaher Sorry, the travis.yml is quite complex and I don't understand it too well :)
When I build that branch on Docker locally it builds with no issues, which makes me suspect it's some configuration issue.

[kzaher]

Ok, so ... what I think is happening is that swift build uses package version of swift, which is 4.0 to build it.

I think that if we ditch 4.0 and only support 4.2 everything should work. We need to ditch 4.0 because if doesn't support the compiler directive.

I've changed it to be:

    #if compiler(>=5.0) 
    let runLoopMode: RunLoop.Mode = .default
    #else
    let runLoopMode: RunLoopMode = .defaultRunLoopMode
    #endif

This looks promising.
https://travis-ci.org/ReactiveX/RxSwift/builds/516554264

[freak4pc]

@kzaher I think the compiler thing won't work under Xcode 10.2, though.

I agree removing support for 4.0 makes sense.

[freak4pc]

Ok so #if compiler was introduced in 4.2, it should work:

https://github.com/apple/swift-evolution/blob/74d62ec62b742848ee03005c478eccdc5f7d504d/proposals/0212-compiler-version-directive.md

[freak4pc]

@kzaher In that case should we keep these Swift 4 CI tests?

[kzaher]

@freak4pc Yes, we are keeping 4.0 support for CocoaPods and Carthage. I've only removed it for swift package manager.

We are using Xcode 10.2 for CI:
https://github.com/ReactiveX/RxSwift/blob/83fee4e3234a04d28b93cbb293d88bfdba537789/.travis.yml#L36-L38

Is there some problem that you see in the develop branch? I'm not sure I understand.

[freak4pc]

There's no problem, You just said you want to Drop Swift 4 but haven't mentioned it's only for SPM. No problems :)

[kzaher]

Looks like we need to ditch 4.0 support completely :)

https://travis-ci.org/ReactiveX/RxSwift/jobs/516555822

Swift conditional compilation works in a different way than Cs.

I don't want to bump major version for this.

[freak4pc]

@kzaher As I suspected Xcode 9.3 tests fail.
Xcode 9.3 only includes Swift 4.1, so #if compiler won't do.

I don't think you have to drop Swift 4 - I think you have to drop Xcode 9. WDYT?

The latest Xcode still support Swift 4 compatibility mode:

[kzaher]

Ok, I guess that the smartest thing we can do is to drop all legacy, clean our CI, clean podfiles and bump the major version, 5.0 that only supports 5.0 and Xcode >= 10.2.

Don't see any downside to that.

Do you want to make a PR for that?

[freak4pc]

It's a bit of a bummer because that means that your new ideas would actually not be RxSwift 5 ? Or they would just be RxSwift 5.1 ?

I'm wondering if simply trying Xcode 10 only would work, let me make a couple more attempts.

[kzaher]

It's a bit of a bummer because that means that your new ideas would actually not be RxSwift 5 ? Or they would just be RxSwift 5.1 ?

I think the for the timeline for the improvements was too ambitious anyway. There is a lot of stuff to think about. We can always include them in 6.0 or 7.0.

[freak4pc]

@kzaher Seems like dropping Xcode 9.x works. If we support Xcode 10 and above we can support Swift 4, Swift 4.2 and Swift 5:
https://travis-ci.org/ReactiveX/RxSwift/builds/516565086

[freak4pc]

@kzaher that build mostly passed - it doesn't pass only with CocoaPods / Swift 5 but I'm not entirely sure it's related. I'm not entirely sure how your validate-podspec.sh script works (or why it does so many things). If you have thoughts, that'd be great.

[freak4pc]

@kzaher I think the Pod Lint stage is wrong for some reason - when I try it locally:

pod lib lint --swift-version=5 RxSwift.podspec

I'm getting a successful build:

This also works locally, very weird:
TARGET=RxSwift SWIFT_VERSION=5 ./scripts/validate-podspec.sh

[kzaher]

@freak4pc xcodebuild crashes randomly.

[kzaher]

Oh, that is something else, probably parameter changed or something.

[freak4pc]

@keith @dduan I think most of these issues were fixed in #1920, closing this for now.

Let us know if there are any issues :)