Add support for building fully-static binaries

[lunaris]

Off the back of much discussion in #379.

This commit implements support for fully-statically-linked binaries by
extending functionality linked to the is_static attribute of an active
Haskell toolchain. In particular:

• Packages built with Cabal will be passed fields that ensure their
  static artifacts are relocatable (-fPIC and
  -fexternal-dynamic-refs).

• Intermediate artifacts (such as binaries built by hsc2hs for the
  purposes of generating Haskell code) will also be built statically to
  avoid issues caused by the absences of dynamic libraries.

• Linker flags for REPLs will be generated so that they correctly find
  static C library dependencies instead of failing/finding dynamic
  counterparts.

[lunaris]

@aherrmann I've pulled out the hsc2hs flags as suggested, which I agree is much nicer. Also I took your recommendation and added path_prefix to link_libraries. bazel test //... is all green for me locally though I'm not sure if CI can catch any more (e.g. Windows, Darwin) cases.

If you think this looks sensible, some ideas I have:

• Thoughts on adding a couple of build @stackage//:<package> s to the test suite? In particular I know that (currently at least) hslua exercises the hsc2hs code path and as seen openssl will trigger some of the GHCi linking code paths.

• I think we should hand-craft an hsc2hs example to make sure that the --lflag=-static works there (I've not tested this but assume it will work as it does in the Cabal wrapper).

• Naturally, lots of documentation for the static use case, which feels fairly complete now! (At least, if you're using Nix/Linux and not a bindist...)

[lunaris]

Yea looks like we might need some guards for Windows at least -- the Azure failures seem to be linker errors, which I assume are genuine and caused by is_static having more aggressive semantics now.

[aherrmann]

Right, --lflag=-static is too strict for the mostly static use-case with the static RTS, and probably also for Windows, as it completely disables linking against dynamic libraries. Sorry, I didn't think of that before. It looks like we'll have to separate the static RTS and fully static use-case.

IIUC the only difference between static RTS and fully static is whether --lflag=-static needs to be passed to hsc2hs, correct? I'm not sure that this justifies an additional dedicated toolchain attribute. Maybe a more generic hsc2hsopt toolchain attribute could handle this case? WDYT?

* Thoughts on adding a couple of `build @stackage//:<package>` s to the test suite? In particular I know that (currently at least) `hslua` exercises the `hsc2hs` code path and as seen `openssl` will trigger some of the GHCi linking code paths.

Yes, not opposed to that. If possible it's better to create a dedicated test-case in tests. That's more stable towards future updates of the Stackage snapshot and also doesn't introduce additional external dependencies. But, if that's not feasible then an additional stack_snapshot dependency is a good alternative. Though, we currently don't have a static RTS or fully static CI pipeline in rules_haskell. So, anything hidden behind if is_static won't be exercised. It would be good to add such a pipeline, though that sounds like a task for a separate PR.

* Naturally, lots of documentation for the static use case, which feels fairly complete now! (At least, if you're using Nix/Linux and not a bindist...)

Absolutely, a use-case section in docs/haskell-use-cases.rst would be a good start for that. I think it's fine that this only covers Nix/Linux for now.

[lunaris]

To elaborate on your comment, as I understand it -- passing -fPIC / -fexternal-dynamic-refs to Cabal and path-prefixing -L flags in REPLs will work for static runtimes with mostly-statically-linked binaries, is that right? If so, then yes, that only leaves --lflag=-static to hsc2hs.

As to how we trigger this:

• I appreciate that renaming is_static is not really an option without an aggressive bump, but in an ideal scenario I feel like I'd lean towards toolchains having static_runtime and static_build attributes (or similarly named).

• Given we likely can't do that, I'd still be inclined to add an attribute (perhaps that implies is_static) to the toolchain. My reasoning here would be that, supposing we discover that we have missed a corner case (e.g. some TH/REPL/gnarly thing we have to patch around), I could see us then needing something alongside the hsc2hsopt attribute, etc. Whereas if we added builds_static (for example, which could, for example, imply is_static) we might allow for that in future.

Just my 2c though -- generally happy to do whatever as long as it works and is documented 👍

[aherrmann]

To elaborate on your comment, as I understand it -- passing -fPIC / -fexternal-dynamic-refs to Cabal and path-prefixing -L flags in REPLs will work for static runtimes with mostly-statically-linked binaries, is that right? If so, then yes, that only leaves --lflag=-static to hsc2hs.

Yes, that's what I meant.

* I'd still be inclined to add an attribute (perhaps that implies `is_static`) to the toolchain. My reasoning here would be that, supposing we discover that we have missed a corner case (e.g. some TH/REPL/gnarly thing we have to patch around), I could see us _then_ needing something alongside the `hsc2hsopt` attribute, etc. Whereas if we added `builds_static` (for example, which could, for example, imply `is_static`) we might allow for that in future.

That's a good point! Okay, let's go with a dedicated attribute.

AFAIK the only other user of the is_static attribute is daml, and a toolchain attribute name is pretty easy to change. So, I'm not opposed to changing it to a more descriptive name. is_static is ambiguous when it comes to distinguishing the static RTS and fully static builds use-cases. So, it makes sense to change it. I agree that static_runtime is a better name for this attribute.

Regarding the fully static mode, looking for precedent in Bazel there is the fully_static_link feature flag. Following that we could name the new GHC toolchain attribute fully_static_link.

[lunaris]

I was going to suggest fully_static_link, but shied away from it since a. it seems to be something of a top-level feature switch that affects all Bazel (which presumably we'd not be hooking into verbatim, since it could have other effects we don't want/know about) and b. it may currently be broken (!) -- bazelbuild/bazel#8672

That said, I do think it is morally the right choice, and if we're not against changing (or at least initially deprecating) is_static then that sounds great -- static_runtime and fully_static_link, which implies static_runtime, correct?

[aherrmann]

a. it seems to be something of a top-level feature switch that affects all Bazel

It's a recognized value for the features attribute of the cc_* rules. If present, then -static will be set during linking. Using it as an attribute name shouldn't interfere with that functionality. Given that fully static linking in Haskell requires support from the GHC toolchain, it makes sense to make this switch an attribute of the toolchain itself, rather than a feature than can be selected on individual targets.

b. it may currently be broken (!) -- bazelbuild/bazel#8672

That's good to know. It looks like building fully statically linked cc_binary targets requires extra flags and configuration next to features = ["fully_static_link"]. This shouldn't interfere with fully static linking of Haskell targets, though.

That said, I do think it is morally the right choice, and if we're not against changing (or at least initially deprecating) is_static then that sounds great

That sounds good. Such a deprecation can be implemented at the nixpkgs toolchain wrapper macro haskell_register_ghc_nixpkgs.

static_runtime and fully_static_link, which implies static_runtime, correct?

Yes, though I think it's best if the user has to explicitly write

    static_runtime = True,
    fully_static_link = True,

and we raise an error in case of static_runtime = False, fully_static_link = True,. My reasoning is this: IIUC the nixpkgs implementation of fully static linking works with a dynamic RTS and links libraries dynamically for GHCi and Template Haskell. So, in principle the combination static_runtime = False, fully_static_link = True, is possible, it's just not supported by rules_haskell at this point. If support for this configuration is ever implemented in rules_haskell, then turning an implicit implication into an explicit setting would be a subtle breaking change.

[aherrmann]

I did a review pass on the changes so far.

Again, thanks a lot for working on this!

[lunaris]

I've made all the changes you suggested -- great to see this patch getting smaller and smaller! I'll take up test cases and documentation when I next have time and then the only question I see left is deprecating is_static. Options I can see:

1. It's just not there in the next release. We have some documentation about it but if you use it you'll get a Bazel error.

2. It's there in the next release but if you use it we throw an error telling you to use static_runtime and perhaps explaining/linking to docs about fully_static_build.

3. It's there in the next release but if you use it we pass it as static_runtime and give you a warning letting you know that is_static is deprecated and that you should move away from it. Possibly also explaining/linking to docs about fully_static_build.

There's also the question of combinations of static_runtime and fully_static_build -- do we want to check and warn about these? If so, do we already have some sort of warnings for "bad" combinations (like bindist/Windows/dynamic runtime)? If not, do we want to add this while we're there? I'm guessing any such logic could go in haskell_toolchain, since I think all roads go through there.

More feedback welcome as always; cheers.

[aherrmann]

I've made all the changes you suggested -- great to see this patch getting smaller and smaller!

Thank you!

the only question I see left is deprecating is_static. Options I can see:

1. It's just not there in the next release. We have some documentation about it but if you use it you'll get a Bazel error.

2. It's there in the next release but if you use it we throw an error telling you to use `static_runtime` and perhaps explaining/linking to docs about `fully_static_build`.

3. It's there in the next release but if you use it we pass it as `static_runtime` and give you a warning letting you know that `is_static` is deprecated and that you should move away from it. Possibly also explaining/linking to docs about `fully_static_build`.

In the past we've usually gone with 3, it sounds reasonably straight-forward to implement in haskell_register_ghc_nixpkgs. is_static is currently only exposed through nixpkgs GHC, so we don't need to worry about bindist.

There's also the question of combinations of static_runtime and fully_static_build -- do we want to check and warn about these?

I'd say, not static_runtime and fully_static_build should raise an error (fail()) for now. As your work has shown, rules_haskell is not currently able to handle that combination. All the other combinations should be fine. static_runtime defaults to False on Unix and True on Windows, fully_static_build defaults to False on all platforms.

If so, do we already have some sort of warnings for "bad" combinations (like bindist/Windows/dynamic runtime)? If not, do we want to add this while we're there?

I don't think we have checks for any such "bad" combinations, yet. TBH I don't know if static_runtime = False on Windows is bad. I've never tried to build a dynamic RTS GHC on Windows.

[lunaris]

@aherrmann I implemented the compatibility checking/deprecation here: https://github.com/tweag/rules_haskell/pull/1390/files#diff-63173affa8c52d7a87df960435e8c641R275; see what you think. I've also pushed a chunk of documentation which hopefully looks OK and will see what the Windows tests say now that (hopefully) everything is gated by appropriate conditionals.

[aherrmann]

Excellent, this looks great! Thank you for the use-case documentation!

I have only two small comments.

I also ran the remaining CI pipelines on this. It's all good except for the formatting check. You can fix these warnings automatically with bazel run //:buildifier-fix.

[lunaris]

Added the missing arguments documentation and hopefully have appeased Buildifier; 🤞 we get a green CI this time!

[aherrmann]

Looks great and CI is all green!
Thank you @lunaris for implementing this feature!