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

fix: remove swiftsettings from binarytarget #121

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

reez
Copy link
Contributor

@reez reez commented Oct 16, 2023

Description

Fixes #120.

Notes to the reviewers

This fixes the reason for the error in #120.

Screenshot 2023-10-16 at 1 55 47 PM

I can follow up in an additional PR how to re-fix not showing the warnings that this code will re-introduce by removing suppress-warnings from .binaryTarget, but this PR's main goal is the fix the issue where a user can not use ldk-swift in their Xcode project, which was because of suppress-warnings in .binaryTarget. Here is the warnings showing up:

Screenshot 2023-10-16 at 2 02 19 PM

Other Thoughts

Why did this error show in .117 but not .116? This question still bugs me because I would like to have seen a specific changelog from Apple relating to this. From documentation I researched though, in the Swift Package Manager swiftSettings is a parameter that is specific to .target instances. From what I read binary targets don't have source files that need to be compiled, so they don't seem to have a swiftSettings parameter; so I'm guessing the swiftSettings parameter isn't supposed to be used with .binaryTarget it seems as per the design of the Swift Package Manager. It's possible that there might have been a change or a bug fix in the Swift Package Manager that now enforces this rule more strictly?

I also looked and tested across LDK Node + BDK Swift + LDK Swift, LDK Swift was the only dependency I was getting an error like this for and one of the main differences between the Package.swift in LDK Swift as compared to LDK Node and BDK Swift is that suppressWarnings was included in the .binaryTarget for LDK Swift but it was included in the .target for both LDK Node and BDK Swift.

So taking all of that into consideration, and also testing 2 things to make sure worked:

  • removing the related line of code worked to remove the error on the ldk-swift when opened in Xcode
  • creating a new Xcode project then added ldk-swift as a local dependency that had the fix included worked for adding the dependency

Submissions:

I did sign all my commits

I did not format my code per .swift-format since that change over 200+ files that were unrelated to my small code change.

I linked to the Issue this PR fixes.

Draft PR

I will just put this PR in Draft mode for now.

Copy link
Collaborator

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM, feel free to undraft I think!

@reez reez marked this pull request as ready for review October 17, 2023 18:04
@arik-so arik-so merged commit 2abea22 into lightningdevkit:main Oct 18, 2023
@reez reez deleted the 2023-10-package_error branch November 11, 2023 23:29
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.

package: error adding ldk-swift to xcode project
3 participants