Skip to content

haskellPackages.llvm-ffi: set LLVM#227424

Merged
maralorn merged 1 commit intoNixOS:haskell-updatesfrom
MaxHearnden:llvm-ffi
Apr 24, 2023
Merged

haskellPackages.llvm-ffi: set LLVM#227424
maralorn merged 1 commit intoNixOS:haskell-updatesfrom
MaxHearnden:llvm-ffi

Conversation

@MaxHearnden
Copy link
Copy Markdown
Contributor

@MaxHearnden MaxHearnden commented Apr 21, 2023

Description of changes

Overrides LLVM from null to llvmPackages_13.libllvm

Fixes #227412

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release 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.

@github-actions github-actions bot added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label Apr 21, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Apr 21, 2023
@thielema
Copy link
Copy Markdown
Contributor

Must llvm-ffi be removed from broken.yaml, too?

@maralorn
Copy link
Copy Markdown
Member

Yeah, that’s correct please remove the package from broken.yaml, because hackage-packages.nix is an auto generated file. Also hydraPlatforms = lib.platforms.none; line above the broken line should also be deleted.

I am not 100% certain about the LLVM override. @sternenseemann is that okay this way?

@MaxHearnden
Copy link
Copy Markdown
Contributor Author

Just managed to properly unmark llvm-ffi as broken.

In case anyone else wants to unmark a package as broken the steps are

  1. update pkgs/development/haskell-modules/configuration-hackage2nix/broken.yaml to remove the fixed package
  2. run maintainers/scripts/haskell/regenerate-transitive-broken-packages.sh to update transitive-broken.yaml
  3. run maintainers/scripts/haskell/regenerate-hackage-packages.sh to update hackage-packages.nix

The documented method in HACKING.md does not cover removing packages from broken.yaml

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Apr 22, 2023
@maralorn
Copy link
Copy Markdown
Member

maralorn commented Apr 22, 2023

@MaxHearnden that’s actually not quite right.

Running regenerate-transitive-broken-packages.sh does not look into the broken.yaml at all but uses the generated hackage-packages.nix file. Thus, if you only change broken.yaml and then run regenerate-transitive-….sh, the changes to broken.yaml won’t be reflected.

Also, the transitive-broken.yaml only affects whether we build packages on hydra, not if they are marked broken, so in general it’s not very urgent to update it, and we always do it before merging haskell-updates into master. So we generally don’t expect people to run it in their PRs, but it’s nice if you do.

So the correct steps are:

  1. update pkgs/development/haskell-modules/configuration-hackage2nix/broken.yaml to remove the fixed package.
  2. Run maintainers/scripts/haskell/regenerate-hackage-packages.sh to update hackage-packages.nix.

and then optionally if you want to go the extra mile and see the effect for all dependencies on hydra as soon as possible:

  1. run maintainers/scripts/haskell/regenerate-transitive-broken-packages.sh to update transitive-broken.yaml
  2. run maintainers/scripts/haskell/regenerate-hackage-packages.sh again.

@MaxHearnden
Copy link
Copy Markdown
Contributor Author

I have hopefully done it correctly this time

@ofborg ofborg bot added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 2.status: merge conflict This PR has merge conflicts with the target branch 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Apr 22, 2023
@sternenseemann
Copy link
Copy Markdown
Member

It is preferred to rebase PR branches on top of the target instead of merging the target branch back in again.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does it more sense to use the default LLVM (i.e. self.llvmPackages.libllvm) used for the compiler (maybe just to reduce closure size in some cases)? Seems like llvm-ffi supports multiple versions.

@thielema
Copy link
Copy Markdown
Contributor

thielema commented Apr 23, 2023 via email

@maralorn
Copy link
Copy Markdown
Member

We currently use llvm 12, and I don’t expect that to change before branch-off.

@thielema
Copy link
Copy Markdown
Contributor

thielema commented Apr 23, 2023 via email

@sternenseemann
Copy link
Copy Markdown
Member

GHC chooses a range of supported LLVM versions and we have different ones per package set as a result. Since the newer GHC versions already support LLVM 14, it seems reasonable to assume that llvm-ffi doesn't keep up, so maybe pinning the version is the best course of action. Worst case is that you have to download LLVM twice when building haskellPackages.llvm-ffi on e.g. aarch64-linux.

@MaxHearnden
Copy link
Copy Markdown
Contributor Author

I have, regenerated haskell packages using the updated script, set llvm to llvm 12 and enabled the cabal flag.

@maralorn
Copy link
Copy Markdown
Member

But if we are pinning for the convenience of llvm-ffi anyway, why not leave it at pinning llvm 13?

@MaxHearnden
Copy link
Copy Markdown
Contributor Author

I have simplified it to only using llvm 13 although pinning on llvm 12 would ensure that it doesn't break when llvm-ffi updates to llvm 14

@maralorn
Copy link
Copy Markdown
Member

Fair point, but then we notice the build error and can update the pin to llvm 14 so that’s basically what we want.

Thank you for bearing with us!

@maralorn maralorn merged commit a3617cb into NixOS:haskell-updates Apr 24, 2023
@thielema
Copy link
Copy Markdown
Contributor

thielema commented Oct 9, 2023

I have updated llvm-ffi and llvm-tf to support LLVM-9 to LLVM-16. What version will be the default for the next NixOS release?

@maralorn
Copy link
Copy Markdown
Member

@thielema Apparently LLVM 16, if I understand this correctly. https://discourse.nixos.org/t/breaking-changes-announcement-for-unstable/17574/37

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants