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

Add new linkingStrategy parameter to .target() dependencies. #17

Open
1 task done
furby-tm opened this issue Oct 18, 2024 · 4 comments
Open
1 task done

Add new linkingStrategy parameter to .target() dependencies. #17

furby-tm opened this issue Oct 18, 2024 · 4 comments
Assignees
Labels
good first issue Good for newcomers

Comments

@furby-tm
Copy link
Member

furby-tm commented Oct 18, 2024

Allow users to specify dynamic library targets in same-package dependencies, which are otherwise always statically linked with the following client-facing API changes to SwiftPM:

products: [
  .library(
    name: "DynamicLib",
    type: .dynamic,
    targets: ["DynamicLib"]
  )
]

// ok, dynamically linking DynamicLib.
.target(name: "DynamicLib", linkingStrategy: .matchProduct)

// ok, statically linking DynamicLib.
.target(name: "DynamicLib", linkingStrategy: .alwaysStatic)

// ok, falling back to default behavior.
.target(name: "DynamicLib")
@furby-tm furby-tm added this to SwiftPM Oct 18, 2024
@furby-tm furby-tm self-assigned this Oct 18, 2024
@furby-tm furby-tm converted this from a draft issue Oct 18, 2024
@furby-tm furby-tm added the good first issue Good for newcomers label Oct 18, 2024
@furby-tm
Copy link
Member Author

furby-tm commented Oct 18, 2024

cc. @stackotter for review:

wabiverse/SwiftPM@5dc346a

@furby-tm
Copy link
Member Author

furby-tm commented Oct 18, 2024

@stackotter after further review, is type: .dynamic along with a .alwaysStatic linking strategy something we want to be doing here?

My C/C++ senses tell me this is a weird thing to do, but given this is the historical default behavior perhaps it makes sense in this context?

Perhaps a tiny grammatical change we can make is to replace .alwaysStatic with .default here which simply fallbacks to the historical behavior of always statically linking (the same behavior that .alwaysStatic is intended to infer), but I suppose a .default option is less clear than having users explicitly declare .alwaysStatic:

// ok, statically linking DynamicLib.
.target(name: "DynamicLib", linkingStrategy: .default)

@furby-tm
Copy link
Member Author

furby-tm commented Oct 18, 2024

Pending #18 to be fixed before we can consider this as completed.

Edit: This bug is unrelated, it's a regression in SwiftPM main:
swiftlang/swift-package-manager#8055

@stackotter
Copy link
Collaborator

@stackotter after further review, is type: .dynamic along with a .alwaysStatic linking strategy something we want to be doing here?

My C/C++ senses tell me this is a weird thing to do, but given this is the historical default behavior perhaps it makes sense in this context?

Perhaps a tiny grammatical change we can make is to replace .alwaysStatic with .default here which simply fallbacks to the historical behavior of always statically linking (the same behavior that .alwaysStatic is intended to infer), but I suppose a .default option is less clear than having users explicitly declare .alwaysStatic:

// ok, statically linking DynamicLib.
.target(name: "DynamicLib", linkingStrategy: .default)

I think targets shouldn't be able to override the library type of products they depend on. So if a library specifically has type: .dynamic then it should always be compiled to a dylib whenever it's needed.

With the context that linkingStrategy is meant to control how the product links against the target, rather than the linking of the parent product, .alwaysStatic seems clear. So perhaps we should try to find a way to make that connotation of linkingStrategy more clear through naming or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: In review
Development

No branches or pull requests

2 participants