Comcast / mamba

Mamba is a Swift iOS, tvOS and macOS framework to parse, validate and write HTTP Live Streaming (HLS) data.
Apache License 2.0
182 stars 40 forks source link

SPM version support #109

Closed gpe08 closed 2 years ago

gpe08 commented 3 years ago

Description

This PR adds a different way to expose the version when exporting the framework through SPM. Unfortunately, the Info.plist cannot be added to packages, so the current way to expose the version (CFBundleShortVersionString) is not friendly to the SPM Users.

Under the Core Video Team in Sky, we are using mamba through SPM, and this is stopping us from using the framework in debug at all (and causing bigger issues in our Unit tests).

This PR just appends a hotfix to the problem, but I am open to updating it with a proper versioning solution were the maintaining team to propose one. Of course having the value hardcoded into Mamba.swift is far from ideal, but I would not want to impose any options either.

Change Notes

Pre-submission Checklist

CLAassistant commented 3 years ago

CLA assistant check
All committers have signed the CLA.

gpe08 commented 2 years ago

@jonnybach that should work fine indeed. I will look into it asap. (Sorry, I was on leave yesterday)

gpe08 commented 2 years ago

@jonnybach Just pushed some commits to fix this problem. The bundling is a bit trickier with SPM than with other processes, so I had to append some suffixes to the path on the bundle.

This has to do with bundle nesting and it's supposed to work with Bundle.module, however, that does not seem to work well in Xcode 12.

jonnybach commented 2 years ago

@gpe08 , sorry i was out on leave yesterday and just had a chance to look at this PR. Thanks for making the code changes. I want to understand more about the problem you faced when attempting to use the Bundle.module.path(forResource: "version", ofType: "txt") approach as opposed to what you have in this PR.

I attempted to recreate the problem by creating a dummy MySwiftPackage project in Xcode 12.5.1 with the following setup:

// swift-tools-version:5.3
// The swift-tools-version declares the minimum version of Swift required to build this package.

import PackageDescription

let package = Package(
    name: "MySwiftPackage",
    products: [
        // Products define the executables and libraries a package produces, and make them visible to other packages.
        .library(
            name: "MySwiftPackage",
            targets: ["MySwiftPackage"]),
    ],
    dependencies: [
        // Dependencies declare other packages that this package depends on.
        .package(name: "mamba", path: "/Users/jbachm182/Projects/mamba_pr"),
    ],
    targets: [
        // Targets are the basic building blocks of a package. A target can define a module or a test suite.
        // Targets can depend on other targets in this package, and on products in packages this package depends on.
        .target(
            name: "MySwiftPackage",
            dependencies: ["mamba"]),
        .testTarget(
            name: "MySwiftPackageTests",
            dependencies: ["MySwiftPackage"]),
    ]
)

I then referenced the mamba.FrameworkInfo.version property in my dummy test app but modified your code in the PR to:

    private static var versionFilePathUrl: String? {
        let verPath = Bundle.module.path(forResource: "version", ofType: "txt")
        return verPath
    }

And had no issues fetching the version file. Perhaps my test package is too trivial and I need to modify how it's configured?

gpe08 commented 2 years ago

@jonnybach Sorry, I've had too many things on my plate lately. When working on xcode 12.5.1, the oacjage itself would neither index the module method nor compile when I added it, so I could not get it to work. This is inside the module, not from the project using it.

I was able to get it working on the debugger, by doing po Bundle.module but in the code itself, the project would not compile for me at all. I had the same when using internal packages in our repository. I am a bit lost to the reasoning behind it.

jonnybach commented 2 years ago

No apologies necessary @gpe08! Do you have a sample project to share that demonstrates this? I'd like to look into this a bit more to get a good understanding.

gpe08 commented 2 years ago

@jonnybach Seems like those issues I had were Xcode being Xcode for me, and I can now get this working. I played around with the version.txt and things seem to work fine in our tests. Seems like it was caching an older mamba version somehow.

So, I'm happy to go with the Bundle.module version, and sorry about all the noise.