top-level, stdenv: Make system and stdenv.system describe the hostPlatform
#46148
Conversation
…tPlatform. Intuitively, one cares mainly about the host platform: Platforms differ in meaningful ways but compilation is morally a pure process and probably doesn't care, or those difference are already abstracted away. @dezgeg also empirically confirmed that > 95% of checks are indeed of the host platform. Yet these attributes in the old cross infrastructure were defined to be the build platform, for expediency. And this was never before changed. (For native builds build and host coincide, so it isn't clear what the intention was.) Fixing this doesn't affect native builds, since again they coincide. It also doesn't affect cross builds of anything in Nixpkgs, as these are no longer used. It could affect external cross builds, but I deem that unlikely as anyone thinking about cross would use more explicit attributes for clarity, all the more so because the rarity of inspecting the build platform.
GrahamcOfBorg
added
6.topic: stdenv
|
Success on x86_64-darwin (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
Ericson2314
force-pushed
the
plain-system-host
branch
from
773233c
to
65558d9
Compare
|
Success on x86_64-darwin (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
…ystem` See the previous commit for details.
Ericson2314
force-pushed
the
plain-system-host
branch
from
65558d9
to
8ae2703
Compare
|
Success on x86_64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on aarch64-linux (full log) Attempted: stdenv Partial log (click to expand)
|
|
Success on x86_64-darwin (full log) Attempted: stdenv Partial log (click to expand)
|
| @@ -127,6 +127,9 @@ let | |||
| "`stdenv.isArm` is deprecated after 18.03. Please use `stdenv.isAarch32` instead" | |||
| hostPlatform.isAarch32; | |||
|
|
|||
| # The derivation's `system` is `buildPlatform.system`. | |||
| inherit (buildPlatform) system; | |||
There was a problem hiding this comment.
Was this supposed to be hostPlatform? I just came across this and thought it seemed like a no-op as is.
There was a problem hiding this comment.
Yes. It is weird at first but this is a Nix thing. Nix doesn't care why you are just building something, just what (possibly remote) builders can be used. It just cares about the build platform.
There was a problem hiding this comment.
Isn't that already done earlier in the file?: https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/default.nix#L88
Based on some quick tests I did, it looks like this the first assignment to system (inside the derivation) is what Nix uses to decide the builder. If you remove that but keep the second assignment (in the attrset override outside the derivation), Nix complains that system is not set.
Based on your commit message, I thought you were trying to correctly keep the derivation system value as buildPlatform.system, but make stdenv.system evaluate to hostPlatform.system. Right now the second assignment just seems redundant.
There was a problem hiding this comment.
I tested changing buildPlatform to hostPlatform in the second assignment and it changed pkgsCross.armv7l-hf-multiplatform.stdenv.system from x86_64-linux to armv7l-linux without rebuilding stdenv.
There was a problem hiding this comment.
Well, that's weird to me. Can you point to lines on master rather than this old PR?
There was a problem hiding this comment.
On master, the first assignment is: https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/default.nix#L88 and the second is: https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/default.nix#L142
There was a problem hiding this comment.
Ah so I think the whole point of the second must be to overwrite the system from the first without changing the derivation hash. And the only reason to do that is to make it the host platform instead of build platform!
So good catch :) if you could make a PR and give them both some comments, that would be great.
Motivation for this change
Intuitively, one cares mainly about the host platform: Platforms differ in meaningful ways but compilation is morally a pure process and probably doesn't care, or those difference are already abstracted away. @dezgeg also empirically confirmed that > 95% of checks are indeed of the host platform.
Yet these attributes in the old cross infrastructure were defined to be the build platform, for expediency. And this was never before changed. (For native builds build and host coincide, so it isn't clear what the
intention was.)
Fixing this doesn't affect native builds, since again they coincide. It also doesn't affect cross builds of anything in Nixpkgs, as these are no longer used. It could affect external cross builds, but I deem that
unlikely as anyone thinking about cross would use more explicit attributes for clarity, all the more so because the rarity of inspecting the build platform.
CC @dezgeg
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)