Ensure plug-in C modules don't mix with target ones. #8083
Conversation
When a Swift module depends on a C library that is also a dependent of plug-in that the module depends on, we were seeing includes of the module map for both versions of that library, host and target, causing types to be redefined. This change filters out the plug-in version of these dependencies.
dschaefer2
requested review from
bnbarham,
MaxDesiatov,
jakepetroules,
francescomikulis,
xedin,
shawnhyam,
bripeticca,
plemarquand and
owenv
as code owners
|
Still have tests to write. |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
MaxDesiatov
added
bug
build system
|
@swift-ci please test |
|
Updated to push this all the way to the dependency walking so we never get plug-in rooted dependencies in this part of the planning. And added a test. Also missed adding the issue link #8060 which points at the @DougGregor's repo where I first reproduced this. |
|
@swift-ci please test |
|
@swift-ci please test windows |
| @@ -20,12 +20,13 @@ extension BuildPlan { | |||
| func plan(swiftTarget: SwiftModuleBuildDescription) throws { | |||
| // We need to iterate recursive dependencies because Swift compiler needs to see all the targets a target | |||
| // depends on. | |||
| for case .module(let dependency, let description) in swiftTarget.recursiveDependencies(using: self) { | |||
| for case .module(let dependency, let description) in swiftTarget.recursiveLinkDependencies(using: self) { | |||
There was a problem hiding this comment.
I think instead of a special method that is filtering out plugins what you want to do is check whether the dependency targets the same destination and if not skip it, that would filter out plugins/macros and other things for "target" destination.
There was a problem hiding this comment.
I did that originally but felt that was hiding the real issue of including plug-in dependencies in my list of recursive dependencies when I don't want them when constructing the command line arguments for my module build.
There was a problem hiding this comment.
Maybe add a optional destination as a parameter to recursive dependencies method then, it would be easier to understand than having to navigate multiple slightly different methods.
There was a problem hiding this comment.
Or maybe it would be okay to adjust recursiveDependencies to always respect destination of the requesting module I don't think we ever want to have things from different destinations used anyway, it's not only plugins though it's also applicable to macros.
There was a problem hiding this comment.
I've been hesitant to rely on destinations. Could a plug-in tool depend on another plug-in tool? Wouldn't both be host?
There was a problem hiding this comment.
Yes, both would be host, just like macros. There are other places in Build which filter based on the destination so it won’t be unique.
There was a problem hiding this comment.
Btw, note that plugins actually have a special description not SwiftModuleBuildDescription.
|
@swift-ci please test |
|
@swift-ci please test windows |
When a Swift module depends on a C library that is also a dependent of plug-in that the module depends on, we were seeing includes of the module map for both versions of that library, host and target, causing types to be redefined.
This change filters out the plug-in version of these dependencies.