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

Swift Package Manager support #482

Closed
wants to merge 4 commits into from

Conversation

RafaelFernandezAlv
Copy link
Contributor

@RafaelFernandezAlv RafaelFernandezAlv commented Nov 9, 2020

Hi, I add support for swift package manager for this awesome library.

I saw the PR #464 but this contains an error with the functions of extensions (you can't use it).

This PR solve this error and change the README.md for explain how to install with Swift Package Manager.

Bassically the support consist in add a new folder "Headers" with all ".h" headers of the library. This ".h" are a symlink to the reals in other paths and they are generated with the new script create_headers.sh in the root repo. The Package.swift use this Header folder as public headers of the library. Feel free to add some exceptions in the script if you want some of this ".h" as private headers.

Also, I added a sample target in the project with FLEX installed with Swift Package Manager

@NSExceptional
Copy link
Collaborator

Do you mind if I edit this PR? I prefer merging one logical change at a time per PR, so I'd like to remove all the code changes that are not related to SPM support, even if it fixes warnings.

@RafaelFernandezAlv
Copy link
Contributor Author

Ok, no problem.

Thanks!

@JackoPlane
Copy link

@NSExceptional Any update on supporting SPM?

@NSExceptional
Copy link
Collaborator

@JackoPlane I actually forgot this PR existed; I will see to it ASAP as more and more people have been asking about it.

@kravtsovguy
Copy link

@NSExceptional I ask you about it as you wished =)

@Claes34
Copy link

Claes34 commented Apr 27, 2021

Hey guys, any update on when this could be merged ? Thanks :D

@NSExceptional
Copy link
Collaborator

Hey @RafaelFernandezAlv,

First of all, I'm sorry I took so long to review this!

I would like to make some small changes before I merge this; would you be okay with giving me write access temporarily?

Normally when you create a PR there is a checkbox to give maintainers write access (I believe just to the specific branch related to the PR, actually) but since the PR is already open, I think you just have to add me as a contributor to your fork

@NSExceptional NSExceptional self-assigned this May 1, 2021
@NSExceptional NSExceptional added awaiting response … from the author, to proceed enhancement labels May 1, 2021
@RafaelFernandezAlv
Copy link
Contributor Author

RafaelFernandezAlv commented May 3, 2021

Hi @NSExceptional. I have given you write permissions to the fork. Tell me if you need anymore

@NSExceptional
Copy link
Collaborator

@RafaelFernandezAlv I just noticed this, but why was it necessary to change the import syntax for SPM? We need to continue using <FLEX/...> syntax for everything else.

@RafaelFernandezAlv
Copy link
Contributor Author

I don't remember very well, but SPM keeps the folder hierarchy intact when resolving imports and modifying the imports was the only solution I could find.

@NSExceptional
Copy link
Collaborator

@RafaelFernandezAlv Well, we can address that later I suppose as I've just run into bigger issues

The script you wrote links ALL headers in the Headers folder, which is NOT what we want. We only want to include the public headers. I have modified the script to do this. However, because of this:

SPM keeps the folder hierarchy intact when resolving imports

FLEX itself will not build. This is really frustrating. This also goes back to my comments in the previous PR, where you must manually specify each folder in the header search paths, which is HORRIBLE 😫

I have updated the script to also output a list of .headerSearchPaths we can copy into Package.swift by hand, in the meantime.

Lastly, we need to use .unsafeFlags to silence warnings (if there is another way, I don't know what it is). This appears to make it impossible to use the package at all for some stupid reason. See: https://forums.swift.org/t/confused-by-unsafe-flags-being-disallowed-in-dependencies/27359/

Any ideas?

NSExceptional and others added 4 commits May 4, 2021 13:53
Includes script to generate headers

Fix script

Update Package.swift
Remove target
@TheCoordinator
Copy link

Hey guys, does anybody have an update on this?

@NSExceptional
Copy link
Collaborator

Not from me. Not sure where to go from here. Don't want to add official SPM support without a way to silence warnings…

@apps4everyone
Copy link

an easy way would be to support SPM support with binary target with pre compiled xcFramework.

@TheCoordinator
Copy link

Agree with @apps4everyone this will be the best way for an application like this. I know Firebase is doing this now, would it be possible to follow what they have done in here too?

@apps4everyone
Copy link

apps4everyone commented Aug 22, 2021

opened a PR: #550
Package.swift file:

// 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: "FLEX",
    platforms: [.iOS(.v13)],
    products: [
        .library(
            name: "FLEX",
            targets: ["FLEX"]
        )
    ],
    targets: [
        .binaryTarget(name: "FLEX", path: "build/FLEX.xcframework")
    ]
)

@NSExceptional
Copy link
Collaborator

I definitely do not want to pre-compile FLEX for every release, I don't have time to do that. Would it be possible to include the Package.swift file in a sub folder so anyone can do that as needed?

@alessaba
Copy link

alessaba commented Aug 22, 2021 via email

@TheCoordinator
Copy link

Yes! It can certainly be done simply on Github actions CI for free.

@apps4everyone
Copy link

I definitely do not want to pre-compile FLEX for every release, I don't have time to do that. Would it be possible to include the Package.swift file in a sub folder so anyone can do that as needed?

it is one command line with: bundle exec fastlane exportXCFramework
but I agree: this could be generated on github CI on release. But I am not an expert in this. Maybe this can work as an solution until someone else can help here?

@NSExceptional NSExceptional removed the awaiting response … from the author, to proceed label Oct 26, 2021
@glassomoss
Copy link

@apps4everyone I've tried this approach at our project, but unfortunately didn't get it to work on simulator. App just crashes when launched without Xcode. I've tried both adding it to another Swift Packet and directly to project. The error is

Dyld Error Message: Library not loaded: /Library/Frameworks/FLEX.framework/FLEX

I haven't tried it out on the device though. Xcode version is 13.1.

@apps4everyone
Copy link

apps4everyone commented Nov 10, 2021

@glassomoss the problem is not the xcFramework it's the version 4.5.0, there was a fix, but there was never a release @NSExceptional

@apps4everyone
Copy link

#547

@apps4everyone
Copy link

can you try master?

@glassomoss
Copy link

glassomoss commented Nov 10, 2021

@apps4everyone oh I see, thank you. I guess I could try right now, but I haven't really built any xcframework before. Could you give me a clue where to start?)

@apps4everyone
Copy link

@glassomoss hopefully this will help:
#551 (comment)

@glassomoss
Copy link

glassomoss commented Nov 10, 2021

@apps4everyone Thanks!

Long story short: I failed at literally every step. I wasn't able to install bundle, it just kept failing with random errors, so I ended up installing fastlane with brew. Then I ran into bielikb/xcframeworks#10 with fastlane and had to do build xcframework manually https://www.appcoda.com/xcframework/.

As I described my initial problem was app just crashing when launched without Xcode. I created an all new project and FLEX appeared to be working just fine. So yes, this solution works. Great job!

As for my first project, I've been struggling for hours and I ended up connecting FLEX to another Swift Package rather than directly to the whole project. So that might be the solution for these problems which potential users should be aware of.

My conclusion is that the crucial difference between my project and the all-new one is having multiple (static?) packages vs having only FLEX.

However, I'm not sure what exactly was the problem in connecting FLEX directly. I guess there might be better solution. And I also doubt the problem was in #547. Didn't test it though.

I forked your fork. You can find the latest master build at a3915ea

Now my goal is to exclude it from release builds somehow. I'm wondering if it's even possible lol

EDIT: I've managed to exclude FLEX by using
EXCLUDED_SOURCE_FILE_NAMES = FLEX*
OTHER_LINKER_FLAGS = -weak_framework FLEX

@apps4everyone
Copy link

@glassomoss thx for the report, sorry for the bundle problem. Yes the real problem is the MISSING release after the fix #547 @NSExceptional

@glassomoss
Copy link

None of my issues were related to that problem

@NSExceptional
Copy link
Collaborator

@RafaelFernandezAlv I ended up writing a bunch of scripts to make it easier to maintain the package manifest, and I also went with a pretty custom manifest compared to yours, so it was just less work to push my own changes instead of merging this PR. You are still the author on some of the commits though I think. Thank you for your help everyone.

Debrief: SPM doesn't allow use of unsafe flags in Xcode projects, so I just removed them. FLEX will just have to generate warnings in SPM. The "public headers" situation is still shitty, but this will do for now.

@NSExceptional
Copy link
Collaborator

Oh, also @apps4everyone @glassomoss I released 4.6.1

@apps4everyone
Copy link

@NSExceptional there is no 4.6.1 release visible only a tag but this tag branch has no Package.swift

@NSExceptional
Copy link
Collaborator

NSExceptional commented Nov 15, 2021

All you need is a tag for a Cocoapods release. As for SPM, just use master for now. 4.6.1 is prior to when I added SPM.

@apps4everyone
Copy link

apps4everyone commented Nov 16, 2021

thx for the information, master is NO valid option for us because if something like #547 is going to master the app will crash, we have to pin a specific version.

@NSExceptional
Copy link
Collaborator

Then use a specific commit hash

@glassomoss
Copy link

thx for the information, master is NO valid option for us because if something like #547 is going to master the app will crash, we have to pin a specific version.

@apps4everyone I've managed to fix this issue, as I mentioned in one of my previous comments

timonus pushed a commit to timonus/FLEX that referenced this pull request May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants