-
Notifications
You must be signed in to change notification settings - Fork 1.4k
bootstrap: allow building with shared BlocksRuntime #1631
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
@@ -579,6 +579,8 @@ class llbuild(object): | |||
if self.args.foundation_path: | |||
link_command.extend(["-L", self.args.foundation_path]) | |||
if self.args.libdispatch_build_dir: | |||
link_command.extend(['-L', self.args.libdispatch_build_dir, | |||
'-Xlinker', '-lBlocksRuntime']) |
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.
Shouldn't this be
link_command.extend(['-L', self.args.libdispatch_build_dir, '-lBlocksRuntime'])
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 seemed that the actual linked library names are passed directly through the driver in the rest of the build. Im just following the pattern. I believe that the driver will forward the options either way.
@swift-ci please test |
The failure is weird, llbuild linkage is failing, which should not be impacted by addition only linkage for BlocksRuntime. |
@swift-ci please test Linux platform |
@aciidb0mb3r - okay to merge this? |
@swift-ci please smoke test |
@@ -1125,6 +1127,9 @@ def main(): | |||
symlink_force(os.path.join(args.foundation_path, 'libFoundation.so'), | |||
libswiftdir) | |||
if args.libdispatch_build_dir: | |||
symlink_force(os.path.join(args.libdispatch_build_dir, | |||
'libBlocksRuntime.so'), | |||
libswiftdir) |
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.
Why do we need this symlink?
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.
Without that symlink, the tests tended to fail. libdispatch would not be found, but by keeping the symlink there, which is one of the paths that linker/loader consult, the tests are happier.
Can you explain what is this and why do clients need to link to it manually? Also, is this a hard dependency? |
libBlocksRuntime is the runtime for blocks. The closures in swift are actually blocks, and need a runtime for to support them. Currently, it was built static and put into libdispatch and a couple of other entities. However, there are globals which need to be uniqued across the image, and that requires that we have the BlocksRuntime be shared. This is setup to convert the build of the Blocks Runtime to a shared library. |
Thanks for the explanation, @compnerd! It sounds like this is a dependency of the libdispatch or the Swift compiler itself. I don't think SwiftPM (or every Swift based Swift.org project) should need to explicitly configure search paths for this dependency. |
@aciidb0mb3r, unfortunately, during the complex CI build, that isnt really feasible. When the components are installed in the proper location ( |
@compnerd Ah, okay. That sounds reasonable to me then! Feel free to merge. |
No description provided.