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#325175RossComputerGuy merged 2 commits intoNixOS:masterfrom
Conversation
5e89fd3 to
24bad6a
Compare
There was a problem hiding this comment.
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; })) versionsAnother 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.
There was a problem hiding this comment.
How would making the version list an attr set work with the officialRelease or gitRelease stuff?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ok, I made it work with an attr set.
245c8f9 to
bcc1b79
Compare
|
If someone nixpkgs-review's this and there's a rebuild count of 0 before Ofborg gets to this, I'll merge this. |
Weird CI error. |
|
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. |
pkgs/top-level/aliases.nix
Outdated
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
(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.)
There was a problem hiding this comment.
linuxPackages_latest is in aliases.nix on purpose
Where is linuxPackages_latest in aliases.nix? I don't see it in there.
There was a problem hiding this comment.
Where is
linuxPackages_latestinaliases.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.
e024141 to
b2df0f7
Compare
b2df0f7 to
8bb4205
Compare
|
@ofborg eval |
840aab0 to
45379cb
Compare
|
Huh, |
45379cb to
71cf049
Compare
|
It seems the recurse into attrs + call packages of the default.nix in |
71cf049 to
297d1b1
Compare
There was a problem hiding this comment.
| && !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.
There was a problem hiding this comment.
Sweet, I've made the change. Not gonna push until #324155 is merged.
297d1b1 to
cfe063d
Compare
|
Backport failed for 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 <[email protected]>
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 <[email protected]>
Description of changes
Drop all
default.nixfiles 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
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.