Factor `Clibsodium` into its own repo and package to work around #233

[jnross]

I'm not sure how seriously you should take this pull request, and I will mark it as a draft to reflect that. But I've bounced off the problem described in #233 for a while, and this is the solution that works and feels least hacky so far. So this is the workaround I'm going to go with until something better comes along.

[FabioTacke]

Based on the discussion in #226 maybe it's time to un-draft this PR as it seems to resolve an important issue with integrating swift-sodium via SPM.

[code28]

Since this seems to work, I consider this a good proof-of-concept. But I think this shouldn't/can't be merged right away. Let me get the steps straight:

1. Move Clibsodium.xcframework into another repository (you let those files in this repo, but I think, they should be moved out of this, right?)
2. Add the new repo (which basically only consists of Clibsodium.xcframework) as a dependency to swift-sodium

(Btw: One advantage would be, that any update to libsodium would only result in an update to the new xcframework-repo, which makes more sense than updating swift-sodium for any libsodium update.)

Did I get this right? Or might it be possible to add the dependency as a local dependency? So no need for a second repository?

[jnross]

I'm leaving this as a draft since we sure don't want to merge it with things pointed to my forks.

@code28: Yes, your comment captures the gist of the fix that worked in my testing. We're using this in our app too. I tried to keep the local dependency but that didn't work for me when I originally authored this - only the external repository. Perhaps I did it wrong or the behavior has changed since then.

Based on the behavior out of SPM, it really seems like a race condition bug where it's overly-aggressively parallelizing parts of the dependency graph. I really really hope this is fixed in a future version. It would be nice if this repo didn't have to be chopped in half before the SPM fix comes in.

[FabioTacke]

This seems to be the bug we are dealing with here: https://bugs.swift.org/browse/SR-13803

[simonmcl]

If this solves the issue can someone please clean it up and get it merged into the main repo + setup the other repo for the .xcframework under the same org.

I'm building a swift package that depends on Sodium. I get this error the odd time when using Xcode locally, but it happens VERY frequently when trying to integrate it with github actions. It's failing randomly, but probably higher than 50% at this stage. I basically can't get a CI pipeline running for my package. We are starting work on our iOS app based around this package next week, and this will cause us a lot of trouble

[jedisct1]

A better thing to do would probably to comment on https://bugs.swift.org/browse/SR-13803 and https://developer.apple.com/forums/thread/662247, providing as much detail as possible in order to get the core issue eventually fixed.

[simonmcl]

@jedisct1 I'll comment on those too in case it helps, but I think this is a more detailed issue than "including an xcframework". I have the TrustWalletCore project in my Package as an xcframework and i'm not facing any similar issues. Its something unique to how its being used in this context ... which likely means a longer time waiting for a fix.

Its only marked as a medium priority and will likely have to wait for an Xcode release. Its causing a lot of issues in the meantime. The workaround posted above seems relatively small and wouldn't change much (correct me if i'm wrong). Is there any chance to get that change made in the short term to avoid this issues?

Looking at my github action ~70% are failing due to this issue

[simonmcl]

@jnross I've used your fork and branch spm to unblock me for now. Archiving, Exporting and uploading to TestFlight all works, but after TestFlight finishes processing I get an email saying:

ITMS-90562: Invalid Bundle - One or more dynamic libraries that are referenced by your app are not present in the dylib search path.

Struggling to find a solution online. Do you have a solution for this by any chance?

EDIT:

Had to disable bitcode in order for it to pass validation, which sucks

[jnross]

I did encounter the same ITMS-90562 error. I know that the solution had to do with how we were specifying which libraries/frameworks were copied into the bundle, and which ones were specified for linking.

Our project contains the app target, and an SDK target, both of which reference the Sodium static library. No Sodium frameworks are copied into the bundle, and there are no explicit references to Clibsodium. My understanding is that all linking is done statically and the bundle doesn't all out any visible reference to sodium. It's just in the binary. We also had to disable bitcode, but Sodium may not be the reason - we have other tricky dependencies too.

[jnross]

I'll comment on the earlier discussion as well.

I agree with @jedisct1 that the most important thing for long-term success is to provide feedback on https://bugs.swift.org/browse/SR-13803 and do everything possible to get the root cause fixed.

But I strongly disagree with the idea of waiting for that bug to be fixed and not trying to work around the issue in the meantime. My past experience tells me that it may take quite some time for Apple to get this issue resolved, and dealing with a high percentage of intermittent CI failure in the meantime is either miserable or untenable.

That's why, until the root cause is fixed, we're using a fork with a workaround that prevents that misery.

[simonmcl]

I did encounter the same ITMS-90562 error. I know that the solution had to do with how we were specifying which libraries/frameworks were copied into the bundle, and which ones were specified for linking.

Our project contains the app target, and an SDK target, both of which reference the Sodium static library. No Sodium frameworks are copied into the bundle, and there are no explicit references to Clibsodium. My understanding is that all linking is done statically and the bundle doesn't all out any visible reference to sodium. It's just in the binary. We also had to disable bitcode, but Sodium may not be the reason - we have other tricky dependencies too.

@jnross Can you provide some more details on this? a link to the repo, copy/paste the code or screenshot xcode settings etc?

I've started using TestFlight now and my app crashes on launch with an error:

Termination Description: DYLD, dyld: Using shared cache: <hash> | dependent dylib `@rpath/Sodium.framework/Sodium` not found for <path-to-ipa>/<appname> tried but didn't find <path-to-sodium>

But works fine when running from Xcode. I'm missing some config setting somewhere and don't know how/where to do that in a Package. All i'm doing at the minute is including your version of sodium in my Package like so:

dependencies: [
        .package(url: "https://github.com/attaswift/BigInt.git", from: "5.2.1"),
    .package(name: "Sodium", url: "https://github.com/junelife/swift-sodium.git", .branch("spm")),
        // ....
],
targets: [
        .target(
            name: "<package-name>",
            dependencies: [
                "Sodium",
                "BigInt",

And then including my package in my app. What else am I missing?

[simonmcl]

@jnross so its because your fork is marked as dynamic thats causing this issue. When using this as part of a Package, there needs to be something to embed the framework inside the bundle, else Xcode just ignores it.

I googled around a lot but couldn't find anything related to this. I ended up having to import that fork of Sodium into the iOS app importing my Package as well. When doing that i'm given the option to Embed & Sign under the general tab of the target. It works now but this is quite messy, and I still sometimes get "can't find CLibSodium" when cleaning and building, thankfully not when archiving though.

Couple of questions for you, or anyone else who can help

1. I know very little about this stuff, but what is the reason that your fork needs to be dynamic? Is there any way to keep it static?

2. Do you know of any way to modify my Package.swift so that it will embed Sodium automatically without having to ask the importing app to include it as well?

3. Trust Wallet's open source library "WalletCore" is written in C, includes the C sodium library, and then has a wrapper for Swift, Kotlin etc. (Sadly the internal Sodium is private and not accessible). They recently distributed their library as a Package, by building everything as .xcframework's. They've managed to do this without having to mark anything as dynamic, or having other compiler issues (Although its iOS only). Would copying some of their approach solve any of these issues? https://github.com/hewigovens/wallet-core-spm

[jnross]

@simonmcl: Sorry I haven't responded earlier. I've been thinking about your comment for a while in the background, but haven't had time to dig into the specifics of any of your questions.

Debugging someone else's project via a game of telephone is always challenging. Could you provide a sample project that demonstrates the issues you're experiencing? That's what I did in https://github.com/jedisct1/swift-sodium/issues/233. Perhaps you could use my sample project as a starting point and adjust it to be more like your project and demonstrate your current issues. That might entice me or someone else to look at the specific details rather than guessing based on a description.

My attempts to answer your questions:

1. I had to mark this as dynamic because when it was static, it was getting linked into both my own framework, as well as the app binary. This resulted in duplicate symbols, and unpredictable linking behavior.
2. No, I don't. I wish I did as I think that would make this simpler to deal with. I can see the benefits of .frameworks and .xcframeworks for binary distribution, but I kind of of wish for the older method of linking everything together statically. This would yield just a single binary, and allow the linker to optimize away unreferenced symbols.
3. Maybe 🤷 . You should try making your own fork and give it a shot. Apple keeps moving the goalposts with changes to Xcode and SPM, so the solution suggested in PR may be obsolete at this point.

[jnross]

Since I just tested the motivating issue #226 with the latest Xcode and no longer encounter this problem, I'm going to close this PR as well. I know some folks have discussed a variety of topics in the comments here. If you're still experiencing issues, consider creating a more focused issue or PR to help isolate your particular issue.