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

Fix GHC not building with musl #129289

Merged
merged 16 commits into from
Jul 12, 2021
Merged

Conversation

nh2
Copy link
Contributor

@nh2 nh2 commented Jul 5, 2021

Motivation for this change

Fixes binary GHCs segfaulting on pkgsMusl. See #129247, #118731.

All Nix-built Haskell compilers build now on pkgsMusl:

NIX_PATH=nixpkgs=. nix-build --no-link --expr 'with import <nixpkgs> {}; { inherit (pkgsMusl.haskell.compiler) ghc884 ghc8104 ghc901 ghcHEAD; }'

Key changes included (see commits for details):

  • Switch ghc8102Binary to use GHC HQ musl bindist instead of patching glibc bindist with musl.
  • Use this better binary bindist to bootstrap all nix-built GHCs for pkgsMusl
  • Add missing hardeningDisable = [ "pie" ] where needed.
  • Swith to build GHC without sphinx by default for musl (cross already does this) to avoid huge amount of build dependencies, some of which may not work with musl yet (or break frequently). This will reduce maintenance burden for projects like static-haskell-nix significantly, and make it cheaper to build musl-based GHC on Hydra.
    • Add new options to control this.
  • Add new structure to declare which ncurses version and similar dependencies the bindists have, and a postUnpack step to ensure at build time that those are actually correct.
    • The build now complains when your expectation is wrong regarding what ncurses a bindist needs.
  • Fix i686 build of ghc8102Binary as a drive-by contribution.
  • Lots of explanatory extra comments.
Extra review needed for
  • My usage of buildPlatform, targetPlatform etc.
  • There are some some cross options that I slightly touch. People who care about cross Haskell should ideally check out if everything remains working.
TODO
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Relase notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

CC people who care about this and may want to try whether it breaks anything, and others qualified to review:

@NixOS/haskell @expipiplus1 @peti @bgamari @nomeata @domenkozar @cdepillabout @dtzWill @lunaris @NorfairKing @maralorn @sternenseemann @dtzWill @Ericson2314 @matthewbauer @aveltras

`glibcLocales` only exists when glibc is used.

Similar to commit:

    8727284 - haskell: only use glibcLocales when using glibc
@nh2
Copy link
Contributor Author

nh2 commented Jul 5, 2021

Looks like I broke the nixpkgs manual with my new doc-related config options:

ghc.mk:184: *** BUILD_SPHINX_PDF=YES, but `xelatex` was not found. Install `xelatex`, then rerun `./configure`.

builder for '/nix/store/zy6asc6hsdrqg8s3nspcx7x99awnibs1-ghc-8.10.4.drv' failed with exit code 2

https://github.com/NixOS/nixpkgs/runs/2986064008?check_suite_focus=true#step:5:3174

Not sure why yet.

Edit:

Probably because GHC's ./configure does dynamic setting of BUILD_SPHINX_PDF:

    if test -n "$XELATEX"; then
        BUILD_SPHINX_PDF=YES
    else
        BUILD_SPHINX_PDF=NO
    fi

https://gitlab.haskell.org/ghc/ghc/-/blob/6a01e28f4204ec17c587931311711fa76e0ea08d/configure.ac#L1383-1387

and by putting it explicitly in buildMK I override that.

@nh2 nh2 force-pushed the issue-129247-ghc-musl-fixes branch from 7123088 to a84215b Compare July 5, 2021 02:04
@nh2
Copy link
Contributor Author

nh2 commented Jul 5, 2021

Will need to target staging as this is a mass rebuild (probably via pandoc? I haven't investigated yet).

Edit: OK, investigated:

Indeed it's via pandoc, e.g.

  /nix/store/4qwgwxajaar8jl2igs1ilii3xcapzszd-pandoc-2.14.0.2.drv
  /nix/store/3rwp9r2x0zap0baf9yv0kgw0r7qwqxws-xen-4.10.4.drv

It's actually "only" 500 non-haskellPackages being rebuilt per architecture.

@maralorn
Copy link
Member

maralorn commented Jul 5, 2021

Well pandoc is not really the point here, I think. I believe we use the 8102 binary to build our 8104 ghc? So this is a full haskellPackages rebuild. We normally do those on haskell-updates, so I don‘t think this needs to target staging, but haskell-updates.

@r-rmcgibbo
Copy link

r-rmcgibbo commented Jul 5, 2021

Result of nixpkgs-review pr 129289 at a84215b run on aarch64-linux 1

29 packages marked as broken and skipped:
  • grass
  • libvmi
  • mvapich
  • python38Packages.dask-gateway
  • python38Packages.dask-jobqueue
  • python38Packages.dask-ml
  • python38Packages.dask-mpi
  • python38Packages.dask-xgboost
  • python38Packages.datashader
  • python38Packages.fipy
  • python38Packages.pygmt
  • python38Packages.streamz
  • python38Packages.stumpy
  • python38Packages.tensorflowWithCuda
  • python39Packages.dask-gateway
  • python39Packages.dask-jobqueue
  • python39Packages.dask-mpi
  • python39Packages.dask-xgboost
  • python39Packages.fipy
  • python39Packages.pygmt
  • python39Packages.streamz
  • python39Packages.tensorflowWithCuda
  • qemu_xen
  • qemu_xen-light
  • qemu_xen_4_10
  • qemu_xen_4_10-light
  • qlandkartegt
  • qubes-core-vchan-xen
  • yosys-bluespec
6987 packages skipped due to time constraints:
  • EBTKS
  • agdaPackages.agda-categories
  • agdaPackages.agda-prelude
  • agdaPackages.cubical
  • agdaPackages.functional-linear-algebra
  • agdaPackages.generic
  • agdaPackages.iowa-stdlib
  • agdaPackages.standard-library
  • apacheHttpdPackages.mod_tile (apacheHttpdPackages_2_4.mod_tile)
  • apostrophe
  • ...
2 packages built successfully:
  • argp-standalone
  • haskell.compiler.ghc8102BinaryMinimal
5 suggestions:
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/argp-standalone/default.nix:31:53:

       |
    31 |        lib.optionals stdenv.hostPlatform.isDarwin [ patch-argp-fmtstream ]
       |                                                     ^
    
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/argp-standalone/default.nix:32:52:

       |
    32 |     ++ lib.optionals stdenv.hostPlatform.isLinux [ patch-throw-in-funcdef patch-shared ];
       |                                                    ^
    
  • warning: name-and-version

    Did you mean to pass pname instead of name to mkDerivation?

    Near pkgs/development/compilers/ghc/8.10.2-binary.nix:135:3:

        |
    135 |   name = "ghc-${version}-binary";
        |   ^
    

    Near pkgs/development/compilers/ghc/8.10.2-binary.nix:133:10:

        |
    133 |   inherit version;
        |          ^
    
  • warning: unclear-gpl

    gpl2 is a deprecated license, please check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

    Near pkgs/development/libraries/argp-standalone/default.nix:59:5:

       |
    59 |     license = licenses.gpl2;
       |     ^
    
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/argp-standalone/default.nix:32:75:

       |
    32 |     ++ lib.optionals stdenv.hostPlatform.isLinux [ patch-throw-in-funcdef patch-shared ];
       |                                                                           ^
    

Result of nixpkgs-review pr 129289 at a84215b run on x86_64-linux 1

2 packages marked as broken and skipped:
  • googleearth-pro
  • grass
7117 packages skipped due to time constraints:
  • EBTKS
  • agda (agdaPackages.agda)
  • agdaPackages.agda-categories
  • agdaPackages.agda-prelude
  • agdaPackages.cubical
  • agdaPackages.functional-linear-algebra
  • agdaPackages.generic
  • agdaPackages.iowa-stdlib
  • agdaPackages.standard-library
  • apacheHttpdPackages.mod_tile (apacheHttpdPackages_2_4.mod_tile)
  • ...
2 packages built successfully:
  • argp-standalone
  • haskell.compiler.ghc865Binary
7 suggestions:
  • warning: unclear-gpl

    gpl2 is a deprecated license, please check if project uses gpl2Plus or gpl2Only and change meta.license accordingly.

    Near pkgs/development/libraries/argp-standalone/default.nix:59:5:

       |
    59 |     license = licenses.gpl2;
       |     ^
    
  • warning: maintainers-missing

    Package does not have a maintainer. Consider adding yourself?

    Near pkgs/development/compilers/ghc/8.6.5-binary.nix:188:3:

        |
    188 |   meta = rec {
        |   ^
    
  • warning: name-and-version

    Did you mean to pass pname instead of name to mkDerivation?

    Near pkgs/development/compilers/ghc/8.10.2-binary.nix:135:3:

        |
    135 |   name = "ghc-${version}-binary";
        |   ^
    

    Near pkgs/development/compilers/ghc/8.10.2-binary.nix:133:10:

        |
    133 |   inherit version;
        |          ^
    
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/argp-standalone/default.nix:32:75:

       |
    32 |     ++ lib.optionals stdenv.hostPlatform.isLinux [ patch-throw-in-funcdef patch-shared ];
       |                                                                           ^
    
  • warning: name-and-version

    Did you mean to pass pname instead of name to mkDerivation?

    Near pkgs/development/compilers/ghc/8.6.5-binary.nix:38:3:

       |
    38 |   name = "ghc-${version}-binary";
       |   ^
    

    Near pkgs/development/compilers/ghc/8.6.5-binary.nix:36:3:

       |
    36 |   version = "8.6.5";
       |   ^
    
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/argp-standalone/default.nix:31:53:

       |
    31 |        lib.optionals stdenv.hostPlatform.isDarwin [ patch-argp-fmtstream ]
       |                                                     ^
    
  • warning: missing-patch-comment

    Consider adding a comment explaining the purpose of this patch on the line preceeding.
    Near pkgs/development/libraries/argp-standalone/default.nix:32:52:

       |
    32 |     ++ lib.optionals stdenv.hostPlatform.isLinux [ patch-throw-in-funcdef patch-shared ];
       |                                                    ^
    

@nh2 nh2 changed the base branch from master to haskell-updates July 5, 2021 12:08
@nh2
Copy link
Contributor Author

nh2 commented Jul 5, 2021

I don‘t think this needs to target staging, but haskell-updates.

Done.

nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Jul 6, 2021
* include fixes for GHC segfaulting: (NixOS/nixpkgs#129247, NixOS/nixpkgs#129289)
* musl 1.2.2, including this important fix from musl 1.2.0:
  #98 (comment)
* workaround for fontforge execution failure due to wrong RPATH:
  NixOS/nixpkgs#94126
* ilmbase musl compilation error:
  NixOS/nixpkgs#94205
* mesa: Fix `-Werror=int-conversion` build error on musl:
  NixOS/nixpkgs#94207

survey:

* Update file in which stackage packages are listed.
  See nixpkgs commit

      7f236bd4 - hackage2nix: Split configuration, auto disable hydra builds
@nh2 nh2 force-pushed the issue-129247-ghc-musl-fixes branch from 8d536f3 to d344593 Compare July 7, 2021 13:05
@nh2 nh2 force-pushed the issue-129247-ghc-musl-fixes branch from 6039bd6 to 1dd108d Compare July 9, 2021 17:43
@nh2
Copy link
Contributor Author

nh2 commented Jul 9, 2021

I've addressed all feedback.

static-haskell-nix is working quite well with these changes: nh2/static-haskell-nix#98

Ready to merge to haskell-updates from my side.

Copy link
Member

@sternenseemann sternenseemann left a comment

Choose a reason for hiding this comment

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

Looks good to me, although I haven't tried building anything with these changes. I'll try building the default compiler and I guess we'll check for extra regressions via the normal haskell-updates workflow.

nh2 added 11 commits July 10, 2021 02:49
With this check, we no longer don't notice when the upstream bindist
changes its dependencies (e.g. because a newer Debian version is used
that uses a new `ncurses` version).
None of the current bindists appear to contain these paths in their
`ghc-stage2` binary.
…OS#118731 NixOS#129247.

This commit replaces the musl + glibc hackery in the GHC bindist
compiler by using the new musl based bindist that GHC HQ provides
(built on Alpine).
We could alternatively also use a nix-built musl boostrap compiler,
but it seems nicer to use the GHC HQ one for now.

This fixes the compiler built by
`pkgsMusl.haskell.compiler.ghc8102Binary` segfaulting (NixOS#118731)
since the commit

    5e2311d - musl: 1.2.1 -> 1.2.2

concretely, musl commit

    01c7920f - remove redundant pthread struct members repeated for layout purposes

which I suspect breaks some glibc/musl ABI compatibility that may have
existed accidentally until then.

The added

    lib.optional stdenv.targetPlatform.isMusl "pie";

also fixes that the packaged bindist compiler cannot create a binary
in its `installCheck` phase (and overall); see detail explanation
in NixOS#129247.


While this does not fix `ghc865Binary` with musl, it at least prevents
that the other, newer errors are shadowed (see NixOS#129247).
…compiler.

This addresses the fact that `ghc865Binary` segfaults on musl
(see NixOS#118731) because of the glibc+musl mix used in there.

With the previous commits, `ghc8102Binary` was changed to use
the musl-based bindist from GHC HQ instead, which works.

With this change, all nix Haskell compilers builds on musl:

    NIX_PATH=nixpkgs=. nix-build --no-link --expr 'with import <nixpkgs> {}; { inherit (pkgsMusl.haskell.compiler) ghc884 ghc8104 ghc901 ghcHEAD; }'
…musl

Adds new package options:

* enableDocs
* enableHaddockProgram

to control whether to build Sphinx docs, and GHC haddocks and the
haddock program.

Unfortunately currently the building of the `haddock `program
and generating GHC docs are mixed into one option, see:
https://gitlab.haskell.org/ghc/ghc/-/issues/20077

Making Sphinx docs disableable, and disabling them by default
for Musl and cross builds, makes it much easier to provide these
builds without having to support Sphinx's enormous dependency
tree for those ways of building.
…and musl.

This allows to implement the "HACK" mentioned in the commit to
build `pkgsMusl` GHCs on Hydra without failing evaluation on Darwin.

Reference of the discussion:
NixOS#129289 (comment)

Patch contributed by @SternI.
@nh2 nh2 force-pushed the issue-129247-ghc-musl-fixes branch from 4b7dc37 to 63b1e6e Compare July 10, 2021 00:50
@nh2 nh2 merged commit c114cd4 into NixOS:haskell-updates Jul 12, 2021
nomeata added a commit to nomeata/ic-hs that referenced this pull request Jul 12, 2021
and update nixpkgs to master, so that that PR applies nicely. We can
worry about going back to stable later (if that fix doesn’t make it
anyways)
ggreif pushed a commit to dfinity/ic-hs that referenced this pull request Jul 13, 2021
* Pull in NixOS/nixpkgs#129289

and update nixpkgs to master, so that that PR applies nicely. We can
worry about going back to stable later (if that fix doesn’t make it
anyways)

* Bump naersk too
* Bump GHC version in github action
nh2 added a commit to nh2/static-haskell-nix that referenced this pull request Jul 20, 2021
* include fixes for GHC segfaulting: (NixOS/nixpkgs#129247, NixOS/nixpkgs#129289)
* musl 1.2.2, including this important fix from musl 1.2.0:
  #98 (comment)
* workaround for fontforge execution failure due to wrong RPATH:
  NixOS/nixpkgs#94126
* ilmbase musl compilation error:
  NixOS/nixpkgs#94205
* mesa: Fix `-Werror=int-conversion` build error on musl:
  NixOS/nixpkgs#94207

survey:

* Update file in which stackage packages are listed.
  See nixpkgs commit

      7f236bd4 - hackage2nix: Split configuration, auto disable hydra builds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants