Skip to content

haskellPackages.*: don’t depend on GHC’s LLVM version#441101

Merged
sternenseemann merged 4 commits intoNixOS:masterfrom
emilazy:push-sqlqyyvworpo
Sep 11, 2025
Merged

haskellPackages.*: don’t depend on GHC’s LLVM version#441101
sternenseemann merged 4 commits intoNixOS:masterfrom
emilazy:push-sqlqyyvworpo

Conversation

@emilazy
Copy link
Member

@emilazy emilazy commented Sep 8, 2025

See commit messages. I’d like this to go to master for the sake of #440273; not sure how to handle the cabal2nix change, as bumping to the latest commit would have unrelated changes creep in.

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@emilazy
Copy link
Member Author

emilazy commented Sep 8, 2025

Theoretically this could result in clashes when combined with forceLlvmCodegenBackend, but in practice that never happens, because forceLlvmCodegenBackend is used for exactly two packages on GHC 9.6.3 and nothing else; see also. (And also there are other existing LLVM dependencies not pinned in this way that would presumably risk similar clashes.)

@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 6.topic: haskell General-purpose, statically typed, purely functional programming language labels Sep 8, 2025
@emilazy
Copy link
Member Author

emilazy commented Sep 8, 2025

Oh no. These actually do force -fllvm.

@emilazy emilazy marked this pull request as draft September 8, 2025 03:18
@emilazy
Copy link
Member Author

emilazy commented Sep 8, 2025

🫠

Suggestions welcome. I still think the cabal2nix change is correct – the other packages are libclang bindings that just so happen to be broken, and don’t set -fllvm – but presumably something needs to be done here. Patching out the -fllvm from both of these and removing the GlomeVec special case from cabal2nix?

@sternenseemann
Copy link
Member

sternenseemann commented Sep 8, 2025

would have unrelated changes creep in

Shouldn't be a problem.

sternenseemann
sternenseemann reviewed Sep 8, 2025
View reviewed changes
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.

Agree in general, but we need to match GHC whenever we pass LLVM tools. Clashes shouldn't be possible in library cases thanks to nativeBuildInputs / buildInput.

@emilazy emilazy force-pushed the push-sqlqyyvworpo branch 2 times, most recently from 7528cd7 to 4250b2e Compare September 8, 2025 17:30
@emilazy emilazy marked this pull request as ready for review September 8, 2025 17:31
sternenseemann
sternenseemann reviewed Sep 8, 2025
View reviewed changes
@sternenseemann sternenseemann mentioned this pull request Sep 8, 2025
Merged
13 tasks
@emilazy emilazy mentioned this pull request Sep 8, 2025
Merged
@sternenseemann sternenseemann mentioned this pull request Sep 8, 2025
Merged
13 tasks
@wolfgangwalther wolfgangwalther mentioned this pull request Sep 9, 2025
Merged
13 tasks
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 9, 2025
@sternenseemann
Copy link
Member

sternenseemann commented Sep 9, 2025

Addressed #440761 (comment) here.

@emilazy
Copy link
Member Author

emilazy commented Sep 9, 2025

Looks like the GlomeVec change got accidentally squashed? (Or at least the resulting commit message looks wrong.)

@sternenseemann
Copy link
Member

sternenseemann commented Sep 9, 2025

Looks like the GlomeVec change got accidentally squashed? (Or at least the resulting commit message looks wrong.)

This is intentional so that there is no intermediate broken commit.

@sternenseemann
Copy link
Member

sternenseemann commented Sep 10, 2025

I think an entry in the release notes would be nice since haskell.packages.*.llvmPackages was documented. I can also write that up. Other than that, I think we can go ahead with this unless someone else has comments?

@wolfgangwalther
Copy link
Contributor

wolfgangwalther commented Sep 10, 2025

No objections, on the LLVM stuff I am going to trust both of you ;)

@emilazy
Copy link
Member Author

emilazy commented Sep 10, 2025

This is intentional so that there is no intermediate broken commit.

Hmm, wouldn’t it just have a useless LLVM dependency, like haskellPackages.spaceprobe? Or is libraryPkgconfigDepends special?

No big deal either way, though. This LGTM, would be happy to write a release note but also happy to let you do it.

emilazy and others added 4 commits September 11, 2025 15:11
@emilazy @sternenseemann
haskellPackages.spaceprobe: don’t force the LLVM backend
9d1b1aa
@emilazy @sternenseemann
haskellPackages.GlomeVec: don’t force the LLVM backend
6409663
@emilazy @sternenseemann
haskellPackages.cabal2nix-unstable: 2025-09-06 -> 2025-09-09
fab98a0 
Co-authored-by: sternenseemann <[email protected]>
@emilazy @sternenseemann
haskellPackages.llvmPackages: drop
921f5db 
There’s no sensible notion of a Haskell package set’s LLVM, only a
GHC’s LLVM. `haskellPackages.ghc.llvmPackages` is still accessible,
although you probably don’t actually want to use it very often.

Co-authored-by: sternenseemann <[email protected]>
@sternenseemann
Copy link
Member

sternenseemann commented Sep 11, 2025

Hmm, wouldn’t it just have a useless LLVM dependency, like haskellPackages.spaceprobe? Or is libraryPkgconfigDepends special?

I guess it was too late when I was thinking about this and I only considered the other ordering. Restored the commit.

@sternenseemann sternenseemann added this pull request to the merge queue Sep 11, 2025
Merged via the queue into NixOS:master with commit 14c1ab0 Sep 11, 2025
29 of 31 checks passed
sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request Sep 11, 2025
@sternenseemann
doc/rl-2511: note removal of llvmPackages from haskell.packages.*
d84b2f1 
This should be the only observable breakage from NixOS#441101 /
921f5db.
@sternenseemann sternenseemann mentioned this pull request Sep 11, 2025
Merged
13 tasks
qweered pushed a commit to qweered/nixpkgs that referenced this pull request Sep 11, 2025
@sternenseemann @qweered
doc/rl-2511: note removal of llvmPackages from haskell.packages.*
94e739e 
This should be the only observable breakage from NixOS#441101 /
921f5db.
@emilazy emilazy deleted the push-sqlqyyvworpo branch September 17, 2025 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sternenseemann sternenseemann sternenseemann approved these changes

@wolfgangwalther wolfgangwalther Awaiting requested review from wolfgangwalther

@maralorn maralorn Awaiting requested review from maralorn

Assignees

No one assigned

Labels

6.topic: haskell General-purpose, statically typed, purely functional programming language 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. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@emilazy @sternenseemann @wolfgangwalther