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

Error when using Kingfisher with SPM and Xcode 11.4 beta 3 #1418

Closed
3 tasks done
marcosatanaka opened this issue Mar 21, 2020 · 19 comments · Fixed by #1420
Closed
3 tasks done

Error when using Kingfisher with SPM and Xcode 11.4 beta 3 #1418

marcosatanaka opened this issue Mar 21, 2020 · 19 comments · Fixed by #1420

Comments

@marcosatanaka
Copy link

Check List

Issue Description

What

I'm using Kingfisher in both my containing app, and two other targets. I've added it using Swift Package Manager on Xcode. On Xcode 11.3 everything works fine, but when updating to Xcode 11.4 beta 3, I started to get the following error:

Swift package product 'Kingfisher' is linked as a static library by {APP NAME} and 2 other targets. This will result in duplication of library code.

Reproduce

  • Create a new project and add Kingfisher as a dependency using Swift Package Manager on Xcode
  • Create a new target and add Kingfisher in the Frameworks and Libraries section
  • Build the project using Xcode 11.4 beta 3

Other Comment

Is this something to do with the way my project is organized, or is something that needs to be fixed on the Kingfisher project? Or is bug in Xcode?

Thanks!

@onevcat
Copy link
Owner

onevcat commented Mar 21, 2020

Ref https://forums.developer.apple.com/thread/128806

Now I am not sure whether it is the intended behavior of Xcode or not. But indeed there could be a problem if linked statically multiple times.

I hope Xcode could be intelligent enough to solve this kind of linking problem for developers, since all the targets are linking to the same library, so it should be not hard for the building system to strip duplicated copies from the final binary. However, it is not happening now.

A possible solution would turn Kingfisher to a dynamic library, which is not the default choice in Swift Package Manager. This requires an update in our Package.swift file. I will keep an eye to see if Apple finally keep this change in the release version. If so, I think we have to do an update to force Kingfisher built as a dynamic library.

@kabouzeid
Copy link

Still not fixed in final Xcode 11.4 (11E146) release from today :/

@OscarGorog
Copy link

Yep, this really sucks

@onevcat
Copy link
Owner

onevcat commented Mar 25, 2020

@kabouzeid @OscarGorog There is a patch branch fix/spm-dynamic. Can you confirm whether it works for you?

Please make sure that using Kingfisher as a dynamic framework would be what you want (since previously it was static linking).

@OscarGorog
Copy link

Working perfectly, amazing quick patch!
Thanks!

@kabouzeid
Copy link

Yes it works. I used the same fix for some other libraries too. I don’t understand why this is necessary though, in the SPM docs it says that it is recommended to leave this property unspecified so SPM can figure it out automatically. And since it worked before, this must be a bug in Xcode, right?

@OscarGorog
Copy link

OscarGorog commented Mar 25, 2020

I would think so but not too sure... is it worth filing a radar or just waiting for the next Xcode release and seeing if it gets fixed?

@kabouzeid
Copy link

I think it’s worth filing a radar. There must be many devs with this problem right now.

@OscarGorog
Copy link

OscarGorog commented Mar 25, 2020

Alrighty, I’ll do it in the morning (Melbourne) and post it here

@simoa
Copy link

simoa commented Mar 26, 2020

Hi, Thanks for this amazing library. Were still not able to submit to the AppStore even after applying the latest patch. We have the error attached. Thanks very much
Screenshot 2020-03-26 at 12 54 13

@marcosatanaka
Copy link
Author

Here I'm having a different problem, when trying to run the app on the device:

Captura de Tela 2020-03-26 às 10 28 21

It doesn't happen when running on the simulator though.

@onevcat
Copy link
Owner

onevcat commented Mar 26, 2020

@marcosatanaka Since it is now a dynamic library, you need to also add it to the Embedded Framework section to make sure it is copied to the final bundle.

@onevcat
Copy link
Owner

onevcat commented Mar 26, 2020

@simoa According to the post here: https://forums.developer.apple.com/thread/128806#411572

After these changes, build & run on device and simulator works, along with archive & export, although I had some errors when exporting until I restarted Xcode and started over, after which it succeeded.

Maybe it worths trying an Xcode restarting?

@onevcat
Copy link
Owner

onevcat commented Mar 26, 2020

@simoa But basically, you should not embed Kingfisher in an extension target. Try to only link them there, and only embed Kingfisher in your main app target.

@simoa
Copy link

simoa commented Mar 26, 2020 via email

@onevcat
Copy link
Owner

onevcat commented Apr 1, 2020

Version 5.13.3 was released, containing changes in this PR #1420. I will leave the branch for a while and delete it eventually to keep the repo clean, so please switch to the version-based dependency soon.

@bguidolim
Copy link
Contributor

I know the issue is closed already. But seems it seems to be a problem with project configuration and not with Xcode or the library. I had to fix the version on my project to 5.13.2 if I don't want to start embedding Kingfisher in every framework I need to use.

@onevcat
Copy link
Owner

onevcat commented Apr 8, 2020

I reverted the default dynamic change since there is no way to workaround the extension targets. I added some suggestions on this topic at #1426 and will provide all build configurations from the next release.

@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

Successfully merging a pull request may close this issue.

6 participants