Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

treewide: stdenv.is -> stdenv.hostPlatform.is #356363

Merged
merged 2 commits into from
Nov 17, 2024

Conversation

JohnRTitor
Copy link
Contributor

@JohnRTitor JohnRTitor commented Nov 16, 2024

Commands ran:

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  "buildStdenv.is" "buildStdenv.hostPlatform.is"
fd --type f "\.nix" | xargs sd  "effectiveStdenv.is" "effectiveStdenv.hostPlatform.is"
fd --type f "\.nix" | xargs sd  "originalStdenv.is" "originalStdenv.hostPlatform.is"

Following #341407

stdenv.isLinux -> stdenv.hostPlatform.isLinux, is pretty much easy expansion of alias. This should cause no rebuilds.

However some packages have stdenv.isLinux in doCheck, meaning package tests have to be run when building in linux. So instead of hostPlatform there use buildPlatform there. This could cause some rebuilds.


Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: python 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: lua 6.topic: jitsi labels Nov 16, 2024
@@ -28,7 +28,7 @@ rustPlatform.buildRustPackage rec {

buildInputs =
[ sqlite ]
++ lib.optionals stdenv.isLinux [ openssl ]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These sneaked back in 5136e37 lol

@Artturin
Copy link
Member

Just do the commands in #341407 instead of only isLinux

@emilazy
Copy link
Member

emilazy commented Nov 16, 2024

I am unsure about the second commit here. Is it going to work with cross? Don’t we want a canExecute thing instead?

@Artturin
Copy link
Member

I am unsure about the second commit here. Is it going to work with cross? Don’t we want a canExecute thing instead?

doCheck' = doCheck && stdenv.buildPlatform.canExecute stdenv.hostPlatform;
canExecute is added by stdenv on top

However not sure if it's added when doing overrideAttrs like done in lua-modules

@JohnRTitor
Copy link
Contributor Author

JohnRTitor commented Nov 16, 2024

I am unsure about the second commit here. Is it going to work with cross?

It should work I suppose, the checks needs to be enabled on the build machine, not on the host machine where it would run.

OKAY I fundamentally misunderstood I believe, let's change it back to hostPlatform

@JohnRTitor JohnRTitor changed the title treewide: stdenv.isLinux -> stdenv.hostPlatform.isLinux; stdenv.isLinux -> stdenv.buildPlatform.isLinux for doCheck treewide: stdenv.is -> stdenv.hostPlatform.is Nov 16, 2024
@github-actions github-actions bot added 6.topic: qt/kde 6.topic: rust 6.topic: TeX Issues regarding texlive and TeX in general labels Nov 16, 2024
@emilazy
Copy link
Member

emilazy commented Nov 16, 2024

Formatter is unhappy.

@JohnRTitor
Copy link
Contributor Author

Not related to this PR, the errors are occuring as the original files are not formatted. We are only touching a line, formatting them here is out of scope

@emilazy
Copy link
Member

emilazy commented Nov 16, 2024

CI only complains if files that were formatted before the PR become unformatted, so I don’t think that’s true.

@jopejoe1
Copy link
Member

It complains about pkgs/applications/radio/sdrangel/default.nix, pkgs/by-name/er/erlang-language-platform/package.nix and pkgs/by-name/im/imhex/package.nix not being formated.

@JohnRTitor
Copy link
Contributor Author

Yeah, it seems like nixfmt will throw errors if line char limits have been exceeded and use an alternate style.

Done, this should make the CI happy, I have still decided to make the commit seperately though, for easier skimming through the diff. The PR can be squash merged if needed.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Nov 16, 2024
Copy link
Contributor Author

@JohnRTitor JohnRTitor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eval successfully passed but a merge conflict is shown. Intend to merge after rebasing, as while we race for the eval to complete again likely another conflict will arise

@JohnRTitor JohnRTitor merged commit e138313 into NixOS:master Nov 17, 2024
11 of 12 checks passed
@JohnRTitor JohnRTitor deleted the stdenv-is-fix branch November 17, 2024 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: jitsi 6.topic: lua 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: python 6.topic: qt/kde 6.topic: rust 6.topic: TeX Issues regarding texlive and TeX in general 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants