Use CcInfo provider for C library dependencies #663
Conversation
There was a problem hiding this comment.
LGTM, but I'm a bit uncomfortable with the boilerplate around gathering transitive library dependencies. Could you try factoring that out a bit more? There's some platform-specific mangling going on there, that i'll be all too easy to miss after some refactoring in the future.
|
@guibou could you test this on your codebase before we merge? |
|
Not quite there, yet. This seems to trigger segfaults in |
|
@mboes tests are OK. |
aherrmann
force-pushed
the
cc_dependencies
branch
from
d49875e to
4dc88e8
Compare
|
I've looked into the A simple workaround in Hazel is to only include the rts headers into cbits targets, not the library. I'll address @mboes 's comments, and then merge this PR, and fix the Hazel issue separately. |
aherrmann
force-pushed
the
cc_dependencies
branch
2 times, most recently
from
bb1b9eb to
cf431a8
Compare
|
I’ll review tomorrow. |
- Extract information on C library dependencies from Bazel's builtin CcInfo provider. This allows to benefit from Bazel's central `_solib` directory which holds all dynamic C library dependencies, and thereby allows to reduce the amount of RUNPATH entries. This also allows to benefit from Bazel's builtin library name mangling without having to reimplement it in rules_haskell - Bazel only mangles names of dynamic library dependencies. If a library dependency is available as both static and dynamic library, then each will have a different name (`.artifact()`). However, GHC expects both static and dynamic libraries to be available under the same name. The reason is that library dependencies have to be listed in the extra-libraries section of the package configuration file. To link the final binary statically, we only provide static archives at linking. However, for GHC's own linker (TemplateHaskell, REPL, etc.) we require dynamic libraries. Both have to match the entries in extra-libraries. We achieve this by manually mangling the static archives based on Baezl's builtin mangling if the static and dynamic library names (`original_artifact()`) differ. - On MacOS GHC's builtin linker hard-codes the expectation that any dynamic library dependencies are `.dylib` files. However, Bazel produces `.so` files, see [1]. To work around this issue we create symlinks to adapt any `.so` dependencies as `.dylib` dependencies. Note, that this is only required when interacting with GHC's builtin linker. I.e. for compilation, doctests, REPL. In particular, this is not required for the final linking. This is good news as it allows us to still benefit from Bazel's central `_solib` directory on MacOS. [1]: https://stackoverflow.com/questions/2339679/what-are-the-differences-between-so-and-dylib-on-osx
Addressing PR review comments. Reducing the risk of future refactorings introducing subtle bugs by missing required steps in get_solibs_for_ghc_linker.
Addressing review comment #663 (comment)
aherrmann
force-pushed
the
cc_dependencies
branch
from
cf431a8 to
34969f2
Compare
|
Rebased and addressed all comments. |
| (cc_link_libs, _cc_solibs) = _link_dependencies( | ||
| hs, | ||
| dep_info, | ||
| True, # dynamic linking mode |
There was a problem hiding this comment.
You can use named arguments in invocations in skylark, dynamic = True.
I strongly suggest we switch all calls to explicitly name arguments.
To improve readability. Addressing review comment #663 (comment)
Implements the
cc_librarydependency part of #593._solibdirectory which holds all dynamic C library dependencies, and thereby allows to reduce the amount of RUNPATH entries. This also allows to benefit from Bazel's builtin library name mangling without having to reimplement it in rules_haskell.dylibfiles. However, Bazel produces.sofiles. To work around this issue we create symlinks to adapt any.sodependencies as.dylibdependencies. Note, that this is only required when interacting with GHC's builtin linker. I.e. for compilation, doctests, REPL. In particular, this is not required for the final linking. This is good news as it allows us to still benefit from Bazel's central_solibdirectory on MacOS.