Skip to content

Comments

llvmPackages_{12,13,14,15,16,17}: drop#440273

Merged
pyrox0 merged 23 commits intoNixOS:masterfrom
emilazy:push-vvwptmwltlny
Sep 15, 2025
Merged

llvmPackages_{12,13,14,15,16,17}: drop#440273
pyrox0 merged 23 commits intoNixOS:masterfrom
emilazy:push-vvwptmwltlny

Conversation

@emilazy
Copy link
Member

@emilazy emilazy commented Sep 4, 2025

We currently package ten stable versions of LLVM going back 4½ years. Older versions of LLVM don’t get upstream fixes, and require significant derivation complexity and patching to keep working, especially the further back we go. Dropping them eases the burden of LLVM maintenance and saves a bunch of expensive compiler builds on Hydra.

The packages marked broken or dropped here will be the only remaining users in the tree after the remaining in‐flight PRs are merged. I made varying degrees of effort to get all of them building with newer versions, as per the list of preceding PRs below, but was not successful in these cases. The upstreams have varying degrees of activity. If anyone can get them building with LLVM ≥ 18 then they can be removed from this PR or re‐added after it is merged. I’ve left some notes in the commit messages and code comments where applicable for anyone who wants to try. I do hope that we will drop old compilers more regularly going forward, though, so I wouldn’t recommend making a one‐off heroic effort if it’s expected that it will end up bitrotting again a couple releases down the line. In particular, I hope that we will be able to drop at least LLVM 18 for 26.05.

This concludes my work to drop old compilers for 25.11 – reviews of open PRs are appreciated:

cc’ing maintainers of packages marked broken or dropped in this PR:

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 emilazy requested review from a team September 4, 2025 23:24
@emilazy emilazy added the 8.has: clean-up This PR removes packages or removes other cruft label Sep 4, 2025
@emilazy
Copy link
Member Author

emilazy commented Sep 5, 2025

Sorry for the typo @niklaskorz 😅

Copy link
Member

@niklaskorz niklaskorz left a comment

Choose a reason for hiding this comment

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

re shader-slang, I'll reference the discussion in #436961

TL;DR: the LLVM mode is in a weird (non-functional) state anyway, so we'll probably have to refactor or remove it. Upstream currently doesn't seem capable to keep up with recent LLVM versions, so removing withLLVM might even be preferred for now.

I'm fine with simply marking it broken here so we can defer that decision.

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 5, 2025
@emilazy
Copy link
Member Author

emilazy commented Sep 5, 2025

Cool, thank you for the pointer! That is one of the packages I was most worried about so it is good to hear it at least won't be getting any more broken. I think I did not spend too long seeing how much work it would be to get it building with newer LLVM versions, so it's possible that it might be an upstreamable patch away from working. But I lack the context to know whether it'd be worth the energy, beyond what they have written down in their docs. I suppose if it's like most use cases for shaders then CPU execution isn't a priority.

Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Thanks, @emilazy.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Sep 6, 2025
Copy link
Member

@alyssais alyssais left a comment

Choose a reason for hiding this comment

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

Lovely :)

@emilazy
Copy link
Member Author

emilazy commented Sep 6, 2025

Thanks for the kind words :) I hope that we can drop old compilers at a more steady pace going forward now that the up‐front cost of catching up has been paid.

I believe I have fixed most of the review comments; would appreciate reviewers double‐checking and marking resolved anything they think has been appropriately handled.

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.

Nice to see some of the ugly conditionals disappear (e.g. from #347887 and #348568 🤐)!

  • 54d55876a456180a837ca467b6f6124e199cab15 seems to prematurely remove some patches that belong in the LLVM 15 removal commit.

Copy link
Member

Choose a reason for hiding this comment

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

This can't be folded into the previous conditional in the LLVM 13 removal, but needs to be its own, unconditional list.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thank you. Will split this up between the two commits.

Copy link
Member

Choose a reason for hiding this comment

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

Do we…?

Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently not, since we’ve gone this long without it? It was added in 99eeff0 without explanation. I guess it was intended to resolve… an issue with Musl on PowerPC defining pt_regs?

I don’t know if the LLDB source would still run into this, but this Musl commit implies that it has not been an issue since 2019, regardless of LLVM version. In any case I think we would not accept a patch for an issue with PowerPC and Musl today without it being sent upstream to LLVM or Musl.

The i686 stuff in that patch, I don’t know. It’s not even related to the <sys/procfs.h> issue so I don’t know why it was added there in b4ee532, but I think LLDB on i686 is also marginal enough that any relevant fixes should go upstream, if they’re even still needed. cc @rrbutani who added it.

In any case it would cause a rebuild to port it forward here, of course.

@emilazy
Copy link
Member Author

emilazy commented Sep 7, 2025

Nice to see some of the ugly conditionals disappear (e.g. from #347887 and #348568 🤐)!

I do appreciate the effort to get those unified with the rest of the LLVM builds :) But it also definitely exposed how much divergence there was across our wide version range, so I’m happy for the packaging to get a lot less tangled. (Though some of it could have been resolved by just eating rebuilds, of course, but there’s still essential stuff around build system differences and tons of patches that is satisfying to drop.)

  • 54d5587 seems to prematurely remove some patches that belong in the LLVM 15 removal commit.

You’re right. getVersionFile has even worse semantics than I thought. I thought that if there are entries in common/patches.nix then it only looks at those, and only otherwise it falls back to looking in the versioned directory directly. But actually any omitted range falls back to a version‐specific directory, and this is seemingly used only for LLVM 15. WTF? I will fix this.

I think the whole patching machinery should probably be done differently. I have ideas for that, but I need a break from looking at this stuff first.

@emilazy emilazy force-pushed the push-vvwptmwltlny branch 2 times, most recently from 3021a45 to c0e02e5 Compare September 7, 2025 19:14
@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 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related labels Sep 7, 2025
@nix-owners nix-owners bot requested a review from Ericson2314 September 7, 2025 19:20
@emilazy
Copy link
Member Author

emilazy commented Sep 14, 2025

Everything required here has now been merged into master, and this rebuilds only tests.cc-wrapper.supported and things downstream of the manual, as expected. I also added a release note. I’ll leave the button for someone else to push :)

@pyrox0 pyrox0 added this pull request to the merge queue Sep 15, 2025
Merged via the queue into NixOS:master with commit f77e9a2 Sep 15, 2025
27 checks passed
@github-project-automation github-project-automation bot moved this from In progress to Done in Picking up garbage Sep 15, 2025
@emilazy emilazy deleted the push-vvwptmwltlny branch September 15, 2025 15:32
@samestep
Copy link
Contributor

samestep commented Oct 7, 2025

@emilazy thank you for the awesome work in this PR!

Regarding Slang, I agree with your and @niklaskorz's assessment that it's not a high priority:

Cool, thank you for the pointer! That is one of the packages I was most worried about so it is good to hear it at least won't be getting any more broken. I think I did not spend too long seeing how much work it would be to get it building with newer LLVM versions, so it's possible that it might be an upstreamable patch away from working. But I lack the context to know whether it'd be worth the energy, beyond what they have written down in their docs. I suppose if it's like most use cases for shaders then CPU execution isn't a priority.

In case someone does decide to try that in the future, though, shader-slang/slang#8038 should be a good starting point. Here are the upstream LLVM changes that accounts for:

As I mentioned in shader-slang/slang#8028 (comment), that PR did build successfully, but some of the tests did not pass, which is why it wasn't merged back then.

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 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 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: 2 This PR was reviewed and approved by two persons.

Projects

Development

Successfully merging this pull request may close these issues.

9 participants