-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Support separate modules for static and dynamic exporting symbols for Windows #8049
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
Conversation
if self.windowsTargetType == .exporting { | ||
path = path.appending("exporting") | ||
} | ||
return path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really don't like how we're putting all the modules in a single directory and adding it to the include path. Since the exporting modules have the same name, I had to create another directory to put them in and add that include path for the products that consume them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah putting all the modules in the same directory is extra annoying because you can end up import
ing things you haven't declared a dependency on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question is, who do I break if I change this?
@swift-ci please test |
Sources/Build/BuildDescription/SwiftModuleBuildDescription.swift
Outdated
Show resolved
Hide resolved
One other issue I'm fighting is when building SwiftPM, I'm still getting the exporting imported symbols warning for the yaml_parser functions, despite having the YAML_DECLARE_STATIC flag being passed to the CYaml target. |
|
if I recall correctly, CSettings on a C target are not passed to Swift targets which depend on that target as |
The warnings I'm seeing are:
Which does imply the issue is on the Swift side. That's something worth trying. |
Yup, @DougGregor ran into that with swift-java too. I've created #8095 to track that. (I'm sure that's another issue on that somewhere but I didn't spend enough time trying to find it :)) |
For Windows triples only, creates a parallel build graph for Swift modules, one for static linking using -static, and one for exporting linking which is the default. DLL products consume their direct target dependencies as exporting. All other dependencies use the static versions to eliminate unnecessary symbol exports. The bulk of this is managed by the SwiftModuleBuildDescription which will create a duplicate of itself for the exporting case and set it's own type to static linking. Both modules are fed to the planner to create llbuild swift command tasks. Code is added for dynamic libraries to hook up the correct inputs for exporting the symbols in the libraries targets.
164e754
to
2e58a0c
Compare
@swift-ci please test |
@swift-ci please test windows |
This is ready for review. I'd like to start giving it some soak time with the snapshot builds once approved. |
Hi @dschaefer2 it seems like the following issue started after this PR/commit: #8110 I'm not sure if it's an issue on the user side or the spm side. Wdyt? |
Thanks! I'll take a look. I am worried about side effects I didn't think about. This could be one. |
I think that this is a modelling issue. The problem is that there are two libraries SwiftWin32UI and SwiftWin32. SwiftWin32UI depends on SwiftWin32. SwiftWin32 however has a dependency on SwiftCOM which is package external and has a dynamic library product. When the SwiftWin32 product is built (dynamic library), we link dynamically to SwiftCOM and everything is happy. When building SwiftWin32UI, we statically link against SwiftWin32. This uses the object library for SwiftWin32. However, this object library was built against the static library (object library) for SwiftCOM. We now attempt to link with |
I've brought that issue over here as #8110 . We can continue diagnosis there. |
Works around a link-time bug in SwiftPM that appears to have been introduced in swiftlang/swift-package-manager#8049.
Works around a link-time bug in SwiftPM that appears to have been introduced in swiftlang/swift-package-manager#8049. Works around swiftlang/swift-package-manager#8111 until swiftlang/swift-package-manager#8112 is in the build. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
On Windows, there is a limit of around 64K to the number of symbols a DLL/EXE can export. We hit that regularly with large projects like SwiftPM itself. This hides those symbols and only exposes the ones requested for a DLL.
For Windows triples only, creates a parallel build graph for Swift modules, one for static linking using -static, and one for exporting linking which is the default. DLL products consume their direct target dependencies as exporting. All other dependencies use the static versions to eliminate unnecessary symbol exports.
The bulk of this is managed by the SwiftModuleBuildDescription which will create a duplicate of itself for the exporting case and set its own type to static linking. Both modules are fed to the planner to create llbuild swift command tasks. Code is added for dynamic libraries to hook up the correct inputs for exporting the symbols in the libraries targets.
This is WIP as we need to do a lot of testing to ensure we didn't break anything. Ensuring this only affects builds for Windows triples helps mitigate that.