Skip to content

Conversation

compnerd
Copy link
Member

No description provided.

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'])
Copy link
Contributor

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'])

Copy link
Member Author

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.

@compnerd
Copy link
Member Author

@swift-ci please test

@compnerd
Copy link
Member Author

The failure is weird, llbuild linkage is failing, which should not be impacted by addition only linkage for BlocksRuntime.

@compnerd
Copy link
Member Author

@swift-ci please test Linux platform

@compnerd
Copy link
Member Author

@aciidb0mb3r - okay to merge this?

@compnerd
Copy link
Member Author

@swift-ci please smoke test

if args.libdispatch_build_dir:
symlink_force(os.path.join(args.libdispatch_build_dir,
'libBlocksRuntime.so'),
libswiftdir)
Copy link
Contributor

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?

Copy link
Member Author

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.

@aciidgh
Copy link
Contributor

aciidgh commented Jul 19, 2018

Can you explain what is this and why do clients need to link to it manually? Also, is this a hard dependency?

@compnerd
Copy link
Member Author

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.

@aciidgh
Copy link
Contributor

aciidgh commented Jul 20, 2018

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.

@compnerd
Copy link
Member Author

@aciidb0mb3r, unfortunately, during the complex CI build, that isnt really feasible. When the components are installed in the proper location (/usr/lib), they will be found without a search path. Switching over to the shared build will allow us to move the library into swift itself, and the search path can be removed at that point. I think it would be more reasonable to try to move this into the driver at that point. This change is basically staging work.

@aciidgh
Copy link
Contributor

aciidgh commented Jul 20, 2018

@compnerd Ah, okay. That sounds reasonable to me then! Feel free to merge.

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