Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReactiveSwift 6.5.0 not binary compatible with 6.4.0 #809

Closed
stevebrambilla opened this issue Nov 26, 2020 · 10 comments
Closed

ReactiveSwift 6.5.0 not binary compatible with 6.4.0 #809

stevebrambilla opened this issue Nov 26, 2020 · 10 comments

Comments

@stevebrambilla
Copy link

The 6.5.0 release included a change to OptionalProtocol (#805):

-  public protocol OptionalProtocol
+  public protocol OptionalProtocol: ExpressibleByNilLiteral

It's a nice API improvement, but unfortunately it seems to break binary compatibility with the previous release, and can cause problems like ReactiveCocoa/ReactiveCocoa#3717.

Adding protocol inheritance isn't explicitly called out as an unsafe change in the Library Evolution docs, but it is causing a bad access at runtime. Here's a small sample project that will fail to launch because of the issue: ReactiveKVOCrash.zip (frameworks compiled using Xcode 12.2).

This means that the latest releases of ReactiveSwift and ReactiveCocoa aren't compatible with each other:

github "ReactiveCocoa/ReactiveCocoa" "11.1.0"
github "ReactiveCocoa/ReactiveSwift" "6.5.0"

Following the SemVer guidelines, we should probably revert that change and release a 6.5.1 update.

@dnadoba
Copy link

dnadoba commented Nov 26, 2020

ReactiveSwift does not declare ABI stability as stated in the readme:

ABI stability release

ReactiveSwift has no plan to declare ABI and module stability at the moment. It will continue to be offered as a source only dependency for the foreseeable future.

@stevebrambilla
Copy link
Author

Hey @dnadoba, that's a fair point, maybe "binary compatible" isn't the best term to use here, it might be better to just say "breaking change".

So in this case, there was a small breaking change between minor versions (6.4 to 6.5) that I don't think the team was aware of based on the versioning, pull request, and release notes.

Right now, if your project uses Carthage to pre-build its frameworks and has dependencies on both RAC and RAS that get resolved to the latest releases of both, then you'll likely have the issue that the sample project above demonstrates when you use OptionalProtocol's init(reconstructing:) API.

Another way to solve this would be to bump ReactiveCocoa's minimum version for ReactiveSwift to 6.5, but that might just be replacing one semver problem with another.

@Marcocanc
Copy link
Member

Doesn't Carthage automatically rebuild all downstream dependencies when there's a new version of e.g. RAS?

@nkristek
Copy link
Contributor

I can confirm that if you upgrade the version of ReactiveSwift from 6.4 to 6.5, Carthage will rebuild ReactiveCocoa as well.

@stevebrambilla
Copy link
Author

The problem is a bit more subtle than that.

Assuming our app's Cartfile.resolved matches the latest versions, (RAS 6.5 / RAC 11.1), the build process with Carthage looks more like this:

Note that ReactiveCocoa 11.1's local ReactiveSwift dependency is resolved to v6.2 in its Cartfile.resolved + submodule.

  • ReactiveSwift.framework v6.5 is built
  • ReactiveCocoa's local dependency is built: ReactiveSwift.framework v6.2
  • ReactiveCocoa.framework v11.1 is built, linking against ReactiveSwift v6.2
  • The app embeds ReactiveSwift.framework v6.5 and ReactiveCocoa.framework v11.1

This should work fine, because v6.5 is expected to be backwards compatible with v6.2, but there's this subtle incompatiblity introduced from that change that results in a bad memory access.

@nkristek
Copy link
Contributor

If I understand you correctly, ReactiveSwift is built twice after being upgraded? Why would it rebuild ReactiveCocoa when it's still linking against the old framework, you wouldn't need to rebuilt it if you're not linking against the new binary?

@stevebrambilla
Copy link
Author

Those steps would be from running carthage bootstrap without anything pre-built yet. If you were just updating ReactiveSwift then you wouldn't have to re-build ReactiveCocoa, but the same crash would happen either way.

That can be reproduced using the sample project, just delete the Carthage directory and run bootstrap, then take a look at the xcodebuild output.

To clarify why it's built twice:

  • ReactiveSwift 6.5 is built once during the Building scheme "ReactiveSwift-iOS" in ReactiveSwift.xcworkspace step, this is the project at Carthage/Checkouts/ReactiveSwift where it builds the framework that the app uses.
  • Then because ReactiveSwift is also a dependency of ReactiveCocoa, v6.2 gets built during the Building scheme "ReactiveCocoa-iOS" in ReactiveCocoa.xcworkspace step, this is the project at Carthage/Checkouts/ReactiveCocoa/Carthage/Checkouts/ReactiveSwift. This is built because it's an implicit dependency of ReactiveCocoa-iOS in the workspace.

@nkristek
Copy link
Contributor

nkristek commented Nov 27, 2020

Thanks for explaining. Would this be mitigated by using a parameter like --cache-builds?

Edit: Probably not, disregard my question.

@kaybutter
Copy link

I've encountered similar problems in the past. The reason is that ReactiveCocoa includes ReactiveSwift as a git submodule instead of relying on the dependency provided by Carthage.

The advantage of that approach is that you can use ReactiveCocoa out of the box without having to install Carthage first. But if you are using Carthage, you will frequently run into issues unless you use the exact same version of ReactiveSwift that ReactiveCocoa's submodule points to. :(

@RuiAAPeres
Copy link
Member

Hello. 👋 Thanks for opening this issue. Due to inactivity, we will soft close the issue. If you feel that it should remain open, please let us know. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants