Skip to content

SwiftDriver: enable response files on LinkJobs #999

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

Merged
merged 1 commit into from
Jan 28, 2022

Conversation

compnerd
Copy link
Member

The non-Darwin targets use clang as the linker driver. As a result, the
non-Darwin targets implicitly support response files (clang will expand
it if the linker does not support response files or file lists).
Unfortunately, the Darwin target manually invokes ld, which requires
re-implementing the handling there. To complicate things further, older
ld64 does not support response files, but does support linker file
lists, while newer ld64 supports both. Ideally, Darwin would also use
clang as the linker driver to homogenise the paths.

This is required for Windows support as the command line limit is much
lower on Windows than on Linux - nearing 4k which is possible to exceed
when building a larger project (e.g. swift-driver).

The non-Darwin targets use clang as the linker driver.  As a result, the
non-Darwin targets implicitly support response files (clang will expand
it if the linker does not support response files or file lists).
Unfortunately, the Darwin target manually invokes ld, which requires
re-implementing the handling there.  To complicate things further, older
ld64 does not support response files, but does support linker file
lists, while newer ld64 supports both.  Ideally, Darwin would also use
clang as the linker driver to homogenise the paths.

This is required for Windows support as the command line limit is much
lower on Windows than on Linux - nearing 4k which is possible to exceed
when building a larger project (e.g. swift-driver).
@compnerd compnerd requested a review from artemcm January 28, 2022 02:00
@compnerd
Copy link
Member Author

@swift-ci please test

@@ -73,7 +72,10 @@ extension Driver {
displayInputs: inputs,
inputs: inputs,
primaryInputs: [],
outputs: [.init(file: outputFile.intern(), type: .image)]
outputs: [.init(file: outputFile.intern(), type: .image)],
// FIXME: newer ld64 supports response files as well, though really,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this mean this could always be enabled at this point? clang now defaults to on for all new versions since xcode 13

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what deployments that this code would run on, and we don't have enough details about what ld64 is in use, so it is safe to limit it to non-Darwin targets. Ideally, we would use clang as the linker driver.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I guess I'm just thinking a new driver version + old xcode tools version is likely considered unsupported? But yea it seems minor enough for now

@compnerd compnerd merged commit f4e80ec into swiftlang:main Jan 28, 2022
@compnerd compnerd deleted the response branch January 28, 2022 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants