-
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
[Build] Add support for building with the integrated Swift driver #2736
Conversation
Cool! |
def add_rpath_for_cmake_build(args, rpath): | ||
"Adds the given rpath to the CMake-built swift-build" | ||
swift_build = os.path.join(args.bootstrap_dir, "bin/swift-build") | ||
add_rpath_cmd = ["install_name_tool", "-add_rpath", rpath, swift_build] |
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 using install_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.
@swift-ci please smoke test |
@neonichu I should have called it out before, but this PR depends on swiftlang/swift#31664 and some Jenkins updates @shahmishal is doing. I expect it to fail bootstrap. |
Yeah, the tests failed with the expected:
|
swiftlang/swift#31664 |
@swift-ci smoke test |
@swift-ci test Linux |
Ahh, a real failure! Excellent:
|
Add bootstrap and build support for building the new Swift driver and importing it (as a library) into SwiftPM. This is an incomplete stepping stone toward integrating the new driver.
Add missing dependencies, rpaths, etc. to get SwiftPM bootstrapping with the new Swift driver. There is no actual integration of SwiftDriver beyond the import.
Add the command-line flag `--use-integrated-swift-driver` to use the integrated Swift driver to create jobs rather than shelling out to the driver executable.
…criptions Unique command names allow us to successfully bootstrap using the integrated Swift driver. Using the Swift driver's job kind to provide more detailed descriptions for the various shell commands generated by the Swift driver, improving the output.
The Swift driver's toolchain and SwiftPM's toolchain might have slightly different paths in them. For now, map to SwiftPM's toolchain. There should be a more direct way to rectify these.
a501d0f
to
c818cdb
Compare
@swift-ci test |
@swift-ci smoke test |
@swift-ci test |
@swift-ci smoke test |
Add bootstrap and build support for building the new Swift driver
and importing it (as a library) into SwiftPM. Then introduce the new
SwiftPM flag
--use-integrated-swift-driver
to enable use of theintegrated Swift driver rather than shelling out to a separate
Swift driver binary. The jobs created by the integrated Swift driver
will be merged into SwiftPM's build graph.