package: add target to swift package

[reez]

Draft: see Thoughts/Issues/Feedback/Before-Merging

tl;dr this removes warnings in ldk-swift, but just some feedback needed.

Describe The Feature

In PR #121 we removed swiftSettings: [.unsafeFlags(["-suppress-warnings"])] due to Issue #120.

This PR adds a .target that can now handle that swiftSettings: [.unsafeFlags(["-suppress-warnings"])] line of code back into ldk-swift without re-creating the Issue #120.

Why Add This Feature

We probably want the swiftSettings: [.unsafeFlags(["-suppress-warnings"])] line somewhere because an end consumer of ldk-swift will show no warnings in their IDE instead of 1000+ without swiftSettings: [.unsafeFlags(["-suppress-warnings"])]

How This Feature Was Added

Instead of including swiftSettings: [.unsafeFlags(["-suppress-warnings"])] in .binaryTarget it is included in .target.

For this a .target had to be added in the Package.swift file in the same .targets section that .binaryTarget is in.

Thinking Around This Feature

ldk-node + bdk-ffi + bdk-swift Package.swift's all include a .target that the swiftSettings: [.unsafeFlags(["-suppress-warnings"])] is used in, and testing this change in ldk-swift does what is expected by removing the warnings and not causing any errors.

Thoughts/Issues/Feedback Before Merging

• No warnings is definitely nice to have for consumers of ldk-swift but not something that technically impedes the use of ldk-swift
• ldk-swift might only have included a .binaryTarget and no .target for good reason
• I think the path I put for the new .target is correct but let me know if not
• If we use .target we just have to figure out a different name in the .target than what I temporarily put ("LightningDevKitTarget").
  • ldk-node + bdk-swift + bdk-ffi use XyzFFI for the name of their .binaryTarget and then Xyz for the name of their .target
  • ldk-swift currently uses Xyz for the name of the .binaryTarget. I'm not sure if we'd want to change the name of .binaryTarget or not to add "FFI" to the end of it if it could cause problems for current consumers of the SDK, and I wasn't sure what we would want for the .target name if LightningDevKit was already taken by the .binaryTarget
• note: Xcode still has some weird bug(?) related to swiftSettings: [.unsafeFlags(["-suppress-warnings"])] but it only shows up if you are importing a local package
  • would not really effect consumers of ldk-swift other than if they were trying to make modifications to ldk-swift locally or something;
  • if someone was using the remote swift package of ldk-swift they will not see that error/bug/etc.
  • Most people would not use ldk-swift as a local swift package imo If they did they would just need to comment out swiftSettings: [.unsafeFlags(["-suppress-warnings"])] in their local swift package

[tnull]

This seems plausible. Do you mind double-checking if this works with the just-released 0.0.118?

[reez]

This seems plausible. Do you mind double-checking if this works with the just-released 0.0.118?

Sure!

Just tested with the just-released 0.0.118 and everything works as expected.

Since having no warnings is a nice-to-have and not a must-have, its totally a fair option to just keep this PR around for whatever day its decided its for sure needed. That might actually be the best thing to do potentially?

But just to recap this PR, it does seem to solve the issue of not getting any warnings, works for the new 0.0.118 release and previous releases, is the best solution I can think of, and is a similar solution to the sister projects like ldk-node+bdk-swift.

Screenshot of this change in the Package.swift working for a local package of ldk-swift-0.0.118 that has the code in this PR applied to it:

[tnull]

Sounds good! I'm however still a bit dubious on what adding a new target would change. For example, would it mean that under certain circumstances we would rebuild the project from the out files instead of using the binary target directly?

[reez]

Sounds good! I'm however still a bit dubious on what adding a new target would change. For example, would it mean that under certain circumstances we would rebuild the project from the out files instead of using the binary target directly?

Good questions and thoughts, thanks those are important for even myself to make sure I have fully fleshed out in my own head.

• The binaryTarget in Swift Package Manager is designed to reference pre-compiled binary libraries, and from my understanding does not involve source compilation where build settings such as suppressing warnings would apply​.

• On the other hand, by introducing a new target for source compilation and moving the suppressWarnings flag there, what can be achieved is the effect of suppressing warnings during the build process which aligns with the typical use of build settings in Swift where they are specified in the target declarations that compile from source​.

  • This idea can be seen in the Package.swift of ldk-node + bdk-ffi + bdk-swift. (the only slight difference in this PR is I have not added LightningDevKitTarget to line 17 yet because I wasn't sure of the naming convention to be used for it yet if it is trying to match the naming convention in ldk-node+bdk-swift, etc, I probably should have included that though or called that out)

That's the high level idea here, but the PR in Draft mode partially because if that general idea is agreed upon for this specific package ldk-swift then there also might be some tweaks to the specific nuance of things like the build process and Package.swift (also naming LightningDevKitTarget something different, adding it to line 17, etc).

But ultimately there might be good reason ldk-swift is only using a .binaryTarget and does not want to include a .target which would make the general idea of this PR a non-starter and is totally cool too.