DeclarativeHub / ReactiveKit

A Swift Reactive Programming Kit
MIT License
1.24k stars 114 forks source link

Dynamic linking for SPM #266

Open npvisual opened 4 years ago

npvisual commented 4 years ago

Added .dynamic in the products definition to make sure the SPM bundle is dynamically linked.

npvisual commented 4 years ago

See #265 for more information about the rational behind this PR.

srdanrasic commented 4 years ago

How about we keep ReactiveKit static and add ReactiveKitDynamic product as an alternative option? I would like to keep the default one static since I think that's the most common use case.

npvisual commented 4 years ago

Sure thing. See the new commit. We could explicitly declare ReactiveKit as .static if you prefer.

npvisual commented 4 years ago

All 3 options could be offered :

    products: [
        .library(name: "ReactiveKit", targets: ["ReactiveKit"])
        .library(name: "ReactiveKitStatic", type: .static, targets: ["ReactiveKit"])
        .library(name: "ReactiveKitDynamic", type: .dynamic, targets: ["ReactiveKit"])
srdanrasic commented 4 years ago

Do you think it would make sense to offer all three options? We could do that.

npvisual commented 4 years ago

The only reason I can think of is if we'd want more control down the road when Xcode can "magically" decide how to link those libraries inside frameworks and it doesn't really work as intended or there's no option to "force" one behavior or the other.

srdanrasic commented 4 years ago

Forward thinking, sounds good to me :) Let's do all three options then. I'll release a new versions when you have the updated PR.

npvisual commented 4 years ago

FYI. Still working through the kinks... Adding 2 libraries seem to be creating some issues. Trying to fix that.

npvisual commented 4 years ago

Ok, after several tests, it seems that the main issue would be wrt Bond's dependencies.

So for example, if we implement the following strategy for Bond :

// swift-tools-version:5.0
import PackageDescription

let package = Package(
    name: "Bond",
    platforms: [
        .macOS(.v10_11), .iOS(.v9), .tvOS(.v9)
    ],
    products: [
        .library(name: "BondDynamic", type: .dynamic, targets: ["Bond"]),
        .library(name: "Bond", targets: ["Bond"])
    ],
    dependencies: [
        .package(url: "https://github.com/npvisual/ReactiveKit.git", .branch("fix/spm-dynamic")),
        .package(url: "https://github.com/tonyarnold/Differ.git", .upToNextMajor(from: "1.4.3"))
    ],
    targets: [
        .target(name: "BNDProtocolProxyBase"),
        .target(name: "Bond", dependencies: ["BNDProtocolProxyBase", "ReactiveKitDynamic", "Differ"]),
        .testTarget(name: "BondTests", dependencies: ["Bond", "ReactiveKit"])
    ]
)

Then the dependencies, for the target, point to :

There's a way to impose conditions on target dependencies, but it's limited to platforms at the moment. I am just not sure if it matters, and if we can just use RK's dynamically-linked library and let SPM decide how it's gonna work when Bond is imported into an app.

npvisual commented 4 years ago

@srdanrasic : Here's the Bond Package.swift that I used to validate the changes (compiles and doesn't crash as long as both ReactiveKitDynamic and BondDynamic are used for the dependencies).

https://github.com/npvisual/Bond/blob/fix/spm-dynamic/Package.swift