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

SPM forced dynamic linking results in distribution error #1423

Closed
3 tasks done
geraldvoit opened this issue Apr 7, 2020 · 5 comments
Closed
3 tasks done

SPM forced dynamic linking results in distribution error #1423

geraldvoit opened this issue Apr 7, 2020 · 5 comments

Comments

@geraldvoit
Copy link

Check List

Thanks for considering to open an issue. Before you submit your issue, please confirm these boxes are checked.

Issue Description

Forced dynamic linking in Kingfisher 5.13.3 Swift Package (#1420) results in an AppStore Upload Error, if your App embeds Kingfisher in multiple Targets. In my case the main iOS App and a Today Extension.

What

Bildschirmfoto 2020-04-07 um 10 31 47

Reproduce

  • Xcode 11.4
  • Targets linking Kingfisher 5.13.3
    • iOS main app
    • Today Extension
  • Archive (Release)
  • Distribute through Organizer to the AppStore

Other Comment

Maybe it should be considered to provide multiple product types (.automatic, .static, .dynamic) in the Package.swift file, see https://developer.apple.com/documentation/swift_packages/product .

On the swift.org forums the user defined build setting DISABLE_DIAMOND_PROBLEM_DIAGNOSTIC is used to allow static SPM libraries in multiple targets like in Xcode prior 11.4.
https://forums.swift.org/t/adding-a-package-to-two-targets-in-one-projects-results-in-an-error/35007

@onevcat
Copy link
Owner

onevcat commented Apr 7, 2020

As a dynamic framework now, you can just remove Kingfisher from the embed phase from your Today extension. Only linking to it and give it the search path should be enough.

@onevcat
Copy link
Owner

onevcat commented Apr 7, 2020

The DISABLE_DIAMOND_PROBLEM_DIAGNOSTIC could work, but I am afraid it is not where intended to be used. Force users to disable it is obviously against the original purpose of the related changes in Xcode 11.4, so I personally do not like that...

Let me know if you still have issue if you stop embedding it to extension target. And I’d like to see a change on either Xcode or SPM to have better “automatic” build type support.

@geraldvoit
Copy link
Author

Xcode does not allow linking without embedding Kingfisher in the Today Extension. Maybe because the Kingfisher Framework is created at build time. There is no local copy of the framework in the project (search path?). Furthermore, the Today Extension is a dependency of the main app's target and will be built first.

@onevcat
Copy link
Owner

onevcat commented Apr 8, 2020

I think you are right...

It seems that Xcode does not link the SPM libraries as dynamic ones. The change in Package.swift only affects partially library checking there. I think this is an out-of-sync development process in Xcode and SPM teams.

Maybe it is the correct way to revert it to automatically and use DISABLE_DIAMOND_PROBLEM_DIAGNOSTIC for now. I will prepare a fix for it and write it to wiki.

Hope Xcode team can fix this soon. :(

@onevcat
Copy link
Owner

onevcat commented Apr 17, 2020

This is fixed in Xcode 11.4.1 as below:

Fixed an issue where an error like “Swift package product A is linked as a static library by B and C. This will result in duplication of library code.” was incorrectly emitted if an app and an embedded app extension or helper tool statically linked the same package product. If you previously set the DISABLE_DIAMOND_PROBLEM_DIAGNOSTIC build setting to work around this issue, you can delete this setting now. (59310009, 61227255)

Now Kingfisher provides all build configurations as Kingfisher (link as .automatic), KingfisherDynamic (link as .dynamic) and KingfisherStatic (link as .static). You can freely choose either. By default, you can just use the automatic one (without any suffix) now.

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

No branches or pull requests

2 participants