-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
ObjC AnyPromise and SwiftPackageManager (SPM) #1193
base: v6
Are you sure you want to change the base?
Conversation
As expected, the tests and/or build on Linux failed. |
My mistake: Not the Linux-Tests are failing. It's the Pretest and I got irritated by the |
I tested it with Xcode 10.3 and got the same results as seen here while building with Travis. I currently don't know how to fix this and I don't have the time to further investigate this issue. However it works fine with Xcode 12. So maybe this PR could be used for some a later or special SPM version of PromiseKit. I don't know. Any help for fixing this Issue is greatly appreciated. |
caddf7d
to
0122f95
Compare
Rebase for new CI infrastructure |
9e44117
to
e6e8f32
Compare
This is very intrusive. I cannot merge. |
Yeah, a lot of changes were necessary to make it work. I’m already using my fork in production code for two of my projects and it works without any problems so far. I can think of a few simplifications but I don’t know if they work. The main problem is, that SPM does not support mixed code targets. So I had to clearly separate Swift and ObjC into to two targets and adapt the code accordingly. Let me know what you find most intrusive. Maybe I find a way to change that. |
@@ -1,3 +1,4 @@ | |||
import Foundation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Presumably unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sure there was a compiler error but I'll check again....
@@ -2,6 +2,7 @@ | |||
#import <dispatch/dispatch.h> | |||
|
|||
@class AnyPromise; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The newline or the forward declaration?
// silence a compile error which would avoid building | ||
// the Swift-Module on first builds. | ||
|
||
#if __has_include("PromiseKit-Swift.h") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC having this in the header is a race condition that thus will fail sometimes, which is the main reason for all the self->d
stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Problem here was, that AnyPromise was not always defined at this point. But it must be defined here, because otherwise I could not declare a category on it. The include path is always different depending on what is currently build. Tests, Package, Xcode Project, etc. So I figured out that this seemed to work. I'm not very satisfied with it but finally it worked. Would appreciate a better solution.
self->d
isn't used anymore. The idea I was following was defining AnyPromise
in Swift and declaring a category in ObjC on that and delegate from the category to the Swift implementation.
We need to run the full CI against this. I'll make that possible with a comment or something. |
…ive-C even when used with Swift Packe Manager.
…and another for Objective-C Targets must be Swift or Objective-C only. Mixing languages in one target is not supported by SPM. Both targets get integrated into the PromiseKit library product
Set public header path so that Objective-C projects can include and find "PromiseKit.h" In Objective-C projects @import PromiseKitObjC; can be used as well.
…mise is sufficient at this point.
…ion to AnyPromise in the file CustomStringConvertible.swift
Fixed formatting
…romiseKitObjC target. This makes AnyPromise available when PromiseKit is included with SPM for Swift versions >= 5.3
I guess we would merge for v6 since v7 is now mainline and we don‘t intend to maintain v6 beyond fixes. Ideally would be a little less intense though… |
Hi, is there any update about this issue? Thanks. |
This PR is too much. I have to maintain the results and this is beyond my ability to comprehend it without significant work. So I can't merge it.
It is not possible without a patch like this. You could use the fork of OP. SwiftPM is decentralized by design. |
This PR makes
AnyPromise
available to Objective-C and Swift code when used with SPM.To achieve this, the PromiseKit package now consists of two targets. PromiseKit and PromiseKitObjC. The existence of PromiseKitObjC is almost irrelevant to the client because a simple
#import PromiseKit.h
can be used in Objective-C files to get access toAnyPromise
. If the client wants to use modules,@import PromiseKitObjC;
must be used.AnyPromise
is now defined in Swift and uses the@objc
attribute. It's made accessible to Objective-C, by adding the category@interface AnyPromise (ObjC)
to it. All ObjC functionality defined in this category, delegates to the SwiftAnyPromise
.Additional changes to other files were necessary to make everything work in the PromiseKit Xcode Project and while build as a SwiftPackage. All tests succeeded in the Xcode Project.