search

erikdoe / ocmock

Mock objects for Objective-C
http://ocmock.org
Apache License 2.0
2.16k stars 606 forks source link

Swift Package Manager #442

Closed paulb777 closed 3 years ago

paulb777 commented 4 years ago

Successor to https://github.com/erikdoe/ocmock/pull/402

The Public folder symbolic links to the public headers. Making a copy would be an alternative implementation that would allow other package manager implementations to be unchanged, but this seems to work and is easier to maintain.

Ideally all implementation would migrate to using a separate Public header folder implementation at some point.

Testing:

Fix #375

mRs- commented 4 years ago

any news on this? would like to use OCMock with Swift Package Manager as soon as possible 😀

paulb777 commented 4 years ago

While the PR is pending, Firebase is using it like https://github.com/firebase/firebase-ios-sdk/blob/master/Package.swift#L123

mRs- commented 4 years ago

Good point, will give it a try!

erikdoe commented 4 years ago

To be honest, after the other two PR's about Swift Package Manager got really confusing, and I didn't have time to learn about Swift Package Manager, I mentally parked this one.

Having looked at it now, I must say that I'm not on board with the approach taken. If SPM really needs all public headers on their own in a folder, then this sounds like part of the build process. It feels completely wrong (to me) to create a folder full of symlinks and check that in.

Also, are you sure about the fact that Swift Package Manager needs the headers on their own in a folder. I don't recall the other two PRs mentioning this.

paulb777 commented 4 years ago

Swift Package Manager can only specify public headers by folder and the previous PRs ended up treating non-public headers as public which caused a few failures in my testing.

From an SPM perspective, it would be more ideal for all build systems to specify the public headers from a single folder, but this change is less intrusive to the existing builds.

erikdoe commented 4 years ago

Thanks for the quick reply. As mentioned, I would like to explore whether this folder for public headers cannot be generated automatically at build time, using a script or something. It feels wrong to me to create this artefact, which is only needed for SPM, and check it in.

Also, I now see that your are specifying .unsafeFlags(["-fno-objc-arc"]). If I remember correctly from the previous discussions any package that uses these becomes "un-importable" (see https://github.com/erikdoe/ocmock/pull/379#issuecomment-614610432).

paulb777 commented 4 years ago

I don't think that SPM has any kind of script capability and it is very opinionated about a 1-1 relationship between the git contents and what gets built - so I don't have any ideas about alternatives other than a larger file structure refactor like we did in Firebase to support building with both SPM and CocoaPods.

I didn't touch the .unsafeFlags(["-fno-objc-arc"]) flag from the previous PRs but it seems to work fine for test targets. I'll investigate removing it ...

paulb777 commented 4 years ago

.unsafeFlags(["-fno-objc-arc"]) is definitely needed. swift build fails with several ARC errors without it.

SPM must have different rules about unsafeFlags for test targets.

dmaclach commented 4 years ago

Paul, you won't be able to remove the .unsafeFlags(["-fno-objc-arc"]). It is critical to OCMock.

On Wed, Aug 19, 2020 at 11:39 AM Paul Beusterien notifications@github.com wrote:

I don't think that SPM has any kind of script capability and it is very opinionated about a 1-1 relationship between the git contents and what gets built - so I don't have any ideas about alternatives other than a larger file structure refactor like we did in Firebase https://github.com/firebase/firebase-ios-sdk/blob/master/HeadersImports.md to support building with both Firebase and CocoaPods.

I didn't touch the .unsafeFlags(["-fno-objc-arc"]) flag from the previous PRs but it seems to work fine for test targets. I'll investigate removing it ...

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/erikdoe/ocmock/pull/442#issuecomment-676593855, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACOFSN5GVK6WHZUGYMTU2LSBQL5NANCNFSM4OVBSIDQ .

erikdoe commented 4 years ago

So, I started down the rabbit hole... It really seems like SwiftPM does not allow to run scripts. At the same time I am unwilling to accept a directory full of symlinks just for this.

And maybe it's just me but I can't seem to find any reasonable documentation by either Apple or on swift.org that explains how to create a Swift package for an Objective-C framework. Could anyone please provide a link to such documentation? I'm not going to piece this together from Stack Overflow answers and the source code of SwiftPM.

paulb777 commented 4 years ago

From https://forums.swift.org/t/picking-and-choosing-public-headers/39558, a custom module map for Swift Package Manager would be an alternative. Would that work?

erikdoe commented 4 years ago

@paulb777, thank you for looking into this option. Reading through the documentation it seems this could work:

In case of complicated include layouts or headers that are not compatible with modules, a custom module.modulemap can be provided in the include directory.

Now, I know nothing about modulemaps...

By the way, in the meantime I made the non-controversial changes on the "master" branch. The imports now use straight quotes and the localised plist string files are gone. Thanks for pointing these out; good changes anyway. And I would have cherry-picked the commits to give credit that way, but the commits were intertwined with changes to the package manager file.

karimhm commented 4 years ago

If any one is interested module maps are extensively documented here.

karimhm commented 4 years ago

Did any one investigated the possibility to distribute OCMock as a Swift Package Manager Binary Dependency rather than via source code?.

It is possible to create a Github Actions pipeline that packages OCMock as a Binary Dependencies.

karimhm commented 4 years ago

Another interesting thing: Uploading Artifacts on Travis CI.

paulb777 commented 4 years ago

@erikdoe Thanks for the follow-up and updates. I'll see if I can put together a module map alternative to this PR.

@karimhm Thanks for sharing the links. I'm not sure a binary distribution provides any extra value to justify its extra maintenance and usability complexity.

karimhm commented 4 years ago

@karimhm Thanks for sharing the links. I'm not sure a binary distribution provides any extra value to justify its extra maintenance and usability complexity.

With binary distribution it would be possible to use OCMock as a dependency from any target. I believe there is some other libraries that depend on this one, one example would be Cuckoo.

paulb777 commented 4 years ago

See #460 for a custom module map implementation

twitterkb commented 4 years ago

just starting to catch up on the changes here w.r.t. SPM.

would like to see some solution that will allow us to start using OCMock as an SPM package …

i don't know which of the alternatives here is preferred at this point.

looking for a tl;dr on where the discussion on this is at and if there's a plan going forward.

twitterkb commented 3 years ago

i think i prefer the other patch with the Package.swift changes to this one … but would like to see a canonical Package.swift file adopted on master here …

erikdoe commented 3 years ago

Closing this as it looks likely that the solution in #460 is the way to go.