Adding Swift Package Manager Support

[jeffctown]

Updating gitignore. Adding Package.swift Adding Swift tests, getting all builds working. Downgrading swift package version to be compatible with Xcode 9.1. Disabling xcode 9.1 tests and redirect tests in other xcode versions. Changing iPhone types depending on Xcode version. Passing conditional compilation flags to swift test to disable redirect tests in CI. Bumping podspec version. Cleaning up podspec. Updating example projects. Adding SPM Example. Moving Example Stubs to shared folder. Adding building example apps to CI. Adding Carthage builds to CI. Updating rake file for SPM tests. Passing Swift Versions to carthage. Updating SPM example to use URLSession instead of URLConnection. Updating documentation.

Checklist

• [x] I've checked that all new and existing tests pass
• [x] I've updated the documentation if necessary
• [x] I've added an entry in the CHANGELOG to credit myself

Description

This PR is for adding Swift Package Manager support.

Motivation and Context

Apple integrated Swift Package Manager into Xcode 11 this year and these are the changes to support it.

File Structure:

The apple docs tell us to have the Package.swift at the root of the repository.

To turn your existing component into a Swift package, you don’t need to create a new Swift package from scratch. Instead, keep your existing project and add a README.md and a Package.swift file inside the root directory of your library project to turn your library into a Swift package.

While SPM does not require a specific structure, OHHTTPStubs's SPM support will be easier to maintain if we conform to the standard style of foldering in SPM: Sources and Tests at the root of the repo, and then folders matching the names of the targets inside of those. This required moving most of the files up a level by removing the OHHTTPStubs folder that was previously at the root of the project and also renaming UnitTests to become Tests.

The pod specs were updated for the new file structure. To validate them, I ran a pod install in the ObjC and Swift Example projects, I verified that the same files exist, their visibility did not change, and that the example projects still work as expected.

The OHHTTPStub project was also updated for the new file structure. To validate it, I ran unit tests on several Xcode versions (9.1, 10.1, 10.2, 11).

Package Naming:

I went through several revisions on the right design for the package targets.

Original Design

I originally went down the path of trying to recreate the subspecs that exist in our Cocoapods integration. This was a bit painful, but worked. Once I started on the SPM example project, I saw the awkwardness of having to import each of the targets to my swift example code.

Second Design

I then decided to modify the package design to be 2 targets: OHHTTPStubs and OHHTTPStubsSwift, but ran into problems with this approach also.

As soon as I created the OHHTTPStubs target, I started receiving warnings about certain headers not being included in our OHHTTPStubs.h umbrella header. SPM started seeing this existing file as our umbrella header and wasn't generating one on its own anymore. I was tempted to rename our OHHTTPStubs.h file to be something else to get SPM to create an umbrella header again, but wanted to retain backwards compatibility with this PR.

Final Design

I then changed the design to be 2 targets: OHHTTPStubsCore and OHHTTPStubsSwift.

Importing these in Swift will look like this:

Example Projects:

• A Swift Package Manager project with OHHTTPStubs integration (Xcode-11 beta 5) was added to the Examples/SwiftPackageManager folder. This example project uses URLSession APIs.
• The Stubs folder and files used in the example apps is now shared to remove the previously duplicate stub files in each example.

CI Support:

I added a lot more (is this overkill?) travis tasks in this PR. These are the new tasks:

• xcodebuild test in Xcode 11 for iOS Static, iOS Dynamic, MacOS, and tvOS
• swift test in Xcode 10.1, 10.2, 11.
• carthage build commands in 9.1, 10.1, 10.2, 11 and several versions of swift: 3.2, 4.0, 4.1, 4.2, 5.0, 5.1.
• xcodebuild build to build all example projects in Xcode 11.

Other:

• Mocktail and HTTPMessage targets were not created in this PR. Mocktail does not support SPM, and hasn't actually been updated in several years. I added a comment in the README for anyone to open an issue if they see a need for these. I would be happy to create a follow-up PR for OHHTTPStubsHTTPMessage if we desire it.
• Fixes #311.

[sunshinejr]

Hey @jeffctown, this is awesome, thanks for taking care of that! I've created a WIP PR in Moya to test the integration and it's building and testing correctly! 🎉

The only problem I'm seeing currently is that you would need a conditional import that depends on package manager:

#if canImport(OHHTTPStubs)
import OHHTTPStubs
#elseif canImport(OHHTTPStubsSwift)
import OHHTTPStubsCore
import OHHTTPStubsSwift
#endif

ideally I'd like to not care about that and just do the usual import:

import OHHTTPStubs

I've read that you had problems with umbrella header - have you looked at how guys at Quick did the integration? They have a base and then Quick target that depends on that base. Maybe that would help?

[jeffctown]

@sunshinejr - That's amazing feedback! Thanks for trying out the changes so quickly. It's great to know that everything is working as expected.

I totally agree that having a cleaner import would be nice, and I am willing to give the umbrella header stuff another shot to see if I can work around it.

I'll also look at what Quick is doing. From a quick glance, i'm not sure that we could do the same as them. It looks like they're not adding any Objective-C in their iOS target, but I don't know if OHHTTPStubs could work without swizzling. With the current limitation in SPM of "no mixed language targets" we may have to have 2 imports. I'll look further into it though.

[nacho4d]

Can SWIFT_VERSION be changed to 4.2 or 5.x everywhere? I see there are still old SWIFT_VERSION = 3.0 occurrences and having this complicates installation with SPM and Carthage.

[jeffctown]

Where do you see that @nacho4d ?

[nacho4d]

@jeffctown Sorry, my mistake. I re-checked and I was trying to build the wrong version. No problem here.

[JoeMatt]

This works for me using swift test!

In Package.swift you can test now with,

    dependencies: [
        .package(url: "https://github.com/AliSoftware/OHHTTPStubs.git", Package.Dependency.Requirement.branch("feature/spm-support"))
    ],
    targets: [
        .target(
            name: "MyFramework",
            dependencies: []),
        .testTarget(
            name: "MyFrameworkTests",
            dependencies: ["MyFramework", "OHHTTPStubsSwift"]),

[AliSoftware]

Regarding the import problem, note that OHHTTPStubs was created long ago, before Swift existed but also even before modular headers were a thing.That also led to some issues and challenges in with umbrella headers (#127 + #131) and <> vs "" imports back in the day, probably similar to the ones that were mentioned in this PR

Especially, even before Swift came around, when modular headers were added in ObjC, I started having some challenges, and I think the main reason was because the name of our module (OHHTTPStubs – coming from the name of the .framework itself) and the name of the main class (@interface OHHTTPStubs) were the same.

Another side effect of that btw is that it's impossible to specify the module in a fully-qualified name for this lib (like when you use Swift.Error as a fully-qualified name containing the name of the module – Swift in front of the type to disambiguate) because the compiler will always try to interpret OHHTTPStubs.foo as a reference to a static function or variable on the OHHTTPStubs class, not as a fully-qualified reference of a global function on the module.

So users of the lib can't use OHHTTPStubs.stub() to refer to the global method provided by OHHTTPStubsSwift extension and to disambiguate with any potential other stub() method they could have in their own codebase, because the compiler will interpret that as "the stub static method on the OHHTTPStubs class instead and spit an error (at least last time I tried a while ago that's what happened)

---

All this to say that having the module and the main class have the same name leads to issues in various parts. So maybe before we expose OHHTTPStubs to the SPM world it would be the right occasion to do some renaming?

Hoping that having a different name for the main class and the package/framework itself would get rid of all those issues altogether, avoid the need for the custom .modulemap and umbrella header we currently had to write in the repo back then, and get back to more modern conventions

Sure that would break the API (which is why I've postponed any renaming until now) by renaming the class. But maybe it's time, instead of continuing dragging that legacy, and I think that would not have a big impact on current users of the library anyway since I think most of our users use the Swift API not the ObjC API, so don't even refer to the OHHTTPStubs class/singleton at call sites in their code anyway.

[mikelupo]

Olivier, I still use objective c. But do not let that stop you from bettering the lib for the sake of a wider audience. We will adapt!

Thanks for this great library!

Mike

On Aug 21, 2019, at 1:38 PM, Olivier Halligon notifications@github.com wrote:

Regarding the import problem, note that OHHTTPStubs was created long ago, before Swift existed but also even before modular headers were a thing.That also led to some issues and challenges in with umbrella headers (#127 + #131) and <> vs "" imports back in the day, probably similar to the ones that were mentioned in this PR

Especially, even before Swift came around, when modular headers were added in ObjC, I started having some challenges, and I think the main reason was because the name of our module (OHHTTPStubs – coming from the name of the .framework itself) and the name of the main class (@interface OHHTTPStubs) were the same.

Another side effect of that btw is that it's impossible to specify the module in a fully-qualified name for this lib (like when you use Swift.Error as a fully-qualified name containing the name of the module – Swift in front of the type to disambiguate) because the compiler will always try to interpret OHHTTPStubs.foo as a reference to a static function or variable on the OHHTTPStubs class, not as a fully-qualified reference of a global function on the module.

So users of the lib can't use OHHTTPStubs.stub() to refer to the global method provided by OHHTTPStubsSwift extension and to disambiguate with any potential other stub() method they could have in their own codebase, because the compiler will interpret that as "the stub static method on the OHHTTPStubs class instead and spit an error (at least last time I tried a while ago that's what happened)

All this to say that having the module and the main class have the same name leads to issues in various parts. So maybe before we expose OHHTTPStubs to the SPM world it would be the right occasion to do some renaming?

Hoping that having a different name for the main class and the package/framework itself would get rid of all those issues altogether, avoid the need for the custom .modulemap and umbrella header we currently had to write in the repo back then, and get back to more modern conventions

Sure that would break the API (which is why I've postponed any renaming until now) by renaming the class. But maybe it's time, instead of continuing dragging that legacy, and I think that would not have a big impact on current users of the library anyway since I think most of our users use the Swift API not the ObjC API, so don't even refer to the OHHTTPStubs class/singleton at call sites in their code anyway.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[AliSoftware]

Related to the issue I describe above with module name being the same as main class name: https://twitter.com/harlanhaskins/status/1168406417113378816?s=21

[jeffctown]

I've gotten a bit busy at work preparing for iOS 13, but I hope to get this worked out soon.

@AliSoftware I appreciate the input. I agree renaming the main class would make things easier to maintain, and now is probably a great time to do that.

I'm leaning towards the OHHTTPStubs class being renamed OHHTTPStub. Any objections here? I'd really like to stay away from the (somewhat common) practice of adding Manager to the end of it to become OHHTTPStubsManager.

[AliSoftware]

So, correct me if I'm wrong but in think the class is now only used by the ObjC interface/API and people using Swift don't usually use the class directly right?

So that means the requirement for the new class name still needs to be unique (as ObjC don't have the concept of module namespaces that would allow disambiguation like in Swift) right?

Saying this because I was thinking of HTTPStubs or even just Stubs … but that might violate that requirement of trying to have a unique name (not risking to clash with an existing symbol in the client app) for ObjC…

[txaiwieser]

updates?

[bielikb]

@AliSoftware HTTPStubs sounds reasonable to me, Stubs is too ambiguous on the other hand

[CatalinMustata]

I'm having a very strange issue with this. When I use is as a dependency to my own framework, everything is just fine. However, when I use my framework as a dependency to my application, SPM complains that my framework contains an incompatible dependency and it points to OHHTTPStubs.

I have yet to figure out what triggers it, but any pointers would help.

EDIT: Deleting the Package.resolved file from my framework seemed to fix it, so I'll write it off as a SPM bug.

EDIT 2: Figured out the issue, with help from the Swift Forums. It seems you cannot add a dependency to a project (my own framework in this case) which has a dependency targeting a branch. While completely logical, it wasn't very obvious (especially given the vague error message).

All in all, please merge this as soon as possible :D

[jakeholland]

Just looking for an update on the progress of this. Is there anything I can help out with?

[jeffctown]

This PR has gone on a bit longer than I would like, so I'd love to finish this soon.

@AliSoftware - After a GitHub search, there are actually lots of people using this class from Swift. I agree that we will need to keep the main class name unique, for Objective-C reasons.

I went ahead with pushing a few more changes:

• Renamed OHHTTPStubs class -> HTTPStubs (Objective-C & Swift).
• Removed custom module map now that the main class and module name are different.
• Updated all example projects.
• Renaming SPM Module OHTTPStubsCore -> OHHTTPStubs, which looks a lot cleaner and consistent:

Remaining Tasks:

• Need to update documentation about the breaking changes.
• Renaming other classes to remove OH prefix: any issues with this (@AliSoftware)? IMO it looks a bit strange to NOT use a prefix for the main class, but then have to use OH for all other classes. I don't think we should remove the OH from the module name itself, just all of the classes.
• Update version number to make this a major version increment since its breaking.

@jakeholland - if you're willing to grab any of those ^^, feel free.

@sunshinejr - I spent a while trying to figure out how to combine the SPM targets, so that people would only need to import one target module when using SPM. The real problem here is that SPM doesn't support mixed languages in one target, so we would have to consolidate this repo to use one language, Objective-C or Swift. The swizzling we use depends heavily on the Objective-C runtime, and we are using a lot of swift only classes in our swift files (AnyHashhable, etc). I don't see a reasonable way to convert the Objective-C to Swift, or the Swift to Objective-C, so I think we will have to live with requiring 2 imports when using SPM.

[jeffctown]

I went ahead with the dropping of OH from class names, but the diff is pretty large. I'd love any thoughts on it.

[rynaardb]

Any plans on getting this PR merged in anytime soon?

[AliSoftware]

I originally thought the class rename was good thing, but after creating it I'm not so sure anymore. I created that as a second PR, because it's a HUGE change

How would SPM work if we don't do the renaming though? Will the hack using the custom modulemap still work — contrary to our initial beliefs and original issues we had when we stated that SPM support? I think I remember that the renaming was originally decided also mainly because having module and class name being the same created issues with this SPM port, isn't that the case anymore? (If so, that's great and let's avoid the renaming then!)

[jeffctown]

Lol. You're right; my memory is failing me. After reading (my own) comments above we did run into issues with the module name.

The other PR is up to date and works with the rename.

[rynaardb]

@jeffctown Thanks and great job! 👌

[jeffctown]

Thanks @rynaardb and everyone else who provided feedback. I won't have another huge PR open for 6 months anytime soon. 🤣