Skip to content
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

haskell: pkgsStatic fixes #162374

Merged
merged 4 commits into from
Mar 17, 2022
Merged

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Mar 1, 2022

Motivation for this change

Fixing common build failures in pkgsStatic.haskellPackages

See the commit messages for details.

Things done
  • built static GHCs 9.0.2 and 9.2.1
  • built pkgsStatic.haskellPackages.{QuickCheck,pretty-simple} (previously failing)
  • built and tested pkgsStatic.haskellPackages.pandoc

@rnhmjoj rnhmjoj changed the title haskell: haskell: pkgsStatic fixes Mar 1, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Mar 1, 2022
@sternenseemann
Copy link
Member

  • I'd drop d8dcb588aed526a632c3060c1b091908c7cf31df, since it changes the meaning of haskell.packages.ghc* / haskellPackages between pkgs and pkgsStatic which I find problematic. Especially since the compiler build itself is not affected, this doesn't seem necessary?
  • Can you add a package to the staticHaskellPackages jobset in pkgs/top-level/release-haskell.nix which doesn't build without -fexternal-dynamic-refs? This would help keeping tabs on it.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 2, 2022

Especially since the compiler build itself is not affected, this doesn't seem necessary?

I see your point, though I'm not sure how useful the default compiler is, given most packages fails to build. Ideally, we should fix the linking errors, but avoiding gmp is the only way I've found so far.

Can you add a package to the staticHaskellPackages jobset in pkgs/top-level/release-haskell.nix which doesn't build without -fexternal-dynamic-refs? This would help keeping tabs on it.

Done.

Comment on lines 110 to 115
GhcLibHcOpts += -fPIC -fexternal-dynamic-refs
GhcRtsHcOpts += -fPIC -fexternal-dynamic-refs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be able to add a comment here why -fexternal-dynamic-refs is needed for enableRelocatedStaticLibs?

From the GHC manual: https://downloads.haskell.org/ghc/latest/docs/html/users_guide/phases.html#ghc-flag--fexternal-dynamic-refs

When generating code, assume that entities imported from a different module might be dynamically linked. This flag is enabled automatically by -dynamic.

It is not clear to me why this would be necessary if we are statically linking things. (Although I don't have a good understanding of static linking, so maybe it should be obvious.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit I'm baffled by this too and was kind of expecting the haskell maintainers would know better.
Basically, I'm implementing the solution proposed in this blog post: https://www.tweag.io/blog/2020-09-30-bazel-static-haskell/.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the link to the blog post. It says:

When evaluating template Haskell splices, GHC will execute compiled expressions in its built-in bytecode interpreter and this code has to be compatible with the RTS of GHC itself. In short, a GHC configured with a dynamic RTS will not be able to load static Haskell libraries to evaluate template Haskell splices.

It then goes on to say:

We need to build GHC with a static RTS and to make sure that Haskell code is compiled as position independent code so that it can be loaded into a running GHC for template Haskell splices.

Both of these statements make sense of course, but they don't seem to explain why -fexternal-dynamic-refs is necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GHC man page reads:

Generate code for linking against dynamic libraries

I assume this means that the RTS will have some kind of dynamic library loading facilities even of static (Haskell?) archives, so the byte code interpreter / whatever used by TH will actually work. -fPIC also serves this purpose.

@cdepillabout
Copy link
Member

Maybe I'm misinterpreting, but it seems that this change allows building Haskell packages from pkgsStatic.haskellPackages.* that use Template Haskell? This seems like quite a large improvement, and would potentially be game-changing for a lot of projects.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 2, 2022

allows building Haskell packages from pkgsStatic.haskellPackages.* that use Template Haskell?

Yes, correct.

Copy link
Member

@cdepillabout cdepillabout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from the above comment suggestions, this LGTM!

I tried nix-build -A pkgsStatic.pretty-simple and it appears to build and run correctly!

@cdepillabout
Copy link
Member

@sternenseemann Does this look alright to merge to you?

@sternenseemann
Copy link
Member

sternenseemann commented Mar 3, 2022

I see your point, though I'm not sure how useful the default compiler is, given most packages fails to build. Ideally, we should fix the linking errors, but avoiding gmp is the only way I've found so far.

Have you tried actually building anything that uses FFI in TH? I guess that would need a dynamically linked library as well?

I'm still opposed to changing the meaning of haskell.packages.ghc*, but I would be fine with changing the meaning of top level haskellPackages as a compromise, i.e.:

  haskellPackages = dontRecurseIntoAttrs (
    if stdenv.hostPlatform.isStatic
    then haskell.packages.native-bignum.ghc902
    else haskell.packages.ghc902
  );

  /* … */
  ghc = targetPackages.haskellPackages.ghc or (
    if stdenv.targetPlatform.isStatic
    then haskell.compiler.native-bignum.ghc902
    else haskell.compiler.ghc902
  );

@sternenseemann Does this look alright to merge to you?

FWIW, eval is failing. Edit: unrelated.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 3, 2022

but I would be fine with changing the meaning of top level haskellPackages as a compromise,

Looks great to me! Thanks for the suggestion.

pkgs/development/haskell-modules/generic-builder.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
@cdepillabout
Copy link
Member

I would be fine with changing the meaning of top level haskellPackages as a compromise

I agree this was a good suggestion. I think the current state of this PR makes a lot of sense, having pkgsStatic.haskellPackages explicitly pointing at pkgsStatic.haskell.packages.native-bignum.ghc-902.

Have you tried actually building anything that uses FFI in TH? I guess that would need a dynamically linked library as well?

That's a good question. Do you happen to know a package that uses FFI in TH off the top of your head? I gave it some thought, but I couldn't come up with anything. (I did try compiling pkgsStatic.haskellPackages.zlib, which does successfully compile, but I imagine it is unlikely to use TH.)

Although even if this doesn't work, I don't feel like it should hold up merging this PR, since this PR currently improves the status quo.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 8, 2022

So, can we merge this?

@sternenseemann
Copy link
Member

Probably, I'll try to have a final look soon. A small request: Could you use haskellPackages.mkDerivation over haskell-generic-builder in the commit message? I'd like to have correspondence to an attribute where possible. Rebasing on haskell-updates would be useful anyways, so CI wouldn't be broken by an unrelated problem.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 8, 2022

Commit message updated and rebased.

This change allows loading statically compiled libreries into a running
GHC, thus fixing the build of haskell packages that use TemplateHaskell.
See [1] for the details.

Fixes issue NixOS#61575, NixOS#124284.

[1]: https://www.tweag.io/blog/2020-09-30-bazel-static-haskell/
This change makes the haskell builder run the haddockPhase only if the
haddock program is availble (for example, it's not when cross-compiling).
This works around several build failures where programs attempt to link
to the libgmp.so (which is not available), such as:

    Error loading shared library libgmp.so: No such file or directory
@sternenseemann sternenseemann merged commit 299bfb1 into NixOS:haskell-updates Mar 17, 2022
@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Mar 17, 2022

@sternenseemann Thank you for reviewing!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/statically-linking-haskell-fails-to-link-libffi/18317/2

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/statically-linking-haskell-fails-to-link-libffi/18317/3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: haskell 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants