llvmPackages_{13,14,15,16,17,18,git}: commonify the default.nix #325175

RossComputerGuy · 2024-07-07T04:45:14Z

Description of changes

Drop all default.nix files for LLVM versions and simply the entrypoint to a common place. This will greatly reduce the amount of work to maintain LLVM and simply updating. Not bothering with LLVM 12 as it is a legacy version and it sits with the pre-monorepo setup which would increase complexity for this PR.

Things done

Add a 👍 reaction to pull requests you find important.

pkgs/top-level/all-packages.nix

pwaller · 2024-07-07T11:10:19Z

pkgs/development/compilers/llvm/default.nix

+        sha256 = "sha256-Tlk+caav7e7H6bha8YQyOl+x2iNk9iH7xKpHQkWQyJ4=";
+      };
+    }
+  ];


There are a couple of ways to implement this and I'm not deadset on anything specific, just adding suggestions.

Suggestion, untested! - feel free to ignore: condensed but possibly a bit opaque:

versions = { "13.0.1".officialRelease.sha256 = "06dv6h5dmvzdxbif2s8njki6h32796v368dyb5945x8gjj72xh7k"; "14.0.6".officialRelease.sha256 = "sha256-vffu4HilvYwtzwgq+NlS26m65DGbp6OSSne2aje1yJE="; "15.0.7".officialRelease.sha256 = "sha256-wjuZQyXQ/jsmvy6y1aksCcEDXGBjuhpgngF3XQJ/T4s="; "16.0.6".officialRelease.sha256 = "sha256-fspqSReX+VD+Nl/Cfq+tDcdPtnQPV1IRopNDfd5VtUs="; "17.0.6".officialRelease.sha256 = "sha256-8MEDLLhocshmxoEBRSKlJ/GzJ8nfuzQ8qn0X/vLA+ag="; "18.1.8".officialRelease.sha256 = "sha256-iiZKMRo/WxJaBXct9GdAcAT3cz9d9pnAcO1mmR6oPNE="; "19.0.0-git".gitRelease = { rev = "9b9405621bcc55b74d2177c960c21f62cc95e6fd"; rev-version = "19.0.0-unstable-2024-06-30"; sha256 = "sha256-Tlk+caav7e7H6bha8YQyOl+x2iNk9iH7xKpHQkWQyJ4="; }; } # ... llvmPackages = lib.mapAttrs' (version: args: mkPackage (args // { inherit version; })) versions

Another thought is to have a mechanism for overriding/extending it: take llvmVersions as a parameter defaulting to empty set. Then do versions // llvmVersions -- this would potentially allow users to override LLVM versions by putting llvmVersions = { ... } in their overlay. The issue I see with this is of keying the llvmPackage_{version} on the major version but having the attribute be a full version.

How would making the version list an attr set work with the officialRelease or gitRelease stuff?

That is already accounted for in my suggested code block, "13.0.1".officialRelease.sha256 expands to "13.0.1" = { officialRelease = { sha256 = ... }; } such that the only missing attribute is the version, which we reinject via mapAttrs' in mkPackage (args // { inherit version; } so that it reads officialRelease = { version = ..., sha256 = ...}. Then the last one is .gitRelease.

Ok, I made it work with an attr set.

RossComputerGuy · 2024-07-07T14:39:53Z

If someone nixpkgs-review's this and there's a rebuild count of 0 before Ofborg gets to this, I'll merge this.

RossComputerGuy · 2024-07-07T15:43:03Z

error: attribute 'hostPlatform' missing

       at /ofborg/checkout/1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-2/pkgs/stdenv/darwin/override-sdk.nix:412:29:

          411|       # deployment target does not require replacing the libc or SDK dependencies.
          412|       // lib.optionalAttrs (old.hostPlatform.darwinSdkVersion != darwinSdkVersion) {
             |                             ^
          413|         allowedRequisites = null;

Weird CI error.

RossComputerGuy · 2024-07-07T18:26:15Z

Couldn't rebase on GitHub website since I can't access one of my systems right now. Will fix up the commits as soon as I can, maybe this will fix the CI fail.

reckenrode · 2024-07-07T19:29:47Z

pkgs/top-level/aliases.nix

-  llvmPackages_git = recurseIntoAttrs (callPackage ../development/compilers/llvm/git {
-    inherit (stdenvAdapters) overrideCC;
-    buildLlvmTools = buildPackages.llvmPackages_git.tools;
-    targetLlvmLibraries = targetPackages.llvmPackages_git.libraries or llvmPackages_git.libraries;
-    targetLlvm = targetPackages.llvmPackages_git.llvm or llvmPackages_git.llvm;
-  });


This was added to aliases to prevent packages in nixpkgs from using it. Dropping it from aliases would change that. Is it intended that llvmPackages_git be usable in nixpkgs going forward?

I don't really care if LLVM git is or isn't in alias. I just moved it so all versions past 12 are using the commonified default.nix. Probably isn't a bad idea either way so Ofborg tests for the weekly git updates.

This was in aliases.nix on purpose — if packages in Nixpkgs start relying on it, it blocks us from keeping it up to date, and the point of llvmPackages_git is to always be up to date. Could you please move it back?

(For the same reason, e.g. linuxPackages_latest is in aliases.nix on purpose, because when it wasn't it created tension between keeping it up to date and keeping dependent packages working.)

linuxPackages_latest is in aliases.nix on purpose

Where is linuxPackages_latest in aliases.nix? I don't see it in there.

Moved back into aliases.nix in #331880

Where is linuxPackages_latest in aliases.nix? I don't see it in there.

Sorry, I was mistaken about that. It's not in aliases.nix — it's been prevented from being used in Nixpkgs entirely through consistent review, and providing an alternative linuxHeaders package that doesn't promise to be "latest", even though in practice we update it as soon as we can.

RossComputerGuy · 2024-07-08T02:03:07Z

@ofborg eval

RossComputerGuy · 2024-07-08T02:59:43Z

Huh, firefox.stdenv.cc.bintools is resulting in a different value that is causing a rebuild.

RossComputerGuy · 2024-07-08T03:31:34Z

It seems the recurse into attrs + call packages of the default.nix in mkPackage was causing issues.

paparodeo · 2024-07-08T20:30:47Z

pkgs/development/compilers/llvm/common/default.nix

+              && !stdenv.targetPlatform.isAarch64
+              && (lib.versionOlder darwin.apple_sdk.sdk.version "11.0")


Suggested change

&& !stdenv.targetPlatform.isAarch64

&& (lib.versionOlder darwin.apple_sdk.sdk.version "11.0")

&& lib.versionOlder stdenv.targetPlatform.darwinSdkVersion "11.0"

will align this with the changes in https://github.com/NixOS/nixpkgs/pull/324155/files

then, which ever change has to deal with the merge conflicts with the deleted files can just re-delete the files and everything should just work.

Sweet, I've made the change. Not gonna push until #324155 is merged.

github-actions · 2024-07-24T17:25:19Z

Backport failed for release-24.05, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.05
git worktree add -d .worktree/backport-325175-to-release-24.05 origin/release-24.05
cd .worktree/backport-325175-to-release-24.05
git switch --create backport-325175-to-release-24.05
git cherry-pick -x 80e3c2c5d0d2138fad54896a19dc64260a066304 cfe063d174d955f88ce73e39a6de2bced18de065

Pull NixOS#320261 introduced the possibility to consistently override dependencies within an llvm package set. I think NixOS#325175 accidentally dropped this, so reinstate it. Signed-off-by: Peter Waller <p@pwaller.net>

github-actions bot added the 6.topic: llvm/clang Issues related to llvmPackages, clangStdenv and related label Jul 7, 2024

RossComputerGuy force-pushed the feat/llvm-version-commonify branch 7 times, most recently from 5e89fd3 to 24bad6a Compare July 7, 2024 05:48

pwaller reviewed Jul 7, 2024

View reviewed changes

pkgs/top-level/all-packages.nix Show resolved Hide resolved

pwaller reviewed Jul 7, 2024

View reviewed changes

RossComputerGuy force-pushed the feat/llvm-version-commonify branch 2 times, most recently from 245c8f9 to bcc1b79 Compare July 7, 2024 14:10

RossComputerGuy marked this pull request as ready for review July 7, 2024 14:32

RossComputerGuy mentioned this pull request Jul 7, 2024

treewide: change various flags to allow x64 darwin to default to sdk 11.0 when ready #324155

Merged

13 tasks

RossComputerGuy marked this pull request as draft July 7, 2024 18:25

RossComputerGuy marked this pull request as ready for review July 7, 2024 18:45

RossComputerGuy marked this pull request as draft July 7, 2024 18:46

reckenrode reviewed Jul 7, 2024

View reviewed changes

RossComputerGuy force-pushed the feat/llvm-version-commonify branch from e024141 to b2df0f7 Compare July 8, 2024 01:14

github-actions bot added the 6.topic: stdenv Standard environment label Jul 8, 2024

RossComputerGuy force-pushed the feat/llvm-version-commonify branch from b2df0f7 to 8bb4205 Compare July 8, 2024 01:30

RossComputerGuy force-pushed the feat/llvm-version-commonify branch 2 times, most recently from 840aab0 to 45379cb Compare July 8, 2024 02:32

RossComputerGuy force-pushed the feat/llvm-version-commonify branch from 45379cb to 71cf049 Compare July 8, 2024 03:30

RossComputerGuy force-pushed the feat/llvm-version-commonify branch from 71cf049 to 297d1b1 Compare July 8, 2024 05:26

ofborg bot added 8.has: clean-up 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 Jul 8, 2024

RossComputerGuy marked this pull request as ready for review July 8, 2024 05:57

paparodeo reviewed Jul 8, 2024

View reviewed changes

RossComputerGuy added 2 commits July 8, 2024 17:21

llvmPackages_{13,14,15,16,17,18,git}: commonify the default.nix

80e3c2c

overrideSDK: fix missing host platform inside of override

cfe063d

RossComputerGuy force-pushed the feat/llvm-version-commonify branch from 297d1b1 to cfe063d Compare July 9, 2024 00:22

RossComputerGuy merged commit cc29ddc into NixOS:master Jul 9, 2024
21 checks passed

RossComputerGuy deleted the feat/llvm-version-commonify branch July 9, 2024 03:38

Ericson2314 added the backport release-24.05 label Jul 24, 2024

Ericson2314 mentioned this pull request Jul 24, 2024

[Backport 24.05] llvmPackages_{13,14,15,16,17,18,git}: commonify the default.nix #329706

Merged

13 tasks

alyssais mentioned this pull request Aug 2, 2024

emscripten: 3.1.55 -> 3.1.64 #326633

Merged

13 tasks

RossComputerGuy mentioned this pull request Aug 8, 2024

Minor LLVM improvements #333146

Merged

13 tasks

Fuuzetsu mentioned this pull request Aug 20, 2024

lldb: Module not found: six.moves #335945

Open

pwaller mentioned this pull request Sep 14, 2024

llvmPackages: Reinstate overriding across whole package set #341855

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

llvmPackages_{13,14,15,16,17,18,git}: commonify the default.nix #325175

llvmPackages_{13,14,15,16,17,18,git}: commonify the default.nix #325175

RossComputerGuy commented Jul 7, 2024 •

edited

Loading

pwaller Jul 7, 2024

RossComputerGuy Jul 7, 2024

pwaller Jul 7, 2024

RossComputerGuy Jul 7, 2024

RossComputerGuy commented Jul 7, 2024

RossComputerGuy commented Jul 7, 2024

RossComputerGuy commented Jul 7, 2024

reckenrode Jul 7, 2024

RossComputerGuy Jul 7, 2024

alyssais Aug 2, 2024

alyssais Aug 2, 2024

RossComputerGuy Aug 3, 2024

RossComputerGuy Aug 3, 2024

alyssais Aug 3, 2024

RossComputerGuy commented Jul 8, 2024

RossComputerGuy commented Jul 8, 2024

RossComputerGuy commented Jul 8, 2024

paparodeo Jul 8, 2024

RossComputerGuy Jul 8, 2024

github-actions bot commented Jul 24, 2024

		&& !stdenv.targetPlatform.isAarch64
		&& (lib.versionOlder darwin.apple_sdk.sdk.version "11.0")

	&& !stdenv.targetPlatform.isAarch64
	&& (lib.versionOlder darwin.apple_sdk.sdk.version "11.0")
	&& lib.versionOlder stdenv.targetPlatform.darwinSdkVersion "11.0"

llvmPackages_{13,14,15,16,17,18,git}: commonify the default.nix #325175

llvmPackages_{13,14,15,16,17,18,git}: commonify the default.nix #325175

Conversation

RossComputerGuy commented Jul 7, 2024 • edited Loading

Description of changes

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RossComputerGuy commented Jul 7, 2024

RossComputerGuy commented Jul 7, 2024

RossComputerGuy commented Jul 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RossComputerGuy commented Jul 8, 2024

RossComputerGuy commented Jul 8, 2024

RossComputerGuy commented Jul 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 24, 2024

RossComputerGuy commented Jul 7, 2024 •

edited

Loading