Skip to content
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

Open
wants to merge 17 commits into
base: v6
Choose a base branch
from
Open

Conversation

DG0BAB
Copy link

@DG0BAB DG0BAB commented Mar 17, 2021

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 to AnyPromise. 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 Swift AnyPromise.

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.

@DG0BAB
Copy link
Author

DG0BAB commented Mar 17, 2021

As expected, the tests and/or build on Linux failed.

@DG0BAB
Copy link
Author

DG0BAB commented Mar 19, 2021

My mistake: Not the Linux-Tests are failing. It's the Pretest and I got irritated by the --generate-linuxmain option....

@DG0BAB
Copy link
Author

DG0BAB commented Mar 29, 2021

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.

@mxcl mxcl force-pushed the master branch 8 times, most recently from caddf7d to 0122f95 Compare May 24, 2021 16:06
@mxcl
Copy link
Owner

mxcl commented May 24, 2021

Rebase for new CI infrastructure

@mxcl
Copy link
Owner

mxcl commented May 26, 2021

This is very intrusive. I cannot merge.

@DG0BAB
Copy link
Author

DG0BAB commented May 26, 2021

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
Copy link
Owner

@mxcl mxcl May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably unnecessary.

Copy link
Author

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;

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary.

Copy link
Author

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")
Copy link
Owner

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.

Copy link
Author

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.

@mxcl
Copy link
Owner

mxcl commented May 26, 2021

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.
…romiseKitObjC target.

This makes AnyPromise available when PromiseKit is included with SPM for Swift versions >= 5.3
@mxcl
Copy link
Owner

mxcl commented Jul 13, 2021

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…

@mxcl mxcl changed the base branch from master to v6 July 15, 2021 13:51
@mrcararia
Copy link

Hi,

is there any update about this issue?
How is possibile to use AnyPromise in an ObjC class using the SPM version?

Thanks.

@mxcl
Copy link
Owner

mxcl commented Sep 22, 2022

is there any update about this issue?

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.

How is possibile to use AnyPromise in an ObjC class using the SPM version?

It is not possible without a patch like this. You could use the fork of OP. SwiftPM is decentralized by design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants