-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[Build] Add support for building with the integrated Swift driver #2736
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
DougGregor
merged 7 commits into
swiftlang:master
from
DougGregor:integrate-swift-driver-build
May 13, 2020
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
8242a78
[Build] Add support for building and importing the new Swift driver
DougGregor 705c1ff
Fix bootstrap and build involving the new Swift driver.
DougGregor 1f1e909
[Swift driver] Add support for the integrated Swift driver
DougGregor 6061214
[Integrated Swift driver] Provide unique command names and better des…
DougGregor 9329793
[bootstrap] Don't test Yams or SwiftDriver as part of bootstrap
DougGregor e836957
[Bootstrap] Fix Linux bootstrap with integrated Swift driver
DougGregor c818cdb
[Integrated Swift driver] Use the tools from SwiftPM's toolchain.
DougGregor File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
Would it be possible to do this with
-rpath
arguments to the linker instead? I realize that the diff in this PR is in line with the existing approach of usinginstall_name_tool
, but I've been investigating some intermittent failures that can occur when there isn't enough space in the Mach-O header for the additional load commands. The approach I was going to use was to switch to passing-rpath
at link time, which seems cleaner. Alternatively, we would need to make sure that the link command leaves enough space by using-headerpad
, but that's a bit messier since we'd have to guess at a number large enough to make sure there's enough space, and it would leave an unnecessary hole in the binary.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.
It should be possible, yeah; we just need to figure out how to drive CMake to do that. That should be a separate pull request, but I could certainly see the argument that we should implement that improvement before merging here, because adding a bunch more rpaths in this manner is likely to push us over the limit.