[Pitch] SPM: Allow targets to depend on products in the same package

I am curious to see the experimental patch you mentioned in the other thread. Do you have a link? (I am skeptical that the implementation changes are as minor as you describe.)

But yes, I have wanted this since SwiftPM first appeared in Swift 3.

big +1 from me on the overall idea

I have a package (Delta Client) which currently requires the executable target to dynamically link with a library target within the same package (so that the plugin system works correctly).

My current solution is to have a second package containing the library target (and dynamic library product) inside a subdirectory. The main package depends on this package as a local dependency, and contains a target that uses @_exported import to reexport the library product to external users who may want to use the library product in their projects (or who need to import it to write plugins).

This solution isn't great because the dependencies are split across two Package.swift files (one of which is hidden in a subdirectory). It also means that swift test has to be run two separate times (in different subdirectories). Xcode also displays the library product with all other dependencies instead of as a sibling of the executable product, which confuses people sometimes.

Given that SwiftPM doesn't support monorepo-style packages (with multiple packages in one repository), this is probably the next best solution.

Another alternative would be allowing dynamic linking between targets, which might more directly address the problems you outlined in the pitch.

I plan to post the code tonight to the fork at https://github.com/tammofreese/swift-package-manager, branch name will be the only branch aside of main (does not exist yet).

1 Like

My take on this:

c) would require a lot of adaptions when upgrading to newer versions—over 90% of library definitions are for a target with the same name (did not dig deep into how many of those have tests referencing the target by name, but I assume it's a lot).
b) would feel too hidden/too magic to me.
a) is what I was going for, but would require lots of , package: "MyPackage" in target dependencies.

What do you think about using .product("Foo") in the target dependencies to reference product Foo in the same package in the future?

Branch is use_internal_product_as_target_dependency_experiment. Changes:

  • Added fixtures to check product resolution for targets, and one package with a cycle (note these are not integrated into the unit tests)
  • Implemented picking up the products from inside the package
  • Extended cycle checker to detect cycles over targets referencing products from the same package
  • Adapted swift-package init to emit the comment change as in my pitch post.

The test testProductDependencyDeclaredInSamePackage now fails which makes sense I think: The by-name dependency there can now be resolved. I can remove the respective error tomorrow.

4 Likes

More flexibility to have control over strategies and how everything is linked within a single Package.swift will be helpful. Within our production codebase, we were forced to define our product with the use of multiple internal Package.swift so we can depend on products and ensure type we need. It is unfortunate that within a single Package.swift everything can be linked only statically unless the dependency is external.
Although I suspect having such infrastructure will help with solving problems like the following — Bug: Linking multiple dynamic libraries that depend on the same target fails with ld failing due to missing files - #3 by Nikita_Leonov

1 Like

This is the crux of the problem that it solves for my project too. Maybe when a proposal is made, this can be the motivation statement (unless some people think that this is solving a different problem).

1 Like

Thanks for the real-world example! I am currently AFK, will check later whether the issue you describe can be tackled by referencing the product instead of the target, and if so add it as a motivating example to the pitch :slightly_smiling_face:

Good point! In my context I stumbled across other issues (the two examples in the pitch): Organizing code locally often requires to pull out extra packages to work around duplication in binaries.

Seems the major pain point in the general context could be allowing dynamic linking inside a package though! :+1:

Just tested out this branch of swiftpm with my project, and allowing targets to depend on local products allowed me to work wonders on my project's structure! I was able to go back to a simple one-package repository and remove a few weird re-exporting tricks i was forced to use to vend the internal package's products to regular users of the package.

The new structure for my project (as enabled by this pitch) can be seen on the single_package branch of my project.

4 Likes

@SDGGiesbrecht Before I put much work into an evolution proposal: Is the experimental implementation acceptable in your opinion, or did I miss something major?

As far as I understand, the two-step loading process can stay as it is, only in the first step we have to allow referencing the package itself, and in the second we have to resolve local products as dependencies as well.

I am amazed it was that simple. Apparently the load order issues resolved themselves at some point in the last few years. (Much has been refactored in that time.)

Two things jump out at me when looking at the implementation.


The first is line 575 of PackageBuilder.swift:

if package == self.manifest.displayName

This is not how the other .product dependencies work. For them, the package argument is the identity, which is usually based on the last path component of the URL these days.

I do not know what the identity of the root package even is. Until now nothing anywhere referenced it, so it was an implementation detail. So I do not think we can use the identity here. If the identity of a root package is its last path component, then its identity would change across checkouts, and cannot be safely referenced from inside the repository. If the identity is some placeholder, then it would be weird to teach users a magic value.

But I do not think it is good to use the display name either, because teaching users when to use the identity and when to use the display name will cause endless confusion.

I think the answer to this might be your suggestion of a new .product(_:) method that lacks a package argument. Similar alternatives to weigh would be making the package argument optional, and treating the same package as a nil, or deprecating the existing method and replacing it with internalProduct(_:) vs externalProduct(_:package:). With any of these options, the identity of the root package remains an implementation detail and we do not need to worry about it.

Fixing this should be trivial as far the implementation; it is just needs closer thought during review over the design question of how the user should be specifying it in the manifest.


The second thing I note from the implementation is that you (understandably) have not touched the #if ENABLE_TARGET_BASED_DEPENDENCY_RESOLUTION sections of PackageModel/Manifest.swift (related to SE‐0226). Those will need updating, otherwise the product your code looks for later on may not be there for it to find. I guess whichever feature gets merged first will cause the other a bit more work.

Either way, I think the required adjustments should be straightforward, so I am not concerned. You do not have to actually implement them now, but would you mind glancing at the file to confirm whether you foresee any difficulties from your perspective?

And with that future in mind, please include a test that has a graph like rootrootTargetdependencyProductdependencyTargetsamePackageProductsamePackageTargettransitiveDependencyProducttransitiveDependencyTarget, where that is the only way any of the nodes are reachable. That fourth link (dependencyTargetsamePackageProduct) is where the two features interact and need to co‐operate.

This isn't quite right, the identity of a root package is also just based on the last component of its path, similar to local packages. It does come up pretty regularly in the context of Xcode, where a root package (any package directly referenced by a workspace or project via a file reference) can override remote dependencies somewhere else in the graph.

Those are not the “root” in the sense I was using the word; for that situation the workspace or project is the root, and the highest‐but‐not‐actually‐top packages are still at fixed paths determined by external reference. The last path component is constant, since it is determined by the external reference, in much the same way a dependency is fixed at the URL in the client manifest.

As an example of the problem I am trying to describe, consider the product SwiftPM-auto from the SwiftPM package. For a client, the dependency declaration must use the identity:

.product(name: "SwiftPM-auto", package: "swift-package-manager")

But by the implementation here, if I were to update SwiftPM’s own package-info example target to depend on the SwiftPM-auto product instead of the Workspace target, its declaration would need to use the package name:

.product(name: "SwiftPM-auto", package: "SwiftPM")

I suspect that discrepancy will confuse many users.

We could attempt to unify them to both use the identity so that it is "swift-package-manager" in both declarations. But then the package only works if it is actually inside such a folder. If someone clones it with a custom directory name, or references it as a submodule in their own repository under a different name (e.g. SwiftPM or SPM), then the manifest will no longer even load, because the identity has changed and the package no longer sees itself as itself, breaking the self‐references. I suspect that will confuse many users too, especially since the directory structure can seem to work for a long time until suddenly it doesn’t because upstream added its first self‐reference.

Instead, I think the best option is to encode nothing in the manifest. Then .product(name:) without the package argument (or one of the other variants I suggested earlier) means “me, whoever I am”, and can be naturally resolved into whatever identity the package happens to have been assigned at load time.

I updated the PR so a product inside the package can now be depended on via .product(name:). I also allow the optional parameters moduleAliases and condition to create a symmetry to external product dependencies.

In the implementation I added an enum case innerProductItem to Target.Dependency and an enum case innerProduct to TargetDescription.Dependency. That allowed me to keep most of the changes as additions to existing code rather than modifying existing code.

Next up: I would like to keep this change as much an addition as possible. For that reason, I will look into making the failing test pass instead of adapting it—as a side effect of the changes it is now possible to reference a product in the same package by name if the package has the same name (example from the test case: product Foo with target FooTarget in package Foo can now be referenced as "Foo"dependency from the test target FooTests).

Changes in the branch:

  • Products in the same package are only found via .product(name:,moduleAliases:,condition) (the last two parameters are optional)
  • By name dependencies as before only work for targets in the package, and for products in the dependencies of the package.
  • .product(name:,package:) and the others specifying a product only work for products in the dependencies of the package, just as before.

Little bit of a nitpick, but inner product is a mathematical term that means something quite different, and we don’t use the term “inner” elsewhere with respect to a package—same-package product is quite self-explanatory, I think (internal could work too but it may suggest a relationship to the internal access level that isn’t quite apt).

2 Likes

I can go for samePackageProduct/samePackageProductItem, those are quite long though.
@SDGGiesbrecht suggested internalProduct, which I agree could be a bit confusing because of the internal keyword in Swift.

I’ll leave the rename for later, now concentrating on testing moduleAlias and condition.

I have implemented it as .product(name:), so referencing products from the same package requires not passing a package name.

Now I wonder about moduleAliases and condition. Currently I pass both through, but I started testing and realized they don’t work (at least moduleAliases, haven’t checked condition yet).

It may be possible to get both to work, now I am wondering whether they are needed.

Pro: It may simplify local package refactorings like merging two packages into one, if those are using moduleAliases and/or condition.

Con: I wonder whether this is needed in the same package, if not I would just add unneeded extra complexity.

I would love to hear opinions on this! :slight_smile:

… I missed something: moduleAliases won't help when merging packages as we would end up with targets having the same name :person_facepalming: . I'll remove it for now, and will look into condition next.