treewide: replace stdenv.is with stdenv.hostPlatform.is#341407
treewide: replace stdenv.is with stdenv.hostPlatform.is#341407emilazy merged 2 commits intoNixOS:masterfrom
stdenv.is with stdenv.hostPlatform.is#341407Conversation
|
Undrafting to get reviews from probably all the To keep the emails to a minimum either approve or thumbs up the PR if the idea is okay, or if it needs changes then let's discuss in https://matrix.to/#/#stdenv:nixos.org |
|
Idea is OK, list of merge conflicts is scary. Maybe do this stepwise? |
lib/tests/test-with-nix.nix
Outdated
There was a problem hiding this comment.
| ] ++ lib.optional pkgs.stdenv.hostPlatform.isLinux pkgs.inotify-tools; | |
| ] ++ lib.optional pkgs.stdenv.buildPlatform.isLinux pkgs.inotify-tools; |
This probably applies to most conditionals for any nativeBuildInputs.
There was a problem hiding this comment.
Let’s maybe fix these patterns in another PR, as it doesn’t match the current behaviour and would disrupt the primary metric (almost no rebuilds) we can judge this PR’s safety by? At least now it will be obviously wrong.
There was a problem hiding this comment.
Good point, but maybe leave the bad ones as is, so they're easy to update in the follow-up PR? Or I guess this PR's diff could be used for that purpose. It's a bit more hidden, but about as good.
SGTM
There was a problem hiding this comment.
I think it would require a smarter script than a simple text substitution to diagnose and avoid these cases, so I’d personally prefer to avoid holding this up over that.
There was a problem hiding this comment.
hostPlatform may just as easily be correct for nativeBuildInputs. I'd just mechanistically replace them. Mistakes are mistakes before and after, they are just more obvious after.
I'd guess in most cases it also sort of doesn't matter since the derivations don't cross compile to begin with or (as is the case here) don't really make sense to cross compile.
|
@Artturin, I think you have 15 thumbs up on this issue. What's the right way to land this, given the scariness of the set of merge conflicts? |
|
Let’s just rebase, wait for the ofborg rebuilds number, and merge. Best to get this in before 24.11 because of backports. |
|
Excited this is finally happening. Thanks @Artturin! |
In preparation for the deprecation of `stdenv.isX`. These shorthands are not conducive to cross-compilation because they hide the platforms. Darwin might get cross-compilation for which the continued usage of `stdenv.isDarwin` will get in the way One example of why this is bad and especially affects compiler packages https://www.github.com/NixOS/nixpkgs/pull/343059 There are too many files to go through manually but a treewide should get users thinking when they see a `hostPlatform.isX` in a place where it doesn't make sense. ``` fd --type f "\.nix" | xargs sd --fixed-strings "stdenv.is" "stdenv.hostPlatform.is" fd --type f "\.nix" | xargs sd --fixed-strings "stdenv'.is" "stdenv'.hostPlatform.is" fd --type f "\.nix" | xargs sd --fixed-strings "clangStdenv.is" "clangStdenv.hostPlatform.is" fd --type f "\.nix" | xargs sd --fixed-strings "gccStdenv.is" "gccStdenv.hostPlatform.is" fd --type f "\.nix" | xargs sd --fixed-strings "stdenvNoCC.is" "stdenvNoCC.hostPlatform.is" fd --type f "\.nix" | xargs sd --fixed-strings "inherit (stdenv) is" "inherit (stdenv.hostPlatform) is" fd --type f "\.nix" | xargs sd --fixed-strings "buildStdenv.is" "buildStdenv.hostPlatform.is" fd --type f "\.nix" | xargs sd --fixed-strings "effectiveStdenv.is" "effectiveStdenv.hostPlatform.is" fd --type f "\.nix" | xargs sd --fixed-strings "originalStdenv.is" "originalStdenv.hostPlatform.is" ```
`treewide: replace stdenv.is with stdenv.hostPlatform.is`
|
Reproduced the commits locally, ofborg has reported low build numbers on previous rebases, seems impossible to race ofborg and merge conflicts. Merging now and will take responsibility for the revert if ofborg throws up a scary result. |
Approve or thumbs up to show that you approve of the idea, or request changes on the first file to discuss why you don't think it's a good idea.
Join https://matrix.to/#/#stdenv:nixos.org to discuss
I'll reset to master and rerun the commands before merge.
Further replacements can be done in the PR where the warnings are added.
The commits can be added to
.git-blame-ignore-revsin a separate PRtreewide: replace
stdenv.iswithstdenv.hostPlatform.isIn preparation for the deprecation of
stdenv.isX.These shorthands are not conducive to cross-compilation because they
hide the platforms.
Darwin might get cross-compilation for which the continued usage of
stdenv.isDarwinwill get in the wayOne example of why this is bad and especially affects compiler packages
#343059
There are too many files to go through manually but a treewide should
get users thinking when they see a
hostPlatform.isXin a place where itdoesn't make sense.
The rebuilds are due to
src ./.or similarhttps://gist.github.com/Artturin/4cc1e73fc612cb7a79459a9bb6bc27a3