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
Implementation of same-package product dependencies #7331
base: main
Are you sure you want to change the base?
Implementation of same-package product dependencies #7331
Conversation
…s (no cycle checks or tests)
I don't think this is the right way to solve the problem. If the goal is to allow controlling how targets and products are linked, it should be done directly at the place of dependency declaration, and not in a roundabout way that allows depending on products in the same package. Let's say I want to link a private target dynamically, is my only option then to expose publicly it in my package as a product? Another problem with this approach is that it constrains users of a package to specific linking kind without any ability to control it at the place of dependency declaration. A slight terminology nit: documents like the one linked from PR description are usually not called evolution proposals until they successfully pass the pitch phase and are scheduled for review by the Swift team. I advise gathering some feedback on Swift Forums and getting to a consensus on a pitch first, before proceeding with the implementation. |
Hey Max thanks for the feedback, I only created the implementation last night (even though it hasn't been pitched since the initial forum thread) so that I could test it out on my own projects and see if it solved my issues in practice. I don't feel like this solution really restricts users of a package to a single linking kind anymore than they already would be, because the issues the pitch is addressing only arises when a package is already imposing dynamic linking of products for one reason or another (e.g. a plugin system as discussed in the pitch). I have thought about the alternative solution of allowing targets to control the linking mode when depending on another target, however that alone wouldn't solve the issues that this pitch is trying to address; let package = Package(
name: "Library",
products: [
.library(name: "LibA", type: .dynamic, targets: ["A"]),
.library(name: "LibB", type: .dynamic, targets: ["B1", "B2]),
],
targets: [
.target(name: "A", dependencies: [
.dynamic("B1") // Ignore the syntax, just for demonstration
]),
.target(name: "B1"),
.target(name: "B2"),
]
) In the above example, The reason that allowing same-package product dependencies works to solve all of the cases that motivated this pitch (of which private target linking wasn't one, but more on that next), is that these issues only arise when your package is a dependency of another package, and someone is depending on more than one product from your package where one of them is a dynamic library and both of them contain some code in common. That's why in my eyes the solution will likely involve products in some way shape or form. Controlling the linking of private targets is an important oversight, thanks for bringing that up. Since a private target wouldn't be directly included in any products, you would likely be able to avoid the limitation with target-target linking that the manifest above demonstrates, so it could be addressed by additionally introducing a way to control linking between targets. Now that I think about it, having both solutions (target-target linking control & target-product same-package dependencies) would provide even more linking flexibility and could provide the tools to solve almost any linking problems that one might face. Apologies if all that would've been better suited to a pitch thread, I'll create one once I've updated the pitch with the points discussed above. |
I realised that the link to the original pitch in the proposal draft is probably easily missed. Here's the original forum pitch thread that sparked this proposal draft: https://forums.swift.org/t/pitch-swiftpm-allow-targets-to-depend-on-products-in-the-same-package/57717 |
My main question here would be then: why do let package = Package(
name: "Library",
products: [
.library(name: "LibA", targets: ["A"]),
.library(name: "LibB", targets: ["B1", "B2]),
],
targets: [
.target(name: "A", dependencies: [
.dynamic("B1") // Ignore the syntax, just for demonstration
]),
.target(name: "B1"),
.target(name: "B2"),
]
) As specified this way, we can infer the right way to link But the main question is: why do you need to link dynamically in the first place? Dynamic linking is ok for system libraries that ship in an SDK that multiple apps can reuse at the same time. If you're building a single app with specific versions of your packages that can't be shared with other apps (and this is always the cases for apps on the iOS App Store), just link everything statically. That will not only avoid the duplication, but will also be beneficial for final binary size and app startup time. And that's the default for |
In my specific use case, the reason is that my app has a plugin system, and plugins written in Swift need to link against the same copy of the plugin API as the app itself (otherwise objects passed between the two just don't work correctly whatsoever, e.g. a plugin wouldn't be able to use an If it was only a thin plugin API that had to be in a separate repo/package, none of this would be an issue for me, but the dylib with the plugin API isn't just the plugin API, it necessarily includes basically the whole implementation of the app except for the UI (plugins can interact with a lot), and many changes that I make span across both the top-level package and the nested package (making splitting them across repos a non-option for maintainability). Additionally, there are multiple other people in the original pitch thread who claim that they've wanted this for quite a while and that they want a way to fix their dynamic linking issues without resorting to multiple nested Package.swift's (which then requires |
I understand there is a need for dynamic linking in certain cases, but I'll reiterate that this should be solved by providing additional linking controls, which is orthogonal to the way that dependencies are specified within a package. Expressing a dependency on products within the same package would have a similar effect only because of the limited way linking controls can be specified right now. We should lift those limitations on linking controls first of all. Not sure it's worth spending time instead on workarounds that bring their own limitations, like the aforementioned inability to keep dynamically linked targets private. |
I agree that a more flexible solution is better. If people aren't opposed to a more overarching set of changes regarding linking behaviour then I'd be happy to explore that avenue instead. I think the original idea was to keep the proposal simple and self-contained (I wasn't the original pitcher so I can't say for sure). If the same-package products idea gets scrapped we could instead (as you've mentioned) introduce controls for configuring linking modes of targets depending on targets (and possibly products depending on targets). I think you'd often end up with more dynamic libraries than strictly required (e.g. in the example above with AsideThe following isn't necessary to solve the code size and type-casting issues (as long as you're happy with .target(
name: "A",
dependencies: [
.dynamicBundle(["B1", "B2"]) // ignore naming for now
]
) which ends up looking a lot like depending on an anonymous product. But I'll drop that idea completely for now. Exploring a new solutionHere's a small example setup where I've explored a new solution and annotated some potential pain points. // The app's Package.swift
let package = Package(
name: "App",
products: [
.executable(name: "App", targets: ["App"])
// If `AppCore` and `AppRenderer` were to be part of the same product,
// SwiftPM would somehow need to get told that `AppCore` and `AppRenderer`
// must be separate dylibs when compiling the composite product for
// dynamic linking. That's only required because `App` depends on both
// targets as separate dylibs and the library is intended for plugins
// to use (which need to link the same way as `App`), it isn't a rule in
// general so it can't and shouldn't be automatically inferred. I haven't
// yet thought of a nice API for specifying when building the product
// as a dynamic library, that certain targets should be separate dylibs. We could
// go with an API such as `[.dynamic("AppCore"), .dynamic("AppRenderer")]`,
// but that implies that they're dynamically linked even under static linking,
// which isn't desirable.
//
// Requiring that the issue is worked around by splitting the product into two
// products like so could be considered an acceptable compromise for a
// first set of changes (or forever if it doesn't bother enough people).
.library(name: "AppCore", dependencies: ["AppCore"]),
.library(name: "AppRenderer", dependencies: ["AppRenderer"]),
],
targets: [
// Both of App's dependencies must always be dynamically linked to,
// so no need to design a flexible auto linking API for this part.
.executableTarget(
name: "App",
dependencies: [
.dynamic("AppCore"),
.dynamic("AppRenderer")
]
),
// This CLI doesn't need the renderer, so rendering is split out into
// a separate target from `AppCore` (`AppRenderer`).
.executableTarget(
name: "CLI",
dependencies: ["AppCore"]
),
// In this example it's possible to infer that `AppCore` must be dynamically
// linked to `AppRenderer` when building `App` (or building the `AppRenderer`
// product as a dylib), but in more complicated cases I would likely have to
// resort to using `.dynamic("AppCore")`, which would enforce dynamic linking
// even when not necessary (not ideal).
.target(
name: "AppRenderer",
dependences: ["AppCore"]
),
.target(name: "AppCore")
]
)
// A plugin's Package.swift
let package = Package(
name: "Plugin",
products: [
// In this case it makes sense to enforce dynamic linking since the plugin
// is only ever built to be loaded by the app, and is as such always compiled
// as a dylib. But perhaps some SwiftPM command line flags like so could be
// useful in other use cases (the existing swiftc -emit-library flag seems
// unreliable with SwiftPM from some initial Googling);
// $ swift build --product Plugin --linking-mode dynamic
.library(name: "Plugin", .dynamic, targets: ["Plugin"])
],
dependencies: [
.package(path: "../App")
],
targets: [
// Targets can specify their own linking modes for dependencies, as long as the
// products they depend on don't explicitly specify their own linking mode.
.target(
name: "Plugin",
dependencies: [
.product(name: "AppCore", package: "App", .dynamic),
.product(name: "AppRenderer", package: "App", .dynamic)
]
)
]
) |
The implementation accompanying a pitch that would allow targets to depend on products within the same package.
Motivation:
These changes allows dynamic linking between sibling targets, alleviating code-size and type-casting issues in certain situations. They also allow for executables to implement plugin systems without resorting to nested packages. See the proposal for motivating examples.
Modifications:
Target.Dependency.product(name:conditions:)
overloadinnerProduct
s) throughout SwiftPM's serialization, package model, and package builder codePackageBuilder.swift
andPackageGraph+Loading.swift
to correctly construct and build package graphs containing same-package product dependenciesResult:
Targets can now depend on products within the same package by using the
Target.Dependency.product(name: ..., conditions: ...)
static method when creating a target's dependencies.Tasks