rules_haskell icon indicating copy to clipboard operation
rules_haskell copied to clipboard

support Bazel 7 cc_shared_library

Open malt3 opened this issue 1 year ago • 1 comments

This change is based on #2180 and adds support for cc_shared_library. This is needed for macOS and Bazel 7+, since cc_library no longer emits dynamic libraries on macOS (feature removal, linker flag removal).

This requires a bit more work, since the tests are now no longer compatible with Bazel < 7 (I'm now using cc_shared_library). We can probably refactor the testing code so that cc_shared_library is used iff we are on Bazel 7 or greater. I need to think about it (suggestions welcome).

This implementation differs from the one in cc_binary in some ways: cc_binary uses a separate attribute dynamic_deps for CcSharedLibraryInfo, while haskell_library / haskell_binary use deps for both CcInfo and CcSharedLibraryInfo. We also do not implement the same deduplication logic as cc_binary where we filter CcInfo deps which are already provided by a CcSharedLibraryInfo. This may lead to duplicate symbols. A more correct implementation would need to use aspects and might require the use of a separate attribute (dynamic_deps).

Open issues:

  • [ ] Fix //tests/recompilation:recompilation_test_nixpkgs_bazel_7, which depends on the macOS Foundation framework. Probably broken by the removal of --undefined dynamic_lookup
  • [ ] Make tests work on Bazel 6 and 7 (use cc_shared_library only conditionally and make //tests/repl-targets:hs_lib_repl_test_nixpkgs_bazel_6 work again)

malt3 avatar Aug 28 '24 14:08 malt3

Adding back the linker flags --undefined dynamic_lookup did not fix the missing Foundation framework at link time. I'll try to investigate why we depend on the framework in the first place and what other things might have changed.

Here is the remaining diff between the two Bazel versions for the failing target:

--- /tmp/linker_flags_bazel_6   2024-08-29 19:06:26.401508228 +0200
+++ /tmp/linker_flags_bazel_7   2024-08-29 19:06:34.941537070 +0200
@@ -1,5 +1,6 @@
--optl-undefined
--optldynamic_lookup
+-optl-mmacosx-version-min=10.11
+-optl-no-canonical-prefixes
+-optl-fobjc-link-runtime
 -optl-headerpad_max_install_names
 -optl-lc++
 -optl-lm

Those might be responsible..

The latter two come from here, as far as I can tell: https://cs.opensource.google/bazel/bazel/+/refs/tags/7.3.1:tools/cpp/unix_cc_toolchain_config.bzl;l=1412-1427

malt3 avatar Aug 29 '24 16:08 malt3

Adding back the linker flags --undefined dynamic_lookup did not fix the missing Foundation framework at link time.

I think that issue occurs when using an automatically configured local cc toolchain which is picking up a C compiler from inside a nix shell. The nix C compiler requires some environment variables to be set in order to locate the frameworks and these are not available inside of the Bazel actions (that is why we write these flags to files in the nix-support directory for the cc toolchain from rules_nixpkgs: https://github.com/tweag/rules_nixpkgs/blob/a11818b2173100ab8122712ae3694b711f956cbd/toolchains/cc/cc.nix#L30C1-L50C7).

The failing test uses haskell_register_ghc_nixpkgs but does not configure a nixpkgs cc toolchain. That is probably not something that should be supported, but it worked with Bazel 6 / older rules_cc versions.

avdv avatar Aug 30 '24 08:08 avdv

The failing test uses haskell_register_ghc_nixpkgs but does not configure a nixpkgs cc toolchain. That is probably not something that should be supported, but it worked with Bazel 6 / older rules_cc versions.

So a better fix would be to add the nixpkgs cc toolchain to that WORKSPACE, correct?

malt3 avatar Aug 30 '24 08:08 malt3

So a better fix would be to add the nixpkgs cc toolchain to that WORKSPACE, correct?

Yes. :crossed_fingers:

avdv avatar Aug 30 '24 09:08 avdv