lib: Clean up how linux and gcc config is specified#107214
lib: Clean up how linux and gcc config is specified#107214Ericson2314 merged 1 commit intoNixOS:masterfrom
Conversation
lheckemann
left a comment
There was a problem hiding this comment.
Some little corrections, but as you said this is generally still quite WIP so I haven't reviewed everything.
Thoughts:
- This seems to make sense
- but what exactly is the purpose of these platforms anyway? Kernel config is quite irrelevant to stdenv, and my impression is that that's the main thing differentiating platforms of the same arch from each other. Meanwhile things like linux's ARCH should probably be implied by
systemor similar…
lib/systems/default.nix
Outdated
There was a problem hiding this comment.
I think this should be separate from platform / linux-kernel. This should be a direct mapping from cpu name to linux kernel name. We already have qemuArch and will soon have a "darwinArch" in #105026. This shouldn't be configurable like platform / kernel-arch is.
lib/systems/default.nix
Outdated
There was a problem hiding this comment.
|
I don't see much benefit in a rename like this. The Linux kernel options really shouldn't be in crossSystem / hostPlatform at all. They're part of the Linux package and really have nothing to do with the whole package set; different kernels will have different configuration values for instance & there's no reason to special case Linux here. GCC, rustc, and linux-headers make sense though because they are part of the toolchain, and that determines what ABI you are going to use. |
55550c6 to
6d7d62a
Compare
|
OK this now builds, thanks for catching all my sloppiness @lheckemann. @matthewbauer I do agree with lots of that, actually. In particular, compiling a kernel is actually cross compilation, so while I do think kernel config could belong in some That said, I'm not really sure where the headers config ends and kernel config begins? Also do you know where to get the info for a complete-ish |
6d7d62a to
630e31b
Compare
b4a4d47 to
f381b01
Compare
|
@matthewbauer What do you think now? Even though the linux stuff might be moving again, still think these changes are worth it. |
f381b01 to
15a6698
Compare
15a6698 to
045d377
Compare
|
I added a release notes for the breaking change. Can we get some final review on this? |
lib/systems/default.nix
Outdated
There was a problem hiding this comment.
Can we continue to export platform for external usage?
There was a problem hiding this comment.
I guess this wouldn't work with the renames though... Maybe it's not worth it.
There was a problem hiding this comment.
Ah, true. I was about to do your change below but I didn't think of that.
There was a problem hiding this comment.
Oh post merge I realize the back-compat was already not attempting the linux stuff, oops.
lib/systems/default.nix
Outdated
There was a problem hiding this comment.
| config = parse.tripleFromSystem final.parsed; | |
| config = parse.tripleFromSystem final.parsed; | |
| platform = final.linux-kernel // { inherit (final) gcc rustc; }; |
The `platform` field is pointless nesting: it's just stuff that happens to be defined together, and that should be an implementation detail. This instead makes `linux-kernel` and `gcc` top level fields in platform configs. They join `rustc` there [all are optional], which was put there and not in `platform` in anticipation of a change like this. `linux-kernel.arch` in particular also becomes `linuxArch`, to match the other `*Arch`es. The next step after is this to combine the *specific* machines from `lib.systems.platforms` with `lib.systems.examples`, keeping just the "multiplatform" ones for defaulting.
045d377 to
8929989
Compare
|
stdenv rebuild which just got merged to m aster |
|
reverted in 0bc275e Please target staging to avoid the situation where people need to rebuild several hundred packages on PR reviews to master EDIT: grammar |
This is now possible, since the `platform` attribute has been removed in PR NixOS#107214. I've been waiting to do a cleanup like this for a long time!
Motivation for this change
The
platformfield is pointless nesting: it's just stuff that happens to be defined together, and that should be an implementation detail.This instead makes
linux-kernelandgcctop level fields in platform configs. They joinrustcthere [all are optional], which was put there and not inplatformin anticipation of a change like this.linux-kernel.archin particular also becomeslinuxArch, to match the other*Arches.The next step after is this to combine the specific machines from
lib.systems.platformswithlib.systems.examples, keeping just the "multiplatform" ones for defaulting.Progress on #34274
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)